[sync] Add Sessions integration tests

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

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/macros.h"
 #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/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;
 using fake_server::SessionsHierarchy;
 using sessions_helper::CheckInitialState;
 using sessions_helper::GetLocalWindows;
 using sessions_helper::GetSessionData;
 using sessions_helper::ModelAssociatorHasTabWithUrl;
+using sessions_helper::MoveTab;
+using sessions_helper::NavigateTab;
+using sessions_helper::NavigateTabBack;
+using sessions_helper::NavigateTabForward;
 using sessions_helper::OpenTabAndGetLocalWindows;
+using sessions_helper::OpenTab;
+using sessions_helper::OpenTabAtIndex;
 using sessions_helper::ScopedWindowMap;
 using sessions_helper::SessionWindowMap;
 using sessions_helper::SyncedSessionVector;
 using sessions_helper::WaitForTabsToLoad;
 using sessions_helper::WindowsMatch;
 using typed_urls_helper::GetUrlFromClient;
 
 namespace {
 
 void ExpectUniqueSampleGE(const HistogramTester& histogram_tester,
@@ skipping 28 matching lines @@
 #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(int browser_index, const std::vector<GURL> urls) {
+    ScopedWindowMap windows;
+    ASSERT_TRUE(GetLocalWindows(browser_index, &windows));

[skym, 2017/02/24 00:14:09]

GetLocalWindows takes a profile_index. This doesn't seem valid.

[Patrick Noland, 2017/02/24 01:00:40]

Good catch. This works in practice for reasons explained above, but the name
should be updated.

[Patrick Noland, 2017/02/27 18:53:26]

I ended up just removing this parameter, which is pointless for a single client
test to supply.

+    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 the
+  // profile at |profile_index|. This will time out if the hierarchy is never
+  // recorded.
+  void WaitForHierarchyOnServer(
+      const fake_server::SessionsHierarchy& hierarchy) {
+    SessionHierarchyMatchChecker checker(GetSyncService(0), hierarchy,
+                                         GetFakeServer());
+    EXPECT_TRUE(checker.Wait());
+  }
+
+  // Shortcut to call WaitForHierarchyOnServer for only |url| in a single
+  // window.
+  void WaitForURLOnServer(const GURL& url) {
+    fake_server::SessionsHierarchy expected_hierarchy;
+    expected_hierarchy.AddWindow(url.spec());
+    WaitForHierarchyOnServer(expected_hierarchy);
+  }
+
  private:
   DISALLOW_COPY_AND_ASSIGN(SingleClientSessionsSyncTest);
 };
 
 IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, Sanity) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
 
   ASSERT_TRUE(CheckInitialState(0));
 
   // 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));
   ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait());
 
   // Get foreign session data from client 0.
   SyncedSessionVector sessions;
   ASSERT_FALSE(GetSessionData(0, &sessions));
   ASSERT_EQ(0U, sessions.size());
 
   // Verify client didn't change.
   ScopedWindowMap new_windows;
   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) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
 
   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));
+  std::vector<GURL> urls({GURL(chrome::kChromeUIHistoryURL)});
   ASSERT_TRUE(WaitForTabsToLoad(0, urls));

[skym, 2017/02/24 00:14:09]

Inline the whole thing!

[Patrick Noland, 2017/02/27 18:53:26]

Done.

 
   // Verify the chrome history page synced.
   ASSERT_TRUE(ModelAssociatorHasTabWithUrl(0,
                                            GURL(chrome::kChromeUIHistoryURL)));
 }
 
 IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, TimestampMatchesHistory) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
 
   ASSERT_TRUE(CheckInitialState(0));
@@ skipping 42 matching lines @@
       for (auto it3 = (*it2)->navigations.begin();
            it3 != (*it2)->navigations.end(); ++it3) {
         EXPECT_EQ(200, it3->http_status_code());
         ++found_navigations;
       }
     }
   }
   ASSERT_EQ(1, found_navigations);
 }
 
+IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, FragmentURLNavigation) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(CheckInitialState(0));
+
+  ScopedWindowMap windows;
+  GURL url = GURL("http://127.0.0.1/bubba");
+  ASSERT_TRUE(OpenTabAndGetLocalWindows(0, url, &windows));
+  WaitForURLOnServer(url);
+
+  GURL fragment_url = GURL("http://127.0.0.1/bubba#fragment");
+  NavigateTab(0, fragment_url);
+  WaitForURLOnServer(fragment_url);
+}
+
+IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest,
+                       NavigationChainForwardBack) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(CheckInitialState(0));
+
+  ScopedWindowMap windows;
+  GURL first_url = GURL("http://127.0.0.1/foobar");
+  ASSERT_TRUE(OpenTabAndGetLocalWindows(0, first_url, &windows));
+  WaitForURLOnServer(first_url);
+
+  GURL second_url = GURL("http://127.0.0.1/barbaz");
+  NavigateTab(0, second_url);
+  WaitForURLOnServer(second_url);
+
+  NavigateTabBack(0);
+  WaitForURLOnServer(first_url);
+
+  ExpectNavigationChain(0, {first_url, second_url});
+
+  NavigateTabForward(0);
+  WaitForURLOnServer(second_url);
+
+  ExpectNavigationChain(0, {first_url, second_url});
+}
+
+IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest,
+                       NavigationChainAlteredDestructively) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(CheckInitialState(0));
+
+  ScopedWindowMap windows;
+  GURL base_url = GURL("http://127.0.0.1/bubba");
+  ASSERT_TRUE(OpenTabAndGetLocalWindows(0, base_url, &windows));
+  WaitForURLOnServer(base_url);
+
+  GURL first_url = GURL("http://127.0.0.1/foobar");
+  ASSERT_TRUE(NavigateTab(0, first_url));
+  WaitForURLOnServer(first_url);
+
+  // Check that the navigation chain matches the above sequence of {base_url,
+  // first_url}.
+  ExpectNavigationChain(0, {base_url, first_url});
+
+  NavigateTabBack(0);
+  WaitForURLOnServer(base_url);
+
+  GURL second_url = GURL("http://127.0.0.1/barbaz");
+  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(0, {base_url, second_url});
+}
+
+IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, OpenNewTab) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(CheckInitialState(0));
+
+  GURL base_url = GURL("http://127.0.0.1/bubba");
+  ASSERT_TRUE(OpenTabAtIndex(0, 0, base_url));
+
+  SessionsHierarchy hierarchy;
+  hierarchy.AddWindow(base_url.spec());
+  WaitForHierarchyOnServer(hierarchy);
+
+  GURL new_tab_url = GURL("http://127.0.0.1/foobar");
+  ASSERT_TRUE(OpenTabAtIndex(0, 1, new_tab_url));
+
+  SessionsHierarchy new_hierarchy;
+  new_hierarchy.AddWindow(
+      std::multiset<std::string>({base_url.spec(), new_tab_url.spec()}));

[skym, 2017/02/24 00:14:09]

Do you need to specify std::multiset<std::string> ?

[Patrick Noland, 2017/02/24 01:00:40]

The compiler complains if you don't :/

[skym, 2017/02/24 16:20:17]

But, why? Also, I don't think you need () and {} even if you are forced to
specify multiset

[Patrick Noland, 2017/02/27 18:53:26]

This is replaced with the initializer_list now, although the braces are required
for that

+  WaitForHierarchyOnServer(new_hierarchy);
+}
+
+IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, OpenNewWindow) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(CheckInitialState(0));
+
+  GURL base_url = GURL("http://127.0.0.1/bubba");
+  ASSERT_TRUE(OpenTab(0, base_url));
+
+  SessionsHierarchy hierarchy;
+  hierarchy.AddWindow(base_url.spec());
+  WaitForHierarchyOnServer(hierarchy);
+
+  GURL new_window_url = GURL("http://127.0.0.1/foobar");
+  AddBrowser(0);
+  ASSERT_TRUE(OpenTab(1, new_window_url));
+
+  SessionsHierarchy new_hierarchy;
+  new_hierarchy.AddWindow({base_url.spec()});

[skym, 2017/02/24 00:14:09]

Why the {}? It seems like it should do the same thing.

[Patrick Noland, 2017/02/27 18:53:26]

Done.

+  new_hierarchy.AddWindow({new_window_url.spec()});
+  WaitForHierarchyOnServer(new_hierarchy);
+}
+
+IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, TabMovedToOtherWindow) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(CheckInitialState(0));
+
+  GURL base_url = GURL("http://127.0.0.1/bubba");
+  GURL moved_tab_url = GURL("http://127.0.0.1/foobar");
+
+  ASSERT_TRUE(OpenTab(0, base_url));
+  ASSERT_TRUE(OpenTabAtIndex(0, 1, moved_tab_url));
+
+  GURL new_window_url = GURL("http://127.0.0.1/barbaz");
+  AddBrowser(0);
+  ASSERT_TRUE(OpenTab(1, new_window_url));
+
+  SessionsHierarchy hierarchy;
+  hierarchy.AddWindow(
+      std::multiset<std::string>({base_url.spec(), moved_tab_url.spec()}));
+  hierarchy.AddWindow({new_window_url.spec()});
+  WaitForHierarchyOnServer(hierarchy);
+
+  // Move tab 1 in browser 0 to browser 1.
+  MoveTab(0, 1, 1);
+
+  SessionsHierarchy changed_hierarchy;

[skym, 2017/02/24 00:14:09]

Doesn't this make you want to have a constructor for SessionsHierarchy that
could take an initializer list of std::strings and/or
std::multiset<std::string>s? It could all get inlined!

[Patrick Noland, 2017/02/24 01:00:40]

Yes it does, I'll just add that

[skym, 2017/02/24 16:20:17]

Bonus points for making a fancy one that can take a combo of strings and
multisets

[Patrick Noland, 2017/02/27 18:53:26]

That requires a variadic template, which should be used judiciously, so I'm
going to stick to a regular initializer_list

+  changed_hierarchy.AddWindow({base_url.spec()});
+  changed_hierarchy.AddWindow(std::multiset<std::string>(
+      {new_window_url.spec(), moved_tab_url.spec()}));
+  WaitForHierarchyOnServer(changed_hierarchy);
+}
+
 IN_PROC_BROWSER_TEST_F(SingleClientSessionsSyncTest, CookieJarMismatch) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
 
   ASSERT_TRUE(CheckInitialState(0));
 
   ScopedWindowMap old_windows;
   sync_pb::ClientToServerMessage message;
 
   // The HistogramTester objects are scoped to allow more precise verification.
   {
@@ skipping 42 matching lines @@
     // Verify the cookie jar mismatch bool is set to false.
     ASSERT_TRUE(GetFakeServer()->GetLastCommitMessage(&message));
     ASSERT_FALSE(message.commit().config_params().cookie_jar_mismatch());
 
     // Verify the histograms were recorded properly.
     ExpectUniqueSampleGE(histogram_tester, "Sync.CookieJarMatchOnNavigation",
                          true, 1);
     histogram_tester.ExpectTotalCount("Sync.CookieJarEmptyOnMismatch", 0);
   }
 }