NTP5: Starting implementation of a native menu for showing other device sessions.

chrome/browser/ui/webui/ntp/foreign_session_handler.cc

Index: chrome/browser/ui/webui/ntp/foreign_session_handler.cc
diff --git a/chrome/browser/ui/webui/ntp/foreign_session_handler.cc b/chrome/browser/ui/webui/ntp/foreign_session_handler.cc
index 3de215c6b91670b84fa1f4a868bb93b21743805d..b0f787c4300b3547d773a17f18b5b06e05d258cf 100644
--- a/chrome/browser/ui/webui/ntp/foreign_session_handler.cc
+++ b/chrome/browser/ui/webui/ntp/foreign_session_handler.cc
@@ -20,6 +20,7 @@
 #include "chrome/browser/sessions/session_restore.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
+#include "chrome/browser/ui/search/other_device_menu.h"
 #include "chrome/browser/ui/webui/ntp/new_tab_ui.h"
 #include "chrome/browser/ui/webui/session_favicon_source.h"
 #include "chrome/browser/ui/webui/web_ui_util.h"
@@ -29,18 +30,14 @@
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
-#include "content/public/browser/web_ui.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace browser_sync {
 
-// Maximum number of session we're going to display on the NTP
+// Maximum number of sessions we're going to display on the NTP
 static const size_t kMaxSessionsToShow = 10;
 
-// Invalid value, used to note that we don't have a tab or window number.
-static const int kInvalidId = -1;
-
 namespace {
 
 // Comparator function for use with std::sort that will sort sessions by
@@ -60,6 +57,87 @@ void ForeignSessionHandler::RegisterUserPrefs(PrefService* prefs) {
                                 PrefService::UNSYNCABLE_PREF);
 }
 
+// static
+void ForeignSessionHandler::OpenForeignSession(
+    content::WebUI* web_ui,
+    const std::string& session_string_value,
+    SessionID::id_type window_num,
+    SessionID::id_type tab_id,
+    const WindowOpenDisposition& disposition) {
+
+  SessionModelAssociator* associator = GetModelAssociator(web_ui);
+  if (!associator)
+    return;
+
+  if (tab_id != kInvalidId) {
+    // We don't actually care about |window_num|, this is just a sanity check.
+    DCHECK_LT(kInvalidId, window_num);
+    const SessionTab* tab;
+    if (!associator->GetForeignTab(session_string_value, tab_id, &tab)) {
+      LOG(ERROR) << "Failed to load foreign tab.";
+      return;
+    }
+    SessionRestore::RestoreForeignSessionTab(
+        web_ui->GetWebContents(), *tab, disposition);
+  } else {
+    std::vector<const SessionWindow*> windows;
+    // Note: we don't own the ForeignSessions themselves.
+    if (!associator->GetForeignSession(session_string_value, &windows)) {
+      LOG(ERROR) << "ForeignSessionHandler failed to get session data from"
+          "SessionModelAssociator.";
+      return;
+    }
+    std::vector<const SessionWindow*>::const_iterator iter_begin =
+        windows.begin() + ((window_num == kInvalidId) ? 0 : window_num);
+    std::vector<const SessionWindow*>::const_iterator iter_end =
+        ((window_num == kInvalidId) ?
+        std::vector<const SessionWindow*>::const_iterator(windows.end()) :
+        iter_begin + 1);
+    SessionRestore::RestoreForeignSessionWindows(
+        Profile::FromWebUI(web_ui), iter_begin, iter_end);
+  }
+}
+
+// static
+bool ForeignSessionHandler::SessionTabToValue(
+    const SessionTab& tab,
+    DictionaryValue* dictionary) {
+  if (tab.navigations.empty())
+    return false;
+  int selected_index = tab.current_navigation_index;

[dhollowa, 2012/10/08 19:15:09]

nit: combine lines 107 and 108?

[jeremycho, 2012/10/09 01:52:13]

Done.

+  selected_index = std::max(
+      0,
+      std::min(selected_index,
+               static_cast<int>(tab.navigations.size() - 1)));
+  const TabNavigation& current_navigation =
+      tab.navigations.at(selected_index);
+  GURL tab_url = current_navigation.virtual_url();
+  if (tab_url == GURL(chrome::kChromeUINewTabURL))
+    return false;
+  NewTabUI::SetUrlTitleAndDirection(dictionary, current_navigation.title(),
+                                    tab_url);
+  dictionary->SetString("type", "tab");
+  dictionary->SetDouble("timestamp",
+                        static_cast<double>(tab.timestamp.ToInternalValue()));
+  // TODO(jeremycho): Rename to tabId?

[dhollowa, 2012/10/08 19:15:09]

What's the answer?

[jeremycho, 2012/10/09 01:52:13]

I think it makes sense, but this name is used in the JS, other handlers, and
possibly other places, so I prefer to that separately.

[dhollowa, 2012/10/09 15:13:42]

Ok, then please reflect this intention in the comment.  Optionally track it with
a bug if you think it might get forgotten.

[jeremycho, 2012/10/09 19:45:38]

Done.

+  dictionary->SetInteger("sessionId", tab.tab_id.id());
+  return true;
+}
+
+// static
+SessionModelAssociator* ForeignSessionHandler::GetModelAssociator(
+    content::WebUI* web_ui) {
+  Profile* profile = Profile::FromWebUI(web_ui);
+  ProfileSyncService* service =
+      ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile);
+
+  // Only return the associator if it exists and it is done syncing sessions.
+  if (service && service->ShouldPushChanges())
+    return service->GetSessionModelAssociator();
+
+  return NULL;
+}
+
 void ForeignSessionHandler::RegisterMessages() {
   Init();
   web_ui()->RegisterMessageCallback("getForeignSessions",
@@ -113,17 +191,6 @@ void ForeignSessionHandler::Observe(
   }
 }
 
-SessionModelAssociator* ForeignSessionHandler::GetModelAssociator() {
-  Profile* profile = Profile::FromWebUI(web_ui());
-  ProfileSyncService* service =
-      ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile);
-
-  // Only return the associator if it exists and it is done syncing sessions.
-  if (service && service->ShouldPushChanges())
-    return service->GetSessionModelAssociator();
-
-  return NULL;
-}
 
 bool ForeignSessionHandler::IsTabSyncEnabled() {
   Profile* profile = Profile::FromWebUI(web_ui());
@@ -138,7 +205,7 @@ string16 ForeignSessionHandler::FormatSessionTime(const base::Time& time) {
 }
 
 void ForeignSessionHandler::HandleGetForeignSessions(const ListValue* args) {
-  SessionModelAssociator* associator = GetModelAssociator();
+  SessionModelAssociator* associator = GetModelAssociator(web_ui());
   std::vector<const SyncedSession*> sessions;
 
   ListValue session_list;
@@ -229,39 +296,11 @@ void ForeignSessionHandler::HandleOpenForeignSession(const ListValue* args) {
     return;
   }
 
-  SessionModelAssociator* associator = GetModelAssociator();
-  if (!associator)
-    return;
+  WindowOpenDisposition disposition =
+      web_ui_util::GetDispositionFromClick(args, 3);
 
-  if (tab_id != kInvalidId) {
-    // We don't actually care about |window_num|, this is just a sanity check.
-    DCHECK_LT(kInvalidId, window_num);
-    const SessionTab* tab;
-    if (!associator->GetForeignTab(session_string_value, tab_id, &tab)) {
-      LOG(ERROR) << "Failed to load foreign tab.";
-      return;
-    }
-    WindowOpenDisposition disposition =
-        web_ui_util::GetDispositionFromClick(args, 3);
-    SessionRestore::RestoreForeignSessionTab(
-        web_ui()->GetWebContents(), *tab, disposition);
-  } else {
-    std::vector<const SessionWindow*> windows;
-    // Note: we don't own the ForeignSessions themselves.
-    if (!associator->GetForeignSession(session_string_value, &windows)) {
-      LOG(ERROR) << "ForeignSessionHandler failed to get session data from"
-          "SessionModelAssociator.";
-      return;
-    }
-    std::vector<const SessionWindow*>::const_iterator iter_begin =
-        windows.begin() + ((window_num == kInvalidId) ? 0 : window_num);
-    std::vector<const SessionWindow*>::const_iterator iter_end =
-        ((window_num == kInvalidId) ?
-        std::vector<const SessionWindow*>::const_iterator(windows.end()) :
-        iter_begin + 1);
-    SessionRestore::RestoreForeignSessionWindows(
-        Profile::FromWebUI(web_ui()), iter_begin, iter_end);
-  }
+  OpenForeignSession(
+      web_ui(), session_string_value, window_num, tab_id, disposition);
 }
 
 void ForeignSessionHandler::HandleSetForeignSessionCollapsed(
@@ -312,31 +351,10 @@ void ForeignSessionHandler::HandleShowOtherDeviceSessionPopup(
   double client_y;
   CHECK(args->GetDouble(2, &client_y));
 
-  // TODO(vadimt): implement this method.
-}
-
-bool ForeignSessionHandler::SessionTabToValue(
-    const SessionTab& tab,
-    DictionaryValue* dictionary) {
-  if (tab.navigations.empty())
-    return false;
-  int selected_index = tab.current_navigation_index;
-  selected_index = std::max(
-      0,
-      std::min(selected_index,
-               static_cast<int>(tab.navigations.size() - 1)));
-  const TabNavigation& current_navigation =
-      tab.navigations.at(selected_index);
-  GURL tab_url = current_navigation.virtual_url();
-  if (tab_url == GURL(chrome::kChromeUINewTabURL))
-    return false;
-  NewTabUI::SetUrlTitleAndDirection(dictionary, current_navigation.title(),
-                                    tab_url);
-  dictionary->SetString("type", "tab");
-  dictionary->SetDouble("timestamp",
-                        static_cast<double>(tab.timestamp.ToInternalValue()));
-  dictionary->SetInteger("sessionId", tab.tab_id.id());
-  return true;
+    OtherDeviceMenu* menu = new OtherDeviceMenu(
+        web_ui(),
+        session_string_value, gfx::Point(client_x, client_y));
+    menu->ShowMenu();
 }
 
 bool ForeignSessionHandler::SessionWindowToValue(