smartdevicelink / sdl_evolution

Tracking and proposing changes to SDL's public APIs.
https://smartdevicelink.github.io/sdl_evolution/
BSD 3-Clause "New" or "Revised" License
33 stars 122 forks source link

[Accepted with Revisions] SDL 0240 - WebEngine support for SDL JavaScript #767

Closed theresalech closed 4 years ago

theresalech commented 5 years ago

Hello SDL community,

The review of the revised proposal "SDL 0240 - WebEngine support for SDL JavaScript" begins now and runs through December 10, 2019. Previous reviews of this proposal took place November 13 - 19, 2019, October 16 - 28, 2019, July 17 - October 8, 2019, and June 19 - 25, 2019. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0240-sdl-js-pwa.md

Reviews are an important part of the SDL evolution process. All reviews should be sent to the associated Github issue at:

https://github.com/smartdevicelink/sdl_evolution/issues/767

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of SDL. When writing your review, here are some questions you might want to answer in your review:

More information about the SDL evolution process is available at

https://github.com/smartdevicelink/sdl_evolution/blob/master/process.md

Thank you, Theresa Lech

Program Manager - Livio theresa@livio.io

joeljfischer commented 5 years ago

As the first commenter, I want to note this first: If this proposal passes, it is step 1 of the biggest change to SDL since its creation. If you have a stake in SDL, review this proposal. It changes the fundamentals of what SDL is.

I see that under 2.2.3, this proposal indicates that this should only be used for first-party apps, but as I say under general note 2, this is fairly weak language. I will therefore address several comments on the basis of the fundamental change that this makes to SDL UI.

I must say that I'm surprised by this proposal, given that it fundamentally alters against the core principles of SDL: that an app only has to do one integration and its design is (1) guaranteed to pass OEM review (every OEM will now have to do design review on every Webengine SDL app), (2) will fit with the rest system (every app will now have a distinct design that won't fit with every OEM system, or any OEM system), (3) is driver distraction safe (apps can now design whatever they want and it's not going to go through the same stringent process that the OEM's own HMI goes through).

Furthermore, this proposal seems to be the opposite of every discussion we have had regarding template customization. Passing this proposal should re-open every previous discussion we've had about template customization that has been opposed by OEMs in the past. If totally open customization is on the table, why wouldn't we first go in the direction of increased template customization? Why not have an XML file that mobile apps or web apps can transfer that declares the layout of the template? Then at least the template would retain some semblance of the OEMs UI.

General Notes

1. Will SDLServer have to change to accommodate these OEM store changes?

2. Under 2.2.3, you write:

With the current depth of this chapter, the new HMI type should be used by 1st party OEM apps only. With future proposals and workshops the SDLC should open the new HMI type to 3rd party by creating and defining proper driver distraction and user interface guidelines.

I don't think this language is strong enough. This is a large amount of work for 1st party only apps. It may still be worth it to the SDLC to only build this for 1st party apps, but I personally think we should at least have some idea of what we're going to do about 3rd party apps. And I'm somewhat skeptical that there's a generic, one-size-fits-all solution to this problem that the SDLC will agree to. It took a year to decide on an english only string for backgrounding a navigation app. When we talked about template customization, after hours of discussion the most we could agree on were three optional colors that the OEM could use or not use however they wished.

Furthermore, this proposal doesn't even really mandate that it can't be used for third party apps. It only says that it should only be used for first-party apps. That leaves the door open for OEMs to go and do exclusive deals with app partners to do apps that only work on their platform, which undermines the very spirit of SDL. If this proposal were to go forward, I would strongly recommend to change should to must. Even then, how would we really enforce it?

3. I don't see the security concerns of running unknown JavaScript code on the head unit addressed. With SDL we have some amount of security due to the protocol based nature of data flow. Now we would have arbitrary code running on the head unit. What sandboxing / security features must be used to allow for this kind of arbitrary code? What about the security risks inherent in having a web browser that is bound to have security flaws running in a head unit for a decade or more? Especially with unknown SDL JavaScript code running on it?

Technical Notes

1. There doesn't seem to be a mechanism to update an app bundle. I'm assuming that the OEM app store would need to know that its installed bundle is out of date and be able to update it. Is that up to the OEM app store backend? But I would assume this would need to be a policies issue to ensure they have the most up to date app bundle that supports the given head unit. (e.g. if v1.0 supports SDL 3.0-5.0 and v2.0 supports SDL 5.1+).

2. Ideally, a change like this would also include a way for a mobile app to transfer / update a PWA app and act as an internet relay for it. That way vehicles without built in modems (or users without modem subscriptions) can access this feature and the apps that use it.

3. Under 2.1 is written:

The default app presentation approach should be template based. With the web app becoming active on the HMI, the HMI should not present the app's HTML document object. The body of the index.html file of the web app should be empty and should not be modified with scripts as the app won't be visible on the HMI.

I'm confused by what is meant by the body if the index.html file should be empty? Is the index file supposed to modify itself based on whether or not the template approach will be used (which it doesn't seem like they can know ahead of time)? Wouldn't it be better for the manifest to point to a different file that the HMI can start in this case? What if an app wants to only support OpenHMI?

4. Under 2.2, you write:

The window capabilities of this open main window will be empty except for the window ID and physical button capabilities.

Shouldn't at least the displayName be made available as well (though it seems that the proposal including WindowCapabilities is missing displayName)? A link to the WindowCapabilities definition would be helpful because it's not yet implemented as well.

5. Under 2.2.2, you write:

Using the new HMI type would be policy controlled. On the policy server this new HMI type can be added to the valid HMI type list per app ID. Only apps with permissions to use this new HMI type would be allowed to register.

Doesn't this conflict with 2.1? Wouldn't the app simply be given a template instead of a WebEngine page? I'm unsure how 2.1 is intended to work here. Is the app supposed to re-register without the OpenHMI type?

6. How is the app supposed to reach out to the internet for data? Are they intended to use a specialized API? The standard JavaScript call? Will Core / HMI route them through the modem? Can they use a connected mobile device as a relay?

7. In 3.1, you write:

The app package should be a compressed zip file

zip is not a great compression format. If OTA size is the main concern, why not consider something like tar + gzip? Though zip is very widely available, and would probably work on all systems in some manner.

8. In 3.5 you write:

The entry point HTML file should refer to the manifest file ( Githubissues.

  • Githubissues is a development platform for aggregating issues.