Make View Source and Search Web For inherit group properly. Original patch by John Spiegel (see http://codereview.chromium.org/466086 ), r=me.

BUG=1922
TEST="View Page Source" in context menu should open to the immediate right of the current tab.

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

4 files changed, 21 insertions, 15 deletions

diff --git a/chrome/browser/tab_contents/render_view_context_menu.cc b/chrome/browser/tab_contents/render_view_context_menu.cc
index 5292a68..aeb4a84 100644
--- a/chrome/browser/tab_contents/render_view_context_menu.cc
+++ b/chrome/browser/tab_contents/render_view_context_menu.cc
@@ -225,7 +225,7 @@ void RenderViewContextMenu::AppendSearchProvider() {
   bool is_search;
   profile_->GetSearchVersusNavigateClassifier()->Classify(
       params_.selection_text, std::wstring(), &is_search,
-      &selection_navigation_url_, &transition_, NULL, NULL);
+      &selection_navigation_url_, NULL, NULL, NULL);
   if (!selection_navigation_url_.is_valid())
     return;
 
@@ -653,7 +653,7 @@ void RenderViewContextMenu::ExecuteItemCommand(int id) {
 
     case IDS_CONTENT_CONTEXT_VIEWPAGESOURCE:
       OpenURL(GURL("view-source:" + params_.page_url.spec()),
-              NEW_FOREGROUND_TAB, PageTransition::GENERATED);
+              NEW_FOREGROUND_TAB, PageTransition::LINK);
       break;
 
     case IDS_CONTENT_CONTEXT_INSPECTELEMENT:
@@ -692,7 +692,7 @@ void RenderViewContextMenu::ExecuteItemCommand(int id) {
 
     case IDS_CONTENT_CONTEXT_VIEWFRAMESOURCE:
       OpenURL(GURL("view-source:" + params_.frame_url.spec()),
-              NEW_FOREGROUND_TAB, PageTransition::GENERATED);
+              NEW_FOREGROUND_TAB, PageTransition::LINK);
       break;
 
     case IDS_CONTENT_CONTEXT_VIEWFRAMEINFO: {
@@ -743,7 +743,8 @@ void RenderViewContextMenu::ExecuteItemCommand(int id) {
 
     case IDS_CONTENT_CONTEXT_SEARCHWEBFOR:
     case IDS_CONTENT_CONTEXT_GOTOURL: {
-      OpenURL(selection_navigation_url_, NEW_FOREGROUND_TAB, transition_);
+      OpenURL(selection_navigation_url_, NEW_FOREGROUND_TAB,
+              PageTransition::LINK);
       break;
     }
 
diff --git a/chrome/browser/tab_contents/render_view_context_menu.h b/chrome/browser/tab_contents/render_view_context_menu.h
index a4bb376..13bf753a 100644
--- a/chrome/browser/tab_contents/render_view_context_menu.h
+++ b/chrome/browser/tab_contents/render_view_context_menu.h
@@ -113,9 +113,6 @@ class RenderViewContextMenu {
   // a text selection.
   GURL selection_navigation_url_;
 
-  // The transition type of |selection_navigation_url_|.
-  PageTransition::Type transition_;
-
   DISALLOW_COPY_AND_ASSIGN(RenderViewContextMenu);
 };
 
diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index b44f01b..ff17975 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -396,28 +396,36 @@ void TabStripModel::AddTabContents(TabContents* contents,
                                    bool force_index,
                                    PageTransition::Type transition,
                                    bool foreground) {
+  // If the newly-opened tab is part of the same task as the parent tab, we want
+  // to inherit the parent's "group" attribute, so that if this tab is then
+  // closed we'll jump back to the parent tab.
+  // TODO(jbs): Perhaps instead of trying to infer this we should expose
+  // inherit_group directly to callers, who may have more context
+  bool inherit_group = false;
+
   if (transition == PageTransition::LINK && !force_index) {
-    // Only try to be clever if we're opening a LINK.
+    // We assume tabs opened via link clicks are part of the same task as their
+    // parent.  Note that when |force_index| is true (e.g. when the user
+    // drag-and-drops a link to the tab strip), callers aren't really handling
+    // link clicks, they just want to score the navigation like a link click in
+    // the history backend, so we don't inherit the group in this case.
     index = order_controller_->DetermineInsertionIndex(
         contents, transition, foreground);
+    inherit_group = true;
   } else {
     // For all other types, respect what was passed to us, normalizing -1s.
     if (index < 0)
       index = count();
   }
 
-  // Tabs opened from links inherit the "group" attribute of the Tab from which
-  // they were opened. This means when they're closed, that Tab will be
-  // selected again.
-  bool inherit_group = transition == PageTransition::LINK;
-  if (!inherit_group) {
+  if (transition == PageTransition::TYPED && index == count()) {
     // Also, any tab opened at the end of the TabStrip with a "TYPED"
     // transition inherit group as well. This covers the cases where the user
     // creates a New Tab (e.g. Ctrl+T, or clicks the New Tab button), or types
     // in the address bar and presses Alt+Enter. This allows for opening a new
     // Tab to quickly look up something. When this Tab is closed, the old one
     // is re-selected, not the next-adjacent.
-    inherit_group = transition == PageTransition::TYPED && index == count();
+    inherit_group = true;
   }
   InsertTabContentsAt(index, contents, foreground, inherit_group);
   if (inherit_group && transition == PageTransition::TYPED)
diff --git a/chrome/browser/tabs/tab_strip_model_order_controller.cc b/chrome/browser/tabs/tab_strip_model_order_controller.cc
index ad4fb22..b91c622 100644
--- a/chrome/browser/tabs/tab_strip_model_order_controller.cc
+++ b/chrome/browser/tabs/tab_strip_model_order_controller.cc
@@ -39,7 +39,7 @@ int TabStripModelOrderController::DetermineInsertionIndex(
     }
     NavigationController* opener =
         &tabstrip_->GetSelectedTabContents()->controller();
-    // Get the index of the next item opened by this tab, and insert before
+    // Get the index of the next item opened by this tab, and insert after
     // it...
     int index = tabstrip_->GetIndexOfLastTabContentsOpenedBy(
         opener, tabstrip_->selected_index());