<webview>: Fix ptr access crash on browser plugin + interstitial teardown.

If browser plugin loaded an interstitial page as view contents
(e.g. loading plugin resulted in ssl-error), upon closing the
containing app, the interstitial would try to show back
(RenderWidgetHostView::WasShown) the original contents of the
RWHViewGuest, whose render_view_host_ was already in the middle of
destruction. We record the destruction phase and avoid calling methods
on RenderViewHost in this case.

BUG=270313
TEST=chrome app <webview>, load a guest pointing a page that results in
ssl-error page showing (Proceed to safety page, etc), close the app. The app
would crash without this change.
Added regression test.

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@218092 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 207 insertions, 14 deletions

diff --git a/chrome/browser/apps/web_view_browsertest.cc b/chrome/browser/apps/web_view_browsertest.cc
index b16d3bd..1aca74e 100644
--- a/chrome/browser/apps/web_view_browsertest.cc
+++ b/chrome/browser/apps/web_view_browsertest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "apps/native_app_window.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/apps/app_browsertest_util.h"
@@ -14,6 +15,8 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/interstitial_page.h"
+#include "content/public/browser/interstitial_page_delegate.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents_delegate.h"
@@ -33,17 +36,83 @@ using prerender::PrerenderLinkManager;
 using prerender::PrerenderLinkManagerFactory;
 
 namespace {
-  const char kEmptyResponsePath[] = "/close-socket";
-  const char kRedirectResponsePath[] = "/server-redirect";
-  const char kRedirectResponseFullPath[] =
-      "/extensions/platform_apps/web_view/shim/guest_redirect.html";
-
-  class EmptyHttpResponse : public net::test_server::HttpResponse {
-  public:
-    virtual std::string ToResponseString() const OVERRIDE {
-      return std::string();
-    }
-  };
+const char kEmptyResponsePath[] = "/close-socket";
+const char kRedirectResponsePath[] = "/server-redirect";
+const char kRedirectResponseFullPath[] =
+    "/extensions/platform_apps/web_view/shim/guest_redirect.html";
+
+class EmptyHttpResponse : public net::test_server::HttpResponse {
+ public:
+  virtual std::string ToResponseString() const OVERRIDE {
+    return std::string();
+  }
+};
+
+class TestInterstitialPageDelegate : public content::InterstitialPageDelegate {
+ public:
+  TestInterstitialPageDelegate() {
+  }
+  virtual ~TestInterstitialPageDelegate() {}
+  virtual std::string GetHTMLContents() OVERRIDE { return std::string(); }
+};
+
+class WebContentsCreatedListener {
+ public:
+  WebContentsCreatedListener() : web_contents_(NULL) {
+    content::WebContents::AddCreatedCallback(
+        base::Bind(&WebContentsCreatedListener::CreatedCallback,
+                   base::Unretained(this)));
+  }
+
+  content::WebContents* WaitForWebContentsCreated() {
+    if (web_contents_)
+      return web_contents_;
+
+    message_loop_runner_ = new content::MessageLoopRunner;
+    message_loop_runner_->Run();
+    return web_contents_;
+  }
+
+ private:
+  void CreatedCallback(content::WebContents* web_contents) {
+    web_contents_ = web_contents;
+
+    if (message_loop_runner_)
+      message_loop_runner_->Quit();
+  }
+
+ private:
+  content::WebContents* web_contents_;
+  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
+
+  DISALLOW_COPY_AND_ASSIGN(WebContentsCreatedListener);
+};
+
+class InterstitialObserver : public content::WebContentsObserver {
+ public:
+  InterstitialObserver(content::WebContents* web_contents,
+                       const base::Closure& attach_callback,
+                       const base::Closure& detach_callback)
+      : WebContentsObserver(web_contents),
+        attach_callback_(attach_callback),
+        detach_callback_(detach_callback) {
+  }
+
+  virtual void DidAttachInterstitialPage() OVERRIDE {
+    attach_callback_.Run();
+  }
+
+  virtual void DidDetachInterstitialPage() OVERRIDE {
+    detach_callback_.Run();
+  }
+
+ private:
+  base::Closure attach_callback_;
+  base::Closure detach_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(InterstitialObserver);
+};
+
 }  // namespace
 
 // This class intercepts media access request from the embedder. The request
@@ -471,6 +540,16 @@ class WebViewTest : public extensions::PlatformAppBrowserTest {
     ASSERT_TRUE(test_run_listener.WaitUntilSatisfied());
   }
 
+  void WaitForInterstitial(content::WebContents* web_contents) {
+    scoped_refptr<content::MessageLoopRunner> loop_runner(
+        new content::MessageLoopRunner);
+    InterstitialObserver observer(web_contents,
+                                  loop_runner->QuitClosure(),
+                                  base::Closure());
+    if (!content::InterstitialPage::GetInterstitialPage(web_contents))
+      loop_runner->Run();
+  }
+
  private:
   scoped_ptr<content::FakeSpeechRecognitionManager>
       fake_speech_recognition_manager_;
@@ -751,6 +830,47 @@ IN_PROC_BROWSER_TEST_F(WebViewTest, Shim_TestRemoveWebviewOnExit) {
   observer.Wait();
 }
 
+// This test makes sure we do not crash if app is closed while interstitial
+// page is being shown in guest.
+IN_PROC_BROWSER_TEST_F(WebViewTest, InterstitialTeardown) {
+  // Start a HTTPS server so we can load an interstitial page inside guest.
+  net::SpawnedTestServer::SSLOptions ssl_options;
+  ssl_options.server_certificate =
+      net::SpawnedTestServer::SSLOptions::CERT_MISMATCHED_NAME;
+  net::SpawnedTestServer https_server(
+      net::SpawnedTestServer::TYPE_HTTPS, ssl_options,
+      base::FilePath(FILE_PATH_LITERAL("chrome/test/data")));
+  ASSERT_TRUE(https_server.Start());
+
+  net::HostPortPair host_and_port = https_server.host_port_pair();
+
+  ExtensionTestMessageListener embedder_loaded_listener("EmbedderLoaded",
+                                                        false);
+  LoadAndLaunchPlatformApp("web_view/interstitial_teardown");
+  ASSERT_TRUE(embedder_loaded_listener.WaitUntilSatisfied());
+
+  WebContentsCreatedListener web_contents_created_listener;
+
+  // Now load the guest.
+  content::WebContents* embedder_web_contents =
+      GetFirstShellWindowWebContents();
+  ExtensionTestMessageListener second("GuestAddedToDom", false);
+  EXPECT_TRUE(content::ExecuteScript(
+      embedder_web_contents,
+      base::StringPrintf("loadGuest(%d);\n", host_and_port.port())));
+  ASSERT_TRUE(second.WaitUntilSatisfied());
+
+  // Wait for interstitial page to be shown in guest.
+  content::WebContents* guest_web_contents =
+      web_contents_created_listener.WaitForWebContentsCreated();
+  ASSERT_TRUE(guest_web_contents->GetRenderProcessHost()->IsGuest());
+  WaitForInterstitial(guest_web_contents);
+
+  // Now close the app while interstitial page being shown in guest.
+  apps::ShellWindow* window = GetFirstShellWindow();
+  window->GetBaseWindow()->Close();
+}
+
 IN_PROC_BROWSER_TEST_F(WebViewTest, ShimSrcAttribute) {
   ASSERT_TRUE(RunPlatformAppTest("platform_apps/web_view/src_attribute"))
       << message_;
diff --git a/chrome/test/data/extensions/platform_apps/web_view/interstitial_teardown/embedder.js b/chrome/test/data/extensions/platform_apps/web_view/interstitial_teardown/embedder.js
new file mode 100644
index 0000000..979044a
--- /dev/null
+++ b/chrome/test/data/extensions/platform_apps/web_view/interstitial_teardown/embedder.js
@@ -0,0 +1,22 @@
+// Copyright 2013 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.
+
+window.loadGuest = function(port) {
+  window.console.log('embedder.loadGuest: ' + port);
+  var webview = document.createElement('webview');
+
+  // This page is not loaded, we just need a https URL.
+  var guestSrcHTTPS = 'https://localhost:' + port +
+      '/files/extensions/platform_apps/web_view/' +
+      'interstitial_teardown/https_page.html';
+  window.console.log('guestSrcHTTPS: ' + guestSrcHTTPS);
+  webview.setAttribute('src', guestSrcHTTPS);
+
+  document.body.appendChild(webview);
+  chrome.test.sendMessage('GuestAddedToDom');
+};
+
+window.onload = function() {
+  chrome.test.sendMessage('EmbedderLoaded');
+};
diff --git a/chrome/test/data/extensions/platform_apps/web_view/interstitial_teardown/https_page.html b/chrome/test/data/extensions/platform_apps/web_view/interstitial_teardown/https_page.html
new file mode 100644
index 0000000..ff9bb8a
--- /dev/null
+++ b/chrome/test/data/extensions/platform_apps/web_view/interstitial_teardown/https_page.html
@@ -0,0 +1,6 @@
+<!doctype html>
+<!--
+ * Copyright 2013 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.
+-->
diff --git a/chrome/test/data/extensions/platform_apps/web_view/interstitial_teardown/main.html b/chrome/test/data/extensions/platform_apps/web_view/interstitial_teardown/main.html
new file mode 100644
index 0000000..9bc5b52
--- /dev/null
+++ b/chrome/test/data/extensions/platform_apps/web_view/interstitial_teardown/main.html
@@ -0,0 +1,12 @@
+<!doctype html>
+<!--
+ * Copyright 2013 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>
+  <webview></webview>
+  <script src="embedder.js"></script>
+</body>
+</html>
diff --git a/chrome/test/data/extensions/platform_apps/web_view/interstitial_teardown/manifest.json b/chrome/test/data/extensions/platform_apps/web_view/interstitial_teardown/manifest.json
new file mode 100644
index 0000000..43f0ea6
--- /dev/null
+++ b/chrome/test/data/extensions/platform_apps/web_view/interstitial_teardown/manifest.json
@@ -0,0 +1,13 @@
+{
+  "name": "<webview> interstitial page check.",
+  "description": "Check teardown path of guest while interstitial page is shown",
+  "version": "1",
+  "permissions": [
+    "webview"
+  ],
+  "app": {
+    "background": {
+      "scripts": ["test.js"]
+    }
+  }
+}
diff --git a/chrome/test/data/extensions/platform_apps/web_view/interstitial_teardown/test.js b/chrome/test/data/extensions/platform_apps/web_view/interstitial_teardown/test.js
new file mode 100644
index 0000000..2f9f855
--- /dev/null
+++ b/chrome/test/data/extensions/platform_apps/web_view/interstitial_teardown/test.js
@@ -0,0 +1,7 @@
+// Copyright 2013 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 () {});
+});
diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc
index 31250b7..ca1b24f 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.cc
+++ b/content/browser/browser_plugin/browser_plugin_guest.cc
@@ -348,7 +348,8 @@ BrowserPluginGuest::BrowserPluginGuest(
       embedder_visible_(true),
       next_permission_request_id_(browser_plugin::kInvalidPermissionRequestID),
       has_render_view_(has_render_view),
-      last_seen_auto_size_enabled_(false) {
+      last_seen_auto_size_enabled_(false),
+      is_in_destruction_(false) {
   DCHECK(web_contents);
   web_contents->SetDelegate(this);
   if (opener)
@@ -422,6 +423,7 @@ int BrowserPluginGuest::RequestPermission(
 }
 
 void BrowserPluginGuest::Destroy() {
+  is_in_destruction_ = true;
   if (!attached() && opener())
     opener()->pending_new_windows_.erase(this);
   DestroyUnattachedWindows();
diff --git a/content/browser/browser_plugin/browser_plugin_guest.h b/content/browser/browser_plugin/browser_plugin_guest.h
index ffe8d56..74624ef 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.h
+++ b/content/browser/browser_plugin/browser_plugin_guest.h
@@ -136,6 +136,7 @@ class CONTENT_EXPORT BrowserPluginGuest
   bool focused() const { return focused_; }
   bool visible() const { return guest_visible_; }
   void clear_damage_buffer() { damage_buffer_.reset(); }
+  bool is_in_destruction() { return is_in_destruction_; }
 
   BrowserPluginGuest* opener() const { return opener_.get(); }
 
@@ -520,6 +521,8 @@ class CONTENT_EXPORT BrowserPluginGuest
   // Last seen autosize attribute state (by OnUpdateRect).
   bool last_seen_auto_size_enabled_;
 
+  bool is_in_destruction_;
+
   // This is a queue of messages that are destined to be sent to the embedder
   // once the guest is attached to a particular embedder.
   std::queue<IPC::Message*> pending_messages_;
diff --git a/content/browser/renderer_host/render_widget_host_view_guest.cc b/content/browser/renderer_host/render_widget_host_view_guest.cc
index 097468f..b49ca41 100644
--- a/content/browser/renderer_host/render_widget_host_view_guest.cc
+++ b/content/browser/renderer_host/render_widget_host_view_guest.cc
@@ -67,14 +67,22 @@ RenderWidgetHost* RenderWidgetHostViewGuest::GetRenderWidgetHost() const {
 }
 
 void RenderWidgetHostViewGuest::WasShown() {
-  if (!is_hidden_)
+  // If the WebContents associated with us showed an interstitial page in the
+  // beginning, the teardown path might call WasShown() while |host_| is in
+  // the process of destruction. Avoid calling WasShown below in this case.
+  // TODO(lazyboy): We shouldn't be showing interstitial pages in guests in the
+  // first place: http://crbug.com/273089.
+  //
+  // |guest_| is NULL during test.
+  if (!is_hidden_ || (guest_ && guest_->is_in_destruction()))
     return;
   is_hidden_ = false;
   host_->WasShown();
 }
 
 void RenderWidgetHostViewGuest::WasHidden() {
-  if (is_hidden_)
+  // |guest_| is NULL during test.
+  if (is_hidden_ || (guest_ && guest_->is_in_destruction()))
     return;
   is_hidden_ = true;
   host_->WasHidden();