Formalise a CHECK for the NULL NavigationEntry in ScriptBadgeController, and

chrome/browser/extensions/script_badge_controller.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/extensions/script_badge_controller.h"
 
+#include "base/logging.h"
+#include "base/string_util.h"
 #include "chrome/browser/extensions/browser_event_router.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_action.h"
 #include "chrome/common/extensions/extension_messages.h"
 #include "chrome/common/extensions/extension_set.h"
 #include "chrome/common/chrome_notification_types.h"
 #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_service.h"
 #include "content/public/browser/web_contents.h"
+#include "googleurl/src/gurl.h"
 #include "ipc/ipc_message.h"
 #include "ipc/ipc_message_macros.h"
 
 namespace extensions {
 
 ScriptBadgeController::ScriptBadgeController(TabContents* tab_contents,
                                              ScriptExecutor* script_executor)
     : ScriptExecutor::Observer(script_executor),
       content::WebContentsObserver(tab_contents->web_contents()),
       tab_contents_(tab_contents) {
@@ skipping 63 matching lines @@
 
       return extension->ShowConfigureContextMenus() ?
           ACTION_SHOW_CONTEXT_MENU : ACTION_NONE;
   }
 
   return ACTION_NONE;
 }
 
 void ScriptBadgeController::OnExecuteScriptFinished(
     const std::string& extension_id,
-    bool success,
-    int32 page_id,
     const std::string& error,
+    int32 on_page_id,
+    const GURL& on_url,
     const base::ListValue& script_results) {
-  if (success && page_id == GetPageID()) {
+  int32 current_page_id = GetPageID();
+
+  // Tracking down http://crbug.com/138323.
+  CHECK_GE(current_page_id, 0) <<
+      "Expect a page ID of " << on_page_id << ", on URL \"" << on_url <<

[Jeffrey Yasskin, 2012/08/06 11:39:02]

Re "buffer on the stack", I assume you'll copy on_url and GetURL to buffers
here? I do see some check failure messages in the crash logs, so maybe it's not
necessary to force the url into the minidump? Ok with me either way.

+      "\", but there was no navigation entry. Current URL=\"" <<
+      tab_contents_->web_contents()->GetURL() << "\", " <<
+      "extension_id=" << extension_id;
+
+  if (error.empty() && on_page_id == current_page_id) {
     if (MarkExtensionExecuting(extension_id))
       NotifyChange();
   }
 }
 
 ExtensionService* ScriptBadgeController::GetExtensionService() {
   return extensions::ExtensionSystem::Get(
       tab_contents_->profile())->extension_service();
 }
 
 int32 ScriptBadgeController::GetPageID() {
-  return tab_contents_->web_contents()->GetController().GetActiveEntry()->
-      GetPageID();
+  content::NavigationEntry* nav_entry =
+      tab_contents_->web_contents()->GetController().GetActiveEntry();
+  return nav_entry ? nav_entry->GetPageID() : -1;
 }
 
 void ScriptBadgeController::NotifyChange() {
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_EXTENSION_LOCATION_BAR_UPDATED,
       content::Source<Profile>(tab_contents_->profile()),
       content::Details<TabContents>(tab_contents_));
 }
 
 void ScriptBadgeController::DidNavigateMainFrame(
@@ skipping 19 matching lines @@
 bool ScriptBadgeController::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(ScriptBadgeController, message)
     IPC_MESSAGE_HANDLER(ExtensionHostMsg_ContentScriptsExecuting,
                         OnContentScriptsExecuting)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
+namespace {
+std::string JoinExtensionIDs(const std::set<std::string>& ids) {
+  std::vector<std::string> as_vector(ids.begin(), ids.end());
+  return "[" + JoinString(as_vector, ',') + "]";
+}
+}  // namespace
+
 void ScriptBadgeController::OnContentScriptsExecuting(
-    const std::set<std::string>& extension_ids, int32 page_id) {
-  if (page_id != GetPageID())
+    const std::set<std::string>& extension_ids,
+    int32 on_page_id,
+    const GURL& on_url) {
+  int32 current_page_id = GetPageID();
+
+  // Tracking down http://crbug.com/138323.
+  CHECK_GE(current_page_id, 0) <<
+      "Expect a page ID of " << on_page_id << ", on URL \"" << on_url <<
+      "\", but there was no navigation entry. Current URL=\"" <<
+      tab_contents_->web_contents()->GetURL() << "\", " <<
+      "extension_ids=" << JoinExtensionIDs(extension_ids);
+
+  if (on_page_id != current_page_id)
     return;
 
   bool changed = false;
   for (std::set<std::string>::const_iterator it = extension_ids.begin();
        it != extension_ids.end(); ++it) {
     changed |= MarkExtensionExecuting(*it);
   }
   if (changed)
     NotifyChange();
 }
@@ skipping 45 matching lines @@
       current_actions_.erase(it);
       break;
     }
   }
 
   CHECK_EQ(size_before, current_actions_.size() + 1);
   return true;
 }
 
 }  // namespace extensions