Invent WebFrameImpl::ClientHandle as a weak reference to WebFrameClient.

Unfortunately, I can't use base::WeakPtr for this since this code will all move
into webkit/api shortly.

R=dglazkov
BUG=10034
TEST=none

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

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

10 files changed, 90 insertions, 67 deletions

diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index a6c8cb5..b0e5ba0 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -1186,18 +1186,6 @@ void RenderView::BindDOMAutomationController(WebFrame* frame) {
                                               L"domAutomationController");
 }
 
-void RenderView::DidCreateScriptContextForFrame(WebFrame* webframe) {
-  EventBindings::HandleContextCreated(webframe, false);
-}
-
-void RenderView::DidDestroyScriptContextForFrame(WebFrame* webframe) {
-  EventBindings::HandleContextDestroyed(webframe);
-}
-
-void RenderView::DidCreateIsolatedScriptContext(WebFrame* webframe) {
-  EventBindings::HandleContextCreated(webframe, true);
-}
-
 bool RenderView::RunJavaScriptMessage(int type,
                                       const std::wstring& message,
                                       const std::wstring& default_value,
@@ -2327,6 +2315,18 @@ void RenderView::didExhaustMemoryAvailableForScript(WebFrame* frame) {
   Send(new ViewHostMsg_JSOutOfMemory(routing_id_));
 }
 
+void RenderView::didCreateScriptContext(WebFrame* frame) {
+  EventBindings::HandleContextCreated(frame, false);
+}
+
+void RenderView::didDestroyScriptContext(WebFrame* frame) {
+  EventBindings::HandleContextDestroyed(frame);
+}
+
+void RenderView::didCreateIsolatedScriptContext(WebFrame* frame) {
+  EventBindings::HandleContextCreated(frame, true);
+}
+
 void RenderView::didChangeContentsSize(WebFrame* frame, const WebSize& size) {
   // We don't always want to send the change messages over IPC, only if we've
   // be put in that mode by getting a |ViewMsg_EnableIntrinsicWidthChangedMode|
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index 42237b8..e11a8e1 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -169,9 +169,6 @@ class RenderView : public RenderWidget,
       const WebKit::WebURLError& error,
       const std::string& html,
       bool replace);
-  virtual void DidCreateScriptContextForFrame(WebKit::WebFrame* webframe);
-  virtual void DidDestroyScriptContextForFrame(WebKit::WebFrame* webframe);
-  virtual void DidCreateIsolatedScriptContext(WebKit::WebFrame* webframe);
   virtual void OnMissingPluginStatus(
       WebPluginDelegateProxy* delegate,
       int status);
@@ -349,6 +346,9 @@ class RenderView : public RenderWidget,
   virtual void didRunInsecureContent(
       WebKit::WebFrame* frame, const WebKit::WebSecurityOrigin& origin);
   virtual void didExhaustMemoryAvailableForScript(WebKit::WebFrame* frame);
+  virtual void didCreateScriptContext(WebKit::WebFrame* frame);
+  virtual void didDestroyScriptContext(WebKit::WebFrame* frame);
+  virtual void didCreateIsolatedScriptContext(WebKit::WebFrame* frame);
   virtual void didChangeContentsSize(
       WebKit::WebFrame* frame, const WebKit::WebSize& size);
   virtual void reportFindInPageMatchCount(
diff --git a/webkit/api/public/WebFrameClient.h b/webkit/api/public/WebFrameClient.h
index 6ef6e2d..24701e5 100644
--- a/webkit/api/public/WebFrameClient.h
+++ b/webkit/api/public/WebFrameClient.h
@@ -215,6 +215,18 @@ namespace WebKit {
         // Script in the page tried to allocate too much memory.
         virtual void didExhaustMemoryAvailableForScript(WebFrame*) = 0;
 
+        // Notifies that a new script context has been created for this frame.
+        // This is similar to didClearWindowObject but only called once per
+        // frame context.
+        virtual void didCreateScriptContext(WebFrame*) = 0;
+
+        // Notifies that this frame's script context has been destroyed.
+        virtual void didDestroyScriptContext(WebFrame*) = 0;
+
+        // Notifies that a garbage-collected context was created - content
+        // scripts.
+        virtual void didCreateIsolatedScriptContext(WebFrame*) = 0;
+
 
         // Geometry notifications ----------------------------------------------
 
diff --git a/webkit/glue/empty_webframeclient.h b/webkit/glue/empty_webframeclient.h
index 70c266d..b36f91b 100644
--- a/webkit/glue/empty_webframeclient.h
+++ b/webkit/glue/empty_webframeclient.h
@@ -84,6 +84,9 @@ class EmptyWebFrameClient : public WebKit::WebFrameClient {
   virtual void didRunInsecureContent(
       WebKit::WebFrame* frame, const WebKit::WebSecurityOrigin& origin) {}
   virtual void didExhaustMemoryAvailableForScript(WebKit::WebFrame* frame) {}
+  virtual void didCreateScriptContext(WebKit::WebFrame* frame) {}
+  virtual void didDestroyScriptContext(WebKit::WebFrame* frame) {}
+  virtual void didCreateIsolatedScriptContext(WebKit::WebFrame* frame) {}
   virtual void didChangeContentsSize(
       WebKit::WebFrame* frame, const WebKit::WebSize& size) {}
   virtual void reportFindInPageMatchCount(
diff --git a/webkit/glue/webframe_impl.cc b/webkit/glue/webframe_impl.cc
index 3172952..7fd7ce9 100644
--- a/webkit/glue/webframe_impl.cc
+++ b/webkit/glue/webframe_impl.cc
@@ -1434,8 +1434,8 @@ void WebFrameImpl::increaseMatchCount(int count, int request_id) {
   total_matchcount_ += count;
 
   // Update the UI with the latest findings.
-  if (client_) {
-    client_->reportFindInPageMatchCount(
+  if (client()) {
+    client()->reportFindInPageMatchCount(
         request_id, total_matchcount_, frames_scoping_count_ == 0);
   }
 }
@@ -1444,8 +1444,8 @@ void WebFrameImpl::ReportFindInPageSelection(const WebRect& selection_rect,
                                              int active_match_ordinal,
                                              int request_id) {
   // Update the UI with the latest selection rect.
-  if (client_) {
-    client_->reportFindInPageSelection(
+  if (client()) {
+    client()->reportFindInPageSelection(
         request_id, OrdinalOfFirstMatchForFrame(this) + active_match_ordinal,
         selection_rect);
   }
@@ -1482,10 +1482,14 @@ WebString WebFrameImpl::contentAsMarkup() const {
 
 int WebFrameImpl::live_object_count_ = 0;
 
-WebFrameImpl::WebFrameImpl(WebFrameClient* client)
+PassRefPtr<WebFrameImpl> WebFrameImpl::create(WebFrameClient* client) {
+  return adoptRef(new WebFrameImpl(ClientHandle::create(client)));
+}
+
+WebFrameImpl::WebFrameImpl(PassRefPtr<ClientHandle> client_handle)
   : ALLOW_THIS_IN_INITIALIZER_LIST(frame_loader_client_(this)),
     ALLOW_THIS_IN_INITIALIZER_LIST(scope_matches_factory_(this)),
-    client_(client),
+    client_handle_(client_handle),
     active_match_frame_(NULL),
     active_match_index_(-1),
     locating_active_rect_(false),
@@ -1514,7 +1518,7 @@ void WebFrameImpl::InitMainFrame(WebViewImpl* webview_impl) {
 
   // Add reference on behalf of FrameLoader.  See comments in
   // WebFrameLoaderClient::frameLoaderDestroyed for more info.
-  AddRef();
+  ref();
 
   // We must call init() after frame_ is assigned because it is referenced
   // during init().
@@ -1525,12 +1529,12 @@ PassRefPtr<Frame> WebFrameImpl::CreateChildFrame(
     const FrameLoadRequest& request, HTMLFrameOwnerElement* owner_element) {
   // TODO(darin): share code with initWithName()
 
-  scoped_refptr<WebFrameImpl> webframe = new WebFrameImpl(client());
+  RefPtr<WebFrameImpl> webframe(adoptRef(new WebFrameImpl(client_handle_)));
 
   // Add an extra ref on behalf of the Frame/FrameLoader, which references the
   // WebFrame via the FrameLoaderClient interface. See the comment at the top
   // of this file for more info.
-  webframe->AddRef();
+  webframe->ref();
 
   RefPtr<Frame> child_frame = Frame::create(
       frame_->page(), owner_element, &webframe->frame_loader_client_);
@@ -1712,14 +1716,14 @@ void WebFrameImpl::SetFindEndstateFocusAndSelection() {
 }
 
 void WebFrameImpl::DidFail(const ResourceError& error, bool was_provisional) {
-  if (!client_)
+  if (!client())
     return;
   const WebURLError& web_error =
       webkit_glue::ResourceErrorToWebURLError(error);
   if (was_provisional) {
-    client_->didFailProvisionalLoad(this, web_error);
+    client()->didFailProvisionalLoad(this, web_error);
   } else {
-    client_->didFailLoad(this, web_error);
+    client()->didFailLoad(this, web_error);
   }
 }
 
diff --git a/webkit/glue/webframe_impl.h b/webkit/glue/webframe_impl.h
index 773422c..b4cff81 100644
--- a/webkit/glue/webframe_impl.h
+++ b/webkit/glue/webframe_impl.h
@@ -37,6 +37,7 @@ MSVC_PUSH_WARNING_LEVEL(0);
 #include "ResourceHandleClient.h"
 #include "Frame.h"
 #include "PlatformString.h"
+#include <wtf/RefCounted.h>
 MSVC_POP_WARNING();
 
 class ChromePrintContext;
@@ -64,8 +65,7 @@ class WebFrameClient;
 }
 
 // Implementation of WebFrame, note that this is a reference counted object.
-class WebFrameImpl : public WebKit::WebFrame,
-                     public base::RefCounted<WebFrameImpl> {
+class WebFrameImpl : public WebKit::WebFrame, public RefCounted<WebFrameImpl> {
  public:
   // WebFrame methods:
   virtual WebKit::WebString name() const;
@@ -169,7 +169,7 @@ class WebFrameImpl : public WebKit::WebFrame,
   virtual WebKit::WebString contentAsText(size_t max_chars) const;
   virtual WebKit::WebString contentAsMarkup() const;
 
-  WebFrameImpl(WebKit::WebFrameClient* client);
+  static PassRefPtr<WebFrameImpl> create(WebKit::WebFrameClient* client);
   ~WebFrameImpl();
 
   static int live_object_count() {
@@ -242,12 +242,29 @@ class WebFrameImpl : public WebKit::WebFrame,
   webkit_glue::PasswordAutocompleteListener* GetPasswordListener(
       WebCore::HTMLInputElement* user_name_input_element);
 
-  WebKit::WebFrameClient* client() const { return client_; }
-  void drop_client() { client_ = NULL; }
+  WebKit::WebFrameClient* client() const { return client_handle_->client(); }
+  void drop_client() { client_handle_->drop_client(); }
 
  protected:
   friend class WebFrameLoaderClient;
 
+  // A weak reference to the WebFrameClient.  Each WebFrame in the hierarchy
+  // owns a reference to a ClientHandle.  When the main frame is destroyed, it
+  // clears the WebFrameClient.
+  class ClientHandle : public RefCounted<ClientHandle> {
+   public:
+    static PassRefPtr<ClientHandle> create(WebKit::WebFrameClient* client) {
+      return adoptRef(new ClientHandle(client));
+    }
+    WebKit::WebFrameClient* client() { return client_; }
+    void drop_client() { client_ = NULL; }
+   private:
+    ClientHandle(WebKit::WebFrameClient* client) : client_(client) {}
+    WebKit::WebFrameClient* client_;
+  };
+
+  WebFrameImpl(PassRefPtr<ClientHandle> client_handle);
+
   // Informs the WebFrame that the Frame is being closed, called by the
   // WebFrameLoaderClient
   void Closing();
@@ -261,7 +278,7 @@ class WebFrameImpl : public WebKit::WebFrame,
   // asynchronously in order to scope string matches during a find operation.
   ScopedRunnableMethodFactory<WebFrameImpl> scope_matches_factory_;
 
-  WebKit::WebFrameClient* client_;
+  RefPtr<ClientHandle> client_handle_;
 
   // This is a weak pointer to our corresponding WebCore frame.  A reference to
   // ourselves is held while frame_ is valid.  See our Closing method.
diff --git a/webkit/glue/webframeloaderclient_impl.cc b/webkit/glue/webframeloaderclient_impl.cc
index ea6b62e..4343fd8 100644
--- a/webkit/glue/webframeloaderclient_impl.cc
+++ b/webkit/glue/webframeloaderclient_impl.cc
@@ -108,7 +108,7 @@ void WebFrameLoaderClient::frameLoaderDestroyed() {
   // FrameLoader calls this method when it's going away.  Therefore, we balance
   // out that extra reference, which may cause 'this' to be deleted.
   webframe_->Closing();
-  webframe_->Release();
+  webframe_->deref();
 }
 
 void WebFrameLoaderClient::windowObjectCleared() {
@@ -129,24 +129,18 @@ void WebFrameLoaderClient::documentElementAvailable() {
 }
 
 void WebFrameLoaderClient::didCreateScriptContextForFrame() {
-  WebViewImpl* webview = webframe_->GetWebViewImpl();
-  WebViewDelegate* d = webview->delegate();
-  if (d)
-    d->DidCreateScriptContextForFrame(webframe_);
+  if (webframe_->client())
+    webframe_->client()->didCreateScriptContext(webframe_);
 }
 
 void WebFrameLoaderClient::didDestroyScriptContextForFrame() {
-  WebViewImpl* webview = webframe_->GetWebViewImpl();
-  WebViewDelegate* d = webview->delegate();
-  if (d)
-    d->DidDestroyScriptContextForFrame(webframe_);
+  if (webframe_->client())
+    webframe_->client()->didDestroyScriptContext(webframe_);
 }
 
 void WebFrameLoaderClient::didCreateIsolatedScriptContext() {
-  WebViewImpl* webview = webframe_->GetWebViewImpl();
-  WebViewDelegate* d = webview->delegate();
-  if (d)
-    d->DidCreateIsolatedScriptContext(webframe_);
+  if (webframe_->client())
+    webframe_->client()->didCreateIsolatedScriptContext(webframe_);
 }
 
 void WebFrameLoaderClient::didPerformFirstNavigation() const {
@@ -197,9 +191,6 @@ void WebFrameLoaderClient::detachedFromParent3() {
   // go to a page and then navigate to a new page without getting any asserts
   // or crashes.
   webframe_->frame()->script()->proxy()->clearForClose();
-
-  // Drop any reference to the client since it may shortly become invalid.
-  webframe_->drop_client();
 }
 
 // This function is responsible for associating the |identifier| with a given
diff --git a/webkit/glue/webview_delegate.h b/webkit/glue/webview_delegate.h
index 5ad7b09..7eff757 100644
--- a/webkit/glue/webview_delegate.h
+++ b/webkit/glue/webview_delegate.h
@@ -70,22 +70,6 @@ class WebViewDelegate : public WebKit::WebViewClient {
   virtual void FocusAccessibilityObject(WebCore::AccessibilityObject* acc_obj) {
   }
 
-  // FrameLoaderClient -------------------------------------------------------
-
-  // Notifies that a new script context has been created for this frame.
-  // This is similar to WindowObjectCleared but only called once per frame
-  // context.
-  virtual void DidCreateScriptContextForFrame(WebKit::WebFrame* webframe) {
-  }
-
-  // Notifies that this frame's script context has been destroyed.
-  virtual void DidDestroyScriptContextForFrame(WebKit::WebFrame* webframe) {
-  }
-
-  // Notifies that a garbage-collected context was created - content scripts.
-  virtual void DidCreateIsolatedScriptContext(WebKit::WebFrame* webframe) {
-  }
-
   // ChromeClient ------------------------------------------------------------
 
   // Queries the browser for suggestions to be shown for the form text field
diff --git a/webkit/glue/webview_impl.cc b/webkit/glue/webview_impl.cc
index 6e98ccc..78a15cef 100644
--- a/webkit/glue/webview_impl.cc
+++ b/webkit/glue/webview_impl.cc
@@ -346,7 +346,7 @@ WebView* WebView::Create(WebViewDelegate* delegate) {
 void WebViewImpl::initializeMainFrame(WebFrameClient* frame_client) {
   // NOTE: The WebFrameImpl takes a reference to itself within InitMainFrame
   // and releases that reference once the corresponding Frame is destroyed.
-  scoped_refptr<WebFrameImpl> main_frame = new WebFrameImpl(frame_client);
+  RefPtr<WebFrameImpl> main_frame = WebFrameImpl::create(frame_client);
 
   main_frame->InitMainFrame(this);
 
@@ -942,11 +942,15 @@ WebViewImpl* WebViewImpl::FromPage(WebCore::Page* page) {
 // WebWidget ------------------------------------------------------------------
 
 void WebViewImpl::close() {
+  RefPtr<WebFrameImpl> main_frame;
+
   if (page_.get()) {
     // Initiate shutdown for the entire frameset.  This will cause a lot of
     // notifications to be sent.
-    if (page_->mainFrame())
+    if (page_->mainFrame()) {
+      main_frame = WebFrameImpl::FromFrame(page_->mainFrame());
       page_->mainFrame()->loader()->frameDetached();
+    }
     page_.reset();
   }
 
@@ -954,6 +958,11 @@ void WebViewImpl::close() {
   if (devtools_agent_.get())
     devtools_agent_.reset(NULL);
 
+  // We drop the client after the page has been destroyed to support the
+  // WebFrameClient::didDestroyScriptContext method.
+  if (main_frame)
+    main_frame->drop_client();
+
   // Reset the delegate to prevent notifications being sent as we're being
   // deleted.
   delegate_ = NULL;
diff --git a/webkit/tools/test_shell/test_webview_delegate.h b/webkit/tools/test_shell/test_webview_delegate.h
index 68a6b46..b7e62d4 100644
--- a/webkit/tools/test_shell/test_webview_delegate.h
+++ b/webkit/tools/test_shell/test_webview_delegate.h
@@ -234,6 +234,9 @@ class TestWebViewDelegate : public WebViewDelegate,
   virtual void didRunInsecureContent(
       WebKit::WebFrame* frame, const WebKit::WebSecurityOrigin& origin);
   virtual void didExhaustMemoryAvailableForScript(WebKit::WebFrame*) {}
+  virtual void didCreateScriptContext(WebKit::WebFrame* frame) {}
+  virtual void didDestroyScriptContext(WebKit::WebFrame* frame) {}
+  virtual void didCreateIsolatedScriptContext(WebKit::WebFrame* frame) {}
   virtual void didChangeContentsSize(
       WebKit::WebFrame*, const WebKit::WebSize&) {}
   virtual void reportFindInPageMatchCount(