Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(856)

Issue 69813005: Fix crash in WebContentsViewAura::WindowObserver that happens when switching tabs. The exact repro … (Closed)

Created:
7 years, 1 month ago by jam
Modified:
7 years, 1 month ago
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org
Visibility:
Public.

Description

Fix crash in WebContentsViewAura::WindowObserver that happens when switching tabs. The exact repro steps were: 1) open an empty page 2) print preview 3) open a new tab 4) go back to original tab 5) close print preview 6) switch to the new tab The crash happened at step 4 because the second tab got removed from the same root window as the print preview's observer. We weren't unobserving all the children of the root window at that point. BUG=318791 R=ben@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=235267 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=235302

Patch Set 1 #

Patch Set 2 : upload after revert #

Patch Set 3 : fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -0 lines) Patch
M chrome/browser/ui/webui/print_preview/print_preview_ui_browsertest.cc View 1 1 chunk +16 lines, -0 lines 0 comments Download
M content/browser/web_contents/web_contents_view_aura.cc View 1 2 1 chunk +7 lines, -0 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
jam
7 years, 1 month ago (2013-11-14 23:47:50 UTC) #1
Ben Goodger (Google)
lgtm
7 years, 1 month ago (2013-11-15 00:09:55 UTC) #2
agable
What was the cause of the revert (https://codereview.chromium.org/63013004) and this reland? Wondering because XP Tests ...
7 years, 1 month ago (2013-11-15 18:21:45 UTC) #3
jam
On 2013/11/15 18:21:45, Aaron Gable wrote: > What was the cause of the revert (https://codereview.chromium.org/63013004) ...
7 years, 1 month ago (2013-11-15 18:35:49 UTC) #4
jam
7 years, 1 month ago (2013-11-15 18:36:49 UTC) #5
Message was sent while issue was closed.
On 2013/11/15 18:35:49, jam wrote:
> On 2013/11/15 18:21:45, Aaron Gable wrote:
> > What was the cause of the revert (https://codereview.chromium.org/63013004)
> and
> > this reland?
> 
> I reverted because I committed the change by accident (I hadn't even run
> trybots, I mixed it up with another cl).
> 
> > Wondering because XP Tests (dbg)(4) is now failing three tests in
> > PrintPreviewWebUITest
> >
>
(http://build.chromium.org/p/chromium.win/builders/XP%252520Tests%252520%25252...)
> > and this looks like a possible culprit, but I don't see any explanation for
> why
> > it was reverted the first time.
> 
> These don't look related to me. The crash that this fixes was triggered by by
> any dialog, I chose print preview because it's easy to test. if this change
was
> causing a problem, it would be a crash. looks like the failure on the xp bots
> are related to 1) dchecks firing and 2) timeouts

also, this change relanded 10 hours ago and there have been many green runs of
those xp bots since then...

Powered by Google App Engine
This is Rietveld 408576698