From 221c4beb0789c9c18f18de0c7383ed9272142671 Mon Sep 17 00:00:00 2001 From: "fsamuel@chromium.org" Date: Wed, 13 Feb 2013 19:10:51 +0000 Subject: Browser Plugin: Navigating to the same URL after crash should load a new guest This patch contains a few other small changes in behavior: 1. Removing the src attribute after navigation in will immediately restore it. 2. browserPlugin.terminate() will not issue an IPC as long as the BrowserPlugin is in a crashed state. 3. WebViewTest.Shim was misbehaving because it had lingering s in the DOM all in the same partition and so terminating one webview terminated them all, and caused multiple exit listeners to fire. This was fixed. 4. Fixed a potential infinite loop that could happen if the src attribute is modified while in a crashed state. BUG=175206 Test=WebViewTest.Shim: webViewAssignSrcAfterCrash, webViewRemoveSrcAttribute Review URL: https://chromiumcodereview.appspot.com/12224094 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@182278 0039d316-1c4b-4281-b951-d872f2087c98 --- content/renderer/browser_plugin/browser_plugin.cc | 12 ++++++++---- content/renderer/browser_plugin/browser_plugin.h | 2 ++ .../renderer/browser_plugin/browser_plugin_bindings.cc | 17 ++++++++++++++++- 3 files changed, 26 insertions(+), 5 deletions(-) (limited to 'content') diff --git a/content/renderer/browser_plugin/browser_plugin.cc b/content/renderer/browser_plugin/browser_plugin.cc index 3a5c1e8..99747c6 100644 --- a/content/renderer/browser_plugin/browser_plugin.cc +++ b/content/renderer/browser_plugin/browser_plugin.cc @@ -413,6 +413,11 @@ void BrowserPlugin::OnGuestContentWindowReady(int instance_id, } void BrowserPlugin::OnGuestGone(int instance_id, int process_id, int status) { + // Set the BrowserPlugin in a crashed state before firing event listeners so + // that operations on the BrowserPlugin within listeners are aware that + // BrowserPlugin is in a crashed state. + guest_crashed_ = true; + // We fire the event listeners before painting the sad graphic to give the // developer an opportunity to display an alternative overlay image on crash. std::string termination_status = TerminationStatusToString( @@ -427,7 +432,6 @@ void BrowserPlugin::OnGuestGone(int instance_id, int process_id, int status) { // but leave other member variables valid below. TriggerEvent(browser_plugin::kEventExit, &props); - guest_crashed_ = true; // We won't paint the contents of the current backing store again so we might // as well toss it out and save memory. backing_store_.reset(); @@ -468,9 +472,9 @@ void BrowserPlugin::OnLoadCommit( // If the guest has just committed a new navigation then it is no longer // crashed. guest_crashed_ = false; - if (params.is_top_level) { + if (params.is_top_level) UpdateDOMAttribute(browser_plugin::kAttributeSrc, params.url.spec()); - } + guest_process_id_ = params.process_id; guest_route_id_ = params.route_id; current_nav_entry_index_ = params.current_entry_index; @@ -797,7 +801,7 @@ void BrowserPlugin::Go(int relative_index) { } void BrowserPlugin::TerminateGuest() { - if (!navigate_src_sent_) + if (!navigate_src_sent_ || guest_crashed_) return; browser_plugin_manager()->Send( new BrowserPluginHostMsg_TerminateGuest(render_view_routing_id_, diff --git a/content/renderer/browser_plugin/browser_plugin.h b/content/renderer/browser_plugin/browser_plugin.h index 17dcdcf..3e63f11 100644 --- a/content/renderer/browser_plugin/browser_plugin.h +++ b/content/renderer/browser_plugin/browser_plugin.h @@ -89,6 +89,8 @@ class CONTENT_EXPORT BrowserPlugin : int guest_process_id() const { return guest_process_id_; } // Returns Chrome's route ID for the current guest. int guest_route_id() const { return guest_route_id_; } + // Returns whether the guest process has crashed. + bool guest_crashed() const { return guest_crashed_; } // Query whether the guest can navigate back to the previous entry. bool CanGoBack() const; diff --git a/content/renderer/browser_plugin/browser_plugin_bindings.cc b/content/renderer/browser_plugin/browser_plugin_bindings.cc index 142adc9..bfeaa8b 100644 --- a/content/renderer/browser_plugin/browser_plugin_bindings.cc +++ b/content/renderer/browser_plugin/browser_plugin_bindings.cc @@ -709,7 +709,18 @@ class BrowserPluginPropertyBindingSrc : public BrowserPluginPropertyBinding { const NPVariant* variant) OVERRIDE { std::string new_value = StringFromNPVariant(*variant); std::string old_value = bindings->instance()->GetSrcAttribute(); - if (old_value != new_value && !new_value.empty()) { + bool guest_crashed = bindings->instance()->guest_crashed(); + if ((old_value != new_value) || guest_crashed) { + if (new_value.empty()) { + // Remove the DOM attribute to trigger 's mutation observer + // when it is restored to its original value again. + bindings->instance()->RemoveDOMAttribute(name()); + new_value = old_value; + } + // If the new value was empty then we're effectively resetting the + // attribute to the old value here. This will be picked up by 's + // mutation observer and will restore the src attribute after it has been + // removed. UpdateDOMAttribute(bindings, new_value); std::string error_message; if (!bindings->instance()->ParseSrcAttribute(&error_message)) { @@ -724,7 +735,11 @@ class BrowserPluginPropertyBindingSrc : public BrowserPluginPropertyBinding { } virtual void RemoveProperty(BrowserPluginBindings* bindings, NPObject* np_obj) OVERRIDE { + std::string old_value = bindings->instance()->GetSrcAttribute(); + // Remove the DOM attribute to trigger the mutation observer when it is + // restored to its original value again. bindings->instance()->RemoveDOMAttribute(name()); + UpdateDOMAttribute(bindings, old_value); } private: DISALLOW_COPY_AND_ASSIGN(BrowserPluginPropertyBindingSrc); -- cgit v1.1