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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week ago by msramek
Modified:
5 days, 4 hours ago
Reviewers:
Dan Beam
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 ...
1 week ago (2017-05-17 12:06:49 UTC) #6
Dan Beam
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 days, 23 hours 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 days, 2 hours ago (2017-05-18 15:16:29 UTC) #8
Dan Beam
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') ...
5 days, 16 hours 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: ...
5 days, 7 hours 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
5 days, 7 hours 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)
5 days, 5 hours 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
5 days, 5 hours ago (2017-05-19 12:03:29 UTC) #17
commit-bot: I haz the power
5 days, 4 hours 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 650457f06