ForeignSessionHelper Changes Needed for NTP Other Devices.

chrome/browser/android/foreign_session_helper.cc

Index: chrome/browser/android/foreign_session_helper.cc
diff --git a/chrome/browser/android/foreign_session_helper.cc b/chrome/browser/android/foreign_session_helper.cc
index 0c8f0d9483bea7f5ebb0d7883c6d2540de1cc761..817c536585d8bb845c58556a38626685a7e50f8e 100644
--- a/chrome/browser/android/foreign_session_helper.cc
+++ b/chrome/browser/android/foreign_session_helper.cc
@@ -43,6 +43,51 @@ SessionModelAssociator* GetSessionModelAssociator(Profile* profile) {
   return service->GetSessionModelAssociator();
 }
 
+bool shouldSkipUrl(const GURL& url) {
+  return (url.SchemeIs(chrome::kChromeNativeScheme) ||

[newt (away), 2013/09/20 17:15:03]

do we want to show other chrome:// pages? chrome://bookmarks or
chrome://history, for example? I'd expect these to be hidden as well.

[apiccion, 2013/09/20 20:47:57]

I can confirm these aren't being shown.

[newt (away), 2013/09/20 21:14:35]

why not? are they not being synced in the first place? if so, we should also
prevent chrome-native URLs from being synced. then they won't show up on any
platform. currently chrome-native:// URLs show up under other devices on Linux,
for example.

+      (url.SchemeIs(chrome::kChromeUIScheme) &&
+          url.host() == chrome::kChromeUINewTabHost));
+}
+
+bool shouldSkipTab(const SessionTab& tab) {
+    if (tab.navigations.empty())
+      return true;
+
+    int selected_index = tab.current_navigation_index;
+    if (selected_index < 0 ||
+        selected_index >= static_cast<int>(tab.navigations.size())) {
+      return true;
+    }
+
+    const ::sessions::SerializedNavigationEntry& current_navigation =
+        tab.navigations.at(selected_index);
+
+    GURL tab_url = current_navigation.virtual_url();
+    if (shouldSkipUrl(tab_url))
+      return true;
+    return false;
+}
+
+bool shouldSkipWindow(const SessionWindow* window) {
+  for (std::vector<SessionTab*>::const_iterator tab_it = window->tabs.begin();
+      tab_it != window->tabs.end(); ++tab_it) {
+    const SessionTab &tab = **tab_it;
+    if (!shouldSkipTab(tab))
+      return false;
+  }
+  return true;
+}
+
+bool shouldSkipSession(const browser_sync::SyncedSession* session) {
+  for (SyncedSession::SyncedWindowMap::const_iterator it =
+      session->windows.begin(); it != session->windows.end(); ++it) {
+    const SessionWindow* window = it->second;
+    if (!shouldSkipWindow(window))
+      return false;
+  }
+  return true;
+}
+
 void CopyTabsToJava(
     JNIEnv* env,
     const SessionWindow* window,
@@ -51,23 +96,26 @@ void CopyTabsToJava(
       tab_it != window->tabs.end(); ++tab_it) {
     const SessionTab &tab = **tab_it;
 
-    if (tab.navigations.empty())
+    if (shouldSkipTab(tab))
       continue;
 
+    int selected_index = tab.current_navigation_index;

[newt (away), 2013/09/20 17:15:03]

you just verified in shouldSkipTab(), that the current_nav_index is valid. no
need to check again.

[apiccion, 2013/09/20 20:47:57]

IS it okay if I put a DCHECK? I'd hate if we changed shouldSkipTab and somehow
forgot to re-add the check here.

[newt (away), 2013/09/20 21:14:35]

sure.

+    selected_index = std::max(
+        0,
+        std::min(selected_index,
+                 static_cast<int>(tab.navigations.size() - 1)));
+
     const ::sessions::SerializedNavigationEntry& current_navigation =
-        tab.navigations.at(tab.current_navigation_index);
+        tab.navigations.at(selected_index);
 
     GURL tab_url = current_navigation.virtual_url();
-    if (tab_url.SchemeIs(chrome::kChromeNativeScheme) ||
-        (tab_url.SchemeIs(chrome::kChromeUIScheme) &&
-            tab_url.host() == chrome::kChromeUINewTabHost))
-      continue;
 
     Java_ForeignSessionHelper_pushTab(
         env, j_window.obj(),
         ConvertUTF8ToJavaString(env, tab_url.spec()).Release(),
         ConvertUTF16ToJavaString(env, current_navigation.title()).Release(),
-        tab.timestamp.ToInternalValue(), tab.tab_id.id());
+        tab.timestamp.ToJavaTime(),
+        tab.tab_id.id());
   }
 }
 
@@ -79,10 +127,14 @@ void CopyWindowsToJava(
       session->windows.begin(); it != session->windows.end(); ++it) {
     const SessionWindow* window = it->second;
 
+    if (shouldSkipWindow(window))
+      continue;
+
     ScopedJavaLocalRef<jobject> last_pushed_window;
     last_pushed_window.Reset(
         Java_ForeignSessionHelper_pushWindow(
-            env, j_session.obj(), window->timestamp.ToInternalValue(),
+            env, j_session.obj(),
+            window->timestamp.ToJavaTime(),
             window->window_id.id()));
 
     CopyTabsToJava(env, window, last_pushed_window);
@@ -101,7 +153,6 @@ ForeignSessionHelper::ForeignSessionHelper(Profile* profile)
     : profile_(profile) {
   ProfileSyncService* service = ProfileSyncServiceFactory::GetInstance()->
       GetForProfile(profile);
-
   registrar_.Add(this, chrome::NOTIFICATION_SYNC_CONFIGURE_DONE,
                  content::Source<ProfileSyncService>(service));
   registrar_.Add(this, chrome::NOTIFICATION_FOREIGN_SESSION_UPDATED,
@@ -179,6 +230,8 @@ jboolean ForeignSessionHelper::GetForeignSessions(JNIEnv* env,
   // Note: we don't own the SyncedSessions themselves.
   for (size_t i = 0; i < sessions.size(); ++i) {
     const browser_sync::SyncedSession* session = sessions[i];
+    if (shouldSkipSession(session))
+      continue;
 
     const bool is_collapsed = collapsed_sessions->HasKey(session->session_tag);
 
@@ -191,9 +244,8 @@ jboolean ForeignSessionHelper::GetForeignSessions(JNIEnv* env,
             result,
             ConvertUTF8ToJavaString(env, session->session_tag).Release(),
             ConvertUTF8ToJavaString(env, session->session_name).Release(),
-            ConvertUTF8ToJavaString(env,
-                                    session->DeviceTypeAsString()).Release(),
-            session->modified_time.ToInternalValue()));
+            session->device_type,
+            session->modified_time.ToJavaTime()));
 
     CopyWindowsToJava(env, session, last_pushed_session);
   }
@@ -257,6 +309,14 @@ void ForeignSessionHelper::SetForeignSessionCollapsed(JNIEnv* env, jobject obj,
     update.Get()->Remove(ConvertJavaStringToUTF8(env, session_tag), NULL);
 }
 
+jboolean ForeignSessionHelper::GetForeignSessionCollapsed(JNIEnv* env,
+                                                          jobject obj,
+                                                          jstring session_tag) {
+  const DictionaryValue* dict = profile_->GetPrefs()->GetDictionary(
+      prefs::kNtpCollapsedForeignSessions);
+  return dict && dict->HasKey(ConvertJavaStringToUTF8(env, session_tag));
+}
+
 void ForeignSessionHelper::DeleteForeignSession(JNIEnv* env, jobject obj,
                                                 jstring session_tag) {
   SessionModelAssociator* associator = GetSessionModelAssociator(profile_);