The WebFrame interface currently supports reference counting, but no one uses.

Internally, WebFrameImpl needs reference counting, so we still define it there.

Same goes for WebView and WebWidget.

While making this change, I noticed that WebInspectorClient was casting WebView
pointers to WebViewImpl pointers too much.  By just starting with WebViewImpl
pointers, things could be cleaned up considerably.

R=brettw

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

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

15 files changed, 46 insertions, 44 deletions

diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 3d74340..b8bd897 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -282,9 +282,7 @@ void RenderView::Init(gfx::NativeViewId parent_hwnd,
     decrement_shared_popup_at_destruction_ = false;
   }
 
-  // Avoid a leak here by not assigning, since WebView::Create addrefs for us.
-  WebWidget* view = WebView::Create(this, webkit_prefs);
-  webwidget_.swap(&view);
+  webwidget_ = WebView::Create(this, webkit_prefs);
 
   // Don't let WebCore keep a B/F list - we have our own.
   // We let it keep 1 entry because FrameLoader::goToItem expects an item in the
diff --git a/chrome/renderer/render_widget.cc b/chrome/renderer/render_widget.cc
index 1b20ee7..7d23d36 100644
--- a/chrome/renderer/render_widget.cc
+++ b/chrome/renderer/render_widget.cc
@@ -76,6 +76,7 @@ DeferredCloses* DeferredCloses::current_ = NULL;
 
 RenderWidget::RenderWidget(RenderThreadBase* render_thread, bool activatable)
     : routing_id_(MSG_ROUTING_NONE),
+      webwidget_(NULL),
       opener_id_(MSG_ROUTING_NONE),
       render_thread_(render_thread),
       host_window_(NULL),
@@ -101,6 +102,7 @@ RenderWidget::RenderWidget(RenderThreadBase* render_thread, bool activatable)
 }
 
 RenderWidget::~RenderWidget() {
+  DCHECK(!webwidget_) << "Leaking our WebWidget!";
   if (current_paint_buf_) {
     RenderProcess::FreeSharedMemory(current_paint_buf_);
     current_paint_buf_ = NULL;
@@ -129,9 +131,7 @@ void RenderWidget::Init(int32 opener_id) {
   if (opener_id != MSG_ROUTING_NONE)
     opener_id_ = opener_id;
 
-  // Avoid a leak here by not assigning, since WebWidget::Create addrefs for us.
-  WebWidget* webwidget = WebWidget::Create(this);
-  webwidget_.swap(&webwidget);
+  webwidget_ = WebWidget::Create(this);
 
   bool result = render_thread_->Send(
       new ViewHostMsg_CreateWidget(opener_id, activatable_, &routing_id_));
diff --git a/chrome/renderer/render_widget.h b/chrome/renderer/render_widget.h
index 56ba30c..4153ce0 100644
--- a/chrome/renderer/render_widget.h
+++ b/chrome/renderer/render_widget.h
@@ -165,7 +165,8 @@ class RenderWidget : public IPC::Channel::Listener,
   // RenderWidgetHost. When MSG_ROUTING_NONE, no messages may be sent.
   int32 routing_id_;
 
-  scoped_refptr<WebWidget> webwidget_;
+  // We are responsible for destroying this object via its Close method.
+  WebWidget* webwidget_;
 
   // Set to the ID of the view that initiated creating this view, if any. When
   // the view was initiated by the browser (the common case), this will be
diff --git a/webkit/glue/inspector_client_impl.cc b/webkit/glue/inspector_client_impl.cc
index c3ac881..f99795b 100644
--- a/webkit/glue/inspector_client_impl.cc
+++ b/webkit/glue/inspector_client_impl.cc
@@ -28,7 +28,7 @@ static const float kDefaultInspectorYPos = 50;
 static const float kDefaultInspectorHeight = 640;
 static const float kDefaultInspectorWidth = 480;
 
-WebInspectorClient::WebInspectorClient(WebView* webView)
+WebInspectorClient::WebInspectorClient(WebViewImpl* webView)
   : inspected_web_view_(webView)
   , inspector_web_view_(0) {
   ASSERT(inspected_web_view_);
@@ -45,7 +45,7 @@ Page* WebInspectorClient::createPage() {
   WebCore::Page* page;
 
   if (inspector_web_view_ != NULL) {
-    page = static_cast<WebViewImpl*>(inspector_web_view_)->page();
+    page = inspector_web_view_->page();
     ASSERT(page != NULL);
     if (page != NULL)
       return page;
@@ -54,17 +54,16 @@ Page* WebInspectorClient::createPage() {
   WebViewDelegate* delegate = inspected_web_view_->GetDelegate();
   if (!delegate)
     return NULL;
-  inspector_web_view_ = delegate->CreateWebView(inspected_web_view_, true);
+  inspector_web_view_ = static_cast<WebViewImpl*>(
+      delegate->CreateWebView(inspected_web_view_, true));
   if (!inspector_web_view_)
     return NULL;
 
   GURL inspector_url(webkit_glue::GetInspectorURL());
   scoped_ptr<WebRequest> request(WebRequest::Create(inspector_url));
-  WebViewImpl* inspector_web_view_impl =
-    static_cast<WebViewImpl*>(inspector_web_view_);
-  inspector_web_view_impl->main_frame()->LoadRequest(request.get());
+  inspector_web_view_->main_frame()->LoadRequest(request.get());
 
-  page = inspector_web_view_impl->page();
+  page = inspector_web_view_->page();
 
   page->chrome()->setToolbarsVisible(false);
   page->chrome()->setStatusbarVisible(false);
@@ -92,31 +91,29 @@ Page* WebInspectorClient::createPage() {
 }
 
 void WebInspectorClient::showWindow() {
-  WebViewImpl* impl = static_cast<WebViewImpl*>(inspected_web_view_.get());
-  InspectorController* inspector = impl->page()->inspectorController();
+  InspectorController* inspector = inspected_web_view_->page()->inspectorController();
   inspector->setWindowVisible(true);
 
   // Notify the webview delegate of how many resources we're inspecting.
-  WebViewDelegate* d = impl->delegate();
+  WebViewDelegate* d = inspected_web_view_->delegate();
   DCHECK(d);
   d->WebInspectorOpened(inspector->resources().size());
 }
 
 void WebInspectorClient::closeWindow() {
   inspector_web_view_ = NULL;
-  WebViewImpl* impl = static_cast<WebViewImpl*>(inspected_web_view_.get());
-  WebFrameImpl* frame = static_cast<WebFrameImpl*>(impl->GetMainFrame());
+  WebFrameImpl* frame = inspected_web_view_->main_frame();
 
   if (frame && frame->inspected_node())
     hideHighlight();
 
-  if (impl->page())
-    impl->page()->inspectorController()->setWindowVisible(false);
+  if (inspected_web_view_->page())
+    inspected_web_view_->page()->inspectorController()->setWindowVisible(false);
 }
 
 bool WebInspectorClient::windowVisible() {
   if (inspector_web_view_ != NULL) {
-    Page* page = static_cast<WebViewImpl*>(inspector_web_view_)->page();
+    Page* page = inspector_web_view_->page();
     ASSERT(page != NULL);
     if (page != NULL)
       return true;
@@ -147,21 +144,19 @@ static void invalidateNodeBoundingRect(WebViewImpl* web_view) {
 }
 
 void WebInspectorClient::highlight(Node* node) {
-  WebViewImpl* web_view = static_cast<WebViewImpl*>(inspected_web_view_.get());
-  WebFrameImpl* frame = static_cast<WebFrameImpl*>(web_view->GetMainFrame());
+  WebFrameImpl* frame = inspected_web_view_->main_frame();
 
   if (frame->inspected_node())
     hideHighlight();
 
-  invalidateNodeBoundingRect(web_view);
+  invalidateNodeBoundingRect(inspected_web_view_);
   frame->selectNodeFromInspector(node);
 }
 
 void WebInspectorClient::hideHighlight() {
-  WebViewImpl* web_view = static_cast<WebViewImpl*>(inspected_web_view_.get());
-  WebFrameImpl* frame = static_cast<WebFrameImpl*>(web_view->GetMainFrame());
+  WebFrameImpl* frame = static_cast<WebFrameImpl*>(inspected_web_view_->GetMainFrame());
 
-  invalidateNodeBoundingRect(web_view);
+  invalidateNodeBoundingRect(inspected_web_view_);
   frame->selectNodeFromInspector(NULL);
 }
 
diff --git a/webkit/glue/inspector_client_impl.h b/webkit/glue/inspector_client_impl.h
index 5d3a817..3f65140 100644
--- a/webkit/glue/inspector_client_impl.h
+++ b/webkit/glue/inspector_client_impl.h
@@ -9,11 +9,11 @@
 #include "base/ref_counted.h"
 
 class WebNodeHighlight;
-class WebView;
+class WebViewImpl;
 
 class WebInspectorClient : public WebCore::InspectorClient {
 public:
-  WebInspectorClient(WebView*);
+  WebInspectorClient(WebViewImpl*);
 
   // InspectorClient
   virtual void inspectorDestroyed();
@@ -43,11 +43,11 @@ public:
 private:
   ~WebInspectorClient();
 
-  // The WebView of the page being inspected; gets passed to the constructor
-  scoped_refptr<WebView> inspected_web_view_;
+  // The WebViewImpl of the page being inspected; gets passed to the constructor
+  scoped_refptr<WebViewImpl> inspected_web_view_;
 
   // The WebView of the Inspector popup window
-  WebView* inspector_web_view_;
+  WebViewImpl* inspector_web_view_;
 };
 
 #endif // WEBKIT_GLUE_INSPECTOR_CLIENT_IMPL_H__
diff --git a/webkit/glue/webframe.h b/webkit/glue/webframe.h
index 830205c..a500bdc 100644
--- a/webkit/glue/webframe.h
+++ b/webkit/glue/webframe.h
@@ -34,10 +34,9 @@ class Size;
 
 // Every frame in a web page is represented by one WebFrame, including the
 // outermost frame.
-class WebFrame : public base::RefCounted<WebFrame> {
+class WebFrame {
  public:
   WebFrame() {}
-  virtual ~WebFrame() {}
 
   // Binds a C++ class to a JavaScript property of the window object.  This
   // should generally be used via CppBoundClass::BindToJavascript() instead of
@@ -371,6 +370,9 @@ class WebFrame : public base::RefCounted<WebFrame> {
   // Only for test_shell
   virtual int PendingFrameUnloadEventCount() const = 0;
 
+ protected:
+  virtual ~WebFrame() {}
+
  private:
   DISALLOW_COPY_AND_ASSIGN(WebFrame);
 };
diff --git a/webkit/glue/webframe_impl.h b/webkit/glue/webframe_impl.h
index c3284ff..3633651 100644
--- a/webkit/glue/webframe_impl.h
+++ b/webkit/glue/webframe_impl.h
@@ -70,7 +70,7 @@ class BitmapPlatformDeviceWin;
 }
 
 // Implementation of WebFrame, note that this is a reference counted object.
-class WebFrameImpl : public WebFrame {
+class WebFrameImpl : public WebFrame, public base::RefCounted<WebFrameImpl> {
  public:
   WebFrameImpl();
   ~WebFrameImpl();
diff --git a/webkit/glue/webview_impl.cc b/webkit/glue/webview_impl.cc
index c2165ea..0259b72 100644
--- a/webkit/glue/webview_impl.cc
+++ b/webkit/glue/webview_impl.cc
@@ -820,6 +820,8 @@ void WebViewImpl::Close() {
       page_->mainFrame()->loader()->frameDetached();
     page_.reset();
   }
+
+  Release();  // Balances AddRef from WebView::Create
 }
 
 WebViewDelegate* WebViewImpl::GetDelegate() {
diff --git a/webkit/glue/webview_impl.h b/webkit/glue/webview_impl.h
index 7ef3686..c511e7b 100644
--- a/webkit/glue/webview_impl.h
+++ b/webkit/glue/webview_impl.h
@@ -44,7 +44,7 @@ class WebMouseEvent;
 class WebMouseWheelEvent;
 class WebViewDelegate;
 
-class WebViewImpl : public WebView {
+class WebViewImpl : public WebView, public base::RefCounted<WebViewImpl> {
  public:
   // WebView
   virtual bool ShouldClose();
@@ -194,6 +194,7 @@ class WebViewImpl : public WebView {
 
  protected:
   friend class WebView;  // So WebView::Create can call our constructor
+  friend class base::RefCounted<WebViewImpl>;
 
   WebViewImpl();
   ~WebViewImpl();
diff --git a/webkit/glue/webwidget.h b/webkit/glue/webwidget.h
index 9d26fb5..0fda0dc 100644
--- a/webkit/glue/webwidget.h
+++ b/webkit/glue/webwidget.h
@@ -16,10 +16,9 @@ class Size;
 class WebInputEvent;
 class WebWidgetDelegate;
 
-class WebWidget : public base::RefCounted<WebWidget> {
+class WebWidget {
  public:
   WebWidget() {}
-  virtual ~WebWidget() {}
 
   // This method creates a WebWidget that is initially invisible and positioned
   // according to the given bounds relative to the specified parent window.
@@ -27,7 +26,7 @@ class WebWidget : public base::RefCounted<WebWidget> {
   // GetViewWindow) once it is ready to have the WebWidget appear on the screen.
   static WebWidget* Create(WebWidgetDelegate* delegate);
 
-  // This method closes the WebWidget.
+  // This method closes and deletes the WebWidget.
   virtual void Close() = 0;
 
   // Called to resize the WebWidget.
@@ -67,6 +66,9 @@ class WebWidget : public base::RefCounted<WebWidget> {
   virtual bool ImeUpdateStatus(bool* enable_ime, const void** node,
                                gfx::Rect* caret_rect) = 0;
 
+ protected:
+  virtual ~WebWidget() {}
+
  private:
   DISALLOW_EVIL_CONSTRUCTORS(WebWidget);
 };
diff --git a/webkit/glue/webwidget_impl.cc b/webkit/glue/webwidget_impl.cc
index bf5aaf4..aff0d1d 100644
--- a/webkit/glue/webwidget_impl.cc
+++ b/webkit/glue/webwidget_impl.cc
@@ -100,6 +100,8 @@ void WebWidgetImpl::Close() {
     widget_->hide();
 
   delegate_ = NULL;
+
+  Release();  // Balances AddRef from WebWidget::Create
 }
 
 void WebWidgetImpl::Resize(const gfx::Size& new_size) {
diff --git a/webkit/glue/webwidget_impl.h b/webkit/glue/webwidget_impl.h
index d3d9b06..7a67044 100644
--- a/webkit/glue/webwidget_impl.h
+++ b/webkit/glue/webwidget_impl.h
@@ -30,7 +30,8 @@ class WebMouseWheelEvent;
 class WebWidgetDelegate;
 
 class WebWidgetImpl : public WebWidget,
-                      public WebCore::FramelessScrollViewClient {
+                      public WebCore::FramelessScrollViewClient,
+                      public base::RefCounted<WebWidgetImpl> {
  public:
   // WebWidget
   virtual void Close();
@@ -69,6 +70,7 @@ class WebWidgetImpl : public WebWidget,
 
  protected:
   friend class WebWidget;  // So WebWidget::Create can call our constructor
+  friend class base::RefCounted<WebWidgetImpl>;
 
   WebWidgetImpl(WebWidgetDelegate* delegate);
   ~WebWidgetImpl();
diff --git a/webkit/tools/test_shell/mac/webwidget_host.mm b/webkit/tools/test_shell/mac/webwidget_host.mm
index bedb9a2..dc1b6c6 100644
--- a/webkit/tools/test_shell/mac/webwidget_host.mm
+++ b/webkit/tools/test_shell/mac/webwidget_host.mm
@@ -136,7 +136,6 @@ WebWidgetHost::~WebWidgetHost() {
   TrackMouseLeave(false);
 
   webwidget_->Close();
-  webwidget_->Release();
 }
 
 void WebWidgetHost::UpdatePaintRect(const gfx::Rect& rect) {
diff --git a/webkit/tools/test_shell/webwidget_host_gtk.cc b/webkit/tools/test_shell/webwidget_host_gtk.cc
index 19f25e3..c3b5cb6 100644
--- a/webkit/tools/test_shell/webwidget_host_gtk.cc
+++ b/webkit/tools/test_shell/webwidget_host_gtk.cc
@@ -285,7 +285,6 @@ WebWidgetHost::WebWidgetHost()
 
 WebWidgetHost::~WebWidgetHost() {
   webwidget_->Close();
-  webwidget_->Release();
 }
 
 void WebWidgetHost::Resize(const gfx::Size &newsize) {
diff --git a/webkit/tools/test_shell/webwidget_host_win.cc b/webkit/tools/test_shell/webwidget_host_win.cc
index 7fd45ca..9fcb96b 100644
--- a/webkit/tools/test_shell/webwidget_host_win.cc
+++ b/webkit/tools/test_shell/webwidget_host_win.cc
@@ -186,7 +186,6 @@ WebWidgetHost::~WebWidgetHost() {
   TrackMouseLeave(false);
 
   webwidget_->Close();
-  webwidget_->Release();
 }
 
 bool WebWidgetHost::WndProc(UINT message, WPARAM wparam, LPARAM lparam) {