[Offline Pages] SnapshotController support for delay after onLoadCompleted and...

[Offline Pages] SnapshotController support for delay after onLoadCompleted and also parameterized constructor so different uses could tune delays.

BUG=649711

Review-Url: https://chromiumcodereview.appspot.com/2380093002
Cr-Commit-Position: refs/heads/master@{#426892}

chrome/browser/android/offline_pages/prerendering_loader_unittest.cc

@@ -223,6 +223,8 @@ TEST_F(PrerenderingLoaderTest, LoadPageLoadSucceededFromPrerenderStopLoading) {
   EXPECT_FALSE(loader()->IsLoaded());
 
   test_adapter()->GetObserver()->OnPrerenderStopLoading();
+  // Skip SnapshotController's wait time and emulate StartSnapshot call.
+  loader()->StartSnapshot();
   PumpLoop();
   EXPECT_FALSE(loader()->IsIdle());
   EXPECT_TRUE(loader()->IsLoaded());

chrome/browser/android/offline_pages/recent_tab_helper_unittest.cc

@@ -212,6 +212,7 @@ TEST_F(RecentTabHelperTest, Basic) {
 TEST_F(RecentTabHelperTest, SimpleCapture) {
   NavigateAndCommit(kTestPageUrl);
   recent_tab_helper()->DocumentOnLoadCompletedInMainFrame();
+  FastForwardSnapshotController();
   RunUntilIdle();
   EXPECT_TRUE(model()->is_loaded());
   GetAllPages();
@@ -226,6 +227,7 @@ TEST_F(RecentTabHelperTest, NoTabIdNoCapture) {
 
   NavigateAndCommit(kTestPageUrl);
   recent_tab_helper()->DocumentOnLoadCompletedInMainFrame();
+  FastForwardSnapshotController();
   RunUntilIdle();
   EXPECT_TRUE(model()->is_loaded());
   GetAllPages();
@@ -250,8 +252,10 @@ TEST_F(RecentTabHelperTest, TwoCapturesSameUrl) {
   EXPECT_EQ(1U, all_pages().size());
   EXPECT_EQ(kTestPageUrl, all_pages()[0].url);
 
-  // Triggers snapshot immediately;
+  // Triggers snapshot after a time delay.
   recent_tab_helper()->DocumentOnLoadCompletedInMainFrame();
+  // Move the snapshot controller's time forward so it gets past timeouts.
+  FastForwardSnapshotController();
   RunUntilIdle();
   EXPECT_EQ(2U, model_changed_count());
   EXPECT_EQ(1U, model_removed_count());
@@ -279,8 +283,10 @@ TEST_F(RecentTabHelperTest, TwoCapturesDifferentUrls) {
   EXPECT_EQ(kTestPageUrl, all_pages()[0].url);
 
   NavigateAndCommit(kTestPageUrlOther);
-  // Triggers snapshot immediately;
+  // Triggers snapshot after a time delay.
   recent_tab_helper()->DocumentOnLoadCompletedInMainFrame();
+  // Move the snapshot controller's time forward so it gets past timeouts.
+  FastForwardSnapshotController();
   RunUntilIdle();
   EXPECT_EQ(2U, model_changed_count());
   EXPECT_EQ(1U, model_removed_count());
@@ -293,6 +299,7 @@ TEST_F(RecentTabHelperTest, TwoCapturesDifferentUrls) {
 TEST_F(RecentTabHelperTest, NoCaptureOnErrorPage) {
   FailLoad(kTestPageUrl);
   recent_tab_helper()->DocumentOnLoadCompletedInMainFrame();
+  FastForwardSnapshotController();
   RunUntilIdle();
   EXPECT_TRUE(model()->is_loaded());
   GetAllPages();
@@ -304,6 +311,7 @@ TEST_F(RecentTabHelperTest, FeatureNotEnabled) {
   scoped_feature_list.Init();
   NavigateAndCommit(kTestPageUrl);
   recent_tab_helper()->DocumentOnLoadCompletedInMainFrame();
+  FastForwardSnapshotController();
   RunUntilIdle();
   EXPECT_TRUE(model()->is_loaded());
   GetAllPages();

components/offline_pages/snapshot_controller.cc

@@ -10,10 +10,14 @@
 #include "base/time/time.h"
 
 namespace {
-// Delay, in milliseconds, between the main document parsed event and snapshot.
-// Note if the "load" event fires before this delay is up, then the snapshot
-// is taken immediately.
-const size_t kDelayAfterDocumentAvailable = 7000;
+// Default delay, in milliseconds, between the main document parsed event and
+// snapshot. Note: this snapshot might not occur if the OnLoad event and
+// OnLoad delay elapses first to trigger a final snapshot.
+const size_t kDefaultDelayAfterDocumentAvailableMs = 7000;
+
+// Default delay, in milliseconds, between the main document OnLoad event and
+// snapshot.
+const size_t kDelayAfterDocumentOnLoadCompletedMs = 1000;
 
 }  // namespace
 
@@ -25,8 +29,25 @@ SnapshotController::SnapshotController(
     : task_runner_(task_runner),
       client_(client),
       state_(State::READY),
-      weak_ptr_factory_(this) {
-}
+      delay_after_document_available_ms_(
+          kDefaultDelayAfterDocumentAvailableMs),
+      delay_after_document_on_load_completed_ms_(
+          kDelayAfterDocumentOnLoadCompletedMs),
+      weak_ptr_factory_(this) {}
+
+SnapshotController::SnapshotController(
+    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+    SnapshotController::Client* client,
+    size_t delay_after_document_available_ms,
+    size_t delay_after_document_on_load_completed_ms)
+    : task_runner_(task_runner),
+      client_(client),
+      state_(State::READY),
+      delay_after_document_available_ms_(
+          delay_after_document_available_ms),
+      delay_after_document_on_load_completed_ms_(
+          delay_after_document_on_load_completed_ms),
+      weak_ptr_factory_(this) {}
 
 SnapshotController::~SnapshotController() {}
 
@@ -49,20 +70,21 @@ void SnapshotController::PendingSnapshotCompleted() {
 }
 
 void SnapshotController::DocumentAvailableInMainFrame() {
-  // Post a delayed task. The snapshot will happen either when the delay
-  // is up, or if the "load" event is dispatched in the main frame.
+  // Post a delayed task to snapshot.
   task_runner_->PostDelayedTask(
-      FROM_HERE,
-      base::Bind(&SnapshotController::MaybeStartSnapshot,
-                 weak_ptr_factory_.GetWeakPtr()),
-      base::TimeDelta::FromMilliseconds(kDelayAfterDocumentAvailable));
+      FROM_HERE, base::Bind(&SnapshotController::MaybeStartSnapshot,
+                            weak_ptr_factory_.GetWeakPtr()),
+      base::TimeDelta::FromMilliseconds(
+          delay_after_document_available_ms_));
 }
 
 void SnapshotController::DocumentOnLoadCompletedInMainFrame() {
-  MaybeStartSnapshot();
-  // No more snapshots after onLoad (there still can be other events
-  // or delayed tasks that can try to start another snapshot)
-  Stop();
+  // Post a delayed task to snapshot and then stop this controller.
+  task_runner_->PostDelayedTask(
+      FROM_HERE, base::Bind(&SnapshotController::MaybeStartSnapshotThenStop,
+                            weak_ptr_factory_.GetWeakPtr()),
+      base::TimeDelta::FromMilliseconds(
+          delay_after_document_on_load_completed_ms_));
 }
 
 void SnapshotController::MaybeStartSnapshot() {
@@ -72,8 +94,17 @@ void SnapshotController::MaybeStartSnapshot() {
   client_->StartSnapshot();
 }
 
+void SnapshotController::MaybeStartSnapshotThenStop() {
+  MaybeStartSnapshot();
+  Stop();
+}
+
 size_t SnapshotController::GetDelayAfterDocumentAvailableForTest() {
-  return kDelayAfterDocumentAvailable;
+  return delay_after_document_available_ms_;
+}
+
+size_t SnapshotController::GetDelayAfterDocumentOnLoadCompletedForTest() {
+  return delay_after_document_on_load_completed_ms_;
 }
 
 

components/offline_pages/snapshot_controller.h

@@ -48,6 +48,11 @@ class SnapshotController {
   SnapshotController(
       const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
       SnapshotController::Client* client);
+  SnapshotController(
+      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+      SnapshotController::Client* client,
+      size_t delay_after_document_available_ms,
+      size_t delay_after_document_on_load_completed_ms);
   virtual ~SnapshotController();
 
   // Resets the 'session', returning controller to initial state.
@@ -69,14 +74,18 @@ class SnapshotController {
   void DocumentOnLoadCompletedInMainFrame();
 
   size_t GetDelayAfterDocumentAvailableForTest();
+  size_t GetDelayAfterDocumentOnLoadCompletedForTest();
 
  private:
   void MaybeStartSnapshot();
+  void MaybeStartSnapshotThenStop();
 
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
   // Client owns this class.
   SnapshotController::Client* client_;
   SnapshotController::State state_;
+  size_t delay_after_document_available_ms_;
+  size_t delay_after_document_on_load_completed_ms_;
 
   base::WeakPtrFactory<SnapshotController> weak_ptr_factory_;
 

components/offline_pages/snapshot_controller_unittest.cc

@@ -77,10 +77,12 @@ void SnapshotControllerTest::FastForwardBy(base::TimeDelta delta) {
 }
 
 TEST_F(SnapshotControllerTest, OnLoad) {
-  // Onload should make snapshot right away.
-  EXPECT_EQ(0, snapshot_count());
+  // Onload should make snapshot after its delay.
   controller()->DocumentOnLoadCompletedInMainFrame();
   PumpLoop();
+  EXPECT_EQ(0, snapshot_count());
+  FastForwardBy(base::TimeDelta::FromMilliseconds(
+      controller()->GetDelayAfterDocumentOnLoadCompletedForTest()));
   EXPECT_EQ(1, snapshot_count());
 }
 
@@ -96,17 +98,22 @@ TEST_F(SnapshotControllerTest, OnDocumentAvailable) {
 }
 
 TEST_F(SnapshotControllerTest, OnLoadSnapshotIsTheLastOne) {
+  // This test assumes DocumentAvailable delay is longer than OnLoadCompleted.
+  EXPECT_GT(controller()->GetDelayAfterDocumentAvailableForTest(),
+            controller()->GetDelayAfterDocumentOnLoadCompletedForTest());
   // OnDOM should make snapshot after a delay.
   controller()->DocumentAvailableInMainFrame();
   PumpLoop();
   EXPECT_EQ(0, snapshot_count());
-  // This should start snapshot immediately.
   controller()->DocumentOnLoadCompletedInMainFrame();
+  // Advance time to OnLoadCompleted delay to trigger snapshot.
+  FastForwardBy(base::TimeDelta::FromMilliseconds(
+      controller()->GetDelayAfterDocumentOnLoadCompletedForTest()));
   EXPECT_EQ(1, snapshot_count());
   // Report that snapshot is completed.
   controller()->PendingSnapshotCompleted();
   // Even though previous snapshot is completed, new one should not start
-  // when this delay expires.
+  // when this DocumentAvailable delay expires.
   FastForwardBy(base::TimeDelta::FromMilliseconds(
       controller()->GetDelayAfterDocumentAvailableForTest()));
   EXPECT_EQ(1, snapshot_count());
@@ -122,8 +129,10 @@ TEST_F(SnapshotControllerTest, OnLoadSnapshotAfterLongDelay) {
   EXPECT_EQ(1, snapshot_count());
   // Report that snapshot is completed.
   controller()->PendingSnapshotCompleted();
-  // This should start snapshot immediately.
+  // OnLoad should make 2nd snapshot after its delay.
   controller()->DocumentOnLoadCompletedInMainFrame();
+  FastForwardBy(base::TimeDelta::FromMilliseconds(
+      controller()->GetDelayAfterDocumentOnLoadCompletedForTest()));
   EXPECT_EQ(2, snapshot_count());
 }
 
@@ -151,6 +160,8 @@ TEST_F(SnapshotControllerTest, ClientReset) {
   // No snapshot since session was reset.
   EXPECT_EQ(0, snapshot_count());
   controller()->DocumentOnLoadCompletedInMainFrame();
+  FastForwardBy(base::TimeDelta::FromMilliseconds(
+      controller()->GetDelayAfterDocumentOnLoadCompletedForTest()));
   EXPECT_EQ(1, snapshot_count());
 
   controller()->Reset();
@@ -165,6 +176,8 @@ TEST_F(SnapshotControllerTest, ClientReset) {
 // as done later. That reporting should have no effect nor crash.
 TEST_F(SnapshotControllerTest, ClientResetWhileSnapshotting) {
   controller()->DocumentOnLoadCompletedInMainFrame();
+  FastForwardBy(base::TimeDelta::FromMilliseconds(
+      controller()->GetDelayAfterDocumentOnLoadCompletedForTest()));
   EXPECT_EQ(1, snapshot_count());
   // This normally happens when navigation starts.
   controller()->Reset();