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 <webview> 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 <webview>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

3 files changed, 26 insertions, 5 deletions

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 <webview>'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 <webview>'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);