New issue
Advanced search Search tips

Issue 863565 link

Starred by 3 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Avoid duplicating resources for base mojo js in multiple webuis

Project Member Reported by jlklein@chromium.org, Jul 13

Issue description

See https://chromium-review.googlesource.com/c/chromium/src/+/1135949/1/chromeos/resources/chromeos_resources.grd#15

Right now, we're pulling in the generated mojo JS for base in multiple webuis which is wasteful of binary size.
 
Is the Mojo JS base you refer to different than chrome://resources/js/mojo_bindings.js ?
I'm not super familiar with this code so maybe Kyle or Steven can correct me, but I *think* it's different. I'm talking about pulling in ${mojom_root}/mojo/public/mojom/base/time.mojom.js. 
Cc: calamity@chromium.org
For time.mojom.js, also see my response at https://chromium-review.googlesource.com/c/chromium/src/+/1135949/1#message-269a957152cdf731cebd5f36cbfdcc7deec79846.
Even as of now, I don't think there is a need for duplication in a grd file though. Could you just register time.mojom.js manually in the data source that backs up your WebUI as done at [1]?

[1] https://cs.chromium.org/chromium/src/chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc?l=267
Re: comment #1: Yes, this is a separate source. mojo_bindings.js initializes the Mojo JS system, and time.mojom.js defines time-related structs/enums/interfaces which communicate over the system created by mojo_bindings.js.

Re comment #4: Personally, I would prefer if we had a Mojo-specific resources bundle (perhaps in //mojo/resources) which included JS source for any *.mojom files in //mojo/public that was needed. I think solution is more scalable than adding a JS file to the data source for the WebUI controller since the boilerplate needs to be written once and can be shared with any WebUI page that needs it instead of being needed once per WebUI page.
time.mojom is defined at [1] along with a ton of other .mojom files that are probably not needed for WebUI. I don't think we should be pre-emptively adding to chrome://resources/ generated JS files that are not actively being used.

> since the boilerplate needs to be written once

Adding a file to a WebUI data source is also a single line (example [2]), so I don't know if this qualifies as "boilerplate". I can see how this can get more involved occasionally, especially if an included mojom.js file has other dependencies that also need to be included, but that's not the case for time.mojom.

Either way, it sounds like we are in agreement in the following:
 - a chrome://resources/mojom/ that only hosts actively re-used *.mojom.js files is justified
 - listing the same JS file in multiple grd files is unnecessary, regardless of wether chrome://resources/mojom exists or not.

[1] https://cs.chromium.org/chromium/src/mojo/public/mojom/base/ 
[2] https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/settings/md_settings_ui.cc?l=349
As mentioned, there's an example CL at https://chromium-review.googlesource.com/c/chromium/src/+/1084370.

I'm OOO for the next 6 weeks starting Monday so I won't be repurposing it any time soon. Feel free to cannibalize it though =)
Components: -UI>ProximityAuth UI>Multidevice

Sign in to add a comment