Fix display:none issue for <webview>.

After we load a <webview>, setting its display to "none" removes the
plugin <object> from the render tree. If we set the display back to "",
we need to properly detect the case and allocate new instance id
and call attach to make things work.

The problem is briefly described here:
https://docs.google.com/a/chromium.org/document/d/1ITPOxS97DsG1HkbTAlpH0YcaTXKZNYUs84p1MppaHg0/view

BUG=399060
Test=Load a <webview> in a chrome app. See it render.
Set <webview>.style.display = 'none', see it go away.
Set <webview>.stlye.display = '', see it render properly.

Review URL: https://codereview.chromium.org/440463002

Cr-Commit-Position: refs/heads/master@{#291990}

chrome/browser/apps/web_view_browsertest.cc

@@ -98,17 +98,26 @@ class TestGuestViewManager : public extensions::GuestViewManager {
  public:
   explicit TestGuestViewManager(content::BrowserContext* context) :
       GuestViewManager(context),
+      seen_guest_removed_(false),
       web_contents_(NULL) {}
 
   content::WebContents* WaitForGuestCreated() {
     if (web_contents_)
       return web_contents_;
 
-    message_loop_runner_ = new content::MessageLoopRunner;
-    message_loop_runner_->Run();
+    created_message_loop_runner_ = new content::MessageLoopRunner;
+    created_message_loop_runner_->Run();
     return web_contents_;
   }
 
+  void WaitForGuestDeleted() {
+    if (seen_guest_removed_)
+      return;
+
+    deleted_message_loop_runner_ = new content::MessageLoopRunner;
+    deleted_message_loop_runner_->Run();
+  }
+
  private:
   // GuestViewManager override:
   virtual void AddGuest(int guest_instance_id,
@@ -116,13 +125,25 @@ class TestGuestViewManager : public extensions::GuestViewManager {
     extensions::GuestViewManager::AddGuest(
         guest_instance_id, guest_web_contents);
     web_contents_ = guest_web_contents;
+    seen_guest_removed_ = false;
 
-    if (message_loop_runner_.get())
-      message_loop_runner_->Quit();
+    if (created_message_loop_runner_.get())
+      created_message_loop_runner_->Quit();
+  }
+
+  virtual void RemoveGuest(int guest_instance_id) OVERRIDE {
+    extensions::GuestViewManager::RemoveGuest(guest_instance_id);
+    web_contents_ = NULL;
+    seen_guest_removed_ = true;
+
+    if (deleted_message_loop_runner_.get())
+      deleted_message_loop_runner_->Quit();
   }
 
+  bool seen_guest_removed_;
   content::WebContents* web_contents_;
-  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
+  scoped_refptr<content::MessageLoopRunner> created_message_loop_runner_;
+  scoped_refptr<content::MessageLoopRunner> deleted_message_loop_runner_;
 };
 
 // Test factory for creating test instances of GuestViewManager.
@@ -679,7 +700,6 @@ class WebViewTest : public extensions::PlatformAppBrowserTest {
   }
 
   void LoadAppWithGuest(const std::string& app_path) {
-
     ExtensionTestMessageListener launched_listener("WebViewTest.LAUNCHED",
                                                    false);
     launched_listener.set_failure_message("WebViewTest.FAILURE");
@@ -842,6 +862,33 @@ IN_PROC_BROWSER_TEST_F(WebViewTest, AutoSize) {
       << message_;
 }
 
+// Tests that a <webview> that is set to "display: none" after load and then
+// setting "display: block" re-renders the plugin properly.
+//
+// Initially after loading the <webview> and the test sets <webview> to
+// "display: none".
+// This causes the browser plugin to be destroyed, we then set the
+// style.display of the <webview> to block again and check that loadstop
+// fires properly.
+IN_PROC_BROWSER_TEST_F(WebViewTest, DisplayNoneAndBack) {
+  LoadAppWithGuest("web_view/display_none_and_back");
+
+  scoped_refptr<content::MessageLoopRunner> loop_runner(
+      new content::MessageLoopRunner);
+  WebContentsHiddenObserver observer(GetGuestWebContents(),
+                                     loop_runner->QuitClosure());
+
+  // Handled in platform_apps/web_view/display_none_and_back/main.js
+  SendMessageToEmbedder("hide-guest");
+  GetGuestViewManager()->WaitForGuestDeleted();
+  ExtensionTestMessageListener test_passed_listener("WebViewTest.PASSED",
+                                                    false);
+
+  SendMessageToEmbedder("show-guest");
+  GetGuestViewManager()->WaitForGuestCreated();
+  EXPECT_TRUE(test_passed_listener.WaitUntilSatisfied());
+}
+
 // http://crbug.com/326332
 IN_PROC_BROWSER_TEST_F(WebViewTest, DISABLED_Shim_TestAutosizeAfterNavigation) {
   TestHelper("testAutosizeAfterNavigation", "web_view/shim", NO_TEST_SERVER);

chrome/renderer/resources/extensions/web_view.js

@@ -106,7 +106,7 @@ function WebViewInternal(webviewNode) {
 
   this.browserPluginNode = this.createBrowserPluginNode();
   var shadowRoot = this.webviewNode.createShadowRoot();
-  shadowRoot.appendChild(this.browserPluginNode);
+  this.partition = new Partition();
 
   this.setupWebviewNodeAttributes();
   this.setupFocusPropagation();
@@ -114,10 +114,9 @@ function WebViewInternal(webviewNode) {
 
   this.viewInstanceId = IdGenerator.GetNextId();
 
-  this.partition = new Partition();
-  this.parseAttributes();
-
   new WebViewEvents(this, this.viewInstanceId);
+
+  shadowRoot.appendChild(this.browserPluginNode);
 }
 
 /**
@@ -563,8 +562,13 @@ WebViewInternal.prototype.handleBrowserPluginAttributeMutation =
   if (name == 'internalinstanceid' && !oldValue && !!newValue) {
     this.browserPluginNode.removeAttribute('internalinstanceid');
     this.internalInstanceId = parseInt(newValue);
-    if (this.deferredAttachState && !!this.guestInstanceId &&
-            this.guestInstanceId != 0) {
+
+    if (!this.deferredAttachState) {
+      this.parseAttributes();
+      return;
+    }
+
+    if (!!this.guestInstanceId && this.guestInstanceId != 0) {
       window.setTimeout(function() {
         var isNewWindow = this.deferredAttachState ?
             this.deferredAttachState.isNewWindow : false;
@@ -575,6 +579,7 @@ WebViewInternal.prototype.handleBrowserPluginAttributeMutation =
             params);
       }.bind(this), 0);
     }
+
     return;
   }
 
@@ -737,6 +742,10 @@ WebViewInternal.prototype.onFrameNameChanged = function(name) {
   }
 };
 
+WebViewInternal.prototype.onPluginDestroyed = function() {
+  this.reset();
+};
+
 WebViewInternal.prototype.dispatchEvent = function(webViewEvent) {
   return this.webviewNode.dispatchEvent(webViewEvent);
 };

chrome/renderer/resources/extensions/web_view_events.js

@@ -20,6 +20,7 @@ var CreateEvent = function(name) {
 };
 
 var FrameNameChangedEvent = CreateEvent('webViewInternal.onFrameNameChanged');
+var PluginDestroyedEvent = CreateEvent('webViewInternal.onPluginDestroyed');
 var WebRequestMessageEvent = CreateEvent('webViewInternal.onMessage');
 
 // WEB_VIEW_EVENTS is a map of stable <webview> DOM event names to their
@@ -192,6 +193,7 @@ function WebViewEvents(webViewInternal, viewInstanceId) {
 // Sets up events.
 WebViewEvents.prototype.setup = function() {
   this.setupFrameNameChangedEvent();
+  this.setupPluginDestroyedEvent();
   this.setupWebRequestEvents();
   this.webViewInternal.setupExperimentalContextMenus();
 
@@ -202,10 +204,15 @@ WebViewEvents.prototype.setup = function() {
 };
 
 WebViewEvents.prototype.setupFrameNameChangedEvent = function() {
-  var self = this;
   FrameNameChangedEvent.addListener(function(e) {
-    self.webViewInternal.onFrameNameChanged(e.name);
-  }, {instanceId: self.viewInstanceId});
+    this.webViewInternal.onFrameNameChanged(e.name);
+  }.bind(this), {instanceId: this.viewInstanceId});
+};
+
+WebViewEvents.prototype.setupPluginDestroyedEvent = function() {
+  PluginDestroyedEvent.addListener(function(e) {
+    this.webViewInternal.onPluginDestroyed();
+  }.bind(this), {instanceId: this.viewInstanceId});
 };
 
 WebViewEvents.prototype.setupWebRequestEvents = function() {

chrome/test/data/extensions/platform_apps/web_view/display_none_and_back/main.html

+<!doctype html>
+<!--
+ * Copyright 2014 The Chromium Authors. All rights reserved.  Use of this
+ * source code is governed by a BSD-style license that can be found in the
+ * LICENSE file.
+-->
+<html>
+<body>
+  <div id="webview-tag-container"></div>
+  <script src="main.js"></script>
+</body>
+</html>

chrome/test/data/extensions/platform_apps/web_view/display_none_and_back/main.js

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+var LOG = function(msg) {
+  window.console.log(msg);
+};
+
+var failTest = function() {
+  chrome.test.sendMessage('WebViewTest.FAILURE');
+};
+
+var firstTime = true;
+
+var startTest = function() {
+  document.querySelector('#webview-tag-container').innerHTML =
+      '<webview style="width: 10px; height: 10px; margin: 0; padding: 0;"' +
+      '></webview>';
+
+  var webview = document.querySelector('webview');
+  var onLoadStop = function(e) {
+    if (firstTime) {
+      firstTime = false;
+      chrome.test.sendMessage('WebViewTest.LAUNCHED');
+    } else {
+      chrome.test.sendMessage('WebViewTest.PASSED');
+    }
+  };
+
+  webview.addEventListener('loadstop', onLoadStop);
+  webview.src = 'data:text/html,<body>Guest</body>';
+};
+
+window.onAppCommand = function(command) {
+  LOG('onAppCommand: ' + command);
+  switch (command) {
+    case 'hide-guest':
+      document.querySelector('webview').style.display = 'none';
+      break;
+    case 'show-guest':
+      document.querySelector('webview').style.display = '';
+      break;
+    default:
+      failTest();
+      break;
+  }
+};
+
+window.onload = startTest;

chrome/test/data/extensions/platform_apps/web_view/display_none_and_back/manifest.json

+{
+  "name": "<webview> check display: none toggling.",
+  "version": "1",
+  "permissions": [
+    "webview"
+  ],
+  "app": {
+    "background": {
+      "scripts": ["test.js"]
+    }
+  }
+}

chrome/test/data/extensions/platform_apps/web_view/display_none_and_back/test.js

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+chrome.app.runtime.onLaunched.addListener(function() {
+  chrome.app.window.create('main.html', {}, function () {});
+});

extensions/browser/guest_view/guest_view_manager.h

@@ -94,7 +94,8 @@ class GuestViewManager : public content::BrowserPluginGuestManager,
   virtual void AddGuest(int guest_instance_id,
                         content::WebContents* guest_web_contents);
 
-  void RemoveGuest(int guest_instance_id);
+  // Can be overriden in tests.
+  virtual void RemoveGuest(int guest_instance_id);
 
   content::WebContents* GetGuestByInstanceID(int guest_instance_id);
 

extensions/browser/guest_view/web_view/web_view_constants.cc

@@ -34,6 +34,7 @@ const char kEventLoadStop[] = "webViewInternal.onLoadStop";
 const char kEventMessage[] = "webViewInternal.onMessage";
 const char kEventNewWindow[] = "webViewInternal.onNewWindow";
 const char kEventPermissionRequest[] = "webViewInternal.onPermissionRequest";
+const char kEventPluginDestroyed[] = "webViewInternal.onPluginDestroyed";
 const char kEventResponsive[] = "webViewInternal.onResponsive";
 const char kEventSizeChanged[] = "webViewInternal.onSizeChanged";
 const char kEventUnresponsive[] = "webViewInternal.onUnresponsive";

extensions/browser/guest_view/web_view/web_view_constants.h

@@ -38,6 +38,7 @@ extern const char kEventLoadStop[];
 extern const char kEventMessage[];
 extern const char kEventNewWindow[];
 extern const char kEventPermissionRequest[];
+extern const char kEventPluginDestroyed[];
 extern const char kEventResponsive[];
 extern const char kEventSizeChanged[];
 extern const char kEventUnresponsive[];

extensions/browser/guest_view/web_view/web_view_guest.cc

@@ -351,6 +351,10 @@ void WebViewGuest::WillDestroy() {
   if (!attached() && GetOpener())
     GetOpener()->pending_new_windows_.erase(this);
   DestroyUnattachedWindows();
+
+  scoped_ptr<base::DictionaryValue> args(new base::DictionaryValue());
+  DispatchEventToEmbedder(
+      new GuestViewBase::Event(webview::kEventPluginDestroyed, args.Pass()));
 }
 
 bool WebViewGuest::AddMessageToConsole(WebContents* source,