Refactor RenderWidgetHost::set_paint_observer()

chrome/browser/instant/instant_loader.cc

 // Copyright (c) 2010 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.
 
 #include "chrome/browser/instant/instant_loader.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 15 matching lines @@
 #include "chrome/browser/tab_contents/navigation_controller.h"
 #include "chrome/browser/tab_contents/navigation_entry.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/tab_contents/tab_contents_delegate.h"
 #include "chrome/browser/tab_contents/tab_contents_view.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/notification_details.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
+#include "chrome/common/notification_service.h"
 #include "chrome/common/notification_source.h"
 #include "chrome/common/notification_type.h"
 #include "chrome/common/page_transition_types.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/renderer_preferences.h"
 #include "gfx/codec/png_codec.h"
 #include "ipc/ipc_message.h"
 #include "net/http/http_util.h"
 
 namespace {
@@ skipping 73 matching lines @@
 
   // unique_id of the NavigationEntry we're waiting on.
   const int unique_id_;
 
   // Registers and unregisters us for notifications.
   NotificationRegistrar registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(FrameLoadObserver);
 };
 
-// PaintObserver implementation. When the RenderWidgetHost paints itself this
-// notifies the TabContentsDelegateImpl which ultimately notifies InstantLoader
-// and shows the preview.
-// The ownership of this class is tricky. It's created and
-// tracked by TabContentsDelegateImpl, but owned by RenderWidgetHost. When
-// deleted this notifies the TabContentsDelegateImpl so that it can clean
-// up appropriately.
-class InstantLoader::PaintObserverImpl
-    : public RenderWidgetHost::PaintObserver {
- public:
-  PaintObserverImpl(TabContentsDelegateImpl* delegate,
-                    RenderWidgetHost* rwh)
-      : delegate_(delegate),
-        rwh_(rwh) {
-    rwh_->set_paint_observer(this);
-  }
-
-  ~PaintObserverImpl();
-
-  // Deletes this object by resetting the PaintObserver on the RenderWidgetHost.
-  void Destroy() {
-    rwh_->set_paint_observer(NULL);
-  }
-
-  virtual void RenderWidgetHostWillPaint(RenderWidgetHost* rwh) {}
-
-  virtual void RenderWidgetHostDidPaint(RenderWidgetHost* rwh);
-
- private:
-  TabContentsDelegateImpl* delegate_;
-  RenderWidgetHost* rwh_;
-
-  DISALLOW_COPY_AND_ASSIGN(PaintObserverImpl);
-};
-
-class InstantLoader::TabContentsDelegateImpl : public TabContentsDelegate {
+class InstantLoader::TabContentsDelegateImpl
+    : public TabContentsDelegate,
+      public NotificationObserver {
  public:
   explicit TabContentsDelegateImpl(InstantLoader* loader)
       : loader_(loader),
-        paint_observer_(NULL),
+        registered_render_widget_host_(NULL),
         waiting_for_new_page_(true),
         is_mouse_down_from_activate_(false),
         user_typed_before_load_(false) {
   }
 
   // Invoked prior to loading a new URL.
   void PrepareForNewLoad() {
     user_typed_before_load_ = false;
     waiting_for_new_page_ = true;
     add_page_vector_.clear();
-    DestroyPaintObserver();
+    UnregisterForPaintNotifications();
   }
 
   // Invoked when removed as the delegate. Gives a chance to do any necessary
   // cleanup.
   void Reset() {
     is_mouse_down_from_activate_ = false;
-    DestroyPaintObserver();
+    UnregisterForPaintNotifications();
   }
 
   // Invoked when the preview paints. Invokes PreviewPainted on the loader.
   void PreviewPainted() {
     loader_->PreviewPainted();
   }
 
-  // Invoked when the PaintObserverImpl is deleted.
-  void PaintObserverDestroyed(PaintObserverImpl* observer) {
-    if (observer == paint_observer_)
-      paint_observer_ = NULL;
-  }
-
   bool is_mouse_down_from_activate() const {
     return is_mouse_down_from_activate_;
   }
 
   void set_user_typed_before_load() { user_typed_before_load_ = true; }
 
   // Sets the last URL that will be added to history when CommitHistory is
   // invoked and removes all but the first navigation.
   void SetLastHistoryURLAndPrune(const GURL& url) {
     if (add_page_vector_.empty())
@@ skipping 45 matching lines @@
         // going to be added to history. We need to also set the favicon for the
         // url we're adding to history (see comment in ReleasePreviewContents
         // for details).
         favicon_service->SetFavicon(add_page_vector_.back()->url,
                                     active_entry->favicon().url(),
                                     image_data);
       }
     }
   }
 
+  void RegisterForPaintNotifications(RenderWidgetHost* render_widget_host) {

[sky, 2011/01/07 21:04:16]

Move the register/unregister methods to the private section.

+    DCHECK(registered_render_widget_host_ == NULL);
+    registered_render_widget_host_ = render_widget_host;
+    Source<RenderWidgetHost> source =
+        Source<RenderWidgetHost>(registered_render_widget_host_);
+    registrar_.Add(this, NotificationType::RENDER_WIDGET_HOST_DID_PAINT,
+        source);
+    registrar_.Add(this, NotificationType::RENDER_WIDGET_HOST_DESTROYED,
+        source);
+  }
+
+  void UnregisterForPaintNotifications() {
+    if (registered_render_widget_host_) {
+      Source<RenderWidgetHost> source =
+          Source<RenderWidgetHost>(registered_render_widget_host_);
+      registrar_.Remove(this, NotificationType::RENDER_WIDGET_HOST_DID_PAINT,
+          source);
+      registrar_.Remove(this, NotificationType::RENDER_WIDGET_HOST_DESTROYED,
+          source);
+      registered_render_widget_host_ = NULL;
+    }
+  }
+
+  void Observe(NotificationType type,

[sky, 2011/01/07 21:04:16]

virtual

+               const NotificationSource& source,
+               const NotificationDetails& details) {
+    if (type == NotificationType::RENDER_WIDGET_HOST_DID_PAINT) {
+      UnregisterForPaintNotifications();
+      PreviewPainted();
+    } else if (type == NotificationType::RENDER_WIDGET_HOST_DESTROYED) {
+      UnregisterForPaintNotifications();
+    } else {
+      NOTREACHED() << "Got a notification we didn't register for.";
+    }
+  }
+
   virtual void OpenURLFromTab(TabContents* source,
                               const GURL& url, const GURL& referrer,
                               WindowOpenDisposition disposition,
                               PageTransition::Type transition) {}
   virtual void NavigationStateChanged(const TabContents* source,
                                       unsigned changed_flags) {
-    if (!loader_->ready() && !paint_observer_ &&
+    if (!loader_->ready() && !registered_render_widget_host_ &&
         source->controller().entry_count()) {
       // The load has been committed. Install an observer that waits for the
       // first paint then makes the preview active. We wait for the load to be
       // committed before waiting on paint as there is always an initial paint
       // when a new renderer is created from the resize so that if we showed the
       // preview after the first paint we would end up with a white rect.
-      paint_observer_ = new PaintObserverImpl(this,
+      RegisterForPaintNotifications(
           source->GetRenderWidgetHostView()->GetRenderWidgetHost());
     }
   }
   virtual std::string GetNavigationHeaders(const GURL& url) {
     std::string header;
     net::HttpUtil::AppendHeaderIfMissing(kPreviewHeader, kPreviewHeaderValue,
                                          &header);
     return header;
   }
   virtual void AddNewContents(TabContents* source,
@@ skipping 10 matching lines @@
   virtual bool ShouldFocusConstrainedWindow() {
     // Return false so that constrained windows are not initially focused. If
     // we did otherwise the preview would prematurely get committed when focus
     // goes to the constrained window.
     return false;
   }
   virtual void WillShowConstrainedWindow(TabContents* source) {
     if (!loader_->ready()) {
       // A constrained window shown for an auth may not paint. Show the preview
       // contents.
-      DestroyPaintObserver();
+      UnregisterForPaintNotifications();
       loader_->ShowPreview();
     }
   }
   virtual void ToolbarSizeChanged(TabContents* source, bool is_animating) {}
   virtual void URLStarredChanged(TabContents* source, bool starred) {}
   virtual void UpdateTargetURL(TabContents* source, const GURL& url) {}
   virtual bool ShouldSuppressDialogs() {
     // Any message shown during instant cancels instant, so we suppress them.
     return true;
   }
@@ skipping 57 matching lines @@
   typedef std::vector<scoped_refptr<history::HistoryAddPageArgs> >
       AddPageVector;
 
   void CommitFromMouseReleaseIfNecessary() {
     bool was_down = is_mouse_down_from_activate_;
     is_mouse_down_from_activate_ = false;
     if (was_down && loader_->ShouldCommitInstantOnMouseUp())
       loader_->CommitInstantLoader();
   }
 
-  // If the PaintObserver is non-null Destroy is invoked on it.
-  void DestroyPaintObserver() {
-    if (paint_observer_) {
-      paint_observer_->Destroy();
-      // Destroy should result in invoking PaintObserverDestroyed and NULLing
-      // out paint_observer_.
-      DCHECK(!paint_observer_);
-    }
-  }
-
   InstantLoader* loader_;
 
-  // Used to listen for paint so that we know when to show the preview. See
-  // comment in NavigationStateChanged for details on this.
-  // Ownership of this is tricky, see comment above PaintObserverImpl class.
-  PaintObserverImpl* paint_observer_;
+  NotificationRegistrar registrar_;
+  RenderWidgetHost* registered_render_widget_host_;

[sky, 2011/01/07 21:04:16]

Comment?

 
   // Used to cache data that needs to be added to history. Normally entries are
   // added to history as the user types, but for instant we only want to add the
   // items to history if the user commits instant. So, we cache them here and if
   // committed then add the items to history.
   AddPageVector add_page_vector_;
 
   // Are we we waiting for a NavigationType of NEW_PAGE? If we're waiting for
   // NEW_PAGE navigation we don't add history items to add_page_vector_.
   bool waiting_for_new_page_;
 
   // True if the mouse is down from an activate.
   bool is_mouse_down_from_activate_;
 
   // True if the user typed in the search box before the page loaded.
   bool user_typed_before_load_;
 
   DISALLOW_COPY_AND_ASSIGN(TabContentsDelegateImpl);
 };
 
-InstantLoader::PaintObserverImpl::~PaintObserverImpl() {
-  delegate_->PaintObserverDestroyed(this);
-}
-
-void InstantLoader::PaintObserverImpl::RenderWidgetHostDidPaint(
-    RenderWidgetHost* rwh) {
-  TabContentsDelegateImpl* delegate = delegate_;
-  // Set the paint observer to NULL, which deletes us. Showing the preview may
-  // reset the paint observer, and delete us. By resetting the delegate first we
-  // know we've been deleted and can deal correctly.
-  rwh->set_paint_observer(NULL);
-  // WARNING: we've been deleted.
-  if (delegate)
-    delegate->PreviewPainted();
-}
-
 InstantLoader::InstantLoader(InstantLoaderDelegate* delegate, TemplateURLID id)
     : delegate_(delegate),
       template_url_id_(id),
       ready_(false),
       last_transition_type_(PageTransition::LINK) {
   preview_tab_contents_delegate_.reset(new TabContentsDelegateImpl(this));
 }
 
 InstantLoader::~InstantLoader() {
   registrar_.RemoveAll();
@@ skipping 155 matching lines @@
   if (preview_contents_.get()) {
     if (type != INSTANT_COMMIT_DESTROY) {
       if (template_url_id_) {
         // The URL used during instant is mostly gibberish, and not something
         // we'll parse and match as a past search. Set it to something we can
         // parse.
         preview_tab_contents_delegate_->SetLastHistoryURLAndPrune(url);
       }
       preview_tab_contents_delegate_->CommitHistory(template_url_id_ != 0);
     }
-    // Destroy the paint observer.
-    // RenderWidgetHostView may be null during shutdown.
     if (preview_contents_->tab_contents()->GetRenderWidgetHostView()) {
-      preview_contents_->tab_contents()->GetRenderWidgetHostView()->
-          GetRenderWidgetHost()->set_paint_observer(NULL);
 #if defined(OS_MACOSX)
       preview_contents_->tab_contents()->GetRenderWidgetHostView()->
           SetTakesFocusOnlyOnMouseDown(false);
       registrar_.Remove(
           this,
           NotificationType::RENDER_VIEW_HOST_CHANGED,
           Source<NavigationController>(&preview_contents_->controller()));
 #endif
     }
     preview_contents_->set_delegate(NULL);
@@ skipping 168 matching lines @@
       Source<NavigationController>(&preview_contents_->controller()));
 #endif
 
   registrar_.Add(
       this,
       NotificationType::NAV_ENTRY_COMMITTED,
       Source<NavigationController>(&preview_contents_->controller()));
 
   preview_contents_->tab_contents()->ShowContents();
 }