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

Issue 1089123002: Fix an NPE in BrowserPluginGuest during guest teardown while showing interstitial. (Closed)

Created:
5 years, 8 months ago by lazyboy
Modified:
5 years, 8 months ago
Reviewers:
Fady Samuel
CC:
chromium-reviews, darin-cc_chromium.org, jam
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix an NPE in BrowserPluginGuest during guest teardown while showing interstitial. r324002 introduced a code path where we call BrowserPluginGuest::SendTextInputTypeChangedToView() during shutdown via BrowserPluginGuest::SetFocus(). This happens when guest is showing interstitial page. There are couple of ways this can happen, either when the embedder page closes or the embedder page navigates to a different page. GuestViewBase::OwnerContentsObserver detects these two cases and calls BrowserPluginGuest shutdown path. During that time if guest was showing an interstitial, then it closes the interstitial which in that case would call: content::InterstitialPageImpl::DontProceed() / content::InterstitialPageImpl::Hide() Hide() ends up calling BrowserPluginGuest::SetFocus() and we hit the NPE. BUG=477126, 477132 Test=Navigate a tab chrome signin page, after changing the system clock to few years back, observe that it is shows interstitial. Either of the following reproduces the behavior: a) close the tab b) navigate the tab to some webpage, e.g. http://test.com Committed: https://crrev.com/8664beb06cdff77b4f02154d47bd1f762d2b895a Cr-Commit-Position: refs/heads/master@{#325193}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -0 lines) Patch
M content/browser/browser_plugin/browser_plugin_guest.cc View 1 chunk +12 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
lazyboy
5 years, 8 months ago (2015-04-15 01:55:41 UTC) #2
Fady Samuel
lgtm
5 years, 8 months ago (2015-04-15 02:11:10 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1089123002/1
5 years, 8 months ago (2015-04-15 03:48:37 UTC) #5
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 8 months ago (2015-04-15 06:12:47 UTC) #6
commit-bot: I haz the power
5 years, 8 months ago (2015-04-15 06:14:38 UTC) #7
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/8664beb06cdff77b4f02154d47bd1f762d2b895a
Cr-Commit-Position: refs/heads/master@{#325193}

Powered by Google App Engine
This is Rietveld 408576698