Remove notifications from PrintPreviewDialogController.

chrome/browser/printing/print_preview_dialog_controller.cc

Index: chrome/browser/printing/print_preview_dialog_controller.cc
diff --git a/chrome/browser/printing/print_preview_dialog_controller.cc b/chrome/browser/printing/print_preview_dialog_controller.cc
index 1f0769c9dc86554ddc0120d255eaa2a3d4085c10..c5a1c1e8d6d2a9f6f3b4989b0353d8c6f265da89 100644
--- a/chrome/browser/printing/print_preview_dialog_controller.cc
+++ b/chrome/browser/printing/print_preview_dialog_controller.cc
@@ -6,10 +6,10 @@
 
 #include <algorithm>
 #include <string>
-#include <vector>
 
 #include "base/auto_reset.h"
 #include "base/path_service.h"
+#include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/browser_process.h"
@@ -31,14 +31,14 @@
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
-#include "content/public/browser/notification_details.h"
-#include "content/public/browser/notification_source.h"
 #include "content/public/browser/plugin_service.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_process_host_observer.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_delegate.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_view.h"
 #include "content/public/common/webplugininfo.h"
 #include "ui/web_dialogs/web_dialog_delegate.h"
@@ -46,6 +46,7 @@
 
 using content::NativeWebKeyboardEvent;
 using content::NavigationController;
+using content::RenderProcessHost;
 using content::WebContents;
 using content::WebUIMessageHandler;
 using ui::WebDialogDelegate;
@@ -209,6 +210,97 @@ void PrintPreviewWebContentDelegate::HandleKeyboardEvent(
 
 namespace printing {
 
+struct PrintPreviewDialogController::Operation {
+  class Observer : public content::WebContentsObserver,
+                   public content::RenderProcessHostObserver {
+   public:
+    Observer();
+    virtual ~Observer();
+
+    void StartObserving(PrintPreviewDialogController* owner,
+                        WebContents* web_contents);
+    void StopObserving();
+
+   private:
+    // content::WebContentsObserver
+    virtual void WebContentsDestroyed(WebContents* web_contents) OVERRIDE;
+    virtual void NavigationEntryCommitted(
+        const content::LoadCommittedDetails& load_details) OVERRIDE;
+    // content::RenderProcessHostObserver
+    virtual void RenderProcessExited(RenderProcessHost* host,
+                                     base::ProcessHandle handle,
+                                     base::TerminationStatus status,
+                                     int exit_code) OVERRIDE;
+    virtual void RenderProcessHostDestroyed(RenderProcessHost* host) OVERRIDE;
+
+    PrintPreviewDialogController* owner_;
+    RenderProcessHost* host_;
+  };
+
+  Operation();
+
+  WebContents* preview_dialog;
+  WebContents* initiator;
+  Observer preview_dialog_observer;
+  Observer initiator_observer;
+
+  DISALLOW_COPY_AND_ASSIGN(Operation);
+};
+
+PrintPreviewDialogController::Operation::Operation() : preview_dialog(NULL),
+                                                       initiator(NULL) {
+}
+
+PrintPreviewDialogController::Operation::Observer::Observer() : owner_(NULL),
+                                                                host_(NULL) {
+}
+
+PrintPreviewDialogController::Operation::Observer::~Observer() {
+  StopObserving();
+}
+
+void PrintPreviewDialogController::Operation::Observer::StartObserving(
+    PrintPreviewDialogController* owner,
+    WebContents* web_contents) {
+  owner_ = owner;
+  Observe(web_contents);
+  host_ = web_contents->GetRenderProcessHost();
+  host_->AddObserver(this);
+}
+
+void PrintPreviewDialogController::Operation::Observer::StopObserving() {
+  Observe(NULL);
+  if (host_) {
+    host_->RemoveObserver(this);
+    host_ = NULL;
+  }
+}
+
+void PrintPreviewDialogController::Operation::Observer::WebContentsDestroyed(
+    WebContents* web_contents) {
+  owner_->OnWebContentsDestroyed(web_contents);
+}
+
+void
+PrintPreviewDialogController::Operation::Observer::NavigationEntryCommitted(
+    const content::LoadCommittedDetails& load_details) {
+  owner_->OnNavigationEntryCommitted(web_contents(), &load_details);
+}
+
+void PrintPreviewDialogController::Operation::Observer::RenderProcessExited(
+    RenderProcessHost* host,
+    base::ProcessHandle handle,
+    base::TerminationStatus status,
+    int exit_code) {
+  owner_->OnRenderProcessExited(host);
+}
+
+void
+PrintPreviewDialogController::Operation::Observer::RenderProcessHostDestroyed(
+    RenderProcessHost* host) {
+  host_ = NULL;
+}
+
 PrintPreviewDialogController::PrintPreviewDialogController()
     : waiting_for_new_preview_page_(false),
       is_creating_print_preview_dialog_(false) {
@@ -249,19 +341,11 @@ WebContents* PrintPreviewDialogController::GetOrCreatePreviewDialog(
 
 WebContents* PrintPreviewDialogController::GetPrintPreviewForContents(
     WebContents* contents) const {
-  // |preview_dialog_map_| is keyed by the preview dialog, so if find()
-  // succeeds, then |contents| is the preview dialog.
-  PrintPreviewDialogMap::const_iterator it = preview_dialog_map_.find(contents);
-  if (it != preview_dialog_map_.end())
-    return contents;
-
-  for (it = preview_dialog_map_.begin();
-       it != preview_dialog_map_.end();
-       ++it) {
-    // If |contents| is an initiator.
-    if (contents == it->second) {
-      // Return the associated preview dialog.
-      return it->first;
+  for (size_t i = 0; i < preview_operations_.size(); ++i) {
+    Operation* operation = preview_operations_[i];
+    if (operation->preview_dialog == contents ||
+        operation->initiator == contents) {
+      return operation->preview_dialog;
     }
   }
   return NULL;
@@ -269,27 +353,12 @@ WebContents* PrintPreviewDialogController::GetPrintPreviewForContents(
 
 WebContents* PrintPreviewDialogController::GetInitiator(
     WebContents* preview_dialog) {
-  PrintPreviewDialogMap::iterator it = preview_dialog_map_.find(preview_dialog);
-  return (it != preview_dialog_map_.end()) ? it->second : NULL;
-}
-
-void PrintPreviewDialogController::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  if (type == content::NOTIFICATION_RENDERER_PROCESS_CLOSED) {
-    OnRendererProcessClosed(
-        content::Source<content::RenderProcessHost>(source).ptr());
-  } else if (type == content::NOTIFICATION_WEB_CONTENTS_DESTROYED) {
-    OnWebContentsDestroyed(content::Source<WebContents>(source).ptr());
-  } else {
-    DCHECK_EQ(content::NOTIFICATION_NAV_ENTRY_COMMITTED, type);
-    WebContents* contents =
-        content::Source<NavigationController>(source)->GetWebContents();
-    OnNavEntryCommitted(
-        contents,
-        content::Details<content::LoadCommittedDetails>(details).ptr());
+  for (size_t i = 0; i < preview_operations_.size(); ++i) {
+    Operation* operation = preview_operations_[i];
+    if (operation->preview_dialog == preview_dialog)
+      return operation->initiator;
   }
+  return NULL;
 }
 
 // static
@@ -305,26 +374,32 @@ bool PrintPreviewDialogController::IsPrintPreviewURL(const GURL& url) {
 
 void PrintPreviewDialogController::EraseInitiatorInfo(
     WebContents* preview_dialog) {
-  PrintPreviewDialogMap::iterator it = preview_dialog_map_.find(preview_dialog);
-  if (it == preview_dialog_map_.end())
-    return;
-
-  RemoveObservers(it->second);
-  preview_dialog_map_[preview_dialog] = NULL;
+  for (size_t i = 0; i < preview_operations_.size(); ++i) {
+    Operation* operation = preview_operations_[i];
+    if (operation->preview_dialog == preview_dialog) {
+      operation->initiator_observer.StopObserving();
+      operation->initiator = NULL;
+      return;
+    }
+  }
 }
 
-PrintPreviewDialogController::~PrintPreviewDialogController() {}
+PrintPreviewDialogController::~PrintPreviewDialogController() {
+  DCHECK_EQ(0U, preview_operations_.size());
+  // Paranoia.
+  STLDeleteElements(&preview_operations_);

[Lei Zhang, 2014/01/06 20:29:22]

Isn't this handling a DCHECK failure, which is something the chromium style
guide says not to do?

[Avi (use Gerrit), 2014/01/06 21:34:05]

True. Removed.

+}
 
-void PrintPreviewDialogController::OnRendererProcessClosed(
-    content::RenderProcessHost* rph) {
+void PrintPreviewDialogController::OnRenderProcessExited(
+    RenderProcessHost* rph) {
   // Store contents in a vector and deal with them after iterating through
-  // |preview_dialog_map_| because RemoveFoo() can change |preview_dialog_map_|.
+  // |preview_operations_| because RemoveFoo() can change |preview_operations_|.
   std::vector<WebContents*> closed_initiators;
   std::vector<WebContents*> closed_preview_dialogs;
-  for (PrintPreviewDialogMap::iterator iter = preview_dialog_map_.begin();
-       iter != preview_dialog_map_.end(); ++iter) {
-    WebContents* preview_dialog = iter->first;
-    WebContents* initiator = iter->second;
+  for (size_t i = 0; i < preview_operations_.size(); ++i) {
+    Operation* operation = preview_operations_[i];
+    WebContents* preview_dialog = operation->preview_dialog;
+    WebContents* initiator = operation->initiator;
     if (preview_dialog->GetRenderProcessHost() == rph) {
       closed_preview_dialogs.push_back(preview_dialog);
     } else if (initiator &&
@@ -359,8 +434,8 @@ void PrintPreviewDialogController::OnWebContentsDestroyed(
     RemoveInitiator(contents);
 }
 
-void PrintPreviewDialogController::OnNavEntryCommitted(
-    WebContents* contents, content::LoadCommittedDetails* details) {
+void PrintPreviewDialogController::OnNavigationEntryCommitted(
+    WebContents* contents, const content::LoadCommittedDetails* details) {
   WebContents* preview_dialog = GetPrintPreviewForContents(contents);
   if (!preview_dialog) {
     NOTREACHED();
@@ -420,12 +495,15 @@ WebContents* PrintPreviewDialogController::CreatePrintPreviewDialog(
   EnableInternalPDFPluginForContents(preview_dialog);
   PrintViewManager::CreateForWebContents(preview_dialog);
 
-  // Add an entry to the map.
-  preview_dialog_map_[preview_dialog] = initiator;
   waiting_for_new_preview_page_ = true;
 
-  AddObservers(initiator);
-  AddObservers(preview_dialog);
+  // Add an entry to the map.
+  Operation* operation = new Operation;
+  operation->preview_dialog = preview_dialog;
+  operation->initiator = initiator;
+  operation->preview_dialog_observer.StartObserving(this, preview_dialog);
+  operation->initiator_observer.StartObserving(this, initiator);
+  preview_operations_.push_back(operation);
 
   return preview_dialog;
 }
@@ -441,49 +519,15 @@ void PrintPreviewDialogController::SaveInitiatorTitle(
   }
 }
 
-void PrintPreviewDialogController::AddObservers(WebContents* contents) {
-  registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
-                 content::Source<WebContents>(contents));
-  registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
-      content::Source<NavigationController>(&contents->GetController()));
-
-  // Multiple sites may share the same RenderProcessHost, so check if this
-  // notification has already been added.
-  content::Source<content::RenderProcessHost> rph_source(
-      contents->GetRenderProcessHost());
-  if (!registrar_.IsRegistered(this,
-      content::NOTIFICATION_RENDERER_PROCESS_CLOSED, rph_source)) {
-    registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
-                   rph_source);
-  }
-}
-
-void PrintPreviewDialogController::RemoveObservers(WebContents* contents) {
-  registrar_.Remove(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
-                    content::Source<WebContents>(contents));
-  registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
-      content::Source<NavigationController>(&contents->GetController()));
-
-  // Multiple sites may share the same RenderProcessHost, so check if this
-  // notification has already been added.
-  content::Source<content::RenderProcessHost> rph_source(
-      contents->GetRenderProcessHost());
-  if (registrar_.IsRegistered(this,
-      content::NOTIFICATION_RENDERER_PROCESS_CLOSED, rph_source)) {
-    registrar_.Remove(this, content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
-                      rph_source);
-  }
-}
-
 void PrintPreviewDialogController::RemoveInitiator(
     WebContents* initiator) {
   WebContents* preview_dialog = GetPrintPreviewForContents(initiator);
   DCHECK(preview_dialog);
+
   // Update the map entry first, so when the print preview dialog gets destroyed
   // and reaches RemovePreviewDialog(), it does not attempt to also remove the
   // initiator's observers.
-  preview_dialog_map_[preview_dialog] = NULL;
-  RemoveObservers(initiator);
+  EraseInitiatorInfo(preview_dialog);
 
   PrintViewManager::FromWebContents(initiator)->PrintPreviewDone();
 
@@ -496,22 +540,30 @@ void PrintPreviewDialogController::RemoveInitiator(
 
 void PrintPreviewDialogController::RemovePreviewDialog(
     WebContents* preview_dialog) {
-  // Remove the initiator's observers before erasing the mapping.
-  WebContents* initiator = GetInitiator(preview_dialog);
-  if (initiator) {
-    RemoveObservers(initiator);
-    PrintViewManager::FromWebContents(initiator)->PrintPreviewDone();
-  }
+  for (size_t i = 0; i < preview_operations_.size(); ++i) {
+    Operation* operation = preview_operations_[i];
+    if (operation->preview_dialog == preview_dialog) {
+      // Remove the initiator's observers before erasing the mapping.
+      if (operation->initiator) {
+        operation->initiator_observer.StopObserving();
+        PrintViewManager::FromWebContents(operation->initiator)->
+            PrintPreviewDone();
+      }
 
-  // Print preview WebContents is destroyed. Notify |PrintPreviewUI| to abort
-  // the initiator preview request.
-  PrintPreviewUI* print_preview_ui =
-      static_cast<PrintPreviewUI*>(preview_dialog->GetWebUI()->GetController());
-  if (print_preview_ui)
-    print_preview_ui->OnPrintPreviewDialogDestroyed();
+      // Print preview WebContents is destroyed. Notify |PrintPreviewUI| to
+      // abort the initiator preview request.
+      PrintPreviewUI* print_preview_ui = static_cast<PrintPreviewUI*>(
+          preview_dialog->GetWebUI()->GetController());
+      if (print_preview_ui)
+        print_preview_ui->OnPrintPreviewDialogDestroyed();
+
+      preview_operations_.erase(preview_operations_.begin() + i);
+      delete operation;
 
-  preview_dialog_map_.erase(preview_dialog);
-  RemoveObservers(preview_dialog);
+      return;
+    }
+  }
+  NOTREACHED();
 }
 
 }  // namespace printing