Take 2: Remove WebView::SetDelegate because I'd like to avoid having a method

like this in the WebKit API.

The only consumer was TestShell.  It was using this method to replace its
TestWebViewDelegate instance.  Instead, with this change, it has a manual Reset
method.  To avoid duplication with the constructor, Reset uses operator=().

This required a couple changes:

1- Remove DISALLOW_COPY_AND_ASSIGN from WebViewDelegate.  Anyways, that didn't
make sense since you cannot 'copy' a class with pure virtual methods.

2- Change scoped_ptr members of TestWebViewDelegate to linked_ptr.  The extra
overhead of the linked_ptr seems warranted in this case.

I also changed TestWebViewDelegate to not be reference counted since it wasn't
necessary.

Finally, to get this to work on Linux, I needed to revise the shutdown paths
for WebWidgetHost.  The problem is that the code wants to support destroying a
WebWidgetHost directly by having its destructor called or indirectly by having
a "destroy" signal handler notice destruction of the GtkWidget.  This actually
mirrors the Windows version as well, and so just like the Windows version I
added a level of indirection to get from the GtkWidget to the WebWidgetHost.
However, I only apply this indirection to the "destroy" signal handler.

R=tony
BUG=none
TEST=none

Review URL: http://codereview.chromium.org/165341

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

14 files changed, 70 insertions, 69 deletions

diff --git a/webkit/glue/webview.h b/webkit/glue/webview.h
index 6e2c794..c329452 100644
--- a/webkit/glue/webview.h
+++ b/webkit/glue/webview.h
@@ -66,9 +66,6 @@ class WebView : public WebKit::WebWidget {
   // it, it will be NULL during closing of the view.
   virtual WebViewDelegate* GetDelegate() = 0;
 
-  // Changes the delegate for this WebView.  It is valid to set this to NULL.
-  virtual void SetDelegate(WebViewDelegate* delegate) = 0;
-
   // Instructs the EditorClient whether to pass editing notifications on to a
   // delegate, if one is present.  This allows embedders that haven't
   // overridden any editor delegate methods to avoid the performance impact of
diff --git a/webkit/glue/webview_delegate.h b/webkit/glue/webview_delegate.h
index 7d421af..826d1a7 100644
--- a/webkit/glue/webview_delegate.h
+++ b/webkit/glue/webview_delegate.h
@@ -845,10 +845,9 @@ class WebViewDelegate : virtual public WebKit::WebWidgetClient {
   virtual void DidAddHistoryItem() { }
 
   WebViewDelegate() { }
-  virtual ~WebViewDelegate() { }
 
- private:
-  DISALLOW_COPY_AND_ASSIGN(WebViewDelegate);
+ protected:
+  ~WebViewDelegate() { }
 };
 
 #endif  // WEBKIT_GLUE_WEBVIEW_DELEGATE_H_
diff --git a/webkit/glue/webview_impl.cc b/webkit/glue/webview_impl.cc
index c1c1f1e..16bc819 100644
--- a/webkit/glue/webview_impl.cc
+++ b/webkit/glue/webview_impl.cc
@@ -1246,10 +1246,6 @@ WebViewDelegate* WebViewImpl::GetDelegate() {
   return delegate_;
 }
 
-void WebViewImpl::SetDelegate(WebViewDelegate* delegate) {
-  delegate_ = delegate;
-}
-
 WebFrame* WebViewImpl::GetMainFrame() {
   return main_frame();
 }
diff --git a/webkit/glue/webview_impl.h b/webkit/glue/webview_impl.h
index f2ea84b..275faed 100644
--- a/webkit/glue/webview_impl.h
+++ b/webkit/glue/webview_impl.h
@@ -76,7 +76,6 @@ class WebViewImpl : public WebView, public base::RefCounted<WebViewImpl> {
   virtual bool ShouldClose();
   virtual void ClosePage();
   virtual WebViewDelegate* GetDelegate();
-  virtual void SetDelegate(WebViewDelegate*);
   virtual void SetUseEditorDelegate(bool value);
   virtual void SetTabKeyCyclesThroughElements(bool value);
   virtual WebKit::WebFrame* GetMainFrame();
diff --git a/webkit/tools/test_shell/mac/test_webview_delegate.mm b/webkit/tools/test_shell/mac/test_webview_delegate.mm
index 7724532..1adb264 100644
--- a/webkit/tools/test_shell/mac/test_webview_delegate.mm
+++ b/webkit/tools/test_shell/mac/test_webview_delegate.mm
@@ -23,9 +23,6 @@ using WebKit::WebWidget;
 
 // WebViewDelegate -----------------------------------------------------------
 
-TestWebViewDelegate::~TestWebViewDelegate() {
-}
-
 WebWidget* TestWebViewDelegate::CreatePopupWidgetWithInfo(
     WebView* webview,
     const WebPopupMenuInfo& info) {
diff --git a/webkit/tools/test_shell/test_shell.cc b/webkit/tools/test_shell/test_shell.cc
index 1d296bf..df1f953 100644
--- a/webkit/tools/test_shell/test_shell.cc
+++ b/webkit/tools/test_shell/test_shell.cc
@@ -112,8 +112,8 @@ TestShell::TestShell()
       test_is_pending_(false),
       is_modal_(false),
       dump_stats_table_on_exit_(false) {
-    delegate_ = new TestWebViewDelegate(this);
-    popup_delegate_ = new TestWebViewDelegate(this);
+    delegate_.reset(new TestWebViewDelegate(this));
+    popup_delegate_.reset(new TestWebViewDelegate(this));
     layout_test_controller_.reset(new LayoutTestController(this));
     event_sending_controller_.reset(new EventSendingController(this));
     text_input_controller_.reset(new TextInputController(this));
@@ -126,6 +126,8 @@ TestShell::TestShell()
 }
 
 TestShell::~TestShell() {
+  delegate_->RevokeDragDrop();
+
   // Navigate to an empty page to fire all the destruction logic for the
   // current page.
   LoadURL(L"about:blank");
@@ -134,7 +136,9 @@ TestShell::~TestShell() {
   CallJSGC();
   CallJSGC();
 
-  webView()->SetDelegate(NULL);
+  // Destroy the WebView before the TestWebViewDelegate.
+  m_webViewHost.reset();
+
   PlatformCleanUp();
 
   StatsTable *table = StatsTable::current();
@@ -502,14 +506,11 @@ void TestShell::SizeToDefault() {
 void TestShell::ResetTestController() {
   layout_test_controller_->Reset();
   event_sending_controller_->Reset();
-
-  // Reset state in the test webview delegate.
-  delegate_ = new TestWebViewDelegate(this);
-  webView()->SetDelegate(delegate_);
+  delegate_->Reset();
 }
 
 void TestShell::LoadURL(const wchar_t* url) {
-    LoadURLForFrame(url, NULL);
+  LoadURLForFrame(url, NULL);
 }
 
 bool TestShell::Navigate(const TestNavigationEntry& entry, bool reload) {
diff --git a/webkit/tools/test_shell/test_shell.h b/webkit/tools/test_shell/test_shell.h
index 1c18e4f..4b6ed14 100644
--- a/webkit/tools/test_shell/test_shell.h
+++ b/webkit/tools/test_shell/test_shell.h
@@ -324,8 +324,8 @@ private:
 
     scoped_ptr<TestNavigationController> navigation_controller_;
 
-    scoped_refptr<TestWebViewDelegate> delegate_;
-    scoped_refptr<TestWebViewDelegate> popup_delegate_;
+    scoped_ptr<TestWebViewDelegate> delegate_;
+    scoped_ptr<TestWebViewDelegate> popup_delegate_;
 
     const TestParams* test_params_;
 
diff --git a/webkit/tools/test_shell/test_shell_gtk.cc b/webkit/tools/test_shell/test_shell_gtk.cc
index 27aafcd..2659f7b 100644
--- a/webkit/tools/test_shell/test_shell_gtk.cc
+++ b/webkit/tools/test_shell/test_shell_gtk.cc
@@ -304,9 +304,6 @@ void TestShell::PlatformShutdown() {
 }
 
 void TestShell::PlatformCleanUp() {
-  // The GTK widgets will be destroyed, which will free the associated
-  // objects.  So we don't need the scoped_ptr to free the webViewHost.
-  m_webViewHost.release();
   if (m_mainWnd) {
     // Disconnect our MainWindowDestroyed handler so that we don't go through
     // the shutdown process more than once.
@@ -369,7 +366,8 @@ bool TestShell::Initialize(const std::wstring& startingURL) {
   gtk_toolbar_insert(GTK_TOOLBAR(toolbar), tool_item, -1 /* append */);
 
   gtk_box_pack_start(GTK_BOX(vbox), toolbar, FALSE, FALSE, 0);
-  m_webViewHost.reset(WebViewHost::Create(vbox, delegate_, *TestShell::web_prefs_));
+  m_webViewHost.reset(
+      WebViewHost::Create(vbox, delegate_.get(), *TestShell::web_prefs_));
 
   // Enables output of "EDDITING DELEGATE: " debugging lines in the layout test
   // output.
@@ -476,7 +474,7 @@ void TestShell::DestroyWindow(gfx::NativeWindow windowHandle) {
 WebWidget* TestShell::CreatePopupWidget(WebView* webview) {
   GtkWidget* popupwindow = gtk_window_new(GTK_WINDOW_POPUP);
   GtkWidget* vbox = gtk_vbox_new(FALSE, 0);
-  WebWidgetHost* host = WebWidgetHost::Create(vbox, popup_delegate_);
+  WebWidgetHost* host = WebWidgetHost::Create(vbox, popup_delegate_.get());
   gtk_container_add(GTK_CONTAINER(popupwindow), vbox);
   m_popupHost = host;
 
diff --git a/webkit/tools/test_shell/test_webview_delegate.cc b/webkit/tools/test_shell/test_webview_delegate.cc
index f3ae7e8..654dc314 100644
--- a/webkit/tools/test_shell/test_webview_delegate.cc
+++ b/webkit/tools/test_shell/test_webview_delegate.cc
@@ -845,6 +845,32 @@ WebScreenInfo TestWebViewDelegate::screenInfo() {
   return WebScreenInfo();
 }
 
+// Public methods ------------------------------------------------------------
+
+TestWebViewDelegate::TestWebViewDelegate(TestShell* shell)
+    : policy_delegate_enabled_(false),
+      policy_delegate_is_permissive_(false),
+      policy_delegate_should_notify_done_(false),
+      shell_(shell),
+      top_loading_frame_(NULL),
+      page_id_(-1),
+      last_page_id_updated_(-1),
+#if defined(OS_LINUX)
+      cursor_type_(GDK_X_CURSOR),
+#endif
+      smart_insert_delete_enabled_(true),
+#if defined(OS_WIN)
+      select_trailing_whitespace_enabled_(true),
+#else
+      select_trailing_whitespace_enabled_(false),
+#endif
+      block_redirects_(false) {
+}
+
+void TestWebViewDelegate::Reset() {
+  *this = TestWebViewDelegate(shell_);
+}
+
 void TestWebViewDelegate::SetSmartInsertDeleteEnabled(bool enabled) {
   smart_insert_delete_enabled_ = enabled;
   // In upstream WebKit, smart insert/delete is mutually exclusive with select
@@ -868,6 +894,12 @@ void TestWebViewDelegate::RegisterDragDrop() {
 #endif
 }
 
+void TestWebViewDelegate::RevokeDragDrop() {
+#if defined(OS_WIN)
+  ::RevokeDragDrop(shell_->webViewWnd());
+#endif
+}
+
 void TestWebViewDelegate::SetCustomPolicyDelegate(bool is_custom,
                                                   bool is_permissive) {
   policy_delegate_enabled_ = is_custom;
diff --git a/webkit/tools/test_shell/test_webview_delegate.h b/webkit/tools/test_shell/test_webview_delegate.h
index 81e7bab..6224b9c 100644
--- a/webkit/tools/test_shell/test_webview_delegate.h
+++ b/webkit/tools/test_shell/test_webview_delegate.h
@@ -21,8 +21,7 @@
 #endif
 
 #include "base/basictypes.h"
-#include "base/ref_counted.h"
-#include "base/scoped_ptr.h"
+#include "base/linked_ptr.h"
 #if defined(OS_MACOSX)
 #include "webkit/api/public/WebRect.h"
 #include "webkit/api/public/WebPopupMenuInfo.h"
@@ -41,8 +40,7 @@ class GURL;
 class TestShell;
 class WebWidgetHost;
 
-class TestWebViewDelegate : public base::RefCounted<TestWebViewDelegate>,
-                            public WebViewDelegate {
+class TestWebViewDelegate : public WebViewDelegate {
  public:
   struct CapturedContextMenuEvent {
     CapturedContextMenuEvent(ContextNodeType in_node_type,
@@ -60,27 +58,6 @@ class TestWebViewDelegate : public base::RefCounted<TestWebViewDelegate>,
 
   typedef std::vector<CapturedContextMenuEvent> CapturedContextMenuEvents;
 
-  TestWebViewDelegate(TestShell* shell)
-    : policy_delegate_enabled_(false),
-      policy_delegate_is_permissive_(false),
-      policy_delegate_should_notify_done_(false),
-      shell_(shell),
-      top_loading_frame_(NULL),
-      page_id_(-1),
-      last_page_id_updated_(-1),
-#if defined(OS_LINUX)
-      cursor_type_(GDK_X_CURSOR),
-#endif
-      smart_insert_delete_enabled_(true),
-#if defined(OS_WIN)
-      select_trailing_whitespace_enabled_(true),
-#else
-      select_trailing_whitespace_enabled_(false),
-#endif
-      block_redirects_(false) {
-  }
-  virtual ~TestWebViewDelegate();
-
   // WebViewDelegate
   virtual WebView* CreateWebView(WebView* webview,
                                  bool user_gesture,
@@ -257,6 +234,9 @@ class TestWebViewDelegate : public base::RefCounted<TestWebViewDelegate>,
   virtual WebKit::WebRect windowResizerRect();
   virtual WebKit::WebScreenInfo screenInfo();
 
+  TestWebViewDelegate(TestShell* shell);
+  void Reset();
+
   void SetSmartInsertDeleteEnabled(bool enabled);
   void SetSelectTrailingWhitespaceEnabled(bool enabled);
 
@@ -283,6 +263,9 @@ class TestWebViewDelegate : public base::RefCounted<TestWebViewDelegate>,
 
   // Sets the webview as a drop target.
   void RegisterDragDrop();
+  void RevokeDragDrop();
+
+  void ResetDragDrop();
 
   void SetCustomPolicyDelegate(bool is_custom, bool is_permissive);
   void WaitForPolicyDelegate();
@@ -350,7 +333,7 @@ class TestWebViewDelegate : public base::RefCounted<TestWebViewDelegate>,
   int page_id_;
   int last_page_id_updated_;
 
-  scoped_ptr<TestShellExtraData> pending_extra_data_;
+  linked_ptr<TestShellExtraData> pending_extra_data_;
 
   // Maps resource identifiers to a descriptive string.
   typedef std::map<uint32, std::string> ResourceMap;
@@ -375,7 +358,7 @@ class TestWebViewDelegate : public base::RefCounted<TestWebViewDelegate>,
 #endif
 
 #if defined(OS_MACOSX)
-  scoped_ptr<WebKit::WebPopupMenuInfo> popup_menu_info_;
+  linked_ptr<WebKit::WebPopupMenuInfo> popup_menu_info_;
   WebKit::WebRect popup_bounds_;
 #endif
 
@@ -387,8 +370,6 @@ class TestWebViewDelegate : public base::RefCounted<TestWebViewDelegate>,
 
   // true if we should block any redirects
   bool block_redirects_;
-
-  DISALLOW_COPY_AND_ASSIGN(TestWebViewDelegate);
 };
 
 #endif  // WEBKIT_TOOLS_TEST_SHELL_TEST_WEBVIEW_DELEGATE_H_
diff --git a/webkit/tools/test_shell/test_webview_delegate_gtk.cc b/webkit/tools/test_shell/test_webview_delegate_gtk.cc
index 66d7a62..460dceb 100644
--- a/webkit/tools/test_shell/test_webview_delegate_gtk.cc
+++ b/webkit/tools/test_shell/test_webview_delegate_gtk.cc
@@ -84,9 +84,6 @@ void SelectionClipboardGetContents(GtkClipboard* clipboard,
 
 // WebViewDelegate -----------------------------------------------------------
 
-TestWebViewDelegate::~TestWebViewDelegate() {
-}
-
 WebPluginDelegate* TestWebViewDelegate::CreatePluginDelegate(
     WebView* webview,
     const GURL& url,
diff --git a/webkit/tools/test_shell/test_webview_delegate_win.cc b/webkit/tools/test_shell/test_webview_delegate_win.cc
index d49cb9d..fe57540 100644
--- a/webkit/tools/test_shell/test_webview_delegate_win.cc
+++ b/webkit/tools/test_shell/test_webview_delegate_win.cc
@@ -41,10 +41,6 @@ using WebKit::WebRect;
 
 // WebViewDelegate -----------------------------------------------------------
 
-TestWebViewDelegate::~TestWebViewDelegate() {
-  RevokeDragDrop(shell_->webViewWnd());
-}
-
 WebPluginDelegate* TestWebViewDelegate::CreatePluginDelegate(
     WebView* webview,
     const GURL& url,
diff --git a/webkit/tools/test_shell/webview_host_gtk.cc b/webkit/tools/test_shell/webview_host_gtk.cc
index 71d6d4b..f9f030f 100644
--- a/webkit/tools/test_shell/webview_host_gtk.cc
+++ b/webkit/tools/test_shell/webview_host_gtk.cc
@@ -21,7 +21,6 @@ WebViewHost* WebViewHost::Create(GtkWidget* parent_view,
 
   host->view_ = WebWidgetHost::CreateWidget(parent_view, host);
   host->plugin_container_manager_.set_host_widget(host->view_);
-  g_object_set_data(G_OBJECT(host->view_), "webwidgethost", host);
 
   host->webwidget_ = WebView::Create(delegate, prefs);
   host->webwidget_->layout();
diff --git a/webkit/tools/test_shell/webwidget_host_gtk.cc b/webkit/tools/test_shell/webwidget_host_gtk.cc
index 0f688b6..77981e1 100644
--- a/webkit/tools/test_shell/webwidget_host_gtk.cc
+++ b/webkit/tools/test_shell/webwidget_host_gtk.cc
@@ -33,6 +33,9 @@ using WebKit::WebWidgetClient;
 
 namespace {
 
+// Used to store a backpointer to WebWidgetHost from our GtkWidget.
+const char kWebWidgetHostKey[] = "webwidgethost";
+
 // In response to an invalidation, we call into WebKit to do layout. On
 // Windows, WM_PAINT is a virtual message so any extra invalidates that come up
 // while it's doing layout are implicitly swallowed as soon as we actually do
@@ -83,7 +86,7 @@ class WebWidgetHostGtkWidget {
     g_signal_connect(widget, "expose-event",
                      G_CALLBACK(&HandleExpose), host);
     g_signal_connect(widget, "destroy",
-                     G_CALLBACK(&HandleDestroy), host);
+                     G_CALLBACK(&HandleDestroy), NULL);
     g_signal_connect(widget, "key-press-event",
                      G_CALLBACK(&HandleKeyPress), host);
     g_signal_connect(widget, "key-release-event",
@@ -103,6 +106,7 @@ class WebWidgetHostGtkWidget {
     g_signal_connect(widget, "scroll-event",
                      G_CALLBACK(&HandleScroll), host);
 
+    g_object_set_data(G_OBJECT(widget), kWebWidgetHostKey, host);
     return widget;
   }
 
@@ -152,8 +156,12 @@ class WebWidgetHostGtkWidget {
   }
 
   // The GdkWindow was destroyed.
-  static gboolean HandleDestroy(GtkWidget* widget, WebWidgetHost* host) {
-    host->WindowDestroyed();
+  static gboolean HandleDestroy(GtkWidget* widget, void* unused) {
+    // The associated WebWidgetHost instance may have already been destroyed.
+    WebWidgetHost* host = static_cast<WebWidgetHost*>(
+        g_object_get_data(G_OBJECT(widget), kWebWidgetHostKey));
+    if (host)
+      host->WindowDestroyed();
     return FALSE;
   }
 
@@ -308,6 +316,7 @@ WebWidgetHost::WebWidgetHost()
 }
 
 WebWidgetHost::~WebWidgetHost() {
+  g_object_set_data(G_OBJECT(view_), kWebWidgetHostKey, NULL);
   webwidget_->close();
 }