[Session restore] Add MRU logic to loading of background pages.

chrome/browser/sessions/session_restore_browsertest.cc

Index: chrome/browser/sessions/session_restore_browsertest.cc
diff --git a/chrome/browser/sessions/session_restore_browsertest.cc b/chrome/browser/sessions/session_restore_browsertest.cc
index 14966d7bc49640176a7daf4bbf1a9593fef1cdb6..5809f011e29b96b6cab0bbc8e83ffb707cc35432 100644
--- a/chrome/browser/sessions/session_restore_browsertest.cc
+++ b/chrome/browser/sessions/session_restore_browsertest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <algorithm>
 #include <vector>
 
 #include "base/command_line.h"
@@ -212,12 +213,16 @@ class SessionRestoreTest : public InProcessBrowserTest {
   const BrowserList* active_browser_list_;
 };
 
-// Activates the smart restore behaviour and can track the loading of tabs.
-class SmartSessionRestoreTest : public SessionRestoreTest,
-                                public content::NotificationObserver {
+// Activates the smart restore behaviour in "simple" mode and tracks the loading
+// of tabs.
+class SmartSessionRestoreSimpleTest : public SessionRestoreTest,
+                                      public content::NotificationObserver {
  public:
-  SmartSessionRestoreTest() {}
+  SmartSessionRestoreSimpleTest() {}
   void StartObserving(int num_tabs) {
+    // Start by clearing everything so it can be reused in the same test.
+    web_contents_.clear();
+    registrar_.RemoveAll();
     num_tabs_ = num_tabs;
     registrar_.Add(this, content::NOTIFICATION_LOAD_START,
                    content::NotificationService::AllSources());
@@ -261,7 +266,24 @@ class SmartSessionRestoreTest : public SessionRestoreTest,
   scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
   int num_tabs_;
 
-  DISALLOW_COPY_AND_ASSIGN(SmartSessionRestoreTest);
+  DISALLOW_COPY_AND_ASSIGN(SmartSessionRestoreSimpleTest);
+};
+
+class SmartSessionRestoreMRUTest : public SmartSessionRestoreSimpleTest {
+ public:
+  SmartSessionRestoreMRUTest() {}
+
+ protected:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+        switches::kForceFieldTrials, "IntelligentSessionRestore/TestGroup/");
+    base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+        switches::kForceFieldTrialParams,
+        "IntelligentSessionRestore.TestGroup:PrioritizeTabs/mru");

[sky, 2015/05/12 21:25:30]

Ouch. Is the old one dumb?

[Georges Khalil, 2015/05/15 16:55:30]

No comments :)

+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SmartSessionRestoreMRUTest);
 };
 
 // Verifies that restored tabs have a root window. This is important
@@ -1333,7 +1355,7 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, SessionStorageAfterTabReplace) {
   EXPECT_EQ(1, new_browser->tab_strip_model()->count());
 }
 
-IN_PROC_BROWSER_TEST_F(SmartSessionRestoreTest, CorrectLoadingOrder) {
+IN_PROC_BROWSER_TEST_F(SmartSessionRestoreSimpleTest, CorrectLoadingOrder) {
   ASSERT_EQ(SessionRestore::SMART_RESTORE_MODE_SIMPLE,
             SessionRestore::GetSmartRestoreMode());
 
@@ -1379,7 +1401,6 @@ IN_PROC_BROWSER_TEST_F(SmartSessionRestoreTest, CorrectLoadingOrder) {
 
   session.push_back(&window);
   Profile* profile = browser()->profile();
-  ui_test_utils::BrowserAddedObserver window_observer;
   std::vector<Browser*> browsers = SessionRestore::RestoreForeignSessionWindows(
       profile, browser()->host_desktop_type(), session.begin(), session.end());
 
@@ -1406,3 +1427,84 @@ IN_PROC_BROWSER_TEST_F(SmartSessionRestoreTest, CorrectLoadingOrder) {
   // here to avoid a crash.
   window.tabs.clear();
 }
+
+IN_PROC_BROWSER_TEST_F(SmartSessionRestoreMRUTest, CorrectLoadingOrder) {
+  const int NumTabs = 6;

[sky, 2015/05/12 21:25:30]

num_tabs

[Georges Khalil, 2015/05/15 16:55:30]

Oops, done. Fixed the other test too.

+
+  GURL urls[] = {GURL("http://google.com/1"),
+                 GURL("http://google.com/2"),
+                 GURL("http://google.com/3"),
+                 GURL("http://google.com/4"),
+                 GURL("http://google.com/5"),
+                 GURL("http://google.com/6")};
+
+  // Create a random activation order so that running the test enough times
+  // gives us complete coverage.
+  std::vector<int> activation_order;
+  for (int i = 0; i < NumTabs; i++)
+    activation_order.push_back(i);
+  std::random_shuffle(activation_order.begin(), activation_order.end());

[sky, 2015/05/12 21:25:30]

It's not a good idea to randomize test data. If there is flake then it can be
hard to repro. Much better if you order it in a specific way. Do multiple runs
of the test if appropriate.

[Georges Khalil, 2015/05/15 16:55:30]

Acknowledged. Changed to a fixed order.

+
+  // Replace the first tab and add the other tabs.
+  ui_test_utils::NavigateToURL(browser(), urls[0]);
+  for (int i = 1; i < NumTabs; i++) {
+    ui_test_utils::NavigateToURLWithDisposition(
+        browser(), urls[i], NEW_FOREGROUND_TAB,
+        ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  }
+
+  ASSERT_EQ(NumTabs, browser()->tab_strip_model()->count());
+
+  // Activate the tabs one by one following the random activation order.
+  for (auto i : activation_order) {

[sky, 2015/05/12 21:25:30]

nit: no {}

[Georges Khalil, 2015/05/15 16:55:30]

Done.

+    browser()->tab_strip_model()->ActivateTabAt(i, true);
+  }
+
+  // Close the browser.
+  g_browser_process->AddRefModule();
+  CloseBrowserSynchronously(browser());
+
+  StartObserving(NumTabs);
+
+  // Create a new window, which should trigger session restore.
+  ui_test_utils::BrowserAddedObserver window_observer;
+  chrome::NewEmptyWindow(browser()->profile(),
+                         chrome::HOST_DESKTOP_TYPE_NATIVE);
+  Browser* new_browser = window_observer.WaitForSingleNewBrowser();
+  WaitForAllTabsToStartLoading();
+  g_browser_process->ReleaseModule();
+
+  ASSERT_EQ(static_cast<size_t>(NumTabs), web_contents().size());
+  // Test that we have observed the tabs being loaded in the inverse order of
+  // their activation (MRU).
+  for (size_t i = 0; i < web_contents().size(); i++) {
+    GURL expected_url = urls[activation_order[NumTabs - i - 1]];
+    ASSERT_EQ(expected_url, web_contents()[i]->GetLastCommittedURL());
+  }
+
+  // Close the browser and open it again, to trigger another session restore.
+  // The goal is to ensure that activation time is persisted between session
+  // restores.
+
+  // Close the browser.
+  g_browser_process->AddRefModule();
+  CloseBrowserSynchronously(new_browser);
+
+  StartObserving(NumTabs);
+
+  // Create a new window, which should trigger session restore.
+  ui_test_utils::BrowserAddedObserver window_observer2;
+  chrome::NewEmptyWindow(browser()->profile(),
+                         chrome::HOST_DESKTOP_TYPE_NATIVE);
+  window_observer2.WaitForSingleNewBrowser();
+  WaitForAllTabsToStartLoading();
+  g_browser_process->ReleaseModule();
+
+  ASSERT_EQ(static_cast<size_t>(NumTabs), web_contents().size());
+  // Test that we have observed the tabs being loaded in the inverse order of
+  // their activation (MRU).
+  for (size_t i = 0; i < web_contents().size(); i++) {
+    GURL expected_url = urls[activation_order[NumTabs - i - 1]];
+    ASSERT_EQ(expected_url, web_contents()[i]->GetLastCommittedURL());
+  }
+}