Introduce RenderView::Observer interface so that RenderView doesn't have to k...

chrome/renderer/render_view.cc

Index: chrome/renderer/render_view.cc
===================================================================
--- chrome/renderer/render_view.cc	(revision 71253)
+++ chrome/renderer/render_view.cc	(working copy)
@@ -507,6 +507,36 @@
 
 ///////////////////////////////////////////////////////////////////////////////
 
+RenderView::Observer::Observer() : render_view_(NULL), routing_id_(0) {
+}
+
+RenderView::Observer::~Observer() {
+  if (render_view_)
+    render_view_->RemoveObserver(this);
+}
+
+void RenderView::Observer::OnDestruct() {
+  delete this;
+}
+
+bool RenderView::Observer::OnMessageReceived(const IPC::Message& message) {
+  return false;
+}
+
+bool RenderView::Observer::Send(IPC::Message* message) {
+  if (render_view_)
+    return render_view_->Send(message);
+
+  delete message;
+  return false;
+}
+
+void RenderView::Observer::SetRenderView(RenderView* render_view) {
+  render_view_ = render_view;
+  if (render_view_)
+    routing_id_ = render_view_->routing_id();
+}
+
 int32 RenderView::next_page_id_ = 1;
 
 struct RenderView::PendingFileChooser {
@@ -548,12 +578,16 @@
       browser_window_id_(-1),
       ALLOW_THIS_IN_INITIALIZER_LIST(pepper_delegate_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(page_info_method_factory_(this)),
-      ALLOW_THIS_IN_INITIALIZER_LIST(autofill_method_factory_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(accessibility_method_factory_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(translate_helper_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(cookie_jar_(this)),
+      devtools_agent_(NULL),
+      devtools_client_(NULL),
       ALLOW_THIS_IN_INITIALIZER_LIST(
-          notification_provider_(new NotificationProvider(this))),
+          notification_provider_(new NotificationProvider())),
+      geolocation_dispatcher_(NULL),
+      speech_input_dispatcher_(NULL),
+      device_orientation_dispatcher_(NULL),
       accessibility_ack_pending_(false),
       pending_app_icon_requests_(0),
       session_storage_namespace_id_(session_storage_namespace_id),
@@ -565,14 +599,7 @@
   // in single-process mode.
   WebKit::WebView::setUseExternalPopupMenus(true);
 #endif
-  password_autocomplete_manager_.reset(new PasswordAutocompleteManager(this));
-  autofill_helper_.reset(new AutoFillHelper(this));
-  page_click_tracker_.reset(new PageClickTracker(this));
-  // Note that the order of insertion of the listeners is important.
-  // The password_autocomplete_manager_ takes the first shot at processing the
-  // notification and can stop the propagation.
-  page_click_tracker_->AddListener(password_autocomplete_manager_.get());
-  page_click_tracker_->AddListener(autofill_helper_.get());
+
   ClearBlockedContentSettings();
   if (CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableClientSidePhishingDetection)) {
@@ -624,6 +651,11 @@
   for (ViewMap::iterator it = views->begin(); it != views->end(); ++it)
     DCHECK_NE(this, it->second) << "Failed to call Close?";
 #endif
+
+  for (size_t i = 0; i < observers_.size(); ++i) {
+    observers_[i]->SetRenderView(NULL);
+    observers_[i]->OnDestruct();
+  }
 }
 
 /*static*/
@@ -674,6 +706,19 @@
   next_page_id_ = next_page_id;
 }
 
+void RenderView::AddObserver(Observer* observer) {
+  observers_.push_back(observer);
+  observer->SetRenderView(this);
+}
+
+void RenderView::RemoveObserver(Observer* observer) {
+  observer->SetRenderView(NULL);
+  for (size_t i = 0; i < observers_.size(); ++i) {
+    if (observers_[i] == observer)
+      observers_.erase(observers_.begin() + i);
+  }
+}
+
 bool RenderView::RendererAccessibilityNotification::ShouldIncludeChildren() {
   typedef ViewHostMsg_AccessibilityNotification_Params params;
   if (type == params::NOTIFICATION_TYPE_CHILDREN_CHANGED ||
@@ -902,6 +947,7 @@
                       const string16& frame_name) {
   DCHECK(!webview());
 
+  routing_id_ = routing_id;
   if (opener_id != MSG_ROUTING_NONE)
     opener_id_ = opener_id;
 
@@ -914,14 +960,13 @@
     decrement_shared_popup_at_destruction_ = false;
   }
 
-  devtools_agent_.reset(new DevToolsAgent(routing_id, this));
+  devtools_agent_ = new DevToolsAgent(routing_id);
+  PasswordAutocompleteManager* password_autocomplete_manager =
+      new PasswordAutocompleteManager();
+  AutoFillHelper* autofill_helper = new AutoFillHelper(
+      password_autocomplete_manager);

[Ilya Sherman, 2011/01/13 02:18:30]

nit: Might be nice to have a quick comment mentioning how memory is managed for
these.

[jam, 2011/01/13 02:53:43]

There's a comment beside the Observer interface, I'd rather not duplicate it

[Ilya Sherman, 2011/01/13 18:34:50]

That comment is really visible when looking at the changes from this patch, but
a month down the line it'll be harder to know to look there when reading these
lines.  Your call, but I think a (very brief) comment here would be helpful.

[jam, 2011/01/13 18:40:53]

My main issue with documenting stuff multiple times is that they'll go out of
sync, which is more confusing than not having comments.  Also, I think if
someone is using an interface, they should be familiar with it.

[Ilya Sherman, 2011/01/13 22:34:51]

It's not reasonable to expect everyone reading this code to know all of the
details of the Observer interface, and the interface has a decidedly
non-standard approach to memory management.  Without a comment, the immediate
reaction is "Huh, I wonder why there's an unprotected memory allocation."  Sure,
it's not very hard to figure out what's actually going on, but it's an
unnecessary distraction.  A comment like

// Observers will be deleted when the RenderView goes away.

alleviates this; and if it goes out of sync, the code probably needs to be
updated as well.

 
-  // TODO(jam): remove this once WebKit is rolled
-#if defined(WEBKIT_HAS_WEB_AUTO_FILL_CLIENT)
-  webwidget_ = WebView::create(this, devtools_agent_.get(), this);
-#else
-  webwidget_ = WebView::create(this, devtools_agent_.get());
-#endif
+  webwidget_ = WebView::create(this, devtools_agent_, autofill_helper);
   g_view_map.Get().insert(std::make_pair(webview(), this));
   webkit_preferences_.Apply(webview());
   webview()->initializeMainFrame(this);
@@ -930,7 +975,6 @@
 
   OnSetRendererPrefs(renderer_prefs);
 
-  routing_id_ = routing_id;
   render_thread_->AddRoute(routing_id_, this);
   // Take a reference on behalf of the RenderThread.  This will be balanced
   // when we receive ViewMsg_Close.
@@ -953,6 +997,20 @@
 
   audio_message_filter_ = new AudioMessageFilter(routing_id_);
   render_thread_->AddFilter(audio_message_filter_);
+
+  PageClickTracker* page_click_tracker = new PageClickTracker();
+  // Note that the order of insertion of the listeners is important.
+  // The password_autocomplete_manager takes the first shot at processing the
+  // notification and can stop the propagation.
+  page_click_tracker->AddListener(password_autocomplete_manager);
+  page_click_tracker->AddListener(autofill_helper);
+
+  AddObserver(autofill_helper);
+  AddObserver(password_autocomplete_manager);
+  AddObserver(page_click_tracker);
+
+  AddObserver(notification_provider_);
+  AddObserver(devtools_agent_);
 }
 
 bool RenderView::OnMessageReceived(const IPC::Message& message) {
@@ -960,25 +1018,10 @@
   if (main_frame)
     child_process_logging::SetActiveURL(main_frame->url());
 
-  // If this is developer tools renderer intercept tools messages first.
-  if (devtools_client_.get() && devtools_client_->OnMessageReceived(message))
-    return true;
-  if (devtools_agent_.get() && devtools_agent_->OnMessageReceived(message))
-    return true;
-  if (notification_provider_->OnMessageReceived(message))
-    return true;
-  if (geolocation_dispatcher_.get() &&
-      geolocation_dispatcher_->OnMessageReceived(message)) {
-    return true;
+  for (size_t i = 0; i < observers_.size(); ++i) {
+    if (observers_[i]->OnMessageReceived(message))
+      return true;
   }
-  if (speech_input_dispatcher_.get() &&
-      speech_input_dispatcher_->OnMessageReceived(message)) {
-    return true;
-  }
-  if (device_orientation_dispatcher_.get() &&
-      device_orientation_dispatcher_->OnMessageReceived(message)) {
-    return true;
-  }
 
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(RenderView, message)
@@ -1024,7 +1067,6 @@
     IPC_MESSAGE_HANDLER(ViewMsg_CSSInsertRequest, OnCSSInsertRequest)
     IPC_MESSAGE_HANDLER(ViewMsg_AddMessageToConsole, OnAddMessageToConsole)
     IPC_MESSAGE_HANDLER(ViewMsg_ReservePageIDRange, OnReservePageIDRange)
-    IPC_MESSAGE_HANDLER(ViewMsg_FillPasswordForm, OnFillPasswordForm)
     IPC_MESSAGE_HANDLER(ViewMsg_DragTargetDragEnter, OnDragTargetDragEnter)
     IPC_MESSAGE_HANDLER(ViewMsg_DragTargetDragOver, OnDragTargetDragOver)
     IPC_MESSAGE_HANDLER(ViewMsg_DragTargetDragLeave, OnDragTargetDragLeave)
@@ -1058,10 +1100,6 @@
                         OnHandleMessageFromExternalHost)
     IPC_MESSAGE_HANDLER(ViewMsg_DisassociateFromPopupCount,
                         OnDisassociateFromPopupCount)
-    IPC_MESSAGE_HANDLER(ViewMsg_AutoFillSuggestionsReturned,
-                        OnAutoFillSuggestionsReturned)
-    IPC_MESSAGE_HANDLER(ViewMsg_AutoFillFormDataFilled,
-                        OnAutoFillFormDataFilled)
     IPC_MESSAGE_HANDLER(ViewMsg_AllowScriptToClose,
                         OnAllowScriptToClose)
     IPC_MESSAGE_HANDLER(ViewMsg_MoveOrResizeStarted, OnMoveOrResizeStarted)
@@ -1383,12 +1421,6 @@
   history_list_offset_ = params.current_history_list_offset;
   history_list_length_ = params.current_history_list_length;
 
-  if (devtools_agent_.get())
-    devtools_agent_->OnNavigate();
-
-  if (notification_provider_.get())
-    notification_provider_->OnNavigate();
-
   child_process_logging::SetActiveURL(params.url);
 
   AboutHandler::MaybeHandle(params.url);
@@ -1509,8 +1541,9 @@
 }
 
 void RenderView::OnSetupDevToolsClient() {
-  DCHECK(!devtools_client_.get());
-  devtools_client_.reset(new DevToolsClient(this));
+  DCHECK(!devtools_client_);
+  devtools_client_ = new DevToolsClient(webview());
+  AddObserver(devtools_client_);
 }
 
 void RenderView::OnUpdateTargetURLAck() {
@@ -1987,21 +2020,6 @@
                                      provider_type));
 }
 
-void RenderView::OnAutoFillSuggestionsReturned(
-    int query_id,
-    const std::vector<string16>& values,
-    const std::vector<string16>& labels,
-    const std::vector<string16>& icons,
-    const std::vector<int>& unique_ids) {
-  autofill_helper_->SuggestionsReceived(
-      query_id, values, labels, icons, unique_ids);
-}
-
-void RenderView::OnAutoFillFormDataFilled(int query_id,
-                                          const webkit_glue::FormData& form) {
-  autofill_helper_->FormDataFilled(query_id, form);
-}
-
 void RenderView::OnAllowScriptToClose(bool script_can_close) {
   script_can_close_ = script_can_close;
 }
@@ -2191,7 +2209,7 @@
 }
 
 WebKit::WebNotificationPresenter* RenderView::notificationPresenter() {
-  return notification_provider_.get();
+  return notification_provider_;
 }
 
 void RenderView::didStartLoading() {
@@ -2296,29 +2314,6 @@
   UserMetricsRecordAction(name);
 }
 
-void RenderView::textFieldDidEndEditing(
-    const WebKit::WebInputElement& element) {
-  password_autocomplete_manager_->TextFieldDidEndEditing(element);
-}
-
-void RenderView::textFieldDidChange(const WebKit::WebInputElement& element) {
-  // We post a task for doing the AutoFill as the caret position is not set
-  // properly at this point (http://bugs.webkit.org/show_bug.cgi?id=16976) and
-  // it is needed to trigger autofill.
-  autofill_method_factory_.RevokeAll();
-  MessageLoop::current()->PostTask(
-        FROM_HERE,
-        autofill_method_factory_.NewRunnableMethod(
-            &RenderView::TextFieldDidChangeImpl, element));
-}
-
-void RenderView::TextFieldDidChangeImpl(
-    const WebKit::WebInputElement& element) {
-  if (password_autocomplete_manager_->TextDidChangeInTextField(element))
-    return;
-  autofill_helper_->TextDidChangeInTextField(element);
-}
-
 void RenderView::SendPendingAccessibilityNotifications() {
   if (!accessibility_.get())
     return;
@@ -2346,13 +2341,6 @@
   accessibility_ack_pending_ = true;
 }
 
-void RenderView::textFieldDidReceiveKeyDown(
-    const WebKit::WebInputElement& element,
-    const WebKit::WebKeyboardEvent& event) {
-  password_autocomplete_manager_->TextFieldHandlingKeyDown(element, event);
-  autofill_helper_->KeyDownInTextField(element, event);
-}
-
 bool RenderView::handleCurrentKeyboardEvent() {
   if (edit_commands_.empty())
     return false;
@@ -2645,43 +2633,6 @@
                                               value.utf8()));
 }
 
-void RenderView::didAcceptAutoFillSuggestion(const WebKit::WebNode& node,
-                                             const WebKit::WebString& value,
-                                             const WebKit::WebString& label,
-                                             int unique_id,
-                                             unsigned index) {
-  autofill_helper_->DidAcceptAutoFillSuggestion(node, value, unique_id, index);
-}
-
-void RenderView::didSelectAutoFillSuggestion(const WebKit::WebNode& node,
-                                             const WebKit::WebString& value,
-                                             const WebKit::WebString& label,
-                                             int unique_id) {
-  autofill_helper_->DidSelectAutoFillSuggestion(node, unique_id);
-}
-
-void RenderView::didClearAutoFillSelection(const WebKit::WebNode& node) {
-  autofill_helper_->DidClearAutoFillSelection(node);
-}
-
-void RenderView::didAcceptAutocompleteSuggestion(
-    const WebKit::WebInputElement& user_element) {
-  bool result = password_autocomplete_manager_->FillPassword(user_element);
-  // Since this user name was selected from a suggestion list, we should always
-  // have password for it.
-  DCHECK(result);
-}
-
-void RenderView::removeAutocompleteSuggestion(const WebKit::WebString& name,
-                                              const WebKit::WebString& value) {
-  autofill_helper_->RemoveAutocompleteSuggestion(name, value);
-}
-
-void RenderView::removeAutofillSuggestions(const WebString& name,
-                                           const WebString& value) {
-  removeAutocompleteSuggestion(name, value);
-}
-
 // WebKit::WebWidgetClient ----------------------------------------------------
 
 void RenderView::didFocus() {
@@ -2937,8 +2888,8 @@
 }
 
 void RenderView::frameDetached(WebFrame* frame) {
-  autofill_helper_->FrameDetached(frame);
-  page_click_tracker_->StopTrackingFrame(frame, true);
+  for (size_t i = 0; i < observers_.size(); ++i)
+    observers_[i]->FrameDetached(frame);
 }
 
 void RenderView::willClose(WebFrame* frame) {
@@ -2948,11 +2899,8 @@
   page_load_histograms_.Dump(frame);
   navigation_state->user_script_idle_scheduler()->Cancel();
 
-  // TODO(jhawkins): Remove once frameDetached is called by WebKit.
-  // NOTE: taking this out results in lots of increased memory usage!  This is
-  // because frameDetached is NOT like wilLClose.  The latter happens between
-  // navigations, but the former only happens when the RenderView is going away.
-  autofill_helper_->FrameWillClose(frame);
+  for (size_t i = 0; i < observers_.size(); ++i)
+    observers_[i]->FrameWillClose(frame);
 }
 
 bool RenderView::allowImages(WebFrame* frame, bool enabled_per_settings) {
@@ -3546,11 +3494,8 @@
 
   Send(new ViewHostMsg_DocumentLoadedInFrame(routing_id_, frame->identifier()));
 
-  page_click_tracker_->StartTrackingFrame(frame);
-  // The document has now been fully loaded.  Scan for forms to be sent up to
-  // the browser.
-  autofill_helper_->FrameContentsAvailable(frame);
-  password_autocomplete_manager_->SendPasswordForms(frame, false);
+  for (size_t i = 0; i < observers_.size(); ++i)
+    observers_[i]->DidFinishDocumentLoad(frame);
 
   // Check whether we have new encoding name.
   UpdateEncoding(frame, frame->view()->pageEncoding().utf8());
@@ -3601,8 +3546,8 @@
   navigation_state->set_finish_load_time(Time::Now());
   navigation_state->user_script_idle_scheduler()->DidFinishLoad();
 
-  // Let the password manager know which password forms are actually visible.
-  password_autocomplete_manager_->SendPasswordForms(frame, true);
+  for (size_t i = 0; i < observers_.size(); ++i)
+    observers_[i]->DidFinishLoad(frame);
 
   Send(new ViewHostMsg_DidFinishLoad(routing_id_, frame->identifier()));
 }
@@ -4716,12 +4661,6 @@
   webview()->dragSourceSystemDragEnded();
 }
 
-void RenderView::OnFillPasswordForm(
-    const webkit_glue::PasswordFormFillData& form_data) {
-  password_autocomplete_manager_->ReceivedPasswordFormFillData(webview(),
-                                                              form_data);
-}
-
 void RenderView::OnDragTargetDragEnter(const WebDropData& drop_data,
                                        const gfx::Point& client_point,
                                        const gfx::Point& screen_point,
@@ -5493,10 +5432,6 @@
   edit_commands_.clear();
 }
 
-void RenderView::DidHandleMouseEvent(const WebKit::WebMouseEvent& event) {
-  page_click_tracker_->DidHandleMouseEvent(event);
-}
-
 #if defined(OS_MACOSX)
 void RenderView::OnWasHidden() {
   RenderWidget::OnWasHidden();
@@ -5652,27 +5587,33 @@
   if (!frame)
     return;
 
-  // The page is translated, so try to extract the form data again.
-  autofill_helper_->FrameContentsAvailable(frame);
+  for (size_t i = 0; i < observers_.size(); ++i)
+    observers_[i]->FrameTranslated(frame);
 }
 
 WebKit::WebGeolocationClient* RenderView::geolocationClient() {
-  if (!geolocation_dispatcher_.get())
-    geolocation_dispatcher_.reset(new GeolocationDispatcher(this));
-  return geolocation_dispatcher_.get();
+  if (!geolocation_dispatcher_) {
+    geolocation_dispatcher_ = new GeolocationDispatcher();
+    AddObserver(geolocation_dispatcher_);
+  }
+  return geolocation_dispatcher_;
 }
 
 WebKit::WebSpeechInputController* RenderView::speechInputController(
     WebKit::WebSpeechInputListener* listener) {
-  if (!speech_input_dispatcher_.get())
-    speech_input_dispatcher_.reset(new SpeechInputDispatcher(this, listener));
-  return speech_input_dispatcher_.get();
+  if (!speech_input_dispatcher_) {
+    speech_input_dispatcher_ = new SpeechInputDispatcher(listener);
+    AddObserver(speech_input_dispatcher_);
+  }
+  return speech_input_dispatcher_;
 }
 
 WebKit::WebDeviceOrientationClient* RenderView::deviceOrientationClient() {
-  if (!device_orientation_dispatcher_.get())
-    device_orientation_dispatcher_.reset(new DeviceOrientationDispatcher(this));
-  return device_orientation_dispatcher_.get();
+  if (!device_orientation_dispatcher_) {
+    device_orientation_dispatcher_ = new DeviceOrientationDispatcher();
+    AddObserver(device_orientation_dispatcher_);
+  }
+  return device_orientation_dispatcher_;
 }
 
 void RenderView::zoomLimitsChanged(double minimum_level, double maximum_level) {