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

Issue 683021 link

Starred by 2 users

Issue metadata

Status: Fixed
Merged: issue 683794
Owner:
Last visit > 30 days ago
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Theme doesn't retain on reloading killed NTP

Project Member Reported by sc00335...@techmahindra.com, Jan 20 2017

Issue description

Chrome Version: 57.0.2987.0 dev
OS: Ubuntu 14.04,Windows

What steps will reproduce the problem?
(1)Launch chrome and add any theme >> Open 2 or more NTPs >> Use chrome://kill in any page
(2)Now go to all NTPs and reload[F5] page and observe. 

Expected: Added theme should retain on reloading killed new tab page.
Actual: Instead NTP with blank theme is seen.

This is a regression issue broken in M57. Will provide bisect info soon.
 
Expected_themes.ogv
2.4 MB View Download
Labels: OS-Mac
Status: Untriaged (was: Unconfirmed)
Able to reproduce this issue on Mac OS 10.12 using chrome latest Dev M57-57.0.2987.0
Labels: ReleaseBlock-Stable
Manual Bisect Info:
===================

Good Build: 57.0.2983.0 dev
Bad Build: 57.0.2984.0 dev

Comment 3 by hdodda@chromium.org, Jan 20 2017

Cc: hdodda@chromium.org
Labels: -Needs-Bisect hasbisect-per-revision
Owner: tibell@chromium.org
Status: Assigned (was: Untriaged)
Using the per-revision bisect providing the bisect results,
Good Build: 57.0.2983.0 dev (revision : 443819)
Bad Build: 57.0.2984.0 dev (revision : 443964)

You are probably looking for a change made after 443837 (known good), but no later than 443838 (first known bad).

CHANGELOG URL:

The script might not always return single CL as suspect as some perf builds might get missing due to failure.

 https://chromium.googlesource.com/chromium/src/+log/dc7d397153bcbccbc21e12f2ea3568f594699220..e3df8cd8ac45539711b1f0b6a00420f87d09d279

From the CL above, assigning the issue to the concern owner 

@tibell - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Review-Url: https://codereview.chromium.org/2086223002

Thanks!

Comment 4 by treib@chromium.org, Jan 20 2017

I can reproduce this. It's not just the theme that is missing; the whole NTP is not functional, e.g. the Most Visited tiles are not populated, and the search box doesn't do anything. It looks like none of the "chrome.embeddedSearch.*" APIs are hooked up.
tibell, any idea what might be going wrong?

Comment 5 by tibell@chromium.org, Jan 22 2017

Status: Started (was: Assigned)
Not sure what's going on. Most likely due to Mojoification. Will try to see if I can fix it or if we need to roll back the CL.

Comment 6 by tibell@chromium.org, Jan 23 2017

Blockedon: 683794

Comment 7 by tibell@chromium.org, Jan 23 2017

This was due a bug in how Mojo was set up in RPH. Fix out for review: http://crrev.com/2652573002.

Comment 8 by tibell@chromium.org, Jan 23 2017

Blockedon: -683794
Mergedinto: 683794
Status: Duplicate (was: Started)

Comment 9 by tibell@chromium.org, Jan 23 2017

Blockedon: 683794
Blockedon: -683794
Status: Assigned (was: Duplicate)
This still reproduces on a current ToT build (r447510).
Curious. I'm on a business trip and will be back in the office on Tuesday next week. Is that enough time to fix this or will we have to roll back and reland for next release?
Well, depends on how long it takes to find and fix the problem ;)

M57 goes to Beta any day now, but we can live with this on Beta for a bit. It goes to Stable on March 14; we should have it fixed *well* before that. I'd say anything within the next week or two will be okay; longer than that and we should probably revert.

I also really hope that this is the last problem with the Mojoification. Test coverage is obviously awful (i.e. non-existent) :(
Labels: zine-triaged
Cc: ranjitkan@chromium.org kkaluri@chromium.org treib@chromium.org nyerramilli@chromium.org msrchandra@chromium.org tibell@chromium.org
 Issue 687855  has been merged into this issue.
Status: Started (was: Assigned)
Project Member

Comment 17 by bugdroid1@chromium.org, Feb 7 2017

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

commit 3c36d59b4bd011c2bbbf28d59275aa082ede1ff5
Author: tibell <tibell@chromium.org>
Date: Tue Feb 07 02:30:37 2017

SearchIPCRouter: Invalidate cached connection ID on renderer crash

BUG= 683021 

Review-Url: https://codereview.chromium.org/2677013004
Cr-Commit-Position: refs/heads/master@{#448519}

[modify] https://crrev.com/3c36d59b4bd011c2bbbf28d59275aa082ede1ff5/chrome/browser/ui/search/search_ipc_router.cc

Labels: Merge-Request-57
Project Member

Comment 19 by sheriffbot@chromium.org, Feb 8 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

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

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

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a5cc619c21b1c41688f2d475dff336e54d47dc11

commit a5cc619c21b1c41688f2d475dff336e54d47dc11
Author: Sam McNally <sammc@chromium.org>
Date: Wed Feb 08 02:37:01 2017

SearchIPCRouter: Invalidate cached connection ID on renderer crash

BUG= 683021 

Review-Url: https://codereview.chromium.org/2677013004
Cr-Commit-Position: refs/heads/master@{#448519}
(cherry picked from commit 3c36d59b4bd011c2bbbf28d59275aa082ede1ff5)

Review-Url: https://codereview.chromium.org/2683943002 .
Cr-Commit-Position: refs/branch-heads/2987@{#378}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/a5cc619c21b1c41688f2d475dff336e54d47dc11/chrome/browser/ui/search/search_ipc_router.cc

Status: Fixed (was: Started)
Labels: TE-Verified-57.0.2987.54 TE-Verified-M57
Rechecked this issue on chrome version 57.0.2987.54 on Windows 10, MAC 10.12.2, Ubuntu 14.04. Fix is working as intended. Theme is retained on reloading after killed a new tab page.

Adding TE-Verified labels.

Sign in to add a comment