Commit a56e7000 authored by sreeram@chromium.org's avatar sreeram@chromium.org

Remove observer when removing delegate, in Instant.

I believe the crash (see bug link) is caused because we remove the delegate
connection, but we never stop observing the WebContents. I haven't reproduced
the crash, but I think this should fix the root cause. I've put in a bandaid as
well, which might be redundant, but good for safety. Will remove it once I can
confirm the fix works.

BUG=141875
R=sky@chromium.org
TEST=This type of crash (see bug) should go away.

Review URL: https://chromiumcodereview.appspot.com/10829284

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@151173 0039d316-1c4b-4281-b951-d872f2087c98
parent c76f58ba
...@@ -200,6 +200,13 @@ void InstantLoader::WebContentsDelegateImpl::OnSetSuggestions( ...@@ -200,6 +200,13 @@ void InstantLoader::WebContentsDelegateImpl::OnSetSuggestions(
int page_id, int page_id,
const std::vector<string16>& suggestions, const std::vector<string16>& suggestions,
InstantCompleteBehavior behavior) { InstantCompleteBehavior behavior) {
DCHECK(loader_->preview_contents() &&
loader_->preview_contents_->web_contents());
// TODO(sreeram): Remove this 'if' bandaid once bug 141875 is confirmed fixed.
if (!loader_->preview_contents() ||
!loader_->preview_contents_->web_contents()) {
return;
}
content::NavigationEntry* entry = loader_->preview_contents_->web_contents()-> content::NavigationEntry* entry = loader_->preview_contents_->web_contents()->
GetController().GetActiveEntry(); GetController().GetActiveEntry();
if (entry && page_id == entry->GetPageID()) { if (entry && page_id == entry->GetPageID()) {
...@@ -211,6 +218,13 @@ void InstantLoader::WebContentsDelegateImpl::OnSetSuggestions( ...@@ -211,6 +218,13 @@ void InstantLoader::WebContentsDelegateImpl::OnSetSuggestions(
void InstantLoader::WebContentsDelegateImpl::OnInstantSupportDetermined( void InstantLoader::WebContentsDelegateImpl::OnInstantSupportDetermined(
int page_id, int page_id,
bool result) { bool result) {
DCHECK(loader_->preview_contents() &&
loader_->preview_contents_->web_contents());
// TODO(sreeram): Remove this 'if' bandaid once bug 141875 is confirmed fixed.
if (!loader_->preview_contents() ||
!loader_->preview_contents_->web_contents()) {
return;
}
content::NavigationEntry* entry = loader_->preview_contents_->web_contents()-> content::NavigationEntry* entry = loader_->preview_contents_->web_contents()->
GetController().GetActiveEntry(); GetController().GetActiveEntry();
if (entry && page_id == entry->GetPageID()) if (entry && page_id == entry->GetPageID())
...@@ -232,15 +246,17 @@ void InstantLoader::WebContentsDelegateImpl ...@@ -232,15 +246,17 @@ void InstantLoader::WebContentsDelegateImpl
if (loader_->supports_instant_) if (loader_->supports_instant_)
return; return;
loader_->supports_instant_ = supports_instant;
loader_->loader_delegate_->InstantSupportDetermined(loader_,
supports_instant);
// If the page doesn't support the Instant API, InstantController schedules // If the page doesn't support the Instant API, InstantController schedules
// the loader for destruction. Stop sending the controller any more messages, // the loader for destruction. Stop sending the controller any more messages,
// by severing the connection from the WebContents to us (its delegate). // by severing the connection from the WebContents to us (its delegate).
if (!supports_instant) if (!supports_instant) {
loader_->preview_contents_->web_contents()->SetDelegate(NULL); loader_->preview_contents_->web_contents()->SetDelegate(NULL);
Observe(NULL);
}
loader_->supports_instant_ = supports_instant;
loader_->loader_delegate_->InstantSupportDetermined(loader_,
supports_instant);
} }
// InstantLoader --------------------------------------------------------------- // InstantLoader ---------------------------------------------------------------
...@@ -254,8 +270,6 @@ InstantLoader::InstantLoader(InstantLoaderDelegate* delegate, ...@@ -254,8 +270,6 @@ InstantLoader::InstantLoader(InstantLoaderDelegate* delegate,
tab_contents->web_contents(), tab_contents->web_contents(),
tab_contents->web_contents()->GetController(). tab_contents->web_contents()->GetController().
GetSessionStorageNamespace()))), GetSessionStorageNamespace()))),
preview_delegate_(new WebContentsDelegateImpl(
ALLOW_THIS_IN_INITIALIZER_LIST(this))),
supports_instant_(false), supports_instant_(false),
instant_url_(instant_url) { instant_url_(instant_url) {
} }
...@@ -323,6 +337,7 @@ void InstantLoader::Observe(int type, ...@@ -323,6 +337,7 @@ void InstantLoader::Observe(int type,
void InstantLoader::SetupPreviewContents() { void InstantLoader::SetupPreviewContents() {
content::WebContents* new_contents = preview_contents_->web_contents(); content::WebContents* new_contents = preview_contents_->web_contents();
preview_delegate_.reset(new WebContentsDelegateImpl(this));
WebContentsDelegateImpl* new_delegate = preview_delegate_.get(); WebContentsDelegateImpl* new_delegate = preview_delegate_.get();
new_contents->SetDelegate(new_delegate); new_contents->SetDelegate(new_delegate);
...@@ -353,6 +368,7 @@ void InstantLoader::SetupPreviewContents() { ...@@ -353,6 +368,7 @@ void InstantLoader::SetupPreviewContents() {
void InstantLoader::CleanupPreviewContents() { void InstantLoader::CleanupPreviewContents() {
content::WebContents* old_contents = preview_contents_->web_contents(); content::WebContents* old_contents = preview_contents_->web_contents();
old_contents->SetDelegate(NULL); old_contents->SetDelegate(NULL);
preview_delegate_.reset();
preview_contents_->blocked_content_tab_helper()->SetAllContentsBlocked(false); preview_contents_->blocked_content_tab_helper()->SetAllContentsBlocked(false);
preview_contents_->constrained_window_tab_helper()->set_delegate(NULL); preview_contents_->constrained_window_tab_helper()->set_delegate(NULL);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment