Miscellaneous cleanups from several months ago I never got around to landing.

chrome/browser/automation/automation_provider_observers.cc

 // Copyright (c) 2012 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/automation/automation_provider_observers.h"
 
 #include <deque>
 #include <string>
 #include <vector>
 
@@ skipping 368 matching lines @@
                  content::NotificationService::AllSources());
 }
 
 TabStripNotificationObserver::~TabStripNotificationObserver() {
 }
 
 void TabStripNotificationObserver::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
-  if (type == notification_) {
-    if (type == chrome::NOTIFICATION_TAB_PARENTED) {
-      ObserveTab(&(content::Source<TabContents>(source).ptr()->
-                     web_contents()->GetController()));
-    } else if (type == content::NOTIFICATION_WEB_CONTENTS_DESTROYED) {
-      ObserveTab(&(content::Source<content::WebContents>(source).ptr()->
-                     GetController()));
-    } else {
-      ObserveTab(content::Source<NavigationController>(source).ptr());
-    }
-    delete this;
+  DCHECK_EQ(notification_, type);
+  if (type == chrome::NOTIFICATION_TAB_PARENTED) {
+    ObserveTab(&content::Source<TabContents>(source)->web_contents()->
+        GetController());
+  } else if (type == content::NOTIFICATION_WEB_CONTENTS_DESTROYED) {
+    ObserveTab(&content::Source<content::WebContents>(source)->GetController());
   } else {
-    NOTREACHED();
+    ObserveTab(content::Source<NavigationController>(source).ptr());
   }
+  delete this;
 }
 
 TabAppendedNotificationObserver::TabAppendedNotificationObserver(
     Browser* parent,
     AutomationProvider* automation,
     IPC::Message* reply_message,
     bool use_json_interface)
     : TabStripNotificationObserver(chrome::NOTIFICATION_TAB_PARENTED,
                                    automation),
       parent_(parent),
@@ skipping 21 matching lines @@
 
 IPC::Message* TabAppendedNotificationObserver::ReleaseReply() {
   return reply_message_.release();
 }
 
 TabClosedNotificationObserver::TabClosedNotificationObserver(
     AutomationProvider* automation,
     bool wait_until_closed,
     IPC::Message* reply_message,
     bool use_json_interface)
-    : TabStripNotificationObserver(
-      wait_until_closed
-          ? static_cast<int>(content::NOTIFICATION_WEB_CONTENTS_DESTROYED)
-          : static_cast<int>(chrome::NOTIFICATION_TAB_CLOSING),
-          automation),
+    : TabStripNotificationObserver((wait_until_closed ?
+          static_cast<int>(content::NOTIFICATION_WEB_CONTENTS_DESTROYED) :
+          static_cast<int>(chrome::NOTIFICATION_TAB_CLOSING)), automation),
       reply_message_(reply_message),
       use_json_interface_(use_json_interface),
       for_browser_command_(false) {
 }
 
 TabClosedNotificationObserver::~TabClosedNotificationObserver() {}
 
 void TabClosedNotificationObserver::ObserveTab(
     NavigationController* controller) {
   if (!automation_)
@@ skipping 344 matching lines @@
   registrar_.Add(this, chrome::NOTIFICATION_BROWSER_OPENED,
                  content::NotificationService::AllBrowserContextsAndSources());
   registrar_.Add(this, content::NOTIFICATION_LOAD_STOP,
                  content::NotificationService::AllBrowserContextsAndSources());
 }
 
 BrowserOpenedNotificationObserver::~BrowserOpenedNotificationObserver() {
 }
 
 void BrowserOpenedNotificationObserver::Observe(
-    int type, const content::NotificationSource& source,
+    int type,
+    const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   if (!automation_) {
     delete this;
     return;
   }
 
   if (type == chrome::NOTIFICATION_BROWSER_OPENED) {
     // Store the new browser ID and continue waiting for a new tab within it
     // to stop loading.
     new_window_id_ = ExtensionTabUtil::GetWindowId(
         content::Source<Browser>(source).ptr());
-  } else if (type == content::NOTIFICATION_LOAD_STOP) {
+  } else {
+    DCHECK_EQ(content::NOTIFICATION_LOAD_STOP, type);
     // Only send the result if the loaded tab is in the new window.
     NavigationController* controller =
         content::Source<NavigationController>(source).ptr();
     TabContents* tab =
         TabContents::FromWebContents(controller->GetWebContents());
     int window_id = tab ? tab->restore_tab_helper()->window_id().id() : -1;
     if (window_id == new_window_id_) {
       if (use_json_interface_) {
         AutomationJSONReply(automation_,
                             reply_message_.release()).SendSuccess(NULL);
       } else {
         if (for_browser_command_) {
           AutomationMsg_WindowExecuteCommand::WriteReplyParams(
               reply_message_.get(), true);
         }
         automation_->Send(reply_message_.release());
       }
       delete this;
       return;
     }
-  } else {
-    NOTREACHED();
   }
 }
 
 void BrowserOpenedNotificationObserver::set_for_browser_command(
     bool for_browser_command) {
   for_browser_command_ = for_browser_command;
 }
 
 BrowserClosedNotificationObserver::BrowserClosedNotificationObserver(
     Browser* browser,
     AutomationProvider* automation,
     IPC::Message* reply_message,
     bool use_json_interface)
     : automation_(automation->AsWeakPtr()),
       reply_message_(reply_message),
       use_json_interface_(use_json_interface),
       for_browser_command_(false) {
   registrar_.Add(this, chrome::NOTIFICATION_BROWSER_CLOSED,
                  content::Source<Browser>(browser));
 }
 
 BrowserClosedNotificationObserver::~BrowserClosedNotificationObserver() {}
 
 void BrowserClosedNotificationObserver::Observe(
     int type, const content::NotificationSource& source,
     const content::NotificationDetails& details) {
-  DCHECK(type == chrome::NOTIFICATION_BROWSER_CLOSED);
+  DCHECK_EQ(chrome::NOTIFICATION_BROWSER_CLOSED, type);
 
   if (!automation_) {
     delete this;
     return;
   }
 
   int browser_count = static_cast<int>(BrowserList::size());
   // We get the notification before the browser is removed from the BrowserList.
   bool app_closing = browser_count == 1;
 
@@ skipping 282 matching lines @@
 
 void DomOperationObserver::Observe(
     int type, const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   if (type == content::NOTIFICATION_DOM_OPERATION_RESPONSE) {
     content::Details<DomOperationNotificationDetails> dom_op_details(details);
     if (dom_op_details->automation_id == automation_id_)
       OnDomOperationCompleted(dom_op_details->json);
   } else if (type == chrome::NOTIFICATION_APP_MODAL_DIALOG_SHOWN) {
     OnModalDialogShown();
-  } else if (type == chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED) {
+  } else {
+    DCHECK_EQ(chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED, type);
     WebContents* web_contents = content::Source<WebContents>(source).ptr();
     if (web_contents) {
       TabContents* tab_contents = TabContents::FromWebContents(web_contents);
-      if (tab_contents &&
-          tab_contents->content_settings() &&
+      if (tab_contents && tab_contents->content_settings() &&
           tab_contents->content_settings()->IsContentBlocked(
-              CONTENT_SETTINGS_TYPE_JAVASCRIPT)) {
+              CONTENT_SETTINGS_TYPE_JAVASCRIPT))
         OnJavascriptBlocked();
-      }
     }
   }
 }
 
 DomOperationMessageSender::DomOperationMessageSender(
     AutomationProvider* automation,
     IPC::Message* reply_message,
     bool use_json_interface)
     : DomOperationObserver(0),
       automation_(automation->AsWeakPtr()),
@@ skipping 989 matching lines @@
     registrar_.Add(this, chrome::NOTIFICATION_BROWSER_WINDOW_READY,
                    content::NotificationService::AllSources());
   }
 }
 
 AppLaunchObserver::~AppLaunchObserver() {}
 
 void AppLaunchObserver::Observe(int type,
                                 const content::NotificationSource& source,
                                 const content::NotificationDetails& details) {
-  if (type == content::NOTIFICATION_LOAD_STOP) {
-    if (launch_container_ == extension_misc::LAUNCH_TAB) {
-      // The app has been launched in the new tab.
-      if (automation_) {
-        AutomationJSONReply(automation_,
-                            reply_message_.release()).SendSuccess(NULL);
-      }
-      delete this;
-      return;
-    } else {
-      // The app has launched only if the loaded tab is in the new window.
-      NavigationController* controller =
-          content::Source<NavigationController>(source).ptr();
-      TabContents* tab =
-          TabContents::FromWebContents(controller->GetWebContents());
-      int window_id = tab ? tab->restore_tab_helper()->window_id().id() : -1;
-      if (window_id == new_window_id_) {
-        if (automation_) {
-          AutomationJSONReply(automation_,
-                              reply_message_.release()).SendSuccess(NULL);
-        }
-        delete this;
-        return;
-      }
+  if (type == chrome::NOTIFICATION_BROWSER_WINDOW_READY) {
+    new_window_id_ =
+        ExtensionTabUtil::GetWindowId(content::Source<Browser>(source).ptr());
+    return;
+  }
+
+  DCHECK_EQ(content::NOTIFICATION_LOAD_STOP, type);
+  TabContents* tab = TabContents::FromWebContents(
+      content::Source<NavigationController>(source)->GetWebContents());
+  if ((launch_container_ == extension_misc::LAUNCH_TAB) ||
+      (tab &&
+          (tab->restore_tab_helper()->window_id().id() == new_window_id_))) {

[Peter Kasting, 2012/08/29 01:21:38]

Note: There is a slight behavior change here in that the old code would behave
like the app launched if both of the following were true:
* We never saw NOTIFICATION_BROWSER_WINDOW_READY, so |new_window_id_| was never
set
* This notification came without an associated tab

While I'm not sure that combination could ever happen, it's clearly not an "app
launch", so that edge was fixed.

+    if (automation_) {
+      AutomationJSONReply(automation_,
+                          reply_message_.release()).SendSuccess(NULL);
     }
-  } else if (type == chrome::NOTIFICATION_BROWSER_WINDOW_READY) {
-    new_window_id_ = ExtensionTabUtil::GetWindowId(
-        content::Source<Browser>(source).ptr());
-  } else {
-    NOTREACHED();
+    delete this;
   }
 }
 
 namespace {
 
 // Returns whether all active notifications have an associated process ID.
 bool AreActiveNotificationProcessesReady() {
   NotificationUIManager* manager = g_browser_process->notification_ui_manager();
   const BalloonCollection::Balloons& balloons =
       manager->balloon_collection()->GetActiveBalloons();
@@ skipping 615 matching lines @@
     if (!profile) {
       AutomationJSONReply(automation_,
           reply_message_.release()).SendError("Profile could not be created.");
       return;
     }
   } else if (type == chrome::NOTIFICATION_BROWSER_OPENED) {
     // Store the new browser ID and continue waiting for a new tab within it
     // to stop loading.
     new_window_id_ = ExtensionTabUtil::GetWindowId(
         content::Source<Browser>(source).ptr());
-  } else if (type == content::NOTIFICATION_LOAD_STOP) {
+  } else {
+    DCHECK_EQ(content::NOTIFICATION_LOAD_STOP, type);
     // Only send the result if the loaded tab is in the new window.
     NavigationController* controller =
         content::Source<NavigationController>(source).ptr();
     TabContents* tab =
         TabContents::FromWebContents(controller->GetWebContents());
     int window_id = tab ? tab->restore_tab_helper()->window_id().id() : -1;
     if (window_id == new_window_id_) {
       if (automation_) {
         AutomationJSONReply(automation_, reply_message_.release())
             .SendSuccess(NULL);
       }
       delete this;
     }
-  } else {
-    NOTREACHED();
   }
 }
 
 ExtensionPopupObserver::ExtensionPopupObserver(
     AutomationProvider* automation,
     IPC::Message* reply_message,
     const std::string& extension_id)
     : automation_(automation->AsWeakPtr()),
       reply_message_(reply_message),
       extension_id_(extension_id) {
@@ skipping 92 matching lines @@
       if (automation_) {
         AutomationJSONReply(automation_, reply_message_.release())
             .SendSuccess(NULL);
       }
       delete this;
     }
   } else {
     NOTREACHED();
   }
 }