Eliminate WebView::GetDelegate and replace RenderViewSet with a map from

WebView to RenderView.

R=mpcomplete
BUG=10033
TEST=none

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

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

11 files changed, 145 insertions, 85 deletions

diff --git a/chrome/renderer/extensions/bindings_utils.cc b/chrome/renderer/extensions/bindings_utils.cc
index ddccf2c..42d3e7c 100644
--- a/chrome/renderer/extensions/bindings_utils.cc
+++ b/chrome/renderer/extensions/bindings_utils.cc
@@ -106,7 +106,7 @@ RenderView* GetRenderViewForCurrentContext() {
   if (!webview)
     return NULL;  // can happen during closing
 
-  RenderView* renderview = static_cast<RenderView*>(webview->GetDelegate());
+  RenderView* renderview = RenderView::FromWebView(webview);
   DCHECK(renderview) << "Encountered a WebView without a WebViewDelegate";
   return renderview;
 }
diff --git a/chrome/renderer/extensions/event_bindings.cc b/chrome/renderer/extensions/event_bindings.cc
index 446cacb..e23c060 100644
--- a/chrome/renderer/extensions/event_bindings.cc
+++ b/chrome/renderer/extensions/event_bindings.cc
@@ -253,8 +253,8 @@ void EventBindings::HandleContextCreated(WebFrame* frame, bool content_script) {
   }
 
   RenderView* render_view = NULL;
-  if (frame->view() && frame->view()->GetDelegate())
-    render_view = static_cast<RenderView*>(frame->view()->GetDelegate());
+  if (frame->view())
+    render_view = RenderView::FromWebView(frame->view());
 
   contexts.push_back(linked_ptr<ContextInfo>(
       new ContextInfo(persistent_context, extension_id, parent_context,
@@ -298,7 +298,7 @@ void EventBindings::CallFunction(const std::string& function_name,
       continue;
     v8::Handle<v8::Value> retval = CallFunctionInContext((*it)->context,
         function_name, argc, argv);
-    // In debug, the js will validate the event parameters and return a 
+    // In debug, the js will validate the event parameters and return a
     // string if a validation error has occured.
     // TODO(rafaelw): Consider only doing this check if function_name ==
     // "Event.dispatchJSON".
diff --git a/chrome/renderer/extensions/extension_process_bindings.cc b/chrome/renderer/extensions/extension_process_bindings.cc
index a409672..197bb42 100644
--- a/chrome/renderer/extensions/extension_process_bindings.cc
+++ b/chrome/renderer/extensions/extension_process_bindings.cc
@@ -98,6 +98,76 @@ static L10nMessagesMap* GetL10nMessagesMap(const std::string extension_id) {
   }
 }
 
+// Used to accumulate the list of views associated with an extension.
+class ExtensionViewAccumulator : public RenderViewVisitor {
+ public:
+  ExtensionViewAccumulator(const std::string& extension_id,
+                           int browser_window_id,
+                           ViewType::Type view_type)
+      : extension_id_(extension_id),
+        browser_window_id_(browser_window_id),
+        view_type_(view_type),
+        views_(v8::Array::New()),
+        index_(0) {
+  }
+
+  virtual bool Visit(RenderView* render_view) {
+    if (!ViewTypeMatches(render_view->view_type(), view_type_))
+      return true;
+
+    GURL url = render_view->webview()->mainFrame()->url();
+    if (!url.SchemeIs(chrome::kExtensionScheme))
+      return true;
+    const std::string& extension_id = url.host();
+    if (extension_id != extension_id_)
+      return true;
+
+    if (browser_window_id_ != -1 &&
+        render_view->browser_window_id() != browser_window_id_)
+      return true;
+
+    v8::Local<v8::Context> context =
+        render_view->webview()->mainFrame()->mainWorldScriptContext();
+    if (!context.IsEmpty()) {
+      v8::Local<v8::Value> window = context->Global();
+      DCHECK(!window.IsEmpty());
+      views_->Set(v8::Integer::New(index_), window);
+      index_++;
+      if (view_type_ == ViewType::EXTENSION_BACKGROUND_PAGE)
+        return false;  // There can be only one...
+    }
+    return true;
+  }
+
+  v8::Local<v8::Array> views() { return views_; }
+
+ private:
+  // Returns true is |type| "isa" |match|.
+  static bool ViewTypeMatches(ViewType::Type type, ViewType::Type match) {
+    if (type == match)
+      return true;
+
+    // INVALID means match all.
+    if (match == ViewType::INVALID)
+      return true;
+
+    // TODO(erikkay) for now, special case mole as a type of toolstrip.
+    // Perhaps this isn't the right long-term thing to do.
+    if (match == ViewType::EXTENSION_TOOLSTRIP &&
+        type == ViewType::EXTENSION_MOLE) {
+      return true;
+    }
+
+    return false;
+  }
+
+  std::string extension_id_;
+  int browser_window_id_;
+  ViewType::Type view_type_;
+  v8::Local<v8::Array> views_;
+  int index_;
+};
+
 class ExtensionImpl : public ExtensionBase {
  public:
   ExtensionImpl() : ExtensionBase(
@@ -154,25 +224,6 @@ class ExtensionImpl : public ExtensionBase {
     return v8::String::New(GetStringResource<IDR_EXTENSION_API_JSON>());
   }
 
-  // Returns true is |type| "isa" |match|.
-  static bool ViewTypeMatches(ViewType::Type type, ViewType::Type match) {
-    if (type == match)
-      return true;
-
-    // INVALID means match all.
-    if (match == ViewType::INVALID)
-      return true;
-
-    // TODO(erikkay) for now, special case mole as a type of toolstrip.
-    // Perhaps this isn't the right long-term thing to do.
-    if (match == ViewType::EXTENSION_TOOLSTRIP &&
-        type == ViewType::EXTENSION_MOLE) {
-      return true;
-    }
-
-    return false;
-  }
-
   static v8::Handle<v8::Value> GetExtensionViews(const v8::Arguments& args) {
     if (args.Length() != 2)
       return v8::Undefined();
@@ -199,44 +250,10 @@ class ExtensionImpl : public ExtensionBase {
       return v8::Undefined();
     }
 
-    v8::Local<v8::Array> views = v8::Array::New();
-    int index = 0;
-    RenderView::RenderViewSet* render_view_set_pointer =
-        Singleton<RenderView::RenderViewSet>::get();
-    DCHECK(render_view_set_pointer->render_view_set_.size() > 0);
-
-    v8::Local<v8::Value> window;
-    std::string current_extension_id = ExtensionIdForCurrentContext();
-    std::set<RenderView* >::iterator it =
-        render_view_set_pointer->render_view_set_.begin();
-    for (; it != render_view_set_pointer->render_view_set_.end(); ++it) {
-      if (!ViewTypeMatches((*it)->view_type(), view_type))
-        continue;
-
-      GURL url = (*it)->webview()->mainFrame()->url();
-      if (!url.SchemeIs(chrome::kExtensionScheme))
-        continue;
-      std::string extension_id = url.host();
-      if (extension_id != current_extension_id)
-        continue;
-
-      if (browser_window_id != -1 &&
-          (*it)->browser_window_id() != browser_window_id) {
-        continue;
-      }
-
-      v8::Local<v8::Context> context =
-          (*it)->webview()->mainFrame()->mainWorldScriptContext();
-      if (!context.IsEmpty()) {
-        v8::Local<v8::Value> window = context->Global();
-        DCHECK(!window.IsEmpty());
-        views->Set(v8::Integer::New(index), window);
-        index++;
-        if (view_type == ViewType::EXTENSION_BACKGROUND_PAGE)
-          break;
-      }
-    }
-    return views;
+    ExtensionViewAccumulator accumulator(
+        ExtensionIdForCurrentContext(), browser_window_id, view_type);
+    RenderView::ForEach(&accumulator);
+    return accumulator.views();
   }
 
   static v8::Handle<v8::Value> GetNextRequestId(const v8::Arguments& args) {
diff --git a/chrome/renderer/external_extension.cc b/chrome/renderer/external_extension.cc
index 777e883..45585b9 100644
--- a/chrome/renderer/external_extension.cc
+++ b/chrome/renderer/external_extension.cc
@@ -45,7 +45,7 @@ class ExternalExtensionWrapper : public v8::Extension {
       WebView* webview = webframe->view();
       if (!webview) return v8::Undefined();  // can happen during closing
 
-      RenderView* renderview = static_cast<RenderView*>(webview->GetDelegate());
+      RenderView* renderview = RenderView::FromWebView(webview);
       if (!renderview) return v8::Undefined();
 
       std::string name = std::string(*v8::String::Utf8Value(args[0]));
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 7c626ad..de7a051 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -179,10 +179,6 @@ static const int kDelayForForcedCaptureMs = 6000;
 // that variable for more.
 const int kDefaultDelaySecondsForFormStateSync = 5;
 
-// The next available page ID to use. This ensures that the page IDs are
-// globally unique in the renderer.
-static int32 next_page_id_ = 1;
-
 // The maximum number of popups that can be spawned from one page.
 static const int kMaximumNumberOfUnacknowledgedPopups = 25;
 
@@ -201,6 +197,8 @@ static void GetRedirectChain(WebDataSource* ds, std::vector<GURL>* result) {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+int32 RenderView::next_page_id_ = 1;
+
 RenderView::RenderView(RenderThreadBase* render_thread,
                        const WebPreferences& webkit_preferences)
     : RenderWidget(render_thread, true),
@@ -235,11 +233,9 @@ RenderView::RenderView(RenderThreadBase* render_thread,
 #endif
       document_tag_(0),
       webkit_preferences_(webkit_preferences) {
-  Singleton<RenderViewSet>()->render_view_set_.insert(this);
 }
 
 RenderView::~RenderView() {
-  Singleton<RenderViewSet>()->render_view_set_.erase(this);
   if (decrement_shared_popup_at_destruction_)
     shared_popup_counter_->data--;
 
@@ -261,6 +257,29 @@ RenderView::~RenderView() {
 
   render_thread_->RemoveFilter(audio_message_filter_);
   render_thread_->RemoveFilter(notification_provider_.get());
+
+#ifndef NDEBUG
+  // Make sure we are no longer referenced by the ViewMap.
+  ViewMap* views = Singleton<ViewMap>::get();
+  for (ViewMap::iterator it = views->begin(); it != views->end(); ++it)
+    DCHECK_NE(this, it->second) << "Failed to call Close?";
+#endif
+}
+
+/*static*/
+void RenderView::ForEach(RenderViewVisitor* visitor) {
+  ViewMap* views = Singleton<ViewMap>::get();
+  for (ViewMap::iterator it = views->begin(); it != views->end(); ++it) {
+    if (!visitor->Visit(it->second))
+      return;
+  }
+}
+
+/*static*/
+RenderView* RenderView::FromWebView(WebView* webview) {
+  ViewMap* views = Singleton<ViewMap>::get();
+  ViewMap::iterator it = views->find(webview);
+  return it == views->end() ? NULL : it->second;
 }
 
 /*static*/
@@ -324,6 +343,7 @@ void RenderView::Init(gfx::NativeViewId parent_hwnd,
   notification_provider_ = new NotificationProvider(this);
 
   webwidget_ = WebView::Create(this);
+  Singleton<ViewMap>::get()->insert(std::make_pair(webview(), this));
   webkit_preferences_.Apply(webview());
   webview()->initializeMainFrame(this);
 
@@ -3616,6 +3636,13 @@ void RenderView::OnExecuteCode(int request_id, const std::string& extension_id,
   Send(new ViewMsg_ExecuteCodeFinished(routing_id_, request_id, true));
 }
 
+void RenderView::Close() {
+  // We need to grab a pointer to the doomed WebView before we destroy it.
+  WebView* doomed = webview();
+  RenderWidget::Close();
+  Singleton<ViewMap>::get()->erase(doomed);
+}
+
 void RenderView::DidHandleKeyEvent() {
   edit_commands_.clear();
 }
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index e11a8e1..babe495 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_RENDERER_RENDER_VIEW_H_
 #define CHROME_RENDERER_RENDER_VIEW_H_
 
+#include <map>
 #include <set>
 #include <string>
 #include <queue>
@@ -31,6 +32,7 @@
 #include "chrome/renderer/external_host_bindings.h"
 #include "chrome/renderer/notification_provider.h"
 #include "chrome/renderer/render_widget.h"
+#include "chrome/renderer/render_view_visitor.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"
 #include "webkit/api/public/WebConsoleMessage.h"
@@ -111,9 +113,12 @@ class RenderView : public RenderWidget,
                    public webkit_glue::DomSerializerDelegate,
                    public base::SupportsWeakPtr<RenderView> {
  public:
-  struct RenderViewSet {
-    std::set<RenderView*> render_view_set_;
-  };
+  // Visit all RenderViews with a live WebView (i.e., RenderViews that have
+  // been closed but not yet destroyed are excluded).
+  static void ForEach(RenderViewVisitor* visitor);
+
+  // Returns the RenderView containing the given WebView.
+  static RenderView* FromWebView(WebView* webview);
 
   // Creates a new RenderView.  The parent_hwnd specifies a HWND to use as the
   // parent of the WebView HWND that will be created.  If this is a constrained
@@ -431,12 +436,11 @@ class RenderView : public RenderWidget,
   bool SendAndRunNestedMessageLoop(IPC::SyncMessage* message);
 
  protected:
-  // RenderWidget override.
+  // RenderWidget overrides:
+  virtual void Close();
   virtual void OnResize(const gfx::Size& new_size,
                         const gfx::Rect& resizer_rect);
-  // RenderWidget override
   virtual void DidPaint();
-  // RenderWidget override.
   virtual void DidHandleKeyEvent();
 
  private:
@@ -810,6 +814,10 @@ class RenderView : public RenderWidget,
   // same page twice in a row.
   int32 last_indexed_page_id_;
 
+  // The next available page ID to use. This ensures that the page IDs are
+  // globally unique in the renderer.
+  static int32 next_page_id_;
+
   // Used for popups.
   bool opened_by_user_gesture_;
   GURL creator_url_;
@@ -982,6 +990,7 @@ class RenderView : public RenderWidget,
   typedef std::set<webkit_glue::ImageResourceFetcher*> ImageResourceFetcherSet;
   ImageResourceFetcherSet image_fetchers_;
 
+  typedef std::map<WebView*, RenderView*> ViewMap;
 
   DISALLOW_COPY_AND_ASSIGN(RenderView);
 };
diff --git a/chrome/renderer/render_view_visitor.h b/chrome/renderer/render_view_visitor.h
new file mode 100644
index 0000000..4793ed2
--- /dev/null
+++ b/chrome/renderer/render_view_visitor.h
@@ -0,0 +1,17 @@
+// Copyright (c) 2009 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.
+
+#ifndef CHROME_RENDERER_RENDER_VIEW_VISITOR_H_
+#define CHROME_RENDERER_RENDER_VIEW_VISITOR_H_
+
+class RenderViewVisitor {
+ public:
+  // Return true to continue visiting RenderViews or false to stop.
+  virtual bool Visit(RenderView* render_view) = 0;
+
+ protected:
+  ~RenderViewVisitor() {}
+};
+
+#endif  // CHROME_RENDERER_RENDER_VIEW_VISITOR_H_
diff --git a/chrome/renderer/render_widget.h b/chrome/renderer/render_widget.h
index 100aee0..3df6147 100644
--- a/chrome/renderer/render_widget.h
+++ b/chrome/renderer/render_widget.h
@@ -97,7 +97,7 @@ class RenderWidget : public IPC::Channel::Listener,
   void GenerateFullRepaint();
 
   // Close the underlying WebWidget.
-  void Close();
+  virtual void Close();
 
  protected:
   // Friend RefCounted so that the dtor can be non-public. Using this class
diff --git a/webkit/glue/webview.h b/webkit/glue/webview.h
index c5c299c..7e8f569 100644
--- a/webkit/glue/webview.h
+++ b/webkit/glue/webview.h
@@ -63,11 +63,6 @@ class WebView : public WebKit::WebView {
   // links.
   static void ResetVisitedLinkState();
 
-  // Returns the delegate for this WebView.  This is the pointer that was
-  // passed to WebView::Initialize. The caller must check this value before
-  // using it, it will be NULL during closing of the view.
-  virtual WebViewDelegate* GetDelegate() = 0;
-
   // Notifies the webview that autofill suggestions are available for a node.
   virtual void AutofillSuggestionsForNode(
       int64 node_id,
diff --git a/webkit/glue/webview_impl.cc b/webkit/glue/webview_impl.cc
index fdf3fbc..3f86229 100644
--- a/webkit/glue/webview_impl.cc
+++ b/webkit/glue/webview_impl.cc
@@ -1661,10 +1661,6 @@ void WebViewImpl::setInspectorSettings(const WebString& settings) {
 
 // WebView --------------------------------------------------------------------
 
-WebViewDelegate* WebViewImpl::GetDelegate() {
-  return delegate_;
-}
-
 bool WebViewImpl::setDropEffect(bool accept) {
   if (drag_target_dispatch_) {
     drop_effect_ = accept ? DROP_EFFECT_COPY : DROP_EFFECT_NONE;
diff --git a/webkit/glue/webview_impl.h b/webkit/glue/webview_impl.h
index f956042..8dafde0 100644
--- a/webkit/glue/webview_impl.h
+++ b/webkit/glue/webview_impl.h
@@ -138,7 +138,6 @@ class WebViewImpl : public WebView, public base::RefCounted<WebViewImpl> {
   virtual void setInspectorSettings(const WebKit::WebString& settings);
 
   // WebView methods:
-  virtual WebViewDelegate* GetDelegate();
   virtual void AutofillSuggestionsForNode(
       int64 node_id,
       const std::vector<std::wstring>& suggestions,