[content shell] change how the main render view is picked for layout tests

The old code would sometimes crash. The reason is that we need to know which is
the main render view when the first render view is created. The old code,
however, would only pick the main view after it was created.

Individual changes:

- move sending the fake cwd from RenderViewReady to RenderViewCreated so that we can guarantee it's available on time
- Don't guess which WebKitTestRunner belongs to a RenderView, but explictely pass the RenderView with the WebTestProxy creation callback
- instead of having the browser set the main render view, the renderer picks the first RenderView and declares it the main view

BUG=111316
TEST=Running editing/pasteboard/data-transfer-items-drag-drop-string.html doesn't crash
TBR=tsepez@chromium.org


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

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

11 files changed, 43 insertions, 67 deletions

diff --git a/content/public/test/layouttest_support.h b/content/public/test/layouttest_support.h
index b625d54..0a83b2a 100644
--- a/content/public/test/layouttest_support.h
+++ b/content/public/test/layouttest_support.h
@@ -13,11 +13,13 @@ class WebTestProxyBase;
 
 namespace content {
 
+class RenderView;
+
 // Enable injecting of a WebTestProxy between WebViews and RenderViews.
 // |callback| is invoked with a pointer to WebTestProxyBase for each created
 // WebTestProxy.
-void EnableWebTestProxyCreation(
-    const base::Callback<void(WebTestRunner::WebTestProxyBase*)>& callback);
+void EnableWebTestProxyCreation(const base::Callback<
+    void(RenderView*, WebTestRunner::WebTestProxyBase*)>& callback);
 
 }  // namespace content
 
diff --git a/content/shell/shell_content_renderer_client.cc b/content/shell/shell_content_renderer_client.cc
index 8ccf74e..01e73b0 100644
--- a/content/shell/shell_content_renderer_client.cc
+++ b/content/shell/shell_content_renderer_client.cc
@@ -21,8 +21,7 @@ using WebTestRunner::WebTestProxyBase;
 
 namespace content {
 
-ShellContentRendererClient::ShellContentRendererClient()
-    : latest_test_runner_(NULL) {
+ShellContentRendererClient::ShellContentRendererClient() {
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kDumpRenderTree)) {
     EnableWebTestProxyCreation(
         base::Bind(&ShellContentRendererClient::WebTestProxyCreated,
@@ -37,14 +36,6 @@ void ShellContentRendererClient::RenderThreadStarted() {
   shell_observer_.reset(new ShellRenderProcessObserver());
 }
 
-void ShellContentRendererClient::RenderViewCreated(RenderView* render_view) {
-  if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kDumpRenderTree))
-    return;
-
-  CHECK(!latest_test_runner_);
-  latest_test_runner_ = new WebKitTestRunner(render_view);
-}
-
 bool ShellContentRendererClient::OverrideCreatePlugin(
     RenderView* render_view,
     WebKit::WebFrame* frame,
@@ -60,11 +51,16 @@ bool ShellContentRendererClient::OverrideCreatePlugin(
   return false;
 }
 
-void ShellContentRendererClient::WebTestProxyCreated(WebTestProxyBase* proxy) {
-  CHECK(latest_test_runner_);
-  proxy->setDelegate(latest_test_runner_);
-  latest_test_runner_->set_proxy(proxy);
-  latest_test_runner_ = NULL;
+void ShellContentRendererClient::WebTestProxyCreated(RenderView* render_view,
+                                                     WebTestProxyBase* proxy) {
+  WebKitTestRunner* test_runner = new WebKitTestRunner(render_view);
+  if (!ShellRenderProcessObserver::GetInstance()->test_delegate()) {
+    ShellRenderProcessObserver::GetInstance()->SetMainWindow(render_view,
+                                                             test_runner);
+  }
+  test_runner->set_proxy(proxy);
+  proxy->setDelegate(
+      ShellRenderProcessObserver::GetInstance()->test_delegate());
   proxy->setInterfaces(
       ShellRenderProcessObserver::GetInstance()->test_interfaces());
 }
diff --git a/content/shell/shell_content_renderer_client.h b/content/shell/shell_content_renderer_client.h
index f9d49d0..1ccf7ac 100644
--- a/content/shell/shell_content_renderer_client.h
+++ b/content/shell/shell_content_renderer_client.h
@@ -23,14 +23,12 @@ namespace content {
 
 class RenderView;
 class ShellRenderProcessObserver;
-class WebKitTestRunner;
 
 class ShellContentRendererClient : public ContentRendererClient {
  public:
   ShellContentRendererClient();
   virtual ~ShellContentRendererClient();
   virtual void RenderThreadStarted() OVERRIDE;
-  virtual void RenderViewCreated(RenderView* render_view) OVERRIDE;
   virtual bool OverrideCreatePlugin(
       RenderView* render_view,
       WebKit::WebFrame* frame,
@@ -38,10 +36,10 @@ class ShellContentRendererClient : public ContentRendererClient {
       WebKit::WebPlugin** plugin) OVERRIDE;
 
  private:
-  void WebTestProxyCreated(WebTestRunner::WebTestProxyBase* proxy);
+  void WebTestProxyCreated(RenderView* render_view,
+                           WebTestRunner::WebTestProxyBase* proxy);
 
   scoped_ptr<ShellRenderProcessObserver> shell_observer_;
-  WebKitTestRunner* latest_test_runner_;
 };
 
 }  // namespace content
diff --git a/content/shell/shell_messages.h b/content/shell/shell_messages.h
index f2d6b98..d770e34 100644
--- a/content/shell/shell_messages.h
+++ b/content/shell/shell_messages.h
@@ -27,10 +27,6 @@ IPC_MESSAGE_ROUTED3(ShellViewMsg_CaptureTextDump,
 IPC_MESSAGE_ROUTED1(ShellViewMsg_CaptureImageDump,
                     std::string /* expected pixel hash */)
 
-// Tells the render view that it should register itself as main window for the
-// layout test.
-IPC_MESSAGE_ROUTED0(ShellViewMsg_SetIsMainWindow)
-
 // Tells the renderer to reset all test runners.
 IPC_MESSAGE_CONTROL0(ShellViewMsg_ResetAll)
 
diff --git a/content/shell/shell_render_process_observer.cc b/content/shell/shell_render_process_observer.cc
index 250c911..7c25ecf 100644
--- a/content/shell/shell_render_process_observer.cc
+++ b/content/shell/shell_render_process_observer.cc
@@ -13,6 +13,7 @@
 #include "webkit/glue/webkit_glue.h"
 #include "webkit/support/gc_extension.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebTestingSupport.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebView.h"
 #include "third_party/WebKit/Tools/DumpRenderTree/chromium/TestRunner/public/WebTestInterfaces.h"
 
 using WebKit::WebFrame;
@@ -32,7 +33,8 @@ ShellRenderProcessObserver* ShellRenderProcessObserver::GetInstance() {
 }
 
 ShellRenderProcessObserver::ShellRenderProcessObserver()
-    : test_delegate_(NULL) {
+    : main_render_view_(NULL),
+      test_delegate_(NULL) {
   CHECK(!g_instance);
   g_instance = this;
   RenderThread::Get()->AddObserver(this);
@@ -46,17 +48,10 @@ ShellRenderProcessObserver::~ShellRenderProcessObserver() {
 void ShellRenderProcessObserver::SetMainWindow(
     RenderView* view,
     WebTestDelegate* delegate) {
-  if (view == NULL) {
-    if (delegate == test_delegate_) {
-      test_interfaces_->setDelegate(NULL);
-      test_interfaces_->setWebView(NULL);
-      test_delegate_ = NULL;
-    }
-  } else {
-    test_interfaces_->setDelegate(delegate);
-    test_interfaces_->setWebView(view->GetWebView());
-    test_delegate_ = delegate;
-  }
+  test_interfaces_->setDelegate(delegate);
+  test_interfaces_->setWebView(view->GetWebView());
+  main_render_view_ = view;
+  test_delegate_ = delegate;
 }
 
 void ShellRenderProcessObserver::BindTestRunnersToWindow(WebFrame* frame) {
@@ -92,7 +87,10 @@ bool ShellRenderProcessObserver::OnControlMessageReceived(
 
 void ShellRenderProcessObserver::OnResetAll() {
   test_interfaces_->resetAll();
-  // We don't reset the WebTestingSupport objects, as we don't reuse WebViews.
+  if (main_render_view_) {
+    WebTestingSupport::resetInternalsObject(
+        main_render_view_->GetWebView()->mainFrame());
+  }
 }
 
 }  // namespace content
diff --git a/content/shell/shell_render_process_observer.h b/content/shell/shell_render_process_observer.h
index 42d2cca..505c6e0 100644
--- a/content/shell/shell_render_process_observer.h
+++ b/content/shell/shell_render_process_observer.h
@@ -38,6 +38,9 @@ class ShellRenderProcessObserver : public RenderProcessObserver {
   virtual void WebKitInitialized() OVERRIDE;
   virtual bool OnControlMessageReceived(const IPC::Message& message) OVERRIDE;
 
+  WebTestRunner::WebTestDelegate* test_delegate() const {
+    return test_delegate_;
+  }
   WebTestRunner::WebTestInterfaces* test_interfaces() const {
     return test_interfaces_.get();
   }
@@ -47,6 +50,7 @@ class ShellRenderProcessObserver : public RenderProcessObserver {
   void OnResetAll();
 
   scoped_ptr<WebTestRunner::WebTestInterfaces> test_interfaces_;
+  RenderView* main_render_view_;
   WebTestRunner::WebTestDelegate* test_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(ShellRenderProcessObserver);
diff --git a/content/shell/webkit_test_runner.cc b/content/shell/webkit_test_runner.cc
index d21bdab..122282a 100644
--- a/content/shell/webkit_test_runner.cc
+++ b/content/shell/webkit_test_runner.cc
@@ -144,13 +144,10 @@ void CopyCanvasToBitmap(SkCanvas* canvas,  SkBitmap* snapshot) {
 
 WebKitTestRunner::WebKitTestRunner(RenderView* render_view)
     : RenderViewObserver(render_view),
-      RenderViewObserverTracker<WebKitTestRunner>(render_view),
-      is_main_window_(false) {
+      RenderViewObserverTracker<WebKitTestRunner>(render_view) {
 }
 
 WebKitTestRunner::~WebKitTestRunner() {
-  if (is_main_window_)
-    ShellRenderProcessObserver::GetInstance()->SetMainWindow(NULL, this);
 }
 
 // WebTestDelegate  -----------------------------------------------------------
@@ -259,7 +256,6 @@ bool WebKitTestRunner::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(ShellViewMsg_CaptureImageDump, OnCaptureImageDump)
     IPC_MESSAGE_HANDLER(ShellViewMsg_SetCurrentWorkingDirectory,
                         OnSetCurrentWorkingDirectory)
-    IPC_MESSAGE_HANDLER(ShellViewMsg_SetIsMainWindow, OnSetIsMainWindow)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
 
@@ -337,11 +333,6 @@ void WebKitTestRunner::OnSetCurrentWorkingDirectory(
   current_working_directory_ = current_working_directory;
 }
 
-void WebKitTestRunner::OnSetIsMainWindow() {
-  is_main_window_ = true;
-  ShellRenderProcessObserver::GetInstance()->SetMainWindow(render_view(), this);
-}
-
 SkCanvas* WebKitTestRunner::GetCanvas() {
   WebView* view = render_view()->GetWebView();
   const WebSize& size = view->size();
diff --git a/content/shell/webkit_test_runner.h b/content/shell/webkit_test_runner.h
index 367a9e7..a6eafaf 100644
--- a/content/shell/webkit_test_runner.h
+++ b/content/shell/webkit_test_runner.h
@@ -67,7 +67,6 @@ class WebKitTestRunner : public RenderViewObserver,
   // Message handlers.
   void OnCaptureTextDump(bool as_text, bool printing, bool recursive);
   void OnCaptureImageDump(const std::string& expected_pixel_hash);
-  void OnSetIsMainWindow();
   void OnSetCurrentWorkingDirectory(const FilePath& current_working_directory);
 
   SkCanvas* GetCanvas();
@@ -77,7 +76,6 @@ class WebKitTestRunner : public RenderViewObserver,
 
   scoped_ptr<SkCanvas> canvas_;
   scoped_ptr<WebKit::WebContextMenuData> last_context_menu_data_;
-  bool is_main_window_;
   FilePath current_working_directory_;
 
   WebTestRunner::WebTestProxyBase* proxy_;
diff --git a/content/shell/webkit_test_runner_host.cc b/content/shell/webkit_test_runner_host.cc
index e2187f4..f8e45e5 100644
--- a/content/shell/webkit_test_runner_host.cc
+++ b/content/shell/webkit_test_runner_host.cc
@@ -164,7 +164,6 @@ bool WebKitTestController::PrepareForLayoutTest(
       NULL,
       MSG_ROUTING_NONE,
       NULL);
-  did_set_as_main_window_ = false;
   Observe(main_window_->web_contents());
   return true;
 }
@@ -181,7 +180,6 @@ bool WebKitTestController::ResetAfterLayoutTest() {
   is_printing_ = false;
   should_stay_on_page_after_handling_before_unload_ = false;
   wait_until_done_ = false;
-  did_set_as_main_window_ = false;
   watchdog_.Cancel();
   if (main_window_) {
     Observe(NULL);
@@ -243,21 +241,16 @@ void WebKitTestController::PluginCrashed(const FilePath& plugin_path) {
   printer_->AddErrorMessage("#CRASHED - plugin");
 }
 
-void WebKitTestController::RenderViewReady() {
-  RenderViewHost* render_view_host =
-      main_window_->web_contents()->GetRenderViewHost();
+void WebKitTestController::RenderViewCreated(RenderViewHost* render_view_host) {
   render_view_host->Send(new ShellViewMsg_SetCurrentWorkingDirectory(
       render_view_host->GetRoutingID(), current_working_directory_));
-  if (did_set_as_main_window_)
-    return;
-  render_view_host->Send(new ShellViewMsg_SetIsMainWindow(
-      render_view_host->GetRoutingID()));
-  did_set_as_main_window_ = true;
 }
 
 void WebKitTestController::RenderViewGone(base::TerminationStatus status) {
-  if (status == base::TERMINATION_STATUS_PROCESS_CRASHED)
+  if (status == base::TERMINATION_STATUS_PROCESS_CRASHED ||
+      status == base::TERMINATION_STATUS_ABNORMAL_TERMINATION) {
     printer_->AddErrorMessage("#CRASHED - renderer");
+  }
 }
 
 void WebKitTestController::WebContentsDestroyed(WebContents* web_contents) {
diff --git a/content/shell/webkit_test_runner_host.h b/content/shell/webkit_test_runner_host.h
index 53573bf..ea09ec2 100644
--- a/content/shell/webkit_test_runner_host.h
+++ b/content/shell/webkit_test_runner_host.h
@@ -111,7 +111,7 @@ class WebKitTestController : public base::NonThreadSafe,
   // WebContentsObserver implementation.
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
   virtual void PluginCrashed(const FilePath& plugin_path) OVERRIDE;
-  virtual void RenderViewReady() OVERRIDE;
+  virtual void RenderViewCreated(RenderViewHost* render_view_host) OVERRIDE;
   virtual void RenderViewGone(base::TerminationStatus status) OVERRIDE;
   virtual void WebContentsDestroyed(WebContents* web_contents) OVERRIDE;
 
@@ -144,8 +144,6 @@ class WebKitTestController : public base::NonThreadSafe,
   bool should_stay_on_page_after_handling_before_unload_;
   bool wait_until_done_;
 
-  bool did_set_as_main_window_;
-
   base::CancelableClosure watchdog_;
 
   DISALLOW_COPY_AND_ASSIGN(WebKitTestController);
diff --git a/content/test/layouttest_support.cc b/content/test/layouttest_support.cc
index ea70318..2cc383c 100644
--- a/content/test/layouttest_support.cc
+++ b/content/test/layouttest_support.cc
@@ -16,7 +16,8 @@ namespace content {
 
 namespace {
 
-base::LazyInstance<base::Callback<void(WebTestProxyBase*)> >::Leaky g_callback;
+base::LazyInstance<base::Callback<void(RenderView*, WebTestProxyBase*)> >::Leaky
+    g_callback;
 
 RenderViewImpl* CreateWebTestProxy(RenderViewImplParams* params) {
   typedef WebTestProxy<RenderViewImpl, RenderViewImplParams*> ProxyType;
@@ -24,7 +25,8 @@ RenderViewImpl* CreateWebTestProxy(RenderViewImplParams* params) {
       reinterpret_cast<RenderViewImplParams*>(params));
   if (g_callback == 0)
     return render_view_proxy;
-  g_callback.Get().Run(render_view_proxy);
+  g_callback.Get().Run(
+      static_cast<RenderView*>(render_view_proxy), render_view_proxy);
   return render_view_proxy;
 }
 
@@ -32,7 +34,7 @@ RenderViewImpl* CreateWebTestProxy(RenderViewImplParams* params) {
 
 
 void EnableWebTestProxyCreation(
-    const base::Callback<void(WebTestProxyBase*)>& callback) {
+    const base::Callback<void(RenderView*, WebTestProxyBase*)>& callback) {
   g_callback.Get() = callback;
   RenderViewImpl::InstallCreateHook(CreateWebTestProxy);
 }