Lazily inflate toolbar for WebAPKs |
|||||||||||
Issue descriptionAs part of its initialization WebApkActivity inflates toolbar (causes ToolbarControlContainer.initWithToolbar() to be called), even though for some WebAPKs (like MapsGo) toolbar is never shown. The call takes 70-140ms on Nokia 1 (1GiB Go device), which is at least a third (sometimes a half) of setContentView() time. The call probably takes even more time on 512MiB Android Go devices. The relevant change here is crrev.com/c/566168 which added WebappActivity.getToolbarLayoutId() method returning custom_tabs_toolbar. Before that WebappActivity was getting ChromeActivity.getToolbarLayoutId() implementation, which returns NO_TOOLBAR_LAYOUT. Lets think of a way to avoid inflating toolbar, or inflate it lazily when it's needed.
,
Mar 7 2018
,
Mar 7 2018
,
Mar 8 2018
,
Mar 15 2018
The mentioned patch was part of the Minimal-UI display mode implementation. See https://developer.mozilla.org/en-US/docs/Web/Manifest#display I expect this is a rarely used display mode, there exists UMAs that you could use to confirm it (Webapp.Install.DisplayMode, Launch.WebAppDisplayMode). It definitely seems reasonable to avoid inflating the toolbar if display mode is not minimal-ui. That's pretty much all knowledge I can offer I think. I moved to a different team and won't be able to help a lot here. CC me again if you're confident I can contribute more.
,
Mar 16 2018
,
Mar 21 2018
,
Mar 22 2018
I have a CL that avoids inflating toolbar UI: crrev.com/c/974462 Attached is screenshot of the UI hierarchy with the CL. I wonder if we can cut even more stuff?
,
May 30 2018
In crrev.com/c/974462 I completely avoid inflating toolbar for 'standalone' and 'fullscreen' modes. However, it turned out that there is a case where we need to show toolbar even in those modes, see issue 771984 . I attempted to lazily inflate toolbar in ToolbarControlContainer when its visibility changes, and I was able to do that transparently to ToolbarControlContainer clients... only to learn that there are places where we get toolbar via findViewById(R.id.toolbar). One of the places is in ToolbarManager, which wants to operate on toolbar during startup (and toolbar is null because it's lazily inflated).
,
May 30 2018
I still think that this direction is worth going, it's just not as easy as I thought.
,
Jun 4 2018
Mohamed is working on this (although on another bug which will be dup'd)
,
Jun 4 2018
,
Jun 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/01cc957dcf2d9cd3df12e0e21596b0aeaf6fb5f0 commit 01cc957dcf2d9cd3df12e0e21596b0aeaf6fb5f0 Author: Mohamed Heikal <mheikal@google.com> Date: Wed Jun 13 20:16:11 2018 [Android] Inflate layout on BG thread for WebApkActivity. Layout inflation takes about 200-300ms on lower end devices. It is currently done during oncreate way early during startup for WebApkActivity (a splashscreen is showing so inflating the UI eagerly is not important. This CL refactors startup to allow for layout inflation in the background for activities subclassing ChromeActvitiy. Bug: 819760 Change-Id: I538baa1abf19c6f53080b1a4552a8920a0ffcf56 Reviewed-on: https://chromium-review.googlesource.com/1072607 Commit-Queue: Mohamed Heikal <mheikal@chromium.org> Reviewed-by: Ted Choc <tedchoc@chromium.org> Reviewed-by: Yaron Friedman <yfriedman@chromium.org> Cr-Commit-Position: refs/heads/master@{#566967} [modify] https://crrev.com/01cc957dcf2d9cd3df12e0e21596b0aeaf6fb5f0/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java [modify] https://crrev.com/01cc957dcf2d9cd3df12e0e21596b0aeaf6fb5f0/chrome/android/java/src/org/chromium/chrome/browser/WarmupManager.java [modify] https://crrev.com/01cc957dcf2d9cd3df12e0e21596b0aeaf6fb5f0/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkAddActivity.java [modify] https://crrev.com/01cc957dcf2d9cd3df12e0e21596b0aeaf6fb5f0/chrome/android/java/src/org/chromium/chrome/browser/browseractions/BrowserActionActivity.java [modify] https://crrev.com/01cc957dcf2d9cd3df12e0e21596b0aeaf6fb5f0/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorView.java [modify] https://crrev.com/01cc957dcf2d9cd3df12e0e21596b0aeaf6fb5f0/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java [modify] https://crrev.com/01cc957dcf2d9cd3df12e0e21596b0aeaf6fb5f0/chrome/android/java/src/org/chromium/chrome/browser/firstrun/LightweightFirstRunActivity.java [modify] https://crrev.com/01cc957dcf2d9cd3df12e0e21596b0aeaf6fb5f0/chrome/android/java/src/org/chromium/chrome/browser/init/AsyncInitializationActivity.java [modify] https://crrev.com/01cc957dcf2d9cd3df12e0e21596b0aeaf6fb5f0/chrome/android/java/src/org/chromium/chrome/browser/init/BrowserParts.java [modify] https://crrev.com/01cc957dcf2d9cd3df12e0e21596b0aeaf6fb5f0/chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java [modify] https://crrev.com/01cc957dcf2d9cd3df12e0e21596b0aeaf6fb5f0/chrome/android/java/src/org/chromium/chrome/browser/init/EmptyBrowserParts.java [modify] https://crrev.com/01cc957dcf2d9cd3df12e0e21596b0aeaf6fb5f0/chrome/android/java/src/org/chromium/chrome/browser/searchwidget/SearchActivity.java [modify] https://crrev.com/01cc957dcf2d9cd3df12e0e21596b0aeaf6fb5f0/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
,
Jul 5
This hasn't been reverted in 3 weeks, so I am closing this bug. |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by dskiba@chromium.org
, Mar 7 2018