Add SessionID::{IdForTab,IdForWindowContainingTab}.

The chain of calls IdForTab is an abbreviation for appears ~17 times, some of
which are their own abbreviations:
http://code.google.com/searchframe#search/&exact_package=chromium&q=%22restore_tab_helper()-%3Esession_id().id()%22

Currently, they're still implemented in terms of the restore_tab_helper, but since tab and window IDs are more fundamental than session restore nowadays, it would be possible to change that if the dependency becomes awkward.

Review URL: https://chromiumcodereview.appspot.com/10680005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@146962 0039d316-1c4b-4281-b951-d872f2087c98

16 files changed, 61 insertions, 44 deletions

diff --git a/chrome/browser/extensions/active_tab_permission_manager.cc b/chrome/browser/extensions/active_tab_permission_manager.cc
index fac7e64..087d447 100644
--- a/chrome/browser/extensions/active_tab_permission_manager.cc
+++ b/chrome/browser/extensions/active_tab_permission_manager.cc
@@ -6,8 +6,8 @@
 
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
-#include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/extensions/extension.h"
@@ -122,7 +122,7 @@ void ActiveTabPermissionManager::InsertActiveURL(const GURL& url) {
 }
 
 int32 ActiveTabPermissionManager::tab_id() {
-  return tab_contents_->extension_tab_helper()->tab_id();
+  return SessionID::IdForTab(tab_contents_);
 }
 
 int32 ActiveTabPermissionManager::GetPageID() {
diff --git a/chrome/browser/extensions/active_tab_unittest.cc b/chrome/browser/extensions/active_tab_unittest.cc
index 8eb6de5..ee93147 100644
--- a/chrome/browser/extensions/active_tab_unittest.cc
+++ b/chrome/browser/extensions/active_tab_unittest.cc
@@ -10,6 +10,7 @@
 #include "base/values.h"
 #include "chrome/browser/extensions/active_tab_permission_manager.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/browser/ui/tab_contents/test_tab_contents.h"
 #include "chrome/common/chrome_notification_types.h"
@@ -73,7 +74,7 @@ class ActiveTabTest : public TabContentsTestHarness {
 
  protected:
   int tab_id() {
-    return tab_contents()->extension_tab_helper()->tab_id();
+    return SessionID::IdForTab(tab_contents());
   }
 
   ActiveTabPermissionManager* active_tab_permission_manager() {
diff --git a/chrome/browser/extensions/api/extension_action/script_badge_apitest.cc b/chrome/browser/extensions/api/extension_action/script_badge_apitest.cc
index e361396..890ab19 100644
--- a/chrome/browser/extensions/api/extension_action/script_badge_apitest.cc
+++ b/chrome/browser/extensions/api/extension_action/script_badge_apitest.cc
@@ -5,7 +5,6 @@
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_browser_event_router.h"
 #include "chrome/browser/extensions/extension_service.h"
-#include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
@@ -25,8 +24,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, ScriptBadge) {
   const ExtensionAction* script_badge = extension->script_badge();
   ASSERT_TRUE(script_badge);
 
-  const int tab_id = chrome::GetActiveTabContents(browser())->
-      extension_tab_helper()->tab_id();
+  const int tab_id = SessionID::IdForTab(
+      chrome::GetActiveTabContents(browser()));
   EXPECT_EQ(GURL(extension->GetResourceURL("default_popup.html")),
             script_badge->GetPopupUrl(tab_id));
 
diff --git a/chrome/browser/extensions/api/tabs/tabs.cc b/chrome/browser/extensions/api/tabs/tabs.cc
index 8aa36f9..75f3faf 100644
--- a/chrome/browser/extensions/api/tabs/tabs.cc
+++ b/chrome/browser/extensions/api/tabs/tabs.cc
@@ -1195,7 +1195,7 @@ bool UpdateTabFunction::RunImpl() {
       error_ = keys::kNoSelectedTabError;
       return false;
     }
-    tab_id = contents->extension_tab_helper()->tab_id();
+    tab_id = SessionID::IdForTab(contents);
   } else {
     EXTENSION_FUNCTION_VALIDATE(tab_value->GetAsInteger(&tab_id));
   }
@@ -1620,7 +1620,7 @@ bool CaptureVisibleTabFunction::RunImpl() {
   // permission to do this.
   if (!GetExtension()->CanCaptureVisiblePage(
         web_contents->GetURL(),
-        tab_contents->extension_tab_helper()->tab_id(),
+        SessionID::IdForTab(tab_contents),
         &error_)) {
     return false;
   }
diff --git a/chrome/browser/extensions/extension_context_menu_model.cc b/chrome/browser/extensions/extension_context_menu_model.cc
index 355e3e7..c9b26cf 100644
--- a/chrome/browser/extensions/extension_context_menu_model.cc
+++ b/chrome/browser/extensions/extension_context_menu_model.cc
@@ -7,7 +7,6 @@
 #include "base/utf_string_conversions.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/extensions/extension_tab_util.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
@@ -79,8 +78,7 @@ bool ExtensionContextMenuModel::IsCommandIdEnabled(int command_id) const {
     if (!tab_contents)
       return false;
 
-    return extension_action_->HasPopup(
-        tab_contents->extension_tab_helper()->tab_id());
+    return extension_action_->HasPopup(SessionID::IdForTab(tab_contents));
   } else if (command_id == DISABLE || command_id == UNINSTALL) {
     // Some extension types can not be disabled or uninstalled.
     return extensions::ExtensionSystem::Get(
diff --git a/chrome/browser/extensions/extension_tab_util.cc b/chrome/browser/extensions/extension_tab_util.cc
index 40ce283..412d5be 100644
--- a/chrome/browser/extensions/extension_tab_util.cc
+++ b/chrome/browser/extensions/extension_tab_util.cc
@@ -5,10 +5,9 @@
 #include "chrome/browser/extensions/extension_tab_util.h"
 
 #include "chrome/browser/extensions/api/tabs/tabs_constants.h"
-#include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/net/url_fixer_upper.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/sessions/restore_tab_helper.h"
+#include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -45,8 +44,7 @@ int ExtensionTabUtil::GetWindowIdOfTabStripModel(
 }
 
 int ExtensionTabUtil::GetTabId(const WebContents* web_contents) {
-  const TabContents* tab = TabContents::FromWebContents(web_contents);
-  return tab ? tab->extension_tab_helper()->tab_id() : -1;
+  return SessionID::IdForTab(TabContents::FromWebContents(web_contents));
 }
 
 std::string ExtensionTabUtil::GetTabStatusText(bool is_loading) {
@@ -54,8 +52,8 @@ std::string ExtensionTabUtil::GetTabStatusText(bool is_loading) {
 }
 
 int ExtensionTabUtil::GetWindowIdOfTab(const WebContents* web_contents) {
-  const TabContents* tab = TabContents::FromWebContents(web_contents);
-  return tab ? tab->extension_tab_helper()->window_id() : -1;
+  return SessionID::IdForWindowContainingTab(
+      TabContents::FromWebContents(web_contents));
 }
 
 DictionaryValue* ExtensionTabUtil::CreateTabValue(const WebContents* contents) {
@@ -186,8 +184,7 @@ bool ExtensionTabUtil::GetTabById(int tab_id,
       TabStripModel* target_tab_strip = target_browser->tab_strip_model();
       for (int i = 0; i < target_tab_strip->count(); ++i) {
         TabContents* target_contents = target_tab_strip->GetTabContentsAt(i);
-        if (target_contents->restore_tab_helper()->session_id().id() ==
-            tab_id) {
+        if (SessionID::IdForTab(target_contents) == tab_id) {
           if (browser)
             *browser = target_browser;
           if (tab_strip)
diff --git a/chrome/browser/extensions/page_action_controller.cc b/chrome/browser/extensions/page_action_controller.cc
index 3c6e3d4..45e196e 100644
--- a/chrome/browser/extensions/page_action_controller.cc
+++ b/chrome/browser/extensions/page_action_controller.cc
@@ -9,6 +9,7 @@
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
+#include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/common/extensions/extension_set.h"
 #include "chrome/common/chrome_notification_types.h"
@@ -92,7 +93,7 @@ void PageActionController::DidNavigateMainFrame(
 
   for (size_t i = 0; i < current_actions.size(); ++i) {
     current_actions[i]->ClearAllValuesForTab(
-        tab_contents_->extension_tab_helper()->tab_id());
+        SessionID::IdForTab(tab_contents_));
   }
 
   NotifyChange();
diff --git a/chrome/browser/extensions/page_action_controller_unittest.cc b/chrome/browser/extensions/page_action_controller_unittest.cc
index e498d88..13a0b95 100644
--- a/chrome/browser/extensions/page_action_controller_unittest.cc
+++ b/chrome/browser/extensions/page_action_controller_unittest.cc
@@ -8,9 +8,9 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "chrome/browser/extensions/extension_service.h"
-#include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/extensions/page_action_controller.h"
 #include "chrome/browser/extensions/test_extension_system.h"
+#include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/browser/ui/tab_contents/test_tab_contents.h"
 #include "chrome/common/extensions/extension.h"
@@ -44,7 +44,7 @@ class PageActionControllerTest : public TabContentsTestHarness {
 
  protected:
   int tab_id() {
-    return tab_contents()->extension_tab_helper()->tab_id();
+    return SessionID::IdForTab(tab_contents());
   }
 
   ExtensionService* extension_service_;
diff --git a/chrome/browser/extensions/script_badge_controller.cc b/chrome/browser/extensions/script_badge_controller.cc
index f1c3673..0400f58 100644
--- a/chrome/browser/extensions/script_badge_controller.cc
+++ b/chrome/browser/extensions/script_badge_controller.cc
@@ -8,6 +8,7 @@
 #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"
@@ -49,8 +50,7 @@ void ScriptBadgeController::GetAttentionFor(
   // TODO(jyasskin): Modify the icon's appearance to indicate that the
   // extension is merely asking for permission to run:
   // http://crbug.com/133142
-  script_badge->SetIsVisible(
-      tab_contents_->extension_tab_helper()->tab_id(), true);
+  script_badge->SetIsVisible(SessionID::IdForTab(tab_contents_), true);
 
   NotifyChange();
 }
@@ -79,7 +79,7 @@ LocationBarController::Action ScriptBadgeController::OnClicked(
       GetExtensionService()->browser_event_router()->ScriptBadgeExecuted(
           tab_contents_->profile(),
           *script_badge,
-          tab_contents_->extension_tab_helper()->tab_id());
+          SessionID::IdForTab(tab_contents_));
       return ACTION_NONE;
     case 3:  // right
       return extension->ShowConfigureContextMenus() ?
@@ -185,8 +185,8 @@ bool ScriptBadgeController::MarkExtensionExecuting(
   if (!script_badge)
     return false;
 
-  script_badge->RunIconAnimation(
-      tab_contents_->extension_tab_helper()->tab_id());
+  script_badge->RunIconAnimation(SessionID::IdForTab(tab_contents_));
+
   return true;
 }
 
diff --git a/chrome/browser/extensions/tab_helper.cc b/chrome/browser/extensions/tab_helper.cc
index 5073e71..fc27c38 100644
--- a/chrome/browser/extensions/tab_helper.cc
+++ b/chrome/browser/extensions/tab_helper.cc
@@ -12,7 +12,7 @@
 #include "chrome/browser/extensions/script_executor.h"
 #include "chrome/browser/extensions/webstore_inline_installer.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/sessions/restore_tab_helper.h"
+#include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
@@ -105,12 +105,8 @@ bool TabHelper::CanCreateApplicationShortcuts() const {
 #endif
 }
 
-int TabHelper::tab_id() const {
-  return tab_contents_->restore_tab_helper()->session_id().id();
-}
-
 int TabHelper::window_id() const {
-  return tab_contents_->restore_tab_helper()->window_id().id();
+  return SessionID::IdForWindowContainingTab(tab_contents_);
 }
 
 void TabHelper::SetExtensionApp(const Extension* extension) {
@@ -146,7 +142,8 @@ SkBitmap* TabHelper::GetExtensionAppIcon() {
 
 void TabHelper::RenderViewCreated(RenderViewHost* render_view_host) {
   render_view_host->Send(
-      new ExtensionMsg_SetTabId(render_view_host->GetRoutingID(), tab_id()));
+      new ExtensionMsg_SetTabId(render_view_host->GetRoutingID(),
+                                SessionID::IdForTab(tab_contents_)));
 }
 
 void TabHelper::DidNavigateMainFrame(
@@ -165,8 +162,7 @@ void TabHelper::DidNavigateMainFrame(
        it != service->extensions()->end(); ++it) {
     ExtensionAction* browser_action = (*it)->browser_action();
     if (browser_action) {
-      browser_action->ClearAllValuesForTab(
-          tab_contents_->restore_tab_helper()->session_id().id());
+      browser_action->ClearAllValuesForTab(SessionID::IdForTab(tab_contents_));
       content::NotificationService::current()->Notify(
           chrome::NOTIFICATION_EXTENSION_BROWSER_ACTION_UPDATED,
           content::Source<ExtensionAction>(browser_action),
diff --git a/chrome/browser/extensions/tab_helper.h b/chrome/browser/extensions/tab_helper.h
index 13cac4e..956d10e 100644
--- a/chrome/browser/extensions/tab_helper.h
+++ b/chrome/browser/extensions/tab_helper.h
@@ -62,9 +62,6 @@ class TabHelper : public content::WebContentsObserver,
     pending_web_app_action_ = action;
   }
 
-  // Gets the ID of the tab.
-  int tab_id() const;
-
   // Gets the window ID of the tab.
   int window_id() const;
 
diff --git a/chrome/browser/sessions/session_id.cc b/chrome/browser/sessions/session_id.cc
index fc7e249..880f3cd 100644
--- a/chrome/browser/sessions/session_id.cc
+++ b/chrome/browser/sessions/session_id.cc
@@ -1,11 +1,22 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// 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/sessions/session_id.h"
 
+#include "chrome/browser/sessions/restore_tab_helper.h"
+#include "chrome/browser/ui/tab_contents/tab_contents.h"
+
 static SessionID::id_type next_id = 1;
 
 SessionID::SessionID() {
   id_ = next_id++;
 }
+
+SessionID::id_type SessionID::IdForTab(const TabContents* tab) {
+  return tab ? tab->restore_tab_helper()->session_id().id() : -1;
+}
+
+SessionID::id_type SessionID::IdForWindowContainingTab(const TabContents* tab) {
+  return tab ? tab->restore_tab_helper()->window_id().id() : -1;
+}
diff --git a/chrome/browser/sessions/session_id.h b/chrome/browser/sessions/session_id.h
index 031fc14..19f1dc4 100644
--- a/chrome/browser/sessions/session_id.h
+++ b/chrome/browser/sessions/session_id.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -7,6 +7,9 @@
 
 #include "base/basictypes.h"
 
+class Browser;
+class TabContents;
+
 // Uniquely identifies a tab or window for the duration of a session.
 class SessionID {
  public:
@@ -15,6 +18,18 @@ class SessionID {
   SessionID();
   ~SessionID() {}
 
+  // This value is immutable for a given tab. It will be unique across Chrome
+  // within the current session, but may be re-used across sessions. Returns -1
+  // for NULL, and will never return -1 for a valid tab.
+  static id_type IdForTab(const TabContents* tab);
+
+  // If the tab has ever been attached to a window, this is the value of
+  // Browser::session_id().id() for some Browser object. Returns -1 for NULL, or
+  // another value that's not equal to any Browser's id for a tab that has never
+  // been attached to a window. IdForWindowContainingTab() returns the old
+  // window for a tab that's currently being dragged between windows.
+  static id_type IdForWindowContainingTab(const TabContents* tab);
+
   // Returns the underlying id.
   void set_id(id_type id) { id_ = id; }
   id_type id() const { return id_; }
diff --git a/chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm b/chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm
index e0f5fa1..4b8106f 100644
--- a/chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm
+++ b/chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm
@@ -13,6 +13,7 @@
 #include "chrome/browser/extensions/location_bar_controller.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/ui/browser.h"
 #import "chrome/browser/ui/cocoa/extensions/extension_action_context_menu.h"
 #import "chrome/browser/ui/cocoa/extensions/extension_popup_controller.h"
@@ -53,7 +54,7 @@ PageActionDecoration::PageActionDecoration(
       preview_enabled_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(scoped_icon_animation_observer_(
           page_action->GetIconAnimation(
-              owner->GetTabContents()->extension_tab_helper()->tab_id()),
+              SessionID::IdForTab(owner->GetTabContents())),
           this)) {
   const Extension* extension = browser->profile()->GetExtensionService()->
       GetExtensionById(page_action->extension_id(), false);
diff --git a/chrome/browser/ui/gtk/location_bar_view_gtk.cc b/chrome/browser/ui/gtk/location_bar_view_gtk.cc
index e81ab3d..71f5613 100644
--- a/chrome/browser/ui/gtk/location_bar_view_gtk.cc
+++ b/chrome/browser/ui/gtk/location_bar_view_gtk.cc
@@ -1630,7 +1630,7 @@ LocationBarViewGtk::PageActionViewGtk::PageActionViewGtk(
       preview_enabled_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(scoped_icon_animation_observer_(
           page_action->GetIconAnimation(
-              owner->GetTabContents()->extension_tab_helper()->tab_id()),
+              SessionID::IdForTab(owner->GetTabContents())),
           this)) {
   event_box_.Own(gtk_event_box_new());
   gtk_widget_set_size_request(event_box_.get(),
diff --git a/chrome/browser/ui/views/location_bar/page_action_image_view.cc b/chrome/browser/ui/views/location_bar/page_action_image_view.cc
index d744442..4d086e0 100644
--- a/chrome/browser/ui/views/location_bar/page_action_image_view.cc
+++ b/chrome/browser/ui/views/location_bar/page_action_image_view.cc
@@ -15,6 +15,7 @@
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
@@ -48,7 +49,7 @@ PageActionImageView::PageActionImageView(LocationBarView* owner,
       popup_(NULL),
       ALLOW_THIS_IN_INITIALIZER_LIST(scoped_icon_animation_observer_(
           page_action->GetIconAnimation(
-              owner->GetTabContents()->extension_tab_helper()->tab_id()),
+              SessionID::IdForTab(owner->GetTabContents())),
           this)) {
   const Extension* extension = owner_->profile()->GetExtensionService()->
       GetExtensionById(page_action->extension_id(), false);