[TabMetrics] Add signals that mark the start and end of session restore.

chrome/browser/sessions/session_restore_observer_unittest.cc

Index: chrome/browser/sessions/session_restore_observer_unittest.cc
diff --git a/chrome/browser/sessions/session_restore_observer_unittest.cc b/chrome/browser/sessions/session_restore_observer_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9743b59f20e82f26bf87d765c84226d64f79854a
--- /dev/null
+++ b/chrome/browser/sessions/session_restore_observer_unittest.cc
@@ -0,0 +1,130 @@
+// Copyright (c) 2017 The Chromium Authors. All rights reserved.

[sky, 2017/06/26 21:38:09]

no (c) (see chromium style guide).

[ducbui, 2017/06/26 22:30:58]

Done.

I got the (c) when I copied the license from another file.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/sessions/session_restore_observer.h"
+
+#include "chrome/browser/sessions/session_restore.h"
+#include "chrome/browser/sessions/tab_loader.h"
+#include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/web_contents_tester.h"
+
+using content::WebContentsTester;
+
+namespace {
+
+const char kDefaultUrl[] = "https://www.google.com";
+
+}  // namespace
+
+class MockSessionRestoreObserver : public SessionRestoreObserver {
+ public:
+  MockSessionRestoreObserver() { SessionRestore::AddObserver(this); }
+
+  ~MockSessionRestoreObserver() { SessionRestore::RemoveObserver(this); }
+
+  enum class SessionRestoreEvent {
+    STARTED_LOADING_TABS,
+    FINISHED_LOADING_TABS
+  };
+
+  SessionRestoreEvent GetSessionRestoreEventAt(size_t index) const {
+    DCHECK(index < session_restore_events_.size());

[sky, 2017/06/26 21:38:09]

This is rather awkward. Why don't you just return the vector? e.g.
const std::vector<SessionRestoreEvent>& session_restore_events() const { return
... }

[ducbui, 2017/06/26 22:30:58]

I am fine with the change. Your method looks cleaner.
I also added a couple of wrappers for frequent uses in the tests.

+    return session_restore_events_[index];
+  }
+
+  // SessionRestoreObserver implementation:
+  void OnSessionRestoreStartedLoadingTabs() override {
+    session_restore_events_.emplace_back(
+        SessionRestoreEvent::STARTED_LOADING_TABS);
+  }
+
+  void OnSessionRestoreFinishedLoadingTabs() override {
+    session_restore_events_.emplace_back(
+        SessionRestoreEvent::FINISHED_LOADING_TABS);
+  }
+
+ private:
+  std::vector<SessionRestoreEvent> session_restore_events_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockSessionRestoreObserver);
+};
+
+class SessionRestoreObserverTest : public ChromeRenderViewHostTestHarness {
+ public:
+  using RestoredTab = SessionRestoreDelegate::RestoredTab;
+
+  SessionRestoreObserverTest() {}
+
+  // testing::Test:
+  void SetUp() override {
+    ChromeRenderViewHostTestHarness::SetUp();
+    restored_tabs_.emplace_back(web_contents(), false, false, false);
+  }
+
+  void TearDown() override {
+    ChromeRenderViewHostTestHarness::TearDown();
+    restored_tabs_.clear();
+  }
+
+ protected:
+  void LoadWebContents(content::WebContents* contents) {
+    WebContentsTester::For(contents)->NavigateAndCommit(GURL(kDefaultUrl));
+    WebContentsTester::For(contents)->TestSetIsLoading(false);
+  }
+
+  MockSessionRestoreObserver mock_observer_;
+  std::vector<RestoredTab> restored_tabs_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SessionRestoreObserverTest);
+};
+
+TEST_F(SessionRestoreObserverTest, SingleSessionRestore) {
+  TabLoader::RestoreTabs(restored_tabs_, base::TimeTicks());
+  EXPECT_EQ(
+      mock_observer_.GetSessionRestoreEventAt(0),

[sky, 2017/06/26 21:38:09]

You should assert the size of the vector, otherwise you'll trigger a crash and
the remaining unit tests won't execute. By that I mean here you want:
ASSERT_EQ(1u, mock_observer_.session_restore_events().size();
and below you want to assert the size is 2u.

[ducbui, 2017/06/26 22:30:58]

Done.

+      MockSessionRestoreObserver::SessionRestoreEvent::STARTED_LOADING_TABS);

[sky, 2017/06/26 21:38:09]

Generally we use the format "expected, actual", so swap the ordering here and of
others.

[ducbui, 2017/06/26 22:30:58]

Done.

+
+  LoadWebContents(web_contents());
+  EXPECT_EQ(
+      mock_observer_.GetSessionRestoreEventAt(1),
+      MockSessionRestoreObserver::SessionRestoreEvent::FINISHED_LOADING_TABS);
+}
+
+TEST_F(SessionRestoreObserverTest, SequentialSessionRestore) {
+  const int number_of_session_restores = 3;
+  size_t event_index = 0;
+  for (int i = 0; i < number_of_session_restores; ++i) {
+    TabLoader::RestoreTabs(restored_tabs_, base::TimeTicks());
+    EXPECT_EQ(
+        mock_observer_.GetSessionRestoreEventAt(event_index++),
+        MockSessionRestoreObserver::SessionRestoreEvent::STARTED_LOADING_TABS);
+
+    LoadWebContents(web_contents());
+    EXPECT_EQ(
+        mock_observer_.GetSessionRestoreEventAt(event_index++),
+        MockSessionRestoreObserver::SessionRestoreEvent::FINISHED_LOADING_TABS);
+  }
+}
+
+TEST_F(SessionRestoreObserverTest, ConcurrentSessionRestore) {
+  std::vector<RestoredTab> another_restored_tabs;
+  std::unique_ptr<content::WebContents> test_contents(
+      WebContentsTester::CreateTestWebContents(browser_context(), nullptr));
+  another_restored_tabs.emplace_back(test_contents.get(), false, false, false);
+
+  TabLoader::RestoreTabs(restored_tabs_, base::TimeTicks());
+  TabLoader::RestoreTabs(another_restored_tabs, base::TimeTicks());
+  EXPECT_EQ(
+      mock_observer_.GetSessionRestoreEventAt(0),
+      MockSessionRestoreObserver::SessionRestoreEvent::STARTED_LOADING_TABS);
+
+  LoadWebContents(web_contents());
+  LoadWebContents(test_contents.get());
+
+  EXPECT_EQ(
+      mock_observer_.GetSessionRestoreEventAt(1),
+      MockSessionRestoreObserver::SessionRestoreEvent::FINISHED_LOADING_TABS);
+}