Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(9)

Issue 2888103002: Speed up the Incognito NTP reload (Closed)

Created:
6 months ago by msramek
Modified:
6 months ago
CC:
chromium-reviews, dbeam+watch-ntp_chromium.org, arv+watch_chromium.org, pedrosimonetti+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Speed up the Incognito NTP reload As described in crbug.com/715042, the new Incognito NTP sometimes flickers on reload. This is likely caused by the fact that it uses JavaScript when computing layout, including two relatively heavy offsetWidth operations which cause a reflow. However, offsetWidth is constant for a given default font size, and only needs to be computed once. It can be then stored in localStorage for the duration of the Incognito session. Testing on my local debug build shows that the JavaScript execution is now sped up from ~22ms to ~5ms. BUG=715042 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2888103002 Cr-Commit-Position: refs/heads/master@{#473176} Committed: https://chromium.googlesource.com/chromium/src/+/24526e022f84a2a625012160b818eef0ebd0b8de

Patch Set 1 #

Total comments: 8

Patch Set 2 : Fixed flickering. #

Total comments: 3

Patch Set 3 : Use the fontSize attribute directly. #

Messages

Total messages: 20 (11 generated)
msramek
Hi Dan! Since you reviewed my previous Incognito NTP CLs, please have a look at ...
6 months ago (2017-05-17 12:06:49 UTC) #6
Dan Beam (no longer on Chrome)
https://codereview.chromium.org/2888103002/diff/20001/chrome/browser/resources/ntp4/md_incognito_tab.js File chrome/browser/resources/ntp4/md_incognito_tab.js (right): https://codereview.chromium.org/2888103002/diff/20001/chrome/browser/resources/ntp4/md_incognito_tab.js#newcode30 chrome/browser/resources/ntp4/md_incognito_tab.js:30: document.defaultView.getComputedStyle(document.body, null) i don't really see a difference between ...
6 months ago (2017-05-17 18:09:04 UTC) #7
msramek
https://codereview.chromium.org/2888103002/diff/20001/chrome/browser/resources/ntp4/md_incognito_tab.js File chrome/browser/resources/ntp4/md_incognito_tab.js (right): https://codereview.chromium.org/2888103002/diff/20001/chrome/browser/resources/ntp4/md_incognito_tab.js#newcode30 chrome/browser/resources/ntp4/md_incognito_tab.js:30: document.defaultView.getComputedStyle(document.body, null) On 2017/05/17 18:09:04, Dan Beam wrote: > ...
6 months ago (2017-05-18 15:16:29 UTC) #8
Dan Beam (no longer on Chrome)
lgtm https://codereview.chromium.org/2888103002/diff/40001/chrome/browser/resources/ntp4/md_incognito_tab.js File chrome/browser/resources/ntp4/md_incognito_tab.js (right): https://codereview.chromium.org/2888103002/diff/40001/chrome/browser/resources/ntp4/md_incognito_tab.js#newcode30 chrome/browser/resources/ntp4/md_incognito_tab.js:30: window.getComputedStyle(document.body).getPropertyValue("font-size"); don't really get why you prefer getPropertyValue('font-size') ...
6 months ago (2017-05-19 01:09:33 UTC) #9
msramek
Thanks, Dan! https://codereview.chromium.org/2888103002/diff/40001/chrome/browser/resources/ntp4/md_incognito_tab.js File chrome/browser/resources/ntp4/md_incognito_tab.js (right): https://codereview.chromium.org/2888103002/diff/40001/chrome/browser/resources/ntp4/md_incognito_tab.js#newcode30 chrome/browser/resources/ntp4/md_incognito_tab.js:30: window.getComputedStyle(document.body).getPropertyValue("font-size"); On 2017/05/19 01:09:32, Dan Beam wrote: ...
6 months ago (2017-05-19 09:50:59 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2888103002/60001
6 months ago (2017-05-19 09:51:25 UTC) #13
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/449749)
6 months ago (2017-05-19 12:01:13 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2888103002/60001
6 months ago (2017-05-19 12:03:29 UTC) #17
commit-bot: I haz the power
6 months ago (2017-05-19 13:27:53 UTC) #20
Message was sent while issue was closed.
Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/24526e022f84a2a625012160b818...

Powered by Google App Engine
This is Rietveld efc10ee0f