recently closed menu: respect middle click

(and other disposition modifiers)

BUG=7678
TEST=manual

Review URL: http://codereview.chromium.org/8678002

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

13 files changed, 98 insertions, 72 deletions

diff --git a/chrome/browser/resources/ntp4/apps_page.js b/chrome/browser/resources/ntp4/apps_page.js
index aff343f..77c076f 100644
--- a/chrome/browser/resources/ntp4/apps_page.js
+++ b/chrome/browser/resources/ntp4/apps_page.js
@@ -446,7 +446,7 @@ cr.define('ntp4', function() {
 
       chrome.send('launchApp',
                   [this.appId, APP_LAUNCH.NTP_APPS_MAXIMIZED, url,
-                   e.altKey, e.ctrlKey, e.metaKey, e.shiftKey, e.button]);
+                   e.button, e.altKey, e.ctrlKey, e.metaKey, e.shiftKey]);
 
       // Don't allow the click to trigger a link or anything
       e.preventDefault();
@@ -461,7 +461,7 @@ cr.define('ntp4', function() {
       if (e.keyIdentifier == 'Enter') {
         chrome.send('launchApp',
                     [this.appId, APP_LAUNCH.NTP_APPS_MAXIMIZED, '',
-                     e.altKey, e.ctrlKey, e.metaKey, e.shiftKey, 0]);
+                     0, e.altKey, e.ctrlKey, e.metaKey, e.shiftKey]);
         e.preventDefault();
         e.stopPropagation();
       }
diff --git a/chrome/browser/resources/ntp4/recently_closed.js b/chrome/browser/resources/ntp4/recently_closed.js
index 95c6126..aaba0a6 100644
--- a/chrome/browser/resources/ntp4/recently_closed.js
+++ b/chrome/browser/resources/ntp4/recently_closed.js
@@ -89,20 +89,20 @@ cr.define('ntp4', function() {
         a.href = data.url;
         a.style.backgroundImage = 'url(chrome://favicon/' + data.url + ')';
         a.textContent = data.title;
-        // TODO(estade): add app ping url.
       }
 
-      function onActivate(e) {
+      function onClick(e) {
         chrome.send('recordAppLaunchByURL',
                     [encodeURIComponent(data.url),
                      ntp4.APP_LAUNCH.NTP_RECENTLY_CLOSED]);
-        // TODO(estade): don't convert to string.
-        chrome.send('reopenTab', [String(data.sessionId)]);
+        var index = Array.prototype.indexOf.call(a.parentNode.children, a);
+        chrome.send('reopenTab', [data.sessionId, index,
+            e.button, e.altKey, e.ctrlKey, e.metaKey, e.shiftKey]);
         // We are likely deleted by this point!
 
         e.preventDefault();
       }
-      a.addEventListener('activate', onActivate);
+      a.addEventListener('click', onClick);
 
       this.menu.appendChild(a);
       cr.ui.decorate(a, MenuItem);
diff --git a/chrome/browser/sessions/tab_restore_service.cc b/chrome/browser/sessions/tab_restore_service.cc
index 99d1153..ca1081c 100644
--- a/chrome/browser/sessions/tab_restore_service.cc
+++ b/chrome/browser/sessions/tab_restore_service.cc
@@ -302,12 +302,12 @@ void TabRestoreService::RestoreMostRecentEntry(
   if (entries_.empty())
     return;
 
-  RestoreEntryById(delegate, entries_.front()->id, false);
+  RestoreEntryById(delegate, entries_.front()->id, UNKNOWN);
 }
 
 void TabRestoreService::RestoreEntryById(TabRestoreServiceDelegate* delegate,
                                          SessionID::id_type id,
-                                         bool replace_existing_tab) {
+                                         WindowOpenDisposition disposition) {
   Entries::iterator i = GetEntryIteratorById(id);
   if (i == entries_.end()) {
     // Don't hoark here, we allow an invalid id.
@@ -339,7 +339,7 @@ void TabRestoreService::RestoreEntryById(TabRestoreServiceDelegate* delegate,
   // new browser into which we restore the tabs.
   if (entry->type == TAB) {
     Tab* tab = static_cast<Tab*>(entry);
-    delegate = RestoreTab(*tab, delegate, replace_existing_tab);
+    delegate = RestoreTab(*tab, delegate, disposition);
     delegate->ShowBrowserWindow();
   } else if (entry->type == WINDOW) {
     TabRestoreServiceDelegate* current_delegate = delegate;
@@ -356,8 +356,8 @@ void TabRestoreService::RestoreEntryById(TabRestoreServiceDelegate* delegate,
             delegate->AddRestoredTab(tab.navigations, delegate->GetTabCount(),
                                      tab.current_navigation_index,
                                      tab.extension_app_id,
-                                     (static_cast<int>(tab_i) ==
-                                         window->selected_tab_index),
+                                     static_cast<int>(tab_i) ==
+                                         window->selected_tab_index,
                                      tab.pinned, tab.from_last_session,
                                      tab.session_storage_namespace);
         if (restored_tab) {
@@ -377,7 +377,7 @@ void TabRestoreService::RestoreEntryById(TabRestoreServiceDelegate* delegate,
            tab_i != window->tabs.end(); ++tab_i) {
         const Tab& tab = *tab_i;
         if (tab.id == id) {
-          delegate = RestoreTab(tab, delegate, replace_existing_tab);
+          delegate = RestoreTab(tab, delegate, disposition);
           window->tabs.erase(tab_i);
           // If restoring the tab leaves the window with nothing else, delete it
           // as well.
@@ -401,7 +401,7 @@ void TabRestoreService::RestoreEntryById(TabRestoreServiceDelegate* delegate,
     }
     delegate->ShowBrowserWindow();
 
-    if (replace_existing_tab && current_delegate &&
+    if (disposition == CURRENT_TAB && current_delegate &&
         current_delegate->GetSelectedTabContents()) {
       current_delegate->CloseTab();
     }
@@ -909,31 +909,35 @@ void TabRestoreService::CreateEntriesFromCommands(
 TabRestoreServiceDelegate* TabRestoreService::RestoreTab(
     const Tab& tab,
     TabRestoreServiceDelegate* delegate,
-    bool replace_existing_tab) {
-  // |browser| will be NULL in cases where one isn't already available (eg,
-  // when invoked on Mac OS X with no windows open). In this case, create a
-  // new browser into which we restore the tabs.
-  if (replace_existing_tab && delegate) {
+    WindowOpenDisposition disposition) {
+  if (disposition == CURRENT_TAB && delegate) {
     delegate->ReplaceRestoredTab(tab.navigations,
                                  tab.current_navigation_index,
                                  tab.from_last_session,
                                  tab.extension_app_id,
                                  tab.session_storage_namespace);
   } else {
-    if (tab.has_browser())
+    // We only respsect the tab's original browser if there's no disposition.
+    if (disposition == UNKNOWN && tab.has_browser())
       delegate = TabRestoreServiceDelegate::FindDelegateWithID(tab.browser_id);
 
     int tab_index = -1;
-    if (delegate) {
+
+    // |delegate| will be NULL in cases where one isn't already available (eg,
+    // when invoked on Mac OS X with no windows open). In this case, create a
+    // new browser into which we restore the tabs.
+    if (delegate && disposition != NEW_WINDOW) {
       tab_index = tab.tabstrip_index;
     } else {
       delegate = TabRestoreServiceDelegate::Create(profile());
-      if (tab.has_browser()) {
+      if (tab.has_browser())
         UpdateTabBrowserIDs(tab.browser_id, delegate->GetSessionID().id());
-      }
     }
 
-    if (tab_index < 0 || tab_index > delegate->GetTabCount()) {
+    // Place the tab at the end if the tab index is no longer valid or
+    // we were passed a specific disposition.
+    if (tab_index < 0 || tab_index > delegate->GetTabCount() ||
+        disposition != UNKNOWN) {
       tab_index = delegate->GetTabCount();
     }
 
@@ -941,7 +945,9 @@ TabRestoreServiceDelegate* TabRestoreService::RestoreTab(
                              tab_index,
                              tab.current_navigation_index,
                              tab.extension_app_id,
-                             true, tab.pinned, tab.from_last_session,
+                             disposition != NEW_BACKGROUND_TAB,
+                             tab.pinned,
+                             tab.from_last_session,
                              tab.session_storage_namespace);
   }
   RecordAppLaunch(profile(), tab);
diff --git a/chrome/browser/sessions/tab_restore_service.h b/chrome/browser/sessions/tab_restore_service.h
index ee8dba1..c318050 100644
--- a/chrome/browser/sessions/tab_restore_service.h
+++ b/chrome/browser/sessions/tab_restore_service.h
@@ -17,6 +17,7 @@
 #include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/sessions/session_types.h"
 #include "content/browser/in_process_webkit/session_storage_namespace.h"
+#include "webkit/glue/window_open_disposition.h"
 
 class NavigationController;
 class Profile;
@@ -153,12 +154,13 @@ class TabRestoreService : public BaseSessionService {
   void RestoreMostRecentEntry(TabRestoreServiceDelegate* delegate);
 
   // Restores an entry by id. If there is no entry with an id matching |id|,
-  // this does nothing. If |replace_existing_tab| is true and id identifies a
-  // tab, the newly created tab replaces the selected tab in |delegate|. If
-  // |delegate| is NULL, this creates a new window for the entry.
+  // this does nothing. If |delegate| is NULL, this creates a new window for the
+  // entry. |disposition| is respected, but the attributes (tabstrip index,
+  // browser window) of the tab when it was closed will be respected if
+  // disposition is UNKNOWN.
   void RestoreEntryById(TabRestoreServiceDelegate* delegate,
                         SessionID::id_type id,
-                        bool replace_existing_tab);
+                        WindowOpenDisposition disposition);
 
   // Loads the tabs and previous session. This does nothing if the tabs
   // from the previous session have already been loaded.
@@ -263,13 +265,13 @@ class TabRestoreService : public BaseSessionService {
       std::vector<Entry*>* loaded_entries);
 
   // This is a helper function for RestoreEntryById() for restoring a single
-  // tab. If |replace_existing_tab| is true, the newly created tab replaces the
-  // selected tab in |delegate|. If |delegate| is NULL, this creates a new
-  // window for the entry. This returns the TabRestoreServiceDelegate into which
-  // the tab was restored.
+  // tab. If |delegate| is NULL, this creates a new window for the entry. This
+  // returns the TabRestoreServiceDelegate into which the tab was restored.
+  // |disposition| will be respected, but if it is UNKNOWN then the tab's
+  // original attributes will be respected instead.
   TabRestoreServiceDelegate* RestoreTab(const Tab& tab,
                                         TabRestoreServiceDelegate* delegate,
-                                        bool replace_existing_tab);
+                                        WindowOpenDisposition disposition);
 
   // Returns true if |tab| has more than one navigation. If |tab| has more
   // than one navigation |tab->current_navigation_index| is constrained based
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index d09cec9..6f51bc4 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -1289,6 +1289,7 @@ TabContents* Browser::AddRestoredTab(
     // contains similar logic.
     new_tab->view()->SizeContents(window_->GetRestoredBounds().size());
     new_tab->HideContents();
+    new_tab->controller().LoadIfNecessary();
   }
   SessionService* session_service =
       SessionServiceFactory::GetForProfileIfExisting(profile_);
diff --git a/chrome/browser/ui/cocoa/history_menu_cocoa_controller.mm b/chrome/browser/ui/cocoa/history_menu_cocoa_controller.mm
index 171cffc..7fa2988 100644
--- a/chrome/browser/ui/cocoa/history_menu_cocoa_controller.mm
+++ b/chrome/browser/ui/cocoa/history_menu_cocoa_controller.mm
@@ -44,7 +44,7 @@
       TabRestoreServiceFactory::GetForProfile(bridge_->profile());
   if (node->session_id && service) {
     service->RestoreEntryById(browser->tab_restore_service_delegate(),
-        node->session_id, false);
+        node->session_id, UNKNOWN);
   } else {
     DCHECK(node->url.is_valid());
     browser->OpenURL(node->url, GURL(), disposition,
diff --git a/chrome/browser/ui/gtk/global_history_menu.cc b/chrome/browser/ui/gtk/global_history_menu.cc
index 22e4170..1967c16 100644
--- a/chrome/browser/ui/gtk/global_history_menu.cc
+++ b/chrome/browser/ui/gtk/global_history_menu.cc
@@ -382,7 +382,7 @@ void GlobalHistoryMenu::OnRecentlyClosedItemActivated(GtkWidget* sender) {
       TabRestoreServiceFactory::GetForProfile(browser_->profile());
   if (item->session_id && service) {
     service->RestoreEntryById(browser_->tab_restore_service_delegate(),
-                              item->session_id, false);
+                              item->session_id, UNKNOWN);
   } else {
     DCHECK(item->url.is_valid());
     browser_->OpenURL(OpenURLParams(item->url, GURL(), disposition,
diff --git a/chrome/browser/ui/webui/ntp/app_launcher_handler.cc b/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
index 71d2ca8..7294940 100644
--- a/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
+++ b/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
@@ -30,6 +30,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/webui/extension_icon_source.h"
 #include "chrome/browser/ui/webui/ntp/new_tab_ui.h"
+#include "chrome/browser/ui/webui/web_ui_util.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
@@ -39,7 +40,6 @@
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/common/web_apps.h"
-#include "content/browser/disposition_utils.h"
 #include "content/browser/tab_contents/tab_contents.h"
 #include "content/public/browser/notification_service.h"
 #include "googleurl/src/gurl.h"
@@ -49,7 +49,6 @@
 #include "ui/base/animation/animation.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/codec/png_codec.h"
-#include "webkit/glue/window_open_disposition.h"
 
 namespace {
 
@@ -509,25 +508,12 @@ void AppLauncherHandler::HandleGetApps(const ListValue* args) {
 
 void AppLauncherHandler::HandleLaunchApp(const ListValue* args) {
   std::string extension_id;
-  double source = -1.0;
-  std::string url;
-  bool alt_key = false;
-  bool ctrl_key = false;
-  bool meta_key = false;
-  bool shift_key = false;
-  double button = 0.0;
-
   CHECK(args->GetString(0, &extension_id));
+  double source = -1.0;
   CHECK(args->GetDouble(1, &source));
+  std::string url;
   if (args->GetSize() > 2)
     CHECK(args->GetString(2, &url));
-  if (args->GetSize() > 3) {
-    CHECK(args->GetBoolean(3, &alt_key));
-    CHECK(args->GetBoolean(4, &ctrl_key));
-    CHECK(args->GetBoolean(5, &meta_key));
-    CHECK(args->GetBoolean(6, &shift_key));
-    CHECK(args->GetDouble(7, &button));
-  }
 
   extension_misc::AppLaunchBucket launch_bucket =
       static_cast<extension_misc::AppLaunchBucket>(
@@ -546,13 +532,8 @@ void AppLauncherHandler::HandleLaunchApp(const ListValue* args) {
 
   Profile* profile = extension_service_->profile();
 
-  // If the user pressed special keys when clicking, override the saved
-  // preference for launch container.
-  bool middle_button = (button == 1.0);
-  WindowOpenDisposition disposition =
-        disposition_utils::DispositionFromClick(middle_button, alt_key,
-                                                ctrl_key, meta_key, shift_key);
-
+  WindowOpenDisposition disposition = args->GetSize() > 3 ?
+        web_ui_util::GetDispositionFromClick(args, 3) : CURRENT_TAB;
   if (extension_id != extension_misc::kWebStoreAppId) {
     RecordAppLaunchByID(launch_bucket);
     extension_service_->apps_promo()->ExpireDefaultApps();
diff --git a/chrome/browser/ui/webui/ntp/recently_closed_tabs_handler.cc b/chrome/browser/ui/webui/ntp/recently_closed_tabs_handler.cc
index 9d364399..575cf7d 100644
--- a/chrome/browser/ui/webui/ntp/recently_closed_tabs_handler.cc
+++ b/chrome/browser/ui/webui/ntp/recently_closed_tabs_handler.cc
@@ -11,6 +11,7 @@
 #include "chrome/browser/sessions/tab_restore_service_delegate.h"
 #include "chrome/browser/sessions/tab_restore_service_factory.h"
 #include "chrome/browser/ui/webui/ntp/new_tab_ui.h"
+#include "chrome/browser/ui/webui/web_ui_util.h"
 #include "chrome/common/url_constants.h"
 #include "content/browser/tab_contents/tab_contents.h"
 
@@ -65,21 +66,21 @@ void RecentlyClosedTabsHandler::HandleReopenTab(const ListValue* args) {
   if (!delegate || !tab_restore_service_)
     return;
 
-  int session_to_restore;
-  if (!ExtractIntegerValue(args, &session_to_restore))
-    return;
+  double index = -1.0;
+  CHECK(args->GetDouble(1, &index));
 
-  const TabRestoreService::Entries& entries = tab_restore_service_->entries();
-  int index = 0;
-  for (TabRestoreService::Entries::const_iterator iter = entries.begin();
-       iter != entries.end(); ++iter, ++index) {
-    if (session_to_restore == (*iter)->id)
-      break;
-  }
   // There are actually less than 20 restore tab items displayed in the UI.
-  UMA_HISTOGRAM_ENUMERATION("NewTabPage.SessionRestore", index, 20);
+  UMA_HISTOGRAM_ENUMERATION("NewTabPage.SessionRestore",
+                            static_cast<int>(index), 20);
+
+  double session_to_restore = 0.0;
+  CHECK(args->GetDouble(0, &session_to_restore));
 
-  tab_restore_service_->RestoreEntryById(delegate, session_to_restore, true);
+  WindowOpenDisposition disposition =
+      web_ui_util::GetDispositionFromClick(args, 2);
+  tab_restore_service_->RestoreEntryById(delegate,
+                                         static_cast<int>(session_to_restore),
+                                         disposition);
   // The current tab has been nuked at this point; don't touch any member
   // variables.
 }
diff --git a/chrome/browser/ui/webui/web_ui_util.cc b/chrome/browser/ui/webui/web_ui_util.cc
index ee854ac..4914de6 100644
--- a/chrome/browser/ui/webui/web_ui_util.cc
+++ b/chrome/browser/ui/webui/web_ui_util.cc
@@ -7,6 +7,8 @@
 #include <vector>
 
 #include "base/base64.h"
+#include "base/values.h"
+#include "content/browser/disposition_utils.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/codec/png_codec.h"
 
@@ -39,4 +41,22 @@ std::string GetImageDataUrlFromResource(int res) {
   return str_url;
 }
 
+WindowOpenDisposition GetDispositionFromClick(const ListValue* args,
+                                              int start_index) {
+  double button = 0.0;
+  bool alt_key = false;
+  bool ctrl_key = false;
+  bool meta_key = false;
+  bool shift_key = false;
+
+  CHECK(args->GetDouble(start_index++, &button));
+  CHECK(args->GetBoolean(start_index++, &alt_key));
+  CHECK(args->GetBoolean(start_index++, &ctrl_key));
+  CHECK(args->GetBoolean(start_index++, &meta_key));
+  CHECK(args->GetBoolean(start_index++, &shift_key));
+  return disposition_utils::DispositionFromClick(button == 1.0, alt_key,
+                                                 ctrl_key, meta_key, shift_key);
+
+}
+
 }  // namespace web_ui_util
diff --git a/chrome/browser/ui/webui/web_ui_util.h b/chrome/browser/ui/webui/web_ui_util.h
index 2a7743f..99a2b46 100644
--- a/chrome/browser/ui/webui/web_ui_util.h
+++ b/chrome/browser/ui/webui/web_ui_util.h
@@ -8,6 +8,9 @@
 
 #include <string>
 
+#include "base/values.h"
+#include "webkit/glue/window_open_disposition.h"
+
 class SkBitmap;
 
 namespace web_ui_util {
@@ -24,6 +27,12 @@ std::string GetImageDataUrl(const SkBitmap& bitmap);
 // |resource_id|.
 std::string GetImageDataUrlFromResource(int resource_id);
 
+// Extracts a disposition from click event arguments. |args| should contain
+// an integer button and booleans alt key, ctrl key, meta key, and shift key
+// (in that order), starting at |start_index|.
+WindowOpenDisposition GetDispositionFromClick(const ListValue* args,
+                                              int start_index);
+
 }  // namespace web_ui_util
 
 #endif  // CHROME_BROWSER_UI_WEBUI_WEB_UI_UTIL_H_
diff --git a/content/browser/disposition_utils.h b/content/browser/disposition_utils.h
index a531350..6838a45 100644
--- a/content/browser/disposition_utils.h
+++ b/content/browser/disposition_utils.h
@@ -6,6 +6,7 @@
 #define CONTENT_BROWSER_DISPOSITION_UTILS_H_
 #pragma once
 
+#include "base/logging.h"
 #include "content/common/content_export.h"
 #include "webkit/glue/window_open_disposition.h"
 
diff --git a/content/browser/webui/web_ui.cc b/content/browser/webui/web_ui.cc
index 38e9839..202e755 100644
--- a/content/browser/webui/web_ui.cc
+++ b/content/browser/webui/web_ui.cc
@@ -224,6 +224,11 @@ bool WebUIMessageHandler::ExtractIntegerValue(const ListValue* value,
   std::string string_value;
   if (value->GetString(0, &string_value))
     return base::StringToInt(string_value, out_int);
+  double double_value;
+  if (value->GetDouble(0, &double_value)) {
+    *out_int = static_cast<int>(double_value);
+    return true;
+  }
   NOTREACHED();
   return false;
 }