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

Issue 775048 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Local NTP OGB: Don't re-load when tiles are reloaded

Project Member Reported by treib@chromium.org, Oct 16 2017

Issue description

In https://crrev.com/c/695302, I moved loading of the OGB to when we get the 'loaded' message from the MV iframe. Turns out we get that message whenever the tiles are reloaded, e.g. because the user blacklisted one. So now on every blacklisting, we load the OGB again, adding another set of script tags etc. Interestingly, this doesn't seem to cause any major problems other than burning some memory.
 

Comment 1 by treib@chromium.org, Oct 16 2017

Labels: -Type-Bug -M-64 M-63 Type-Bug-Regression

Comment 2 by fi...@chromium.org, Oct 17 2017

Labels: zine-triaged

Comment 3 by treib@chromium.org, Oct 17 2017

Status: Started (was: Assigned)
Project Member

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

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

commit f9974a8c5369686a01afbcc685351549d8202239
Author: Marc Treib <treib@chromium.org>
Date: Wed Oct 18 17:01:08 2017

Local NTP: Load the OGB only once

CL https://crrev.com/c/695302 moved loading the OGB to when we get the
'loaded' message from the MV iframe. Turns out we can get that multiple
times. So before loading the OGB (again), check that we haven't yet.

Bug:  775048 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I1888032f4227369f64b6c0c8c6ff6e0572b076fb
Reviewed-on: https://chromium-review.googlesource.com/722999
Reviewed-by: Chris Pickel <sfiera@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509796}
[modify] https://crrev.com/f9974a8c5369686a01afbcc685351549d8202239/chrome/browser/resources/local_ntp/local_ntp.js

Comment 5 by treib@chromium.org, Oct 19 2017

Status: Fixed (was: Started)
Fix verified in Canary 64.0.3244.0 (r509944)

Comment 6 by treib@chromium.org, Oct 19 2017

Labels: Merge-Request-63
Project Member

Comment 7 by sheriffbot@chromium.org, Oct 20 2017

Labels: -Merge-Request-63 Hotlist-Merge-Approved Merge-Approved-63
Your change meets the bar and is auto-approved for M63. Please go ahead and merge the CL to branch 3239 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

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

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aa22cda92a7e5ff4852d97c26cff474fdc16e406

commit aa22cda92a7e5ff4852d97c26cff474fdc16e406
Author: Marc Treib <treib@chromium.org>
Date: Fri Oct 20 08:59:41 2017

Local NTP: Load the OGB only once

CL https://crrev.com/c/695302 moved loading the OGB to when we get the
'loaded' message from the MV iframe. Turns out we can get that multiple
times. So before loading the OGB (again), check that we haven't yet.

Bug:  775048 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I1888032f4227369f64b6c0c8c6ff6e0572b076fb
Reviewed-on: https://chromium-review.googlesource.com/722999
Reviewed-by: Chris Pickel <sfiera@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509796}(cherry picked from commit f9974a8c5369686a01afbcc685351549d8202239)
Reviewed-on: https://chromium-review.googlesource.com/730323
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#104}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/aa22cda92a7e5ff4852d97c26cff474fdc16e406/chrome/browser/resources/local_ntp/local_ntp.js

Sign in to add a comment