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] SDL 0275 - WebEngine App Testing #914

Closed theresalech closed 4 years ago

theresalech commented 4 years ago

Hello SDL community,

The review of "SDL 0275 - WebEngine App Testing" begins now and runs through January 21, 2020. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0275-webengine-app-testing.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/914

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

Jack-Byrne commented 4 years ago

The zip file may be up to 50MB zipped (200MB unzipped).

Is there a limitation to the developer portal that this specific upload size cap was mentioned? 50mb seems pretty low. If I zip the sdl_javascript_suite repository it is already at 47.9MB.

crokita commented 4 years ago

@JackLivio In regards to your question:

It was a decision made in order to limit the amount of bandwidth used with Manticore with file uploading for costs. It's a value that can be negotiated if needed. And in regards to the repository's size, a developer would only use the bundled distribution files in the lib/js/dist and lib/node/dist folders, which amount to 4.2 MB and 6.5 MB respectively.

kshala-ford commented 4 years ago

I'm not quite sure if I am understanding the proposal correctly. It sounds like the proposed solution is to load the webengine application in an invisible iframe in the developer's browser, specifically the tab showing manticore frontend. If that is true can you describe the differences to run the webengine app in another tab in the developer's browser?

Ideally the webengine app should run in an headless browser in the Manticore instance just as described in the first alternative you considered. Having both, Core and the WebEngine run on the same host is closer to the proposal SDL-0240 but I understand the downsides of such an approach reducing the amount of Manticore instances. Still, if the alternative is a considerable solution I would like to understand the impact to Manticore and the number of instances.

Manticore is supposed to support testing the application and if using the developer's browser as the webengine what if we consider showing the app in a new tab opened by the Manticore frontend? This way the developer could see the app being launched and also test the app with the browser's development tools.

@JackLivio Just tried the same on my machine but I only get a couple megabytes including the generated rpc files. In production the sdl library for webengine apps should be unified and minified in a single js file which heavily reduces the script size.

nickschwab commented 4 years ago

@kshala-ford

Yes, the proposal is to load the WebEngine app in an invisible iFrame using the the developer's browser. By loading it in an invisible iFrame rather than a new tab, we remove the risk of the developer accidentally closing the tab which would cause the app to become disconnected. There's also a possibility that popular ad-blockers would prevent auto-opening of a new tab.

I support running the WebEngine app in a headless browser, but in order to do so I believe the SDLC needs to define the minimum system and browser requirements for running WebEngine apps. These details were not included in the WebEngine proposal. If we go this route, it would also make more sense (in my opinion) for the headless browser to be part of Core so that WebEngine apps could be supported more easily by OEMs.

kshala-ford commented 4 years ago

Ad blockers can block iframes but allow open new windows and load a page from the same domain. It heavily depends on the active filters. If there are any issues with the invisible iframe in the devs browser I'm afraid the developer will only see an empty app list and have a hard time to find the issue.

I don't think that accidentally closing a tab is an issue. It could also be a test case to see the app behavior upon close. Also it's much more easy to test multi-app behavior as each app can be launched per tab. Core prevents a single app to run multiple times as it would refuse RegitsterAppInterface from the same name/id/host. Different to the iframe I think the benefits for debugging the app using dev tools are outstanding. The tools would focus on the app only and not include anything related to the manticore frontend. I think these are testing capabilities that cannot be ignored. Therefore my suggestion is to have webengine apps to open in a new tab.

nickschwab commented 4 years ago

@kshala-ford

Closing tabs would 100% be an issue. If I'm on a page and it forcefully opens a new, blank tab, I will close it immediately every single time and assume it is a bug. Happy to have the Project Maintainer's UI/UX expert weigh in on this, if needed.

From a testing and debugging perspective, uploading a compressed WebEngine package is not meant for debugging, but rather for quick production testing. If a developer wishes to debug their WebEngine app, they should open their local file in their browser and have it connect to Core's exposed websocket host and port. We should not encourage the poor workflow and increased resource consumption (bandwidth, CPU, memory, disk) of repeatedly compressing and re-uploading WebEngine packages for the purpose of debugging.

kshala-ford commented 4 years ago

You're explaining that an empty tab would be an issue as it would be rated as a bug by the developer who should be aware by the behavior as the developer has uploaded the app package but I couldn't see why closing a tab is an issue. Anyways the blank tab can be described, explained and documented on the SDL website, specifically the manticore section.

There's a huge difference between manually launching the webengine app from my local file:// and have Manticore launch it automatically and apps may behave differently. Not having the ability to debug the uploaded package due to misbehavior is an issue. If the SDLC decides not to use development tools though they are easy to reach doesn't sound like something developers would understand.

We should not encourage the poor workflow and increased resource consumption (bandwidth, CPU, memory, disk) of repeatedly compressing and re-uploading WebEngine packages for the purpose of debugging.

I am not understanding where this concern is coming. Is this a response to https://github.com/smartdevicelink/sdl_evolution/issues/914#issuecomment-576739404 regarding hosting and running the web app in the Manticore instance?

crokita commented 4 years ago

@kshala-ford I have a few things to add.

This link has information regarding resource requirements for core and generic HMI and the effects those have on the hosting machines. Memory is the current bottleneck for our AWS machines, and the total memory usage for a pair of these containers is 275 MB. While I don't know how much memory a browser instance like chromium would require, if it happens to need at least 275 MB, for example, then that means we'd lose at least half our available "slots" for each machine running these containers. This is my main concern for the argument of adding a headless browser for every user that connects to Manticore.

Anything that we can do to reduce bandwidth, CPU, memory, or disk requirements for our hosted machines will help with hosting costs. It is why I'm leaning towards the option of just having the uploaded bundle file be accessible by the Manticore user and having the app(s) run on the user's browser.

In regards to how we would want to run the apps (in a new tab versus invisible iframe on the same tab) the main advantage of having the direct connection to core's websocket server is so that quick debugging can be done with the app, and extra information that wouldn't be visible in a production environment can be displayed in a browser tab, for example. I wouldn't expect that uploading a web engine bundled app to Manticore would cause the same effect of opening a new tab to happen. Is there really a significant difference between the direct connection and uploading the bundle where the debugging couldn't just be done with the direct connection method?

theresalech commented 4 years ago

During the 2020-01-21 Steering Committee meeting, the committee voted to keep this proposal in review to allow the author time to respond to the comments, and for the conversation to continue on the review issue.

kshala-ford commented 4 years ago

Hi @crokita thanks for the explanation to AWS. The reason why I was asking is to understand the concerns of running the web application in Manticore and if it could be a considerable solution if hardware and budget allows. Using the developer's browser to run the app causes some more network traffic but it sounds like Manticore's limited RAM availability is the real limiting factor. I am OK with the PM decision to not run the web application on the same Manticore instance but on the developer's browser.

In regards to how we would want to run the apps (in a new tab versus invisible iframe on the same tab) the main advantage of having the direct connection to core's websocket server is so that quick debugging can be done with the app, and extra information that wouldn't be visible in a production environment can be displayed in a browser tab, for example.

I'm afraid I don't fully understand. Can you please elaborate on what's "direct connection"?

crokita commented 4 years ago

@kshala-ford By a direct connection, I mean that Manticore will expose core's websocket server port to the user. So, in addition to uploading a WebEngine app to Manticore, a user would be able to run their JS application on their computer, connecting to core over WS like how a mobile app connects over TCP.

kshala-ford commented 4 years ago

@crokita I do see two (or may be three) reasons why a Manticore driven launch of a hosted app is different to a developer driven launch of a local machine app (direct connection).

  1. The domain who's hosting the app is different. Rules like Same-Origin-Policy may affect the app behavior when being loaded into the browser.
  2. A developer driven launch registers the app getting "not-activated". Manticore driven launch would happen when the app is being activated. Existing state machines in SDL may behave differently.
  3. Manticore can set the query string for the websocket url and port properly.

A developer driven launch of a local machine app isn't the point of discussion as it's a feature accepted in the original WebEngine proposal. The topic is only about a Manticore driven launch of a hosted app and more specifically: a launch into an iframe or a separate tab. If you launch the app into a new tab you can use the browser developer tools without being overloaded by Manticore.

I know Manticore doesn't support video or audio streaming. However, I'm curious if the author and the PM looked into SDL-0272 WebEngine Projection mode with this proposal in mind.

crokita commented 4 years ago

@kshala-ford If the concern is being able to inspect app behavior through browser developer tools, I did a quick check through Chrome and Firefox, and those browser have tools that allows one to isolate the console logs to a specific iframe. With Firefox, there's a button which allows selecting a specific iframe, and only the relevant HTML and console logs will show. In Chrome, there's a context selector for the iframe, and the logs can be filtered in the console tab depending on which context (iframe) you selected (Settings button -> 'Selected context only' checked).

I bring up the original WebEngine proposal because it has been accepted and is closely related to this topic, so the advantages that it brings can be considered to the current issue of debugging JS applications. The WebEngine Projection proposal, on the other hand, has not been accepted, so the implications of projection mode have not been considered for this proposal.

nickschwab commented 4 years ago

@kshala-ford

  1. Are we all in agreement that WebEngine apps should be executed using the client's browser rather than a headless browser running on Manticore's servers?
  2. If the answer to the above question is "yes", then let's focus on resolving the disagreement of running the WebEngine app in an iFrame vs opening a blank new browser tab/window. @crokita provides some interesting research above which seems to address your concern about viewing the console logs of an iFrame.
  3. SDL-0272 WebEngine Projection Mode is not an accepted proposal and is not listed as a dependency of this proposal. For these reasons it should not have any impact on the discussion and outcome of this proposal.
kshala-ford commented 4 years ago

@crokita @nickschwab

  1. Yes, the apps should be executed using the client's browser.

I stated you can use the browser developer tools without being overloaded by Manticore. The ability to select a specific iframe doesn't improve the load of Manticore assets, logs or anything which is not related to the app. It will still be much more difficult to test your specific app compared to having the app opened in a new tab. I am aware of the mentioned console selector and every browser behaves slightly different with this feature but I don't believe this justifies the use of iframes because developers wouldn't realize that their apps is loaded in an iframe. Opening tabs also solves the case of testing multiple apps at the same time as each app opens in a separate tab.

To me the tab based solution clearly has benefits for testing apps.

If you look at the proposal SDL-0273 I provided feedback on SDL-0272 as requested by the PM although is not accepted yet. I think it's more than fair if the PM members follow this lead but I can start with providing some feedback:

Streaming apps are not supported with Manticore yet. With the WebEngine projection mode it could change, at least for WebEngine apps. Presenting an activated projection app in an iframe within the HMI could be an option but abilities to test the app with different screen sizes would not be possible. Also all concerns that I already mentioned would apply to projection apps as well.

There's no need for a vote on this item but it has impact on 2. Opening apps in a new tab independent of the presentation mode (template apps or projection apps) helps with app testing especially when testing the app rendering for projection apps.

crokita commented 4 years ago

@kshala-ford

  1. I understand the benefits of having separate tabs opened for each app in the browser when it comes to debugging. I don't like the idea of having a blank tab opening up on every uploaded bundle, which the browser would automatically focus to every time (I believe this is the default behavior for browsers, but this is something the user would have configured). I think that the benefits of separate tabs can still be fully utilized with the developer-drive app launching method.

Regarding the user knowing about their loaded apps, this is something the Manticore UI can manage like how it manages and sets up cloud apps, as it would be the component that manages the uploading of these bundles. There's an additional nuance I can think of when a user refreshes the tab where Manticore is running. On refreshing/reopening the tab to a running instance, reconnections will attempt to happen to core, the generic hmi, the log stream, etc. By giving agency to the user over their apps through their tabs instead of giving control to the Manticore UI, I don't think the Manticore UI can make assumptions about the connection state of any of the bundled apps on the user's browser. This means that when reconnecting, either the Manticore UI would have to open additional tabs for every uploaded bundle, which the user could already have the tabs open for, or Manticore would not open new tabs for those bundles, and the assumption is the user would have kept those tabs open the whole time.

  1. If the idea is to have the projected apps being displayable on a separate tab for testing resolution sizes, I don't see why a user couldn't just use the developer-driven app launching method to have their app connect to Manticore, and then the benefits of new tabs can be used there.

I understand that there are differences between both methods of JS app connections to core's websocket server but I'm still not convinced that the differences justify having WebEngine apps open in a new tab. I don't mind having new tabs open with the developer-driven method of launching JS apps to Manticore, and all the benefits new tabs brings can be utilized there, as that method gives the developer full control over how they want to launch their app anyway. With the WebEngine bundle process, I consider it as a different scenario, and I believe the Manticore UI should have more control over the bundled apps.

theresalech commented 4 years ago

The Steering Committee voted to keep this proposal in review, to allow for additional discussion on the issue. It was greatly encouraged that all SDLC Members review this proposal and provide their input on the item in question of running a WebEngine app in an invisible iFrame versus opening a new blank browser tab when using Manticore.

There was also conversation in the Steering Committee meeting regarding making sure representatives are aware of their company's stance on each proposal prior to the call, to avoid delaying decisions.

kshala-ford commented 4 years ago

Thank you @crokita for the explanation. As I try to think through the lifecycle of tabbed apps you technically can communicate across tabs but the effort is not reasonable. I'm OK to "close" the 2. and 3. point and go with iframes.

I believe some sort of communication between the iframe and the parent window may be needed to overcome issues when the uploaded application doesn't connect to Core for whatever reason. The developer tools can tell you reasons but you'll have to actively search for it as a developer.

One reason for your uploaded app to "not connect" is because a developer driven launch in a tab prevents it from connecting. Core may reply with APPLICATION_REGISTERED_ALREADY. Developers will have a hard time to understand which application (the tab app or the uploaded app) is connected and registered. I think to overcome this issue and also to provide at least some information to the developer do you plan to list the apps running? I'm not talking about making the iframe visible but have a simple list with the app name and at least an [X] button to "force close" the application. That way developers can close the app from the uploaded bundle and continue testing modifications in a developer driven tab. Any other solution that helps making it clear to developers and to take action which app should register is also appreciated.

crokita commented 4 years ago

@kshala-ford Yes, the idea is to display the list of WebEngine apps submitted to Manticore on the sidebar, and it can operate very similarly to how submitting a cloud application works. See the below image: there can be an additional element for submitting web engine apps, and the connected apps can appear on the top of the list (maybe Manticore will have to explicitly state which apps are cloud apps and which are WebEngine apps now).

Screenshot at Feb 04 13-17-33

Regarding potential connection issues, it will be possible to inform the developer of the information of connected WebEngine apps through the connected apps list. In the image below, the cloud app "BOCKS" is selected and information about its location, nicknames, and app id are displayed. There would be plenty of window space for whatever information may be useful for the developer for each submitted app.

Screenshot at Feb 04 13-30-48

theresalech commented 4 years ago

A quorum was not present at the 2020-02-04 Steering Committee meeting, so a vote on this proposal could not take place. This proposal will remain in review until our next meeting on 2020-02-11.

theresalech commented 4 years ago

The Steering Committee fully agreed to accept this proposal.

theresalech commented 4 years ago

Implementation issue entered: [SDL 0275] WebEngine App Testing