reland of [sync] Add Sessions integration tests

chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc

Index: chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc
diff --git a/chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc b/chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc
index 69c51bc0f108aa593ab06e87307ff52c5bd97e41..509b1998034bbbcdd20fa1334f7b1d8753e82ef8 100644
--- a/chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc
+++ b/chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc
@@ -6,24 +6,19 @@
 #include "base/test/histogram_tester.h"
 #include "chrome/browser/sessions/session_service.h"
 #include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
+#include "chrome/browser/sync/test/integration/session_hierarchy_match_checker.h"
 #include "chrome/browser/sync/test/integration/sessions_helper.h"
 #include "chrome/browser/sync/test/integration/sync_test.h"
 #include "chrome/browser/sync/test/integration/typed_urls_helper.h"
 #include "chrome/browser/sync/test/integration/updated_progress_marker_checker.h"
-#include "chrome/common/chrome_switches.h"
 #include "chrome/common/url_constants.h"
 #include "components/browser_sync/profile_sync_service.h"
 #include "components/history/core/browser/history_types.h"
 #include "components/sessions/core/session_types.h"
 #include "components/sync/base/time.h"
 #include "components/sync/driver/sync_driver_switches.h"
-#include "components/sync/test/fake_server/fake_server_verifier.h"
 #include "components/sync/test/fake_server/sessions_hierarchy.h"
 
-#if defined(OS_CHROMEOS)
-#include "chromeos/chromeos_switches.h"
-#endif
-
 using base::HistogramBase;
 using base::HistogramSamples;
 using base::HistogramTester;
@@ -32,7 +27,12 @@ using sessions_helper::CheckInitialState;
 using sessions_helper::GetLocalWindows;
 using sessions_helper::GetSessionData;
 using sessions_helper::ModelAssociatorHasTabWithUrl;
-using sessions_helper::OpenTabAndGetLocalWindows;
+using sessions_helper::MoveTab;
+using sessions_helper::NavigateTab;
+using sessions_helper::NavigateTabBack;
+using sessions_helper::NavigateTabForward;
+using sessions_helper::OpenTab;
+using sessions_helper::OpenTabAtIndex;
 using sessions_helper::ScopedWindowMap;
 using sessions_helper::SessionWindowMap;
 using sessions_helper::SyncedSessionVector;
@@ -49,7 +49,7 @@ void ExpectUniqueSampleGE(const HistogramTester& histogram_tester,
   std::unique_ptr<HistogramSamples> samples =
       histogram_tester.GetHistogramSamplesSinceCreation(name);
   int sample_count = samples->GetCount(sample);
-  EXPECT_GE(expected_inclusive_lower_bound, sample_count);
+  EXPECT_GE(sample_count, expected_inclusive_lower_bound);
   EXPECT_EQ(sample_count, samples->TotalCount());
 }
 
@@ -60,31 +60,44 @@ class SingleClientSessionsSyncTest : public SyncTest {
   SingleClientSessionsSyncTest() : SyncTest(SINGLE_CLIENT) {}
   ~SingleClientSessionsSyncTest() override {}
 
-  void SetUpCommandLine(base::CommandLine* cl) override {
-    // This is a hacky override of the switches set in
-    // SyncTest::SetUpCommandLine() to avoid the switch that speeds up nudge
-    // delays. CookieJarMismatch asserts exact histogram counts assuming that
-    // sync is relatively slow, so we preserve that assumption.
-    if (!cl->HasSwitch(switches::kDisableBackgroundNetworking))
-      cl->AppendSwitch(switches::kDisableBackgroundNetworking);
-
-    if (!cl->HasSwitch(switches::kSyncShortInitialRetryOverride))
-      cl->AppendSwitch(switches::kSyncShortInitialRetryOverride);
-
-#if defined(OS_CHROMEOS)
-    // kIgnoreUserProfileMappingForTests will let UserManager always return
-    // active user. If this switch is not set, sync test's profile will not
-    // match UserManager's active user, then UserManager won't return active
-    // user to our tests.
-    if (!cl->HasSwitch(chromeos::switches::kIgnoreUserProfileMappingForTests))
-      cl->AppendSwitch(chromeos::switches::kIgnoreUserProfileMappingForTests);
-#endif
+  void ExpectNavigationChain(const std::vector<GURL>& urls) {
+    ScopedWindowMap windows;
+    ASSERT_TRUE(GetLocalWindows(0, &windows));
+    ASSERT_EQ(windows.begin()->second->tabs.size(), 1u);
+    sessions::SessionTab* tab = windows.begin()->second->tabs[0].get();
+
+    int index = 0;
+    EXPECT_EQ(urls.size(), tab->navigations.size());
+    for (auto it = tab->navigations.begin(); it != tab->navigations.end();
+         ++it, ++index) {
+      EXPECT_EQ(urls[index], it->virtual_url());
+    }
+  }
+
+  // Block until the expected hierarchy is recorded on the FakeServer for
+  // profile 0. This will time out if the hierarchy is never
+  // recorded.
+  void WaitForHierarchyOnServer(
+      const fake_server::SessionsHierarchy& hierarchy) {
+    SessionHierarchyMatchChecker checker(hierarchy, GetSyncService(0),
+                                         GetFakeServer());
+    EXPECT_TRUE(checker.Wait());
+  }
+
+  // Shortcut to call WaitForHierarchyOnServer for only |url| in a single
+  // window.
+  void WaitForURLOnServer(const GURL& url) {
+    WaitForHierarchyOnServer({{url.spec()}});
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(SingleClientSessionsSyncTest);
 };
 
+static const char* kURL1 = "data:text/html,<html><title>Test</title></html>";

[skym, 2017/03/02 22:59:43]

Can you make these the first thing that's declared in the anonymous namespace?

[Patrick Noland, 2017/03/02 23:32:48]

Done.

+static const char* kURL2 = "data:text/html,<html><title>Test2</title></html>";
+static const char* kURL3 = "data:text/html,<html><title>Test3</title></html>";
+
 IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, Sanity) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
 
@@ -92,8 +105,9 @@ IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, Sanity) {
 
   // Add a new session to client 0 and wait for it to sync.
   ScopedWindowMap old_windows;
-  GURL url = GURL("http://127.0.0.1/bubba");
-  ASSERT_TRUE(OpenTabAndGetLocalWindows(0, url, &old_windows));
+  GURL url = GURL(kURL1);
+  ASSERT_TRUE(OpenTab(0, url));
+  ASSERT_TRUE(GetLocalWindows(0, &old_windows));
   ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait());
 
   // Get foreign session data from client 0.
@@ -106,18 +120,13 @@ IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, Sanity) {
   ASSERT_TRUE(GetLocalWindows(0, &new_windows));
   ASSERT_TRUE(WindowsMatch(old_windows, new_windows));
 
-  fake_server::FakeServerVerifier fake_server_verifier(GetFakeServer());
-  SessionsHierarchy expected_sessions;
-  expected_sessions.AddWindow(url.spec());
-  ASSERT_TRUE(fake_server_verifier.VerifySessions(expected_sessions));
+  WaitForURLOnServer(url);
 }
 
 IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, NoSessions) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
 
-  fake_server::FakeServerVerifier fake_server_verifier(GetFakeServer());
-  SessionsHierarchy expected_sessions;
-  ASSERT_TRUE(fake_server_verifier.VerifySessions(expected_sessions));
+  WaitForHierarchyOnServer(SessionsHierarchy());
 }
 
 IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, ChromeHistory) {
@@ -125,17 +134,8 @@ IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, ChromeHistory) {
 
   ASSERT_TRUE(CheckInitialState(0));
 
-  // Add a new session to client 0 and wait for it to sync.
-  ScopedWindowMap old_windows;
-  ASSERT_TRUE(OpenTabAndGetLocalWindows(0, GURL(chrome::kChromeUIHistoryURL),
-                                        &old_windows));
-  std::vector<GURL> urls;
-  urls.push_back(GURL(chrome::kChromeUIHistoryURL));
-  ASSERT_TRUE(WaitForTabsToLoad(0, urls));
-
-  // Verify the chrome history page synced.
-  ASSERT_TRUE(ModelAssociatorHasTabWithUrl(0,
-                                           GURL(chrome::kChromeUIHistoryURL)));
+  ASSERT_TRUE(OpenTab(0, GURL(chrome::kChromeUIHistoryURL)));
+  WaitForURLOnServer(GURL(chrome::kChromeUIHistoryURL));
 }
 
 IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, TimestampMatchesHistory) {
@@ -147,7 +147,8 @@ IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, TimestampMatchesHistory) {
   const GURL url("data:text/html,<html><title>Test</title></html>");
 
   ScopedWindowMap windows;
-  ASSERT_TRUE(OpenTabAndGetLocalWindows(0, url, &windows));
+  ASSERT_TRUE(OpenTab(0, url));
+  ASSERT_TRUE(GetLocalWindows(0, &windows));
 
   int found_navigations = 0;
   for (auto it = windows.begin(); it != windows.end(); ++it) {
@@ -175,10 +176,11 @@ IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, ResponseCodeIsPreserved) {
   ASSERT_TRUE(CheckInitialState(0));
 
   // We want a URL that doesn't 404 and has a non-empty title.

[skym, 2017/03/02 22:59:43]

I don't think we want to repeat this every time we use a data: url.

[Patrick Noland, 2017/03/02 23:32:48]

Done.

-  const GURL url("data:text/html,<html><title>Test</title></html>");
+  const GURL url(kURL1);
 
   ScopedWindowMap windows;
-  ASSERT_TRUE(OpenTabAndGetLocalWindows(0, url, &windows));
+  ASSERT_TRUE(OpenTab(0, url));
+  ASSERT_TRUE(GetLocalWindows(0, &windows));
 
   int found_navigations = 0;
   for (auto it = windows.begin(); it != windows.end(); ++it) {
@@ -194,6 +196,137 @@ IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, ResponseCodeIsPreserved) {
   ASSERT_EQ(1, found_navigations);
 }
 
+IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, FragmentURLNavigation) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(CheckInitialState(0));
+
+  // We want a URL that doesn't 404 and has a valid fragment.

[skym, 2017/03/02 22:59:43]

see above

[Patrick Noland, 2017/03/02 23:32:48]

Done.

+  const GURL url(
+      "data:text/html,<html><title>Test</title><body><div "

[skym, 2017/03/02 22:59:43]

constant? Also, like what is this thing really testing? Is there special
fragment logic that it exercises? Cause it sure doesn't expect any different
outward behavior than the other tests. You navigate to a different url and see
it show up on the server. I would kind of like this test to be deleted.

[Patrick Noland, 2017/03/02 23:32:48]

I think it's fair to check this. Fragments are processed entirely client side
and aren't included in the GET request to the server. This test ensures that
despite this specialness, the entire URL is still synced.

+      "name='fragment'></div></body></html>");
+  ASSERT_TRUE(OpenTab(0, url));
+  WaitForURLOnServer(url);
+
+  const GURL fragment_url(
+      "data:text/html,<html><title>Test</title><body><div "

[skym, 2017/03/02 22:59:43]

you get a constant, you get a constant, everyone gets a constant!

[Patrick Noland, 2017/03/02 23:32:48]

Done.

+      "name='fragment'></div></body></html>#fragment");
+  NavigateTab(0, fragment_url);
+  WaitForURLOnServer(fragment_url);
+}
+
+IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest,
+                       NavigationChainForwardBack) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(CheckInitialState(0));
+
+  GURL first_url = GURL(kURL1);
+  ASSERT_TRUE(OpenTab(0, first_url));
+  WaitForURLOnServer(first_url);
+
+  GURL second_url = GURL(kURL2);
+  NavigateTab(0, second_url);
+  WaitForURLOnServer(second_url);
+
+  NavigateTabBack(0);
+  WaitForURLOnServer(first_url);
+
+  ExpectNavigationChain({first_url, second_url});
+
+  NavigateTabForward(0);
+  WaitForURLOnServer(second_url);
+
+  ExpectNavigationChain({first_url, second_url});
+}
+
+IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest,
+                       NavigationChainAlteredDestructively) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(CheckInitialState(0));
+
+  GURL base_url = GURL(kURL1);
+  ASSERT_TRUE(OpenTab(0, base_url));
+  WaitForURLOnServer(base_url);
+
+  GURL first_url = GURL(kURL2);
+  ASSERT_TRUE(NavigateTab(0, first_url));
+  WaitForURLOnServer(first_url);
+
+  // Check that the navigation chain matches the above sequence of {base_url,
+  // first_url}.
+  ExpectNavigationChain({base_url, first_url});
+
+  NavigateTabBack(0);
+  WaitForURLOnServer(base_url);
+
+  GURL second_url = GURL(kURL3);
+  NavigateTab(0, second_url);
+  WaitForURLOnServer(second_url);
+
+  NavigateTabBack(0);
+  WaitForURLOnServer(base_url);
+
+  // Check that the navigation chain contains second_url where first_url was
+  // before.
+  ExpectNavigationChain({base_url, second_url});
+}
+
+IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, OpenNewTab) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(CheckInitialState(0));
+
+  GURL base_url = GURL(kURL1);
+  ASSERT_TRUE(OpenTabAtIndex(0, 0, base_url));
+
+  WaitForURLOnServer(base_url);
+
+  GURL new_tab_url = GURL(kURL2);
+  ASSERT_TRUE(OpenTabAtIndex(0, 1, new_tab_url));
+
+  WaitForHierarchyOnServer(
+      SessionsHierarchy({{base_url.spec(), new_tab_url.spec()}}));
+}
+
+IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, OpenNewWindow) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(CheckInitialState(0));
+
+  GURL base_url = GURL(kURL1);
+  ASSERT_TRUE(OpenTab(0, base_url));
+
+  WaitForURLOnServer(base_url);
+
+  GURL new_window_url = GURL(kURL2);
+  AddBrowser(0);
+  ASSERT_TRUE(OpenTab(1, new_window_url));
+
+  WaitForHierarchyOnServer(
+      SessionsHierarchy({{base_url.spec()}, {new_window_url.spec()}}));
+}
+
+IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, TabMovedToOtherWindow) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(CheckInitialState(0));
+
+  GURL base_url = GURL(kURL1);
+  GURL moved_tab_url = GURL(kURL2);
+
+  ASSERT_TRUE(OpenTab(0, base_url));
+  ASSERT_TRUE(OpenTabAtIndex(0, 1, moved_tab_url));
+
+  GURL new_window_url = GURL(kURL3);
+  AddBrowser(0);
+  ASSERT_TRUE(OpenTab(1, new_window_url));
+
+  WaitForHierarchyOnServer(SessionsHierarchy(
+      {{base_url.spec(), moved_tab_url.spec()}, {new_window_url.spec()}}));
+
+  // Move tab 1 in browser 0 to browser 1.
+  MoveTab(0, 1, 1);
+
+  WaitForHierarchyOnServer(SessionsHierarchy(
+      {{base_url.spec()}, {new_window_url.spec(), moved_tab_url.spec()}}));
+}
+
 IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, CookieJarMismatch) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
 
@@ -207,8 +340,8 @@ IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, CookieJarMismatch) {
     HistogramTester histogram_tester;
 
     // Add a new session to client 0 and wait for it to sync.
-    GURL url = GURL("http://127.0.0.1/bubba");
-    ASSERT_TRUE(OpenTabAndGetLocalWindows(0, url, &old_windows));
+    GURL url = GURL(kURL1);
+    ASSERT_TRUE(OpenTab(0, url));
     TriggerSyncForModelTypes(0, syncer::ModelTypeSet(syncer::SESSIONS));
     ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait());
 
@@ -219,7 +352,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, CookieJarMismatch) {
     ASSERT_TRUE(message.commit().config_params().cookie_jar_mismatch());
 
     // It is possible that multiple sync cycles occured during the call to
-    // OpenTabAndGetLocalWindows, which would cause multiple identical samples.
+    // OpenTab, which would cause multiple identical samples.
     ExpectUniqueSampleGE(histogram_tester, "Sync.CookieJarMatchOnNavigation",
                          false, 1);
     ExpectUniqueSampleGE(histogram_tester, "Sync.CookieJarEmptyOnMismatch",
@@ -241,8 +374,9 @@ IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, CookieJarMismatch) {
     HistogramTester histogram_tester;
 
     // Trigger a sync and wait for it.
-    GURL url = GURL("http://127.0.0.1/bubba2");
-    ASSERT_TRUE(OpenTabAndGetLocalWindows(0, url, &old_windows));
+    GURL url = GURL(kURL2);
+    ASSERT_TRUE(OpenTab(0, url));
+    ASSERT_TRUE(GetLocalWindows(0, &old_windows));
     TriggerSyncForModelTypes(0, syncer::ModelTypeSet(syncer::SESSIONS));
     ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait());