New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 764126 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 770392
issue 778877
issue 782514

Blocking:
issue 780164



Sign in to add a comment

MD Extensions: performance benchmark

Project Member Reported by scottchen@chromium.org, Sep 11 2017

Issue description

We should check the MD Extensions first-meaningful impression time against the existing extensions page before launch.
 
Blocking: 769402
Owner: dpa...@chromium.org
Status: Started (was: Available)
Any first-meaningful-paint measurements pre-optimization are not that useful. So, I started the necessary work to optimize (meaning using polymer-bundler) the Extensions page, so that we can see how much more optimization is needed after that.


Status update: I am able to run MD Extensions optimized by polymer-bundler locally. The necessary work is currently split among 3 CLs.

1) Move MD files to their own grd file, https://chromium-review.googlesource.com/c/chromium/src/+/696670.
2) Use relative URLs where possible, https://chromium-review.googlesource.com/c/chromium/src/+/699245
3) Invoke polymer-bundler, https://chromium-review.googlesource.com/#/c/chromium/src/+/699743.

There are still some errors thrown in the console that I need to investigate though.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/100ff11f74141c368bbb3f8be76892a899569841

commit 100ff11f74141c368bbb3f8be76892a899569841
Author: dpapad <dpapad@chromium.org>
Date: Wed Oct 04 20:05:27 2017

MD Extensions: Move resources to their own grd file.

This is a necessary preparatory step for optimizing the MD Extensions
page with polymer-bundler.

 Side benefits of this change:
 - No need for the repeated "resources/md_extensions" prefix on every file's
   path declaration in the grd file.
 - Faster build times when a file is touched (no longer need to rebuild
   the gigantic browser_resources.pak)
 - No longer need to call source->AddResourcePath() repeatedly in
   extensions_ui.cc.

Also moved two HTML files that were misplaced in the old Extensions folder
even though they are only used in the new MD version.

Bug:  764126 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ie9b13ed4f46fa14f27d9280eca40c9cd79a6d4b4
Reviewed-on: https://chromium-review.googlesource.com/696670
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506492}
[modify] https://crrev.com/100ff11f74141c368bbb3f8be76892a899569841/chrome/BUILD.gn
[modify] https://crrev.com/100ff11f74141c368bbb3f8be76892a899569841/chrome/browser/browser_resources.grd
[modify] https://crrev.com/100ff11f74141c368bbb3f8be76892a899569841/chrome/browser/resources/BUILD.gn
[rename] https://crrev.com/100ff11f74141c368bbb3f8be76892a899569841/chrome/browser/resources/md_extensions/drag_and_drop_handler.html
[add] https://crrev.com/100ff11f74141c368bbb3f8be76892a899569841/chrome/browser/resources/md_extensions/extensions_resources.grd
[rename] https://crrev.com/100ff11f74141c368bbb3f8be76892a899569841/chrome/browser/resources/md_extensions/shortcut_util.html
[modify] https://crrev.com/100ff11f74141c368bbb3f8be76892a899569841/chrome/browser/ui/webui/extensions/extensions_ui.cc
[modify] https://crrev.com/100ff11f74141c368bbb3f8be76892a899569841/chrome/chrome_paks.gni
[modify] https://crrev.com/100ff11f74141c368bbb3f8be76892a899569841/tools/gritsettings/resource_ids

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0

commit b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0
Author: dpapad <dpapad@chromium.org>
Date: Thu Oct 05 23:55:39 2017

MD Extensions: Use relative URLs where possible.

This is in preparation of using optimizing with polymer-bundler. 

Bug:  764126 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I96a88dff699389366122d39344044023d5b20b3c
Reviewed-on: https://chromium-review.googlesource.com/699245
Reviewed-by: Scott Chen <scottchen@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506921}
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/code_section.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/detail_view.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/drop_overlay.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/error_page.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/extensions.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/extensions_resources.grd
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/item.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/item_list.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/item_util.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/keyboard_shortcuts.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/kiosk_dialog.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/load_error.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/manager.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/navigation_helper.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/options_dialog.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/pack_dialog.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/pack_dialog_alert.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/service.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/shortcut_input.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/sidebar.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/strings.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/toolbar.html
[modify] https://crrev.com/b693f2dcbf0ced2bddbdbaf65080ac09ab7252a0/chrome/browser/resources/md_extensions/view_manager.html

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d993020f713b4658701bc6831be0312b0a6d520e

commit d993020f713b4658701bc6831be0312b0a6d520e
Author: dpapad <dpapad@chromium.org>
Date: Fri Oct 06 04:26:38 2017

MD Extensions: Stop loading HTML files dynamically during testing.

It is unnecessary since all tests did load chrome://extensions as
|browsePreload| anyway, but most importantly this causes the extensions
page to load twice during optimized builds, which leads to runtime errors.

This is in preparation of turning on optimize_webui for MD Extensions.

Bug:  764126 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I5e8b903bd4b5589af26fea6b3a25290d56188a7a
Reviewed-on: https://chromium-review.googlesource.com/703861
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506964}
[modify] https://crrev.com/d993020f713b4658701bc6831be0312b0a6d520e/chrome/browser/resources/md_extensions/detail_view.html
[modify] https://crrev.com/d993020f713b4658701bc6831be0312b0a6d520e/chrome/test/data/webui/extensions/cr_extensions_browsertest.js
[modify] https://crrev.com/d993020f713b4658701bc6831be0312b0a6d520e/chrome/test/data/webui/extensions/extension_code_section_test.js
[modify] https://crrev.com/d993020f713b4658701bc6831be0312b0a6d520e/chrome/test/data/webui/extensions/extension_detail_view_test.js
[modify] https://crrev.com/d993020f713b4658701bc6831be0312b0a6d520e/chrome/test/data/webui/extensions/extension_error_page_test.js
[modify] https://crrev.com/d993020f713b4658701bc6831be0312b0a6d520e/chrome/test/data/webui/extensions/extension_item_list_test.js
[modify] https://crrev.com/d993020f713b4658701bc6831be0312b0a6d520e/chrome/test/data/webui/extensions/extension_service_test.js

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 7 2017

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1a0712091b87bdd91ef44f39dbe951aa32176ac8

commit 1a0712091b87bdd91ef44f39dbe951aa32176ac8
Author: Demetrios Papadopoulos <dpapad@chromium.org>
Date: Tue Oct 10 07:09:36 2017

MD Extensions: Remove navigation listeners when element gets detached.

Currently even when <extensions-manager> was detached from the page, it kept
receiving navigation updates, which causes test failures in optimized mode.

Also changing navigation_helper_test to use a new NavigationHelper instance,
instead of re-using the existing singleton instance, which is more error prone.

This is in preparation for turning on optimized_webui for MD Extensions.

Bug:  764126 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I0a2b553f2e1eda7e4588ff6b78b10e7a6127e3b8
Reviewed-on: https://chromium-review.googlesource.com/707916
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Scott Chen <scottchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507602}
[modify] https://crrev.com/1a0712091b87bdd91ef44f39dbe951aa32176ac8/chrome/browser/resources/md_extensions/manager.js
[modify] https://crrev.com/1a0712091b87bdd91ef44f39dbe951aa32176ac8/chrome/browser/resources/md_extensions/navigation_helper.js
[modify] https://crrev.com/1a0712091b87bdd91ef44f39dbe951aa32176ac8/chrome/test/data/webui/extensions/extension_item_test.js
[modify] https://crrev.com/1a0712091b87bdd91ef44f39dbe951aa32176ac8/chrome/test/data/webui/extensions/extension_navigation_helper_test.js
[modify] https://crrev.com/1a0712091b87bdd91ef44f39dbe951aa32176ac8/chrome/test/data/webui/extensions/extension_sidebar_test.js

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/799677b79e4cc02af3f569e5596be73a584eefdc

commit 799677b79e4cc02af3f569e5596be73a584eefdc
Author: dpapad <dpapad@chromium.org>
Date: Tue Oct 10 21:24:04 2017

MD Extensions: Optimize with polymer-bundler.

 - Add all necessary GN setup.
 - Move two files reused by MD Extensions and old Extensions under
   md_extensions/ since it is easier to re-use from there.
 - Make MD Extensions respect the optimize_webui GN config flag.

This reduces the FMP (first meeaningful paint, from about 800ms to 450ms
on my workstation).

Bug:  764126 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ia31a4790b7a349d65b67f07acafa249170207911
Reviewed-on: https://chromium-review.googlesource.com/699743
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507775}
[modify] https://crrev.com/799677b79e4cc02af3f569e5596be73a584eefdc/chrome/browser/browser_resources.grd
[modify] https://crrev.com/799677b79e4cc02af3f569e5596be73a584eefdc/chrome/browser/resources/BUILD.gn
[modify] https://crrev.com/799677b79e4cc02af3f569e5596be73a584eefdc/chrome/browser/resources/extensions/compiled_resources2.gyp
[modify] https://crrev.com/799677b79e4cc02af3f569e5596be73a584eefdc/chrome/browser/resources/extensions/extension_command_list.js
[modify] https://crrev.com/799677b79e4cc02af3f569e5596be73a584eefdc/chrome/browser/resources/extensions/extensions.js
[add] https://crrev.com/799677b79e4cc02af3f569e5596be73a584eefdc/chrome/browser/resources/md_extensions/BUILD.gn
[modify] https://crrev.com/799677b79e4cc02af3f569e5596be73a584eefdc/chrome/browser/resources/md_extensions/compiled_resources2.gyp
[modify] https://crrev.com/799677b79e4cc02af3f569e5596be73a584eefdc/chrome/browser/resources/md_extensions/drag_and_drop_handler.html
[rename] https://crrev.com/799677b79e4cc02af3f569e5596be73a584eefdc/chrome/browser/resources/md_extensions/drag_and_drop_handler.js
[modify] https://crrev.com/799677b79e4cc02af3f569e5596be73a584eefdc/chrome/browser/resources/md_extensions/extensions_resources.grd
[add] https://crrev.com/799677b79e4cc02af3f569e5596be73a584eefdc/chrome/browser/resources/md_extensions/extensions_resources_vulcanized.grd
[modify] https://crrev.com/799677b79e4cc02af3f569e5596be73a584eefdc/chrome/browser/resources/md_extensions/shortcut_util.html
[rename] https://crrev.com/799677b79e4cc02af3f569e5596be73a584eefdc/chrome/browser/resources/md_extensions/shortcut_util.js
[modify] https://crrev.com/799677b79e4cc02af3f569e5596be73a584eefdc/chrome/browser/ui/webui/extensions/extensions_ui.cc
[modify] https://crrev.com/799677b79e4cc02af3f569e5596be73a584eefdc/tools/gritsettings/resource_ids

Uploading screencasts of before/after for upcoming https://chromium-review.googlesource.com/c/chromium/src/+/711095 change.
refresh_perceived_speed_before.mp4
149 KB View Download
refresh_perceived_speed_after.mp4
125 KB View Download
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/988655c32149893423e683430cdc8c271381c3da

commit 988655c32149893423e683430cdc8c271381c3da
Author: dpapad <dpapad@chromium.org>
Date: Thu Oct 12 03:47:45 2017

MD Extensions: Improve perceived speed of page load/refresh.

 - Pushing pixels on the screen faster by moving all <link> tags outside
   of <head>.
 - Faking the page layout (toolbar + background) until Polymer elements are
   loaded.

This is the same trick used in other WebUI pages, and completely eliminates the
white flash currently seen when refreshing/loading the page.

Bug:  764126 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Icc12935f10c8a9f3dda5eeab73ca5d91c06804c0
Reviewed-on: https://chromium-review.googlesource.com/711095
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Dan Beam (no longer on Chrome) <dbeam@chromium.org>
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508250}
[modify] https://crrev.com/988655c32149893423e683430cdc8c271381c3da/chrome/browser/resources/md_extensions/extensions.html
[modify] https://crrev.com/988655c32149893423e683430cdc8c271381c3da/chrome/browser/resources/md_extensions/manager.js

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f991780a2256c9fe13c14f89c4da9125829ede1b

commit f991780a2256c9fe13c14f89c4da9125829ede1b
Author: Keishi Hattori <keishi@chromium.org>
Date: Thu Oct 12 06:11:40 2017

Revert "MD Extensions: Improve perceived speed of page load/refresh."

This reverts commit 988655c32149893423e683430cdc8c271381c3da.

Reason for revert: Suspecting cause for CrExtensionsErrorPageTest.Layout CrExtensionsShortcutTest.Basic failures
 crbug.com/773977 

Original change's description:
> MD Extensions: Improve perceived speed of page load/refresh.
> 
>  - Pushing pixels on the screen faster by moving all <link> tags outside
>    of <head>.
>  - Faking the page layout (toolbar + background) until Polymer elements are
>    loaded.
> 
> This is the same trick used in other WebUI pages, and completely eliminates the
> white flash currently seen when refreshing/loading the page.
> 
> Bug:  764126 
> Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
> Change-Id: Icc12935f10c8a9f3dda5eeab73ca5d91c06804c0
> Reviewed-on: https://chromium-review.googlesource.com/711095
> Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
> Reviewed-by: Dan Beam (no longer on Chrome) <dbeam@chromium.org>
> Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#508250}

TBR=dbeam@chromium.org,dpapad@chromium.org,dschuyler@chromium.org

Change-Id: Ib2a06bd9ff37d5944d56db52ae91bf5936f4eba2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  764126 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Reviewed-on: https://chromium-review.googlesource.com/715236
Reviewed-by: Keishi Hattori <keishi@chromium.org>
Commit-Queue: Keishi Hattori <keishi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508265}
[modify] https://crrev.com/f991780a2256c9fe13c14f89c4da9125829ede1b/chrome/browser/resources/md_extensions/extensions.html
[modify] https://crrev.com/f991780a2256c9fe13c14f89c4da9125829ede1b/chrome/browser/resources/md_extensions/manager.js

Project Member

Comment 13 by bugdroid1@chromium.org, Oct 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d04c934260e5762ebbe19939aa2dd46e7dda7aec

commit d04c934260e5762ebbe19939aa2dd46e7dda7aec
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Thu Oct 12 07:33:13 2017

Revert "MD Extensions: Improve perceived speed of page load/refresh."

This reverts commit 988655c32149893423e683430cdc8c271381c3da.

Reason for revert: Consistent CrExtensionsErrorPageTest.Layout and CrExtensionsShortcutTest.Basic failures starting with https://uberchromegw.corp.google.com/i/chromium.linux/builders/Linux%20Tests%20%28dbg%29%281%29/builds/67359

Original change's description:
> MD Extensions: Improve perceived speed of page load/refresh.
> 
>  - Pushing pixels on the screen faster by moving all <link> tags outside
>    of <head>.
>  - Faking the page layout (toolbar + background) until Polymer elements are
>    loaded.
> 
> This is the same trick used in other WebUI pages, and completely eliminates the
> white flash currently seen when refreshing/loading the page.
> 
> Bug:  764126 
> Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
> Change-Id: Icc12935f10c8a9f3dda5eeab73ca5d91c06804c0
> Reviewed-on: https://chromium-review.googlesource.com/711095
> Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
> Reviewed-by: Dan Beam (no longer on Chrome) <dbeam@chromium.org>
> Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#508250}

TBR=dbeam@chromium.org,dpapad@chromium.org,dschuyler@chromium.org

Change-Id: I8facb8d911d345a3a44ebd3422581dbe5d530586
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  764126 ,  773992 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Reviewed-on: https://chromium-review.googlesource.com/715456
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508274}

Project Member

Comment 14 by bugdroid1@chromium.org, Oct 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/92a2e568b06de2e2b63db8fc8e2731cc5519e032

commit 92a2e568b06de2e2b63db8fc8e2731cc5519e032
Author: dpapad <dpapad@chromium.org>
Date: Fri Oct 13 21:25:13 2017

MD Extensions: Fix tests that are causing problems in debug mode.

Bug:  764126 , 773977 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ic18b4c1315812bbf38d45b77b091710814f113c0
Reviewed-on: https://chromium-review.googlesource.com/717539
Reviewed-by: Scott Chen <scottchen@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508824}
[modify] https://crrev.com/92a2e568b06de2e2b63db8fc8e2731cc5519e032/chrome/browser/resources/md_extensions/shortcut_input.html
[modify] https://crrev.com/92a2e568b06de2e2b63db8fc8e2731cc5519e032/chrome/test/data/webui/extensions/cr_extensions_browsertest.js

Project Member

Comment 15 by bugdroid1@chromium.org, Oct 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1da6ce3789d2450f130d70385e6c9809e5919bb5

commit 1da6ce3789d2450f130d70385e6c9809e5919bb5
Author: dpapad <dpapad@chromium.org>
Date: Sat Oct 14 01:49:19 2017

Reland "MD Extensions: Improve perceived speed of page load/refresh."

This is a reland of 988655c32149893423e683430cdc8c271381c3da
Original change's description:
> MD Extensions: Improve perceived speed of page load/refresh.
> 
>  - Pushing pixels on the screen faster by moving all <link> tags outside
>    of <head>.
>  - Faking the page layout (toolbar + background) until Polymer elements are
>    loaded.
> 
> This is the same trick used in other WebUI pages, and completely eliminates the
> white flash currently seen when refreshing/loading the page.
> 
> Bug:  764126 
> Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
> Change-Id: Icc12935f10c8a9f3dda5eeab73ca5d91c06804c0
> Reviewed-on: https://chromium-review.googlesource.com/711095
> Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
> Reviewed-by: Dan Beam (no longer on Chrome) <dbeam@chromium.org>
> Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#508250}

Bug:  764126 
Change-Id: I7d3f4f79f0d979c0faa7a3fc75981cd73d8a0af8
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Reviewed-on: https://chromium-review.googlesource.com/716800
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508910}
[modify] https://crrev.com/1da6ce3789d2450f130d70385e6c9809e5919bb5/chrome/browser/resources/md_extensions/extensions.html
[modify] https://crrev.com/1da6ce3789d2450f130d70385e6c9809e5919bb5/chrome/browser/resources/md_extensions/manager.js

Status update: I have identified a few separate improvements so far

1) Eliminate unnecessary call to getComputedStyle(), in review at [1].
2) Strip non-Chromium prefixed CSS rules from third_party/polymer (started a 
   discussion thread with more details).
3) Eliminate usage of iron-flex-layout, see [2]. This shaves a few more KB from the final bundled file.
4) Eliminate usage of paper-item. A TODO already exists at [3]

At the same time @dschuyler has been working on [4] that makes non-primary views render lazily.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/737445.
[2] https://cs.chromium.org/search/?q=iron-flex-layout+file:%5Esrc/chrome/browser/resources/md_extensions/+package:%5Echromium$&type=cs
[3] https://cs.chromium.org/chromium/src/chrome/browser/resources/md_extensions/sidebar.html?l=54-62.
[4] https://chromium-review.googlesource.com/c/chromium/src/+/731884
Blockedon: 778877
Blockedon: 770392
Project Member

Comment 19 by bugdroid1@chromium.org, Oct 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2abf8ffcac5aafafe40083d75e82590b043f2597

commit 2abf8ffcac5aafafe40083d75e82590b043f2597
Author: dpapad <dpapad@chromium.org>
Date: Fri Oct 27 02:53:56 2017

WebUI cr-toggle: Remove costly call to getComputedStyle().

getComputedStyle() was used in JS to detect whether the page is in RTL mode.
Element#matches() can achieve the same without invoking a style recalculation.

Bug:  764126 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I39a7d3a303cbfd86c3277157438ab5b3c9916d2c
Reviewed-on: https://chromium-review.googlesource.com/737445
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512074}
[modify] https://crrev.com/2abf8ffcac5aafafe40083d75e82590b043f2597/ui/webui/resources/cr_elements/cr_toggle/cr_toggle.js

Project Member

Comment 20 by bugdroid1@chromium.org, Oct 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7c8a2b41208083b19d6d6b0b11c3408524049057

commit 7c8a2b41208083b19d6d6b0b11c3408524049057
Author: dpapad <dpapad@chromium.org>
Date: Mon Oct 30 22:58:35 2017

Polymer: Automatically remove prefixed CSS from third_party/polymer.

Bug:  764126 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I10e75f00e152c7ae6314e4671fca8f6c601bdf65
Reviewed-on: https://chromium-review.googlesource.com/740573
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512661}
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/README.chromium
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/components-chromium/iron-autogrow-textarea/iron-autogrow-textarea.html
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/components-chromium/iron-flex-layout/iron-flex-layout-classes.html
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/components-chromium/iron-flex-layout/iron-flex-layout.html
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/components-chromium/paper-button/paper-button.html
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/components-chromium/paper-checkbox/paper-checkbox.html
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/components-chromium/paper-fab/paper-fab.html
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/components-chromium/paper-icon-button/paper-icon-button.html
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/components-chromium/paper-input/paper-input-container.html
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/components-chromium/paper-progress/paper-progress.html
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/components-chromium/paper-radio-button/paper-radio-button.html
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/components-chromium/paper-slider/paper-slider.html
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/components-chromium/paper-spinner/paper-spinner-styles.html
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/components-chromium/paper-styles/classes/shadow-layout.html
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/components-chromium/paper-tabs/paper-tab.html
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/components-chromium/paper-tabs/paper-tabs.html
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/components-chromium/paper-toggle-button/paper-toggle-button.html
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/components-chromium/paper-tooltip/paper-tooltip.html
[add] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/css_strip_prefixes.py
[add] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/css_strip_prefixes_test.py
[modify] https://crrev.com/7c8a2b41208083b19d6d6b0b11c3408524049057/third_party/polymer/v1_0/reproduce.sh

Project Member

Comment 21 by bugdroid1@chromium.org, Oct 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/47e8a219f67b800bdc54d8621112d7755f78ed20

commit 47e8a219f67b800bdc54d8621112d7755f78ed20
Author: dpapad <dpapad@chromium.org>
Date: Tue Oct 31 21:20:09 2017

MD Extensions: Remove invalid </iron-list> closing tag.

Also remove dependency on iron-list since it is not actually used. This
reduces crisper.js output by 16% (from 173kb to 145kb)!

Bug:  764126 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Iab7e53b4e67e06f5ab54693e0c7b9df5ec0f677a
Reviewed-on: https://chromium-review.googlesource.com/747127
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512955}
[modify] https://crrev.com/47e8a219f67b800bdc54d8621112d7755f78ed20/chrome/browser/resources/md_extensions/error_page.html

Project Member

Comment 22 by bugdroid1@chromium.org, Oct 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/32bb3b904f4d8475a977603c282d0f477ba48eb1

commit 32bb3b904f4d8475a977603c282d0f477ba48eb1
Author: dpapad <dpapad@chromium.org>
Date: Tue Oct 31 23:03:15 2017

MD Extensions: Drop unnecessary work when first populating the list.

During initial population of the extensions and apps lists, addItem()
was called N times, which internally performs a linear search N times,
as well as triggres splice() notifications that are unnecessary.

As revealed by DevTools (for a total of 30 extensions and apps):
 - before: the N calls to addItem() were taking ~22ms total
 - after: The initialization of the lists takes ~5ms total 

Bug:  764126 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: If66d752cd3e7ac4b4ecd01ea4964ca6fd8d8f0db
Reviewed-on: https://chromium-review.googlesource.com/746967
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512989}
[modify] https://crrev.com/32bb3b904f4d8475a977603c282d0f477ba48eb1/chrome/browser/resources/md_extensions/manager.js
[modify] https://crrev.com/32bb3b904f4d8475a977603c282d0f477ba48eb1/chrome/browser/resources/md_extensions/service.js

Status update: Several optimization CLs have already landed. Pending CLs

 1) Trigger chunked rendering of items list, https://chromium-review.googlesource.com/c/chromium/src/+/748061.
 2) Reduce the amount of hidden rendered <paper-button>s, https://chromium-review.googlesource.com/c/chromium/src/+/748427.

Both of the above reduce the FMP time (noticeably). I also experimented with
 3) Split the app to two modules, basic and lazy, similar to Settings and History, https://chromium-review.googlesource.com/c/chromium/src/+/747051, WIP.
 4)  Polymer: Update css_strip_prefixes to catch multiline CSS rules, https://chromium-review.googlesource.com/c/chromium/src/+/748428.

I will re-evaluate 3 and 4, after all other improvements have landed.
Project Member

Comment 24 by bugdroid1@chromium.org, Nov 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/94084d2ff74097ce33600f6d0de71758bf2a92bb

commit 94084d2ff74097ce33600f6d0de71758bf2a92bb
Author: dpapad <dpapad@chromium.org>
Date: Thu Nov 02 01:09:55 2017

MD Extensions: Trigger chunked rendering of items list.

Before this change, the dom-repeat was rendered all at once. By specifiying
the initial-count property, chunked rendering is triggered, which causes
a few items to be rendered first, and the rest later.

Overall this improves the FMP time by ~18% (675ms -> 552ms, tried on Linux with 30
extensions, averaged over 10 runs).

Bug:  764126 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I10be163ea10f6b4ca84e9f754f188117f58c7f17
Reviewed-on: https://chromium-review.googlesource.com/748061
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513352}
[modify] https://crrev.com/94084d2ff74097ce33600f6d0de71758bf2a92bb/chrome/browser/resources/md_extensions/item_list.html

Project Member

Comment 25 by bugdroid1@chromium.org, Nov 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ac60addbf5a2bb54a16ee3c3b4a41629b28d5732

commit ac60addbf5a2bb54a16ee3c3b4a41629b28d5732
Author: dpapad <dpapad@chromium.org>
Date: Thu Nov 02 02:38:35 2017

MD Extensions: Reduce the amount of hidden rendered <paper-button>s.

Before this CL, for 30 total extensions, there were
paper-button instances: 153, hidden 94

After this CL
paper-button instances: 63, hidden 4

This improves FMP time by ~12% (686ms -> 603ms, on Linux).

Bug:  764126 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I203334058ad93d5a5decfd81f7583fb92a41c8ce
Reviewed-on: https://chromium-review.googlesource.com/748427
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513383}
[modify] https://crrev.com/ac60addbf5a2bb54a16ee3c3b4a41629b28d5732/chrome/browser/resources/md_extensions/item.html
[modify] https://crrev.com/ac60addbf5a2bb54a16ee3c3b4a41629b28d5732/chrome/browser/resources/md_extensions/item.js
[modify] https://crrev.com/ac60addbf5a2bb54a16ee3c3b4a41629b28d5732/chrome/test/data/webui/extensions/extension_item_test.js

I discovered an unwanted side effect caused by the usage of initial-count with <dom-repeat>, introduced at https://chromium-review.googlesource.com/748061.

Also see attached screencasts.
Dragging the enable/disable toggle left right with the mouse no longer works as before, because initial-count causes the dom-repeat to re-render in a way that is problematic. I was expecting that initial-count only affects the first render, but that does not seem to be the case.

Will revert for now, and investigate a better approach later (which is turning off initial-count after the 1st render, by setting it to zero).
without-initial-count.mp4
245 KB View Download
initial-count-issue.mp4
413 KB View Download
Project Member

Comment 27 by bugdroid1@chromium.org, Nov 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3187908e99f70ae440ee7656aed7abee5bcb4a2b

commit 3187908e99f70ae440ee7656aed7abee5bcb4a2b
Author: Demetrios Papadopoulos <dpapad@chromium.org>
Date: Thu Nov 02 18:39:49 2017

Revert "MD Extensions: Trigger chunked rendering of items list."

This reverts commit 94084d2ff74097ce33600f6d0de71758bf2a92bb.

Reason for revert: Caused chunked renderings even after the first render, interfering with user interaction, see details at https://bugs.chromium.org/p/chromium/issues/detail?id=764126#c26.

Original change's description:
> MD Extensions: Trigger chunked rendering of items list.
> 
> Before this change, the dom-repeat was rendered all at once. By specifiying
> the initial-count property, chunked rendering is triggered, which causes
> a few items to be rendered first, and the rest later.
> 
> Overall this improves the FMP time by ~18% (675ms -> 552ms, tried on Linux with 30
> extensions, averaged over 10 runs).
> 
> Bug:  764126 
> Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
> Change-Id: I10be163ea10f6b4ca84e9f754f188117f58c7f17
> Reviewed-on: https://chromium-review.googlesource.com/748061
> Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
> Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#513352}

TBR=dpapad@chromium.org,dschuyler@chromium.org

Change-Id: I7af068abe09d9ecc5d0cbbc3a93038fef4fab88c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  764126 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Reviewed-on: https://chromium-review.googlesource.com/751882
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513561}
[modify] https://crrev.com/3187908e99f70ae440ee7656aed7abee5bcb4a2b/chrome/browser/resources/md_extensions/item_list.html

Blocking: -769402 780164
Performance of MD Extensions loading page was improved quite a bit already, (on my machine it is about 1.5-1.7x of the old Extensions UI). There are a few more improvements that have not landed yet, but I think it is OK to not consider performance as a Dev blocker anymore.
FYI, I just retried the "split modules" approach https://chromium-review.googlesource.com/c/chromium/src/+/747051, with latest build, and it is looking very promising, old and new FMP is at about 1.0x, see attached screencast.

Based on that, I think that we should most likely proceed with the "split modules" approach.

old_new_comparison.mp4
1.1 MB View Download
Project Member

Comment 30 by bugdroid1@chromium.org, Nov 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/162bc197a192766c43dab76bcd672b3ca90c8a23

commit 162bc197a192766c43dab76bcd672b3ca90c8a23
Author: dpapad <dpapad@chromium.org>
Date: Mon Nov 06 20:10:49 2017

MD Extensions: Remove dom-if around simple span in item.html.

<dom-if> is worth using when wrapping a non-trivial DOM subtree. Wrapping a
simple <span> with a <dom-if> is worse for performance than just rendering the
<span> and hiding with attribute |hidden|.

Bug:  764126 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Idd9751cc26a82c9cc6d47735ca3386aa9f1f2cf6
Reviewed-on: https://chromium-review.googlesource.com/753137
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514222}
[modify] https://crrev.com/162bc197a192766c43dab76bcd672b3ca90c8a23/chrome/browser/resources/md_extensions/item.html

Project Member

Comment 31 by bugdroid1@chromium.org, Nov 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3d47a3ce3b95de98021d1c82edf8c9108c625a08

commit 3d47a3ce3b95de98021d1c82edf8c9108c625a08
Author: dpapad <dpapad@chromium.org>
Date: Tue Nov 07 01:43:22 2017

MD Extensions: Wrap non-trivial DOM subtree with dom-if.

The subtree is usually non-visible for most cards, so it makes sense to hide it
behind a dom-if to optimize for the common case. Locally, this improves FMP time
by about 6% (with 30 extensions installed).

Bug:  764126 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: If220093d7d037bf92880ccbd7aa9a9f9921399c6
Reviewed-on: https://chromium-review.googlesource.com/753431
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514343}
[modify] https://crrev.com/3d47a3ce3b95de98021d1c82edf8c9108c625a08/chrome/browser/resources/md_extensions/item.html

Blockedon: 782514
Status: Fixed (was: Started)
Status update: The last optimization I had in mind was to split the app into a basic and a lazy-loaded module (similar to Settings and History).

The CL is at https://chromium-review.googlesource.com/c/chromium/src/+/747051. When I had first implemented this CL the improvement was very significant at about 36%. Now that other improvements have already landed, the improvement (on my Linux workstation) is about 8.6%, which is still not negligible, but not as much as before.

So, I'll keep the CL around, in case we discover a case where the improvement percentage is more significant (Windows? slower computers?), but for now marking this bug as Fixed.

Sign in to add a comment