Mash: Avoid shipping duplicated strings and resources for Mash and Chrome. |
||||||||
Issue descriptionMash: Avoid shipping duplicated strings and resources for Mash and Chrome. When we release mash behind with a flag, both Mash and Chrome will need overlapping strings and resources. Currently, Mash and Chrome each repacks ash, ui, views, etc. strings and resources separately. chrome/BUILD.gn and chrome/chrome_repack_chrome_*_percent.gypi repacks ash_resources_*_percent.pak chrome/tools/build/repack_locales.py and chrome/chrome_repack_locales.gni repack ash_strings_*.pak ash/mus/BUILD.gn repacks the strings and resources needed for mash. ash_resources_100_percent.pak is 50323 bytes ash_resources_200_percent.pak is 100485 bytes ash_strings_en-US.pak is 8159 bytes, other languages are larger, up to 23885 bytes ui_resources_100_percent.pak is 92381 bytes ui_resources_200_percent.pak is 156316 bytes ui_resources_300_percent.pak is 95823 bytes views_resources_100_percent.pak is 37304 bytes views_resources_200_percent.pak is 60292 bytes views_resources_300_percent.pak is 39848 bytes app_locale_settings_en-US.pak is 66 bytes ui_strings_en-US.pak is 2904 bytes That's 643901 bytes altogether, not too bad, but more overlapping strings and resources may be needed. (ui_chromeos_resources_100_percent.pak is 1882087 bytes and ui_chromeos_resources_200_percent.pak is 6092260 bytes) To avoid shipping duplicated assets, we could to ship separate ash, etc. pak files that both apps can load. I'm not sure if there's a tradeoff in performance or overall file sizes when splitting up pak files.
,
Sep 29 2016
,
Sep 30 2016
I think for the short term, as long as --mash is a flag in chrome:flags, we should try to use only the resource paks that non-mash chrome uses. i.e. if we can avoid deploying any additional resources on the device at all, that would be ideal. If there are mus-specific resources (do we have any like that?), we can maybe include them in non-mus pak? With this approach, the only additional files we will need to deploy on device for mash would be the manifest files, and that would be pretty small, and totally doable trivially.
,
Sep 30 2016
Do you mean having mojo:ash load exe:chrome's pak files for string and image resources? That's certainly possible, and perhaps okay for mash as an experimental flag option, but we generally don't want ash/ to depend on chrome/ for resources like that...
,
Sep 30 2016
Pretty much, yes. But not just mojo:ash, the other mus apps too (e.g. touch_hud, autoclick, quick_launch etc.). I was thinking something along the lines of https://codereview.chromium.org/2377203005 (the changes in aura_init do not actually work [yet]). Of course, we do want ash to be completely decoupled and independent of chrome at some point, yes. But I think we want to be able to switch to mustash before that, and avoiding deploying unnecessary files which take up disk space on the device would be useful.
,
Sep 30 2016
We shouldn't have to build chrome to run ash. It's true a bunch of stuff won't work if you don't build chrome, but it shouldn't crash because of missing/outdated resources.
,
Sep 30 2016
Do you mean run ash via mojo_runner? ash_mus_resources* combined are ~500K, and looks like mus also duplicates some resources for cursors (it's pretty big, ~250+K). It's probably small enough that we can deploy these on device? Although it would be pretty nice to not have to.
,
Sep 30 2016
Yes, I was referring to running ash via mojo_runner. When running via --mash it seems totally reasonable to use the chrome resources.
,
Oct 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6a570def6dbb73675bdbee3766e636f0f2218439 commit 6a570def6dbb73675bdbee3766e636f0f2218439 Author: sadrul <sadrul@chromium.org> Date: Tue Oct 04 03:44:27 2016 chrome/mash: Load resources before running the mus app. When running 'chrome --mash' on device, all the resource files necessary for ash, mus etc. apps are already in the resource files used for regular chrome. So instead of deploying separate resource files for these apps, just load chrome's resource files, before initializing the app. BUG= 628715 , 633656 Review-Url: https://codereview.chromium.org/2387233002 Cr-Commit-Position: refs/heads/master@{#422694} [modify] https://crrev.com/6a570def6dbb73675bdbee3766e636f0f2218439/chrome/app/mash/mash_runner.cc [modify] https://crrev.com/6a570def6dbb73675bdbee3766e636f0f2218439/ui/views/mus/aura_init.cc
,
Oct 4 2016
,
Oct 4 2016
,
Sep 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/28e3c66582bef326723f8644893a8957c5748318 commit 28e3c66582bef326723f8644893a8957c5748318 Author: Evan Stade <estade@chromium.org> Date: Wed Sep 13 20:51:02 2017 Move screenshot notification strings from ash/ and ui/ to chrome/. Bug: 628715 Change-Id: I9431f3231f4e3989f10611ce856edbd2ca0ef4d0 Reviewed-on: https://chromium-review.googlesource.com/665348 Reviewed-by: James Cook <jamescook@chromium.org> Commit-Queue: Evan Stade <estade@chromium.org> Cr-Commit-Position: refs/heads/master@{#501745} [modify] https://crrev.com/28e3c66582bef326723f8644893a8957c5748318/ash/ash_strings.grd [modify] https://crrev.com/28e3c66582bef326723f8644893a8957c5748318/chrome/app/chromeos_strings.grdp [modify] https://crrev.com/28e3c66582bef326723f8644893a8957c5748318/chrome/browser/ui/ash/chrome_screenshot_grabber.cc
,
Sep 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8b2e4001fd571ebf1031d5978ad7b30d70e7cb52 commit 8b2e4001fd571ebf1031d5978ad7b30d70e7cb52 Author: Evan Stade <estade@chromium.org> Date: Thu Sep 14 14:57:07 2017 Move some chromeos/ash specific strings from ash/ to chrome/. Bug: 628715 Change-Id: I9e917d71ae65c037e6b18e328b535a0d91806e99 Reviewed-on: https://chromium-review.googlesource.com/665260 Reviewed-by: James Cook <jamescook@chromium.org> Commit-Queue: Evan Stade <estade@chromium.org> Cr-Commit-Position: refs/heads/master@{#501949} [modify] https://crrev.com/8b2e4001fd571ebf1031d5978ad7b30d70e7cb52/ash/ash_strings.grd [modify] https://crrev.com/8b2e4001fd571ebf1031d5978ad7b30d70e7cb52/chrome/app/chromeos_strings.grdp [modify] https://crrev.com/8b2e4001fd571ebf1031d5978ad7b30d70e7cb52/chrome/browser/chromeos/login/quick_unlock/quick_unlock_notification_controller.cc [modify] https://crrev.com/8b2e4001fd571ebf1031d5978ad7b30d70e7cb52/chrome/browser/chromeos/status/network_menu.cc [modify] https://crrev.com/8b2e4001fd571ebf1031d5978ad7b30d70e7cb52/chrome/browser/ui/ash/launcher/launcher_context_menu.cc [modify] https://crrev.com/8b2e4001fd571ebf1031d5978ad7b30d70e7cb52/ui/strings/ui_strings.grd
,
Oct 17 2017
For references to ash strings from Chrome, I think a good number of the strings can be just moved to Chrome and only a few are truly shared (see commits above). If they're really shared, can we just create duplicate strings in both ash strings and chrome strings grd files? Would they get de-duped in the translation console? Who knows how grit/TC works these days? If we do this we might accidentally update one string but not the other. To avoid that I think we can create a grdp file (grd part) which is included via <part> in both ash_strings.grd and generated_resources.grd.
,
Oct 19 2017
+flackr@ for thoughts/advice.
,
Feb 26 2018
,
Feb 26 2018
,
Apr 10 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/10ab4f912a76758187ac332d79aaed3c75740910 commit 10ab4f912a76758187ac332d79aaed3c75740910 Author: Evan Stade <estade@chromium.org> Date: Tue Apr 10 17:48:31 2018 Move some resources from ash_resources.grd to c/a/theme_resources.grd Bug: 628715 Change-Id: I612e5043407ccb7396d222e33e713d1da90c47ce Reviewed-on: https://chromium-review.googlesource.com/1002545 Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Commit-Queue: Evan Stade <estade@chromium.org> Cr-Commit-Position: refs/heads/master@{#549582} [modify] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/ash/resources/ash_resources.grd [delete] https://crrev.com/0f7ed208c67aebb227a68f72cf0e7efabcfd3d56/ash/resources/default_100_percent/cros/network/notification_datasaver.png [delete] https://crrev.com/0f7ed208c67aebb227a68f72cf0e7efabcfd3d56/ash/resources/default_100_percent/cros/network/status_network_failed.png [delete] https://crrev.com/0f7ed208c67aebb227a68f72cf0e7efabcfd3d56/ash/resources/default_200_percent/cros/network/notification_datasaver.png [delete] https://crrev.com/0f7ed208c67aebb227a68f72cf0e7efabcfd3d56/ash/resources/default_200_percent/cros/network/status_network_failed.png [rename] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/app/theme/default_100_percent/cros/avatar_holder.png [rename] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/app/theme/default_100_percent/cros/avatar_holder_mask.png [rename] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/app/theme/default_100_percent/cros/notification_3g.png [rename] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/app/theme/default_100_percent/cros/notification_lte.png [rename] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/app/theme/default_100_percent/cros/status_cellular_failed.png [rename] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/app/theme/default_100_percent/cros/task_manager.png [rename] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/app/theme/default_100_percent/cros/window_switcher_icon_incognito.png [rename] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/app/theme/default_100_percent/cros/window_switcher_icon_normal.png [rename] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/app/theme/default_200_percent/cros/avatar_holder.png [rename] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/app/theme/default_200_percent/cros/avatar_holder_mask.png [rename] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/app/theme/default_200_percent/cros/notification_3g.png [rename] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/app/theme/default_200_percent/cros/notification_lte.png [rename] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/app/theme/default_200_percent/cros/status_cellular_failed.png [rename] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/app/theme/default_200_percent/cros/task_manager.png [rename] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/app/theme/default_200_percent/cros/window_switcher_icon_incognito.png [rename] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/app/theme/default_200_percent/cros/window_switcher_icon_normal.png [modify] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/app/theme/theme_resources.grd [modify] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/browser/ui/ash/launcher/browser_shortcut_launcher_item_controller.cc [modify] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc [modify] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/browser/ui/ash/network/network_state_notifier.cc [modify] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/browser/ui/ash/session_util.cc [modify] https://crrev.com/10ab4f912a76758187ac332d79aaed3c75740910/chrome/browser/ui/views/task_manager_view.cc
,
Apr 12 2018
+agrieve, I believe we get this with your resource alias change. This just happens by default anytime we repack files together right?
,
Apr 12 2018
There's still some overhead for storing the IDs (and the overhead exists for each translation), but repack() will ensure than no identical payloads exist.
,
Aug 14
I believe this is no longer an issue now that we launch via the utility process. Mike, if I'm wrong, please reopen. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by bugdroid1@chromium.org
, Jul 18 2016