Split SnapshotController into online and offline versions.
To make the Resource Percentage work easier, we are splitting the
snapshot controller into a foreground and a background version.
Bug: 699313
Change-Id: Ideeea7d644a903290d9eeae2d7ab700ab0ff50b6
Reviewed-on: https://chromium-review.googlesource.com/1136023
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Commit-Queue: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578351}diff --git a/chrome/browser/offline_pages/background_loader_offliner.cc b/chrome/browser/offline_pages/background_loader_offliner.cc
index 34123a3..448dbb3 100644
--- a/chrome/browser/offline_pages/background_loader_offliner.cc
+++ b/chrome/browser/offline_pages/background_loader_offliner.cc
@@ -239,7 +239,7 @@
// Load page attempt.
loader_.get()->LoadPage(request.url());
- snapshot_controller_ = SnapshotController::CreateForBackgroundOfflining(
+ snapshot_controller_ = std::make_unique<BackgroundSnapshotController>(
base::ThreadTaskRunnerHandle::Get(), this, (bool)page_renovator_);
return true;
@@ -327,7 +327,6 @@
}
void BackgroundLoaderOffliner::DocumentAvailableInMainFrame() {
- snapshot_controller_->DocumentAvailableInMainFrame();
is_low_bar_met_ = true;
// Add this signal to signal_data_.
@@ -413,8 +412,8 @@
RecordOffliningPreviewsUMA(pending_request_->client_id(), navigation_data);
}
-void BackgroundLoaderOffliner::SetSnapshotControllerForTest(
- std::unique_ptr<SnapshotController> controller) {
+void BackgroundLoaderOffliner::SetBackgroundSnapshotControllerForTest(
+ std::unique_ptr<BackgroundSnapshotController> controller) {
snapshot_controller_ = std::move(controller);
}
diff --git a/chrome/browser/offline_pages/background_loader_offliner.h b/chrome/browser/offline_pages/background_loader_offliner.h
index 06b39313..ae020b0 100644
--- a/chrome/browser/offline_pages/background_loader_offliner.h
+++ b/chrome/browser/offline_pages/background_loader_offliner.h
@@ -13,8 +13,8 @@
#include "components/offline_pages/content/background_loader/background_loader_contents.h"
#include "components/offline_pages/core/background/load_termination_listener.h"
#include "components/offline_pages/core/background/offliner.h"
+#include "components/offline_pages/core/background_snapshot_controller.h"
#include "components/offline_pages/core/offline_page_types.h"
-#include "components/offline_pages/core/snapshot_controller.h"
#include "content/public/browser/web_contents_observer.h"
namespace content {
@@ -43,7 +43,7 @@
: public Offliner,
public background_loader::BackgroundLoaderContents::Delegate,
public content::WebContentsObserver,
- public SnapshotController::Client,
+ public BackgroundSnapshotController::Client,
public ResourceLoadingObserver {
public:
BackgroundLoaderOffliner(
@@ -78,12 +78,12 @@
void DidFinishNavigation(
content::NavigationHandle* navigation_handle) override;
- // SnapshotController::Client implementation.
+ // BackgroundSnapshotController::Client implementation.
void StartSnapshot() override;
void RunRenovations() override;
- void SetSnapshotControllerForTest(
- std::unique_ptr<SnapshotController> controller);
+ void SetBackgroundSnapshotControllerForTest(
+ std::unique_ptr<BackgroundSnapshotController> controller);
// ResourceLoadingObserver implemenation
void ObserveResourceLoading(ResourceLoadingObserver::ResourceDataType type,
@@ -140,7 +140,7 @@
// Tracks pending request, if any.
std::unique_ptr<SavePageRequest> pending_request_;
// Handles determining when a page should be snapshotted.
- std::unique_ptr<SnapshotController> snapshot_controller_;
+ std::unique_ptr<BackgroundSnapshotController> snapshot_controller_;
// Callback when pending request completes.
CompletionCallback completion_callback_;
// Callback to report progress.
diff --git a/chrome/browser/offline_pages/background_loader_offliner_unittest.cc b/chrome/browser/offline_pages/background_loader_offliner_unittest.cc
index 30825c8..b6a43ad 100644
--- a/chrome/browser/offline_pages/background_loader_offliner_unittest.cc
+++ b/chrome/browser/offline_pages/background_loader_offliner_unittest.cc
@@ -5,6 +5,7 @@
#include "chrome/browser/offline_pages/background_loader_offliner.h"
#include "base/bind.h"
+#include "base/command_line.h"
#include "base/run_loop.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
@@ -34,6 +35,11 @@
#include "net/http/http_response_headers.h"
#include "testing/gtest/include/gtest/gtest.h"
+namespace {
+char kShortSnapshotDelayForTest[] =
+ "short-offline-page-snapshot-delay-for-test";
+}; // namespace
+
namespace offline_pages {
namespace {
@@ -209,15 +215,12 @@
void CompleteLoading() {
// Reset snapshot controller.
- std::unique_ptr<SnapshotController> snapshot_controller(
- new SnapshotController(base::ThreadTaskRunnerHandle::Get(),
- offliner_.get(),
- 0L /* DelayAfterDocumentAvailable */,
- 0L /* DelayAfterDocumentOnLoad */,
- 0L /* DelayAfterRenovationsCompleted */,
- false /* DocumentAvailableTriggersSnapshot */,
- false /* RenovationsEnabled */));
- offliner_->SetSnapshotControllerForTest(std::move(snapshot_controller));
+ std::unique_ptr<BackgroundSnapshotController> snapshot_controller(
+ new BackgroundSnapshotController(base::ThreadTaskRunnerHandle::Get(),
+ offliner_.get(),
+ false /* RenovationsEnabled */));
+ offliner_->SetBackgroundSnapshotControllerForTest(
+ std::move(snapshot_controller));
// Call complete loading.
offliner()->DocumentOnLoadCompletedInMainFrame();
PumpLoop();
@@ -265,6 +268,10 @@
BackgroundLoaderOfflinerTest::~BackgroundLoaderOfflinerTest() {}
void BackgroundLoaderOfflinerTest::SetUp() {
+ // Set the snapshot controller delay command line switch to short delays.
+ base::CommandLine* cl = base::CommandLine::ForCurrentProcess();
+ cl->AppendSwitch(kShortSnapshotDelayForTest);
+
std::unique_ptr<TestLoadTerminationListener> listener =
std::make_unique<TestLoadTerminationListener>();
load_termination_listener_ = listener.get();
diff --git a/chrome/browser/offline_pages/recent_tab_helper.cc b/chrome/browser/offline_pages/recent_tab_helper.cc
index 2a42230..9028d0f 100644
--- a/chrome/browser/offline_pages/recent_tab_helper.cc
+++ b/chrome/browser/offline_pages/recent_tab_helper.cc
@@ -167,7 +167,7 @@
if (snapshot_controller_) // Initialized already.
return snapshots_enabled_;
- snapshot_controller_ = SnapshotController::CreateForForegroundOfflining(
+ snapshot_controller_ = std::make_unique<SnapshotController>(
base::ThreadTaskRunnerHandle::Get(), this);
snapshot_controller_->Stop(); // It is reset when navigation commits.
@@ -407,10 +407,6 @@
snapshot_controller_->PendingSnapshotCompleted();
}
-void RecentTabHelper::RunRenovations() {
- snapshot_controller_->RenovationsCompleted();
-}
-
void RecentTabHelper::SaveSnapshotForDownloads(bool replace_latest) {
DCHECK_NE(PageQuality::POOR, snapshot_controller_->current_page_quality());
diff --git a/chrome/browser/offline_pages/recent_tab_helper.h b/chrome/browser/offline_pages/recent_tab_helper.h
index 0c5bea7..0b16952 100644
--- a/chrome/browser/offline_pages/recent_tab_helper.h
+++ b/chrome/browser/offline_pages/recent_tab_helper.h
@@ -63,7 +63,6 @@
// SnapshotController::Client
void StartSnapshot() override;
- void RunRenovations() override;
// Delegate that is used by RecentTabHelper to get external dependencies.
// Default implementation lives in .cc file, while tests provide an override.
diff --git a/components/offline_pages/core/BUILD.gn b/components/offline_pages/core/BUILD.gn
index b3c96cc..8199697 100644
--- a/components/offline_pages/core/BUILD.gn
+++ b/components/offline_pages/core/BUILD.gn
@@ -12,6 +12,8 @@
"archive_manager.h",
"archive_validator.cc",
"archive_validator.h",
+ "background_snapshot_controller.cc",
+ "background_snapshot_controller.h",
"client_id.cc",
"client_id.h",
"client_namespace_constants.cc",
@@ -158,6 +160,7 @@
sources = [
"archive_manager_unittest.cc",
"archive_validator_unittest.cc",
+ "background_snapshot_controller_unittest.cc",
"client_policy_controller_unittest.cc",
"model/add_page_task_unittest.cc",
"model/add_page_to_download_manager_task_unittest.cc",
diff --git a/components/offline_pages/core/background_snapshot_controller.cc b/components/offline_pages/core/background_snapshot_controller.cc
new file mode 100644
index 0000000..87244f5
--- /dev/null
+++ b/components/offline_pages/core/background_snapshot_controller.cc
@@ -0,0 +1,111 @@
+// Copyright 2018 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 "components/offline_pages/core/background_snapshot_controller.h"
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/logging.h"
+#include "base/time/time.h"
+#include "components/offline_pages/core/offline_page_feature.h"
+
+namespace {
+// Default delay, in milliseconds, between the main document OnLoad event and
+// snapshot.
+const int64_t kDelayAfterDocumentOnLoadCompletedMsBackground = 2000;
+
+// Default delay, in milliseconds, between renovations finishing and
+// taking a snapshot. Allows for page to update in response to the
+// renovations.
+const int64_t kDelayAfterRenovationsCompletedMs = 2000;
+
+// Delay for testing to keep polling times reasonable.
+const int64_t kDelayForTests = 0;
+
+} // namespace
+
+namespace offline_pages {
+
+BackgroundSnapshotController::BackgroundSnapshotController(
+ const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+ BackgroundSnapshotController::Client* client,
+ bool renovations_enabled)
+ : task_runner_(task_runner),
+ client_(client),
+ state_(State::READY),
+ delay_after_document_on_load_completed_ms_(
+ kDelayAfterDocumentOnLoadCompletedMsBackground),
+ delay_after_renovations_completed_ms_(kDelayAfterRenovationsCompletedMs),
+ renovations_enabled_(renovations_enabled),
+ weak_ptr_factory_(this) {
+ if (offline_pages::ShouldUseTestingSnapshotDelay()) {
+ delay_after_document_on_load_completed_ms_ = kDelayForTests;
+ delay_after_renovations_completed_ms_ = kDelayForTests;
+ }
+}
+
+BackgroundSnapshotController::~BackgroundSnapshotController() {}
+
+void BackgroundSnapshotController::Reset() {
+ // Cancel potentially delayed tasks that relate to the previous 'session'.
+ weak_ptr_factory_.InvalidateWeakPtrs();
+ state_ = State::READY;
+}
+
+void BackgroundSnapshotController::Stop() {
+ state_ = State::STOPPED;
+}
+
+void BackgroundSnapshotController::RenovationsCompleted() {
+ if (renovations_enabled_) {
+ task_runner_->PostDelayedTask(
+ FROM_HERE,
+ base::BindOnce(
+ &BackgroundSnapshotController::MaybeStartSnapshotThenStop,
+ weak_ptr_factory_.GetWeakPtr()),
+ base::TimeDelta::FromMilliseconds(
+ delay_after_renovations_completed_ms_));
+ }
+}
+
+void BackgroundSnapshotController::DocumentOnLoadCompletedInMainFrame() {
+ if (renovations_enabled_) {
+ // Run renovations. After renovations complete, a snapshot will be
+ // triggered after a delay.
+ client_->RunRenovations();
+ } else {
+ // Post a delayed task to snapshot and then stop this controller.
+ task_runner_->PostDelayedTask(
+ FROM_HERE,
+ base::BindOnce(
+ &BackgroundSnapshotController::MaybeStartSnapshotThenStop,
+ weak_ptr_factory_.GetWeakPtr()),
+ base::TimeDelta::FromMilliseconds(
+ delay_after_document_on_load_completed_ms_));
+ }
+}
+
+void BackgroundSnapshotController::MaybeStartSnapshot() {
+ if (state_ != State::READY)
+ return;
+ state_ = State::SNAPSHOT_PENDING;
+ client_->StartSnapshot();
+}
+
+void BackgroundSnapshotController::MaybeStartSnapshotThenStop() {
+ MaybeStartSnapshot();
+ Stop();
+}
+
+int64_t
+BackgroundSnapshotController::GetDelayAfterDocumentOnLoadCompletedForTest() {
+ return delay_after_document_on_load_completed_ms_;
+}
+
+int64_t
+BackgroundSnapshotController::GetDelayAfterRenovationsCompletedForTest() {
+ return delay_after_renovations_completed_ms_;
+}
+
+} // namespace offline_pages
diff --git a/components/offline_pages/core/background_snapshot_controller.h b/components/offline_pages/core/background_snapshot_controller.h
new file mode 100644
index 0000000..202b8a4
--- /dev/null
+++ b/components/offline_pages/core/background_snapshot_controller.h
@@ -0,0 +1,102 @@
+// Copyright 2018 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.
+
+#ifndef COMPONENTS_OFFLINE_PAGES_CORE_BACKGROUND_SNAPSHOT_CONTROLLER_H_
+#define COMPONENTS_OFFLINE_PAGES_CORE_BACKGROUND_SNAPSHOT_CONTROLLER_H_
+
+#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "base/single_thread_task_runner.h"
+
+namespace offline_pages {
+
+// Takes various signals and produces StartSnapshot calls following a specific
+// policy.
+// Main invariants:
+// - Some signals prevent more snapshots to be taken.
+// OnLoad is currently such signal.
+// - Once Reset() is called on the BackgroundSnapshotController, the delayed
+// tasks are reset so no StartSnapshot calls is made 'cross-session'.
+class BackgroundSnapshotController {
+ public:
+ enum class State {
+ READY, // Listening to input, will start snapshot when needed.
+ SNAPSHOT_PENDING, // Snapshot is in progress, don't start another.
+ STOPPED, // Terminal state, no snapshots until reset.
+ };
+ // The expected quality of a page based on its current loading progress.
+ enum class PageQuality {
+ // Not loaded enough to reach a minimum level of quality. Snapshots taken at
+ // this point are expected to be useless.
+ POOR = 0,
+ // A minimal level of quality has been attained but the page is still
+ // loading so its quality is continuously increasing. One or more snapshots
+ // could be taken at this stage as later ones are expected to have higher
+ // quality.
+ FAIR_AND_IMPROVING,
+ // The page is loaded enough and has attained its peak expected quality.
+ // Snapshots taken at this point are not expected to increase in quality
+ // after the first one.
+ HIGH,
+ };
+
+ // Client of the BackgroundSnapshotController.
+ class Client {
+ public:
+ // Invoked at a good moment to start a snapshot.
+ virtual void StartSnapshot() = 0;
+
+ // Invoked when the page is sufficiently loaded for running
+ // renovations. The client should call the RenovationsCompleted()
+ // when they finish.
+ virtual void RunRenovations() = 0;
+
+ protected:
+ virtual ~Client() {}
+ };
+
+ BackgroundSnapshotController(
+ const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+ BackgroundSnapshotController::Client* client,
+ bool renovations_enabled);
+ virtual ~BackgroundSnapshotController();
+
+ // Resets the 'session', returning controller to initial state.
+ void Reset();
+
+ // Stops current session, no more Client::StartSnapshot calls will be
+ // invoked from the BackgroundSnapshotController until current session is
+ // Reset(). Called by Client, for example when it encounters an error loading
+ // the page.
+ void Stop();
+
+ // The Client calls this when renovations have completed.
+ void RenovationsCompleted();
+
+ // Invoked from WebContentObserver::DocumentOnLoadCompletedInMainFrame
+ void DocumentOnLoadCompletedInMainFrame();
+
+ int64_t GetDelayAfterDocumentOnLoadCompletedForTest();
+ int64_t GetDelayAfterRenovationsCompletedForTest();
+
+ private:
+ void MaybeStartSnapshot();
+ void MaybeStartSnapshotThenStop();
+
+ scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+ // Client owns this class.
+ BackgroundSnapshotController::Client* client_;
+ BackgroundSnapshotController::State state_;
+ int64_t delay_after_document_on_load_completed_ms_;
+ int64_t delay_after_renovations_completed_ms_;
+ bool renovations_enabled_;
+
+ base::WeakPtrFactory<BackgroundSnapshotController> weak_ptr_factory_;
+
+ DISALLOW_COPY_AND_ASSIGN(BackgroundSnapshotController);
+};
+
+} // namespace offline_pages
+
+#endif // COMPONENTS_OFFLINE_PAGES_CORE_BACKGROUND_SNAPSHOT_CONTROLLER_H_
diff --git a/components/offline_pages/core/background_snapshot_controller_unittest.cc b/components/offline_pages/core/background_snapshot_controller_unittest.cc
new file mode 100644
index 0000000..91c579b
--- /dev/null
+++ b/components/offline_pages/core/background_snapshot_controller_unittest.cc
@@ -0,0 +1,125 @@
+// Copyright 2018 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 "components/offline_pages/core/background_snapshot_controller.h"
+
+#include "base/bind.h"
+#include "base/run_loop.h"
+#include "base/single_thread_task_runner.h"
+#include "base/test/test_mock_time_task_runner.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "base/time/time.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace offline_pages {
+
+class BackgroundSnapshotControllerTest
+ : public testing::Test,
+ public BackgroundSnapshotController::Client {
+ public:
+ BackgroundSnapshotControllerTest();
+ ~BackgroundSnapshotControllerTest() override;
+
+ BackgroundSnapshotController* controller() { return controller_.get(); }
+ int snapshot_count() { return snapshot_count_; }
+
+ // testing::Test
+ void SetUp() override;
+ void TearDown() override;
+
+ // BackgroundSnapshotController::Client
+ void StartSnapshot() override;
+ void RunRenovations() override;
+
+ // Utility methods.
+ // Runs until all of the tasks that are not delayed are gone from the task
+ // queue.
+ void PumpLoop();
+ // Fast-forwards virtual time by |delta|, causing tasks with a remaining
+ // delay less than or equal to |delta| to be executed.
+ void FastForwardBy(base::TimeDelta delta);
+
+ private:
+ std::unique_ptr<BackgroundSnapshotController> controller_;
+ scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
+ bool snapshot_started_;
+ int snapshot_count_;
+};
+
+BackgroundSnapshotControllerTest::BackgroundSnapshotControllerTest()
+ : task_runner_(new base::TestMockTimeTaskRunner),
+ snapshot_started_(true),
+ snapshot_count_(0) {}
+
+BackgroundSnapshotControllerTest::~BackgroundSnapshotControllerTest() {}
+
+void BackgroundSnapshotControllerTest::SetUp() {
+ controller_ =
+ std::make_unique<BackgroundSnapshotController>(task_runner_, this, false);
+ snapshot_started_ = true;
+}
+
+void BackgroundSnapshotControllerTest::TearDown() {
+ controller_.reset();
+}
+
+void BackgroundSnapshotControllerTest::StartSnapshot() {
+ snapshot_count_++;
+}
+
+void BackgroundSnapshotControllerTest::RunRenovations() {
+ controller_->RenovationsCompleted();
+}
+
+void BackgroundSnapshotControllerTest::PumpLoop() {
+ task_runner_->RunUntilIdle();
+}
+
+void BackgroundSnapshotControllerTest::FastForwardBy(base::TimeDelta delta) {
+ task_runner_->FastForwardBy(delta);
+}
+
+TEST_F(BackgroundSnapshotControllerTest, OnLoad) {
+ // 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());
+}
+
+TEST_F(BackgroundSnapshotControllerTest, Stop) {
+ // OnDOM should make snapshot after a delay.
+ controller()->DocumentOnLoadCompletedInMainFrame();
+ PumpLoop();
+ EXPECT_EQ(0, snapshot_count());
+ controller()->Stop();
+ FastForwardBy(base::TimeDelta::FromMilliseconds(
+ controller()->GetDelayAfterDocumentOnLoadCompletedForTest()));
+ // Should not start snapshots
+ EXPECT_EQ(0, snapshot_count());
+ // Also should not start snapshot.
+ controller()->DocumentOnLoadCompletedInMainFrame();
+ EXPECT_EQ(0, snapshot_count());
+}
+
+// This simulated a Reset while there is ongoing snapshot, which is reported
+// as done later. That reporting should have no effect nor crash.
+TEST_F(BackgroundSnapshotControllerTest, ClientReset) {
+ controller()->DocumentOnLoadCompletedInMainFrame();
+ FastForwardBy(base::TimeDelta::FromMilliseconds(
+ controller()->GetDelayAfterDocumentOnLoadCompletedForTest()));
+ EXPECT_EQ(1, snapshot_count());
+ // This normally happens when navigation starts.
+ controller()->Reset();
+ // Next snapshot should be initiated when new document is loaded.
+ controller()->DocumentOnLoadCompletedInMainFrame();
+ FastForwardBy(base::TimeDelta::FromMilliseconds(
+ controller()->GetDelayAfterDocumentOnLoadCompletedForTest()));
+ // No snapshot since session was reset.
+ EXPECT_EQ(2, snapshot_count());
+}
+
+} // namespace offline_pages
diff --git a/components/offline_pages/core/snapshot_controller.cc b/components/offline_pages/core/snapshot_controller.cc
index c1cb13d..d14f5fb 100644
--- a/components/offline_pages/core/snapshot_controller.cc
+++ b/components/offline_pages/core/snapshot_controller.cc
@@ -11,8 +11,6 @@
#include "components/offline_pages/core/offline_page_feature.h"
namespace {
-const bool kDocumentAvailableTriggersSnapshot = true;
-
// 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.
@@ -21,12 +19,6 @@
// Default delay, in milliseconds, between the main document OnLoad event and
// snapshot.
const int64_t kDelayAfterDocumentOnLoadCompletedMsForeground = 1000;
-const int64_t kDelayAfterDocumentOnLoadCompletedMsBackground = 2000;
-
-// Default delay, in milliseconds, between renovations finishing and
-// taking a snapshot. Allows for page to update in response to the
-// renovations.
-const int64_t kDelayAfterRenovationsCompletedMs = 2000;
// Delay for testing to keep polling times reasonable.
const int64_t kDelayForTests = 0;
@@ -35,55 +27,19 @@
namespace offline_pages {
-// static
-std::unique_ptr<SnapshotController>
-SnapshotController::CreateForForegroundOfflining(
- const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
- SnapshotController::Client* client) {
- return std::unique_ptr<SnapshotController>(new SnapshotController(
- task_runner, client, kDefaultDelayAfterDocumentAvailableMs,
- kDelayAfterDocumentOnLoadCompletedMsForeground,
- kDelayAfterRenovationsCompletedMs, kDocumentAvailableTriggersSnapshot,
- false));
-}
-
-// static
-std::unique_ptr<SnapshotController>
-SnapshotController::CreateForBackgroundOfflining(
- const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
- SnapshotController::Client* client,
- bool renovations_enabled) {
- return std::unique_ptr<SnapshotController>(new SnapshotController(
- task_runner, client, kDefaultDelayAfterDocumentAvailableMs,
- kDelayAfterDocumentOnLoadCompletedMsBackground,
- kDelayAfterRenovationsCompletedMs, !kDocumentAvailableTriggersSnapshot,
- renovations_enabled));
-}
-
SnapshotController::SnapshotController(
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
- SnapshotController::Client* client,
- int64_t delay_after_document_available_ms,
- int64_t delay_after_document_on_load_completed_ms,
- int64_t delay_after_renovations_completed_ms,
- bool document_available_triggers_snapshot,
- bool renovations_enabled)
+ SnapshotController::Client* client)
: task_runner_(task_runner),
client_(client),
state_(State::READY),
- delay_after_document_available_ms_(delay_after_document_available_ms),
+ delay_after_document_available_ms_(kDefaultDelayAfterDocumentAvailableMs),
delay_after_document_on_load_completed_ms_(
- delay_after_document_on_load_completed_ms),
- delay_after_renovations_completed_ms_(
- delay_after_renovations_completed_ms),
- document_available_triggers_snapshot_(
- document_available_triggers_snapshot),
- renovations_enabled_(renovations_enabled),
+ kDelayAfterDocumentOnLoadCompletedMsForeground),
weak_ptr_factory_(this) {
if (offline_pages::ShouldUseTestingSnapshotDelay()) {
delay_after_document_available_ms_ = kDelayForTests;
delay_after_document_on_load_completed_ms_ = kDelayForTests;
- delay_after_renovations_completed_ms_ = kDelayForTests;
}
}
@@ -108,44 +64,25 @@
state_ = State::READY;
}
-void SnapshotController::RenovationsCompleted() {
- if (renovations_enabled_) {
- task_runner_->PostDelayedTask(
- FROM_HERE,
- base::BindOnce(&SnapshotController::MaybeStartSnapshotThenStop,
- weak_ptr_factory_.GetWeakPtr()),
- base::TimeDelta::FromMilliseconds(
- delay_after_renovations_completed_ms_));
- }
-}
-
void SnapshotController::DocumentAvailableInMainFrame() {
- if (document_available_triggers_snapshot_) {
- DCHECK_EQ(PageQuality::POOR, current_page_quality_);
- // Post a delayed task to snapshot.
- task_runner_->PostDelayedTask(
- FROM_HERE,
- base::BindOnce(&SnapshotController::MaybeStartSnapshot,
- weak_ptr_factory_.GetWeakPtr(),
- PageQuality::FAIR_AND_IMPROVING),
- base::TimeDelta::FromMilliseconds(delay_after_document_available_ms_));
- }
+ DCHECK_EQ(PageQuality::POOR, current_page_quality_);
+ // Post a delayed task to snapshot.
+ task_runner_->PostDelayedTask(
+ FROM_HERE,
+ base::BindOnce(&SnapshotController::MaybeStartSnapshot,
+ weak_ptr_factory_.GetWeakPtr(),
+ PageQuality::FAIR_AND_IMPROVING),
+ base::TimeDelta::FromMilliseconds(delay_after_document_available_ms_));
}
void SnapshotController::DocumentOnLoadCompletedInMainFrame() {
- if (renovations_enabled_) {
- // Run renovations. After renovations complete, a snapshot will be
- // triggered after a delay.
- client_->RunRenovations();
- } else {
- // Post a delayed task to snapshot and then stop this controller.
- task_runner_->PostDelayedTask(
- FROM_HERE,
- base::BindOnce(&SnapshotController::MaybeStartSnapshotThenStop,
- weak_ptr_factory_.GetWeakPtr()),
- base::TimeDelta::FromMilliseconds(
- delay_after_document_on_load_completed_ms_));
- }
+ // Post a delayed task to snapshot and then stop this controller.
+ task_runner_->PostDelayedTask(
+ FROM_HERE,
+ base::BindOnce(&SnapshotController::MaybeStartSnapshotThenStop,
+ weak_ptr_factory_.GetWeakPtr()),
+ base::TimeDelta::FromMilliseconds(
+ delay_after_document_on_load_completed_ms_));
}
void SnapshotController::MaybeStartSnapshot(PageQuality updated_page_quality) {
@@ -170,8 +107,4 @@
return delay_after_document_on_load_completed_ms_;
}
-int64_t SnapshotController::GetDelayAfterRenovationsCompletedForTest() {
- return delay_after_renovations_completed_ms_;
-}
-
} // namespace offline_pages
diff --git a/components/offline_pages/core/snapshot_controller.h b/components/offline_pages/core/snapshot_controller.h
index ff89102..bae959c 100644
--- a/components/offline_pages/core/snapshot_controller.h
+++ b/components/offline_pages/core/snapshot_controller.h
@@ -56,35 +56,14 @@
// it is assumed that later snapshots are better then previous.
virtual void StartSnapshot() = 0;
- // Invoked when the page is sufficiently loaded for running
- // renovations. The client should call the RenovationsCompleted()
- // when they finish.
- virtual void RunRenovations() = 0;
-
protected:
virtual ~Client() {}
};
- // Creates a SnapshotController with document available delay = 7s,
- // document on load delay = 1s and triggers snapshot on document available.
- static std::unique_ptr<SnapshotController> CreateForForegroundOfflining(
- const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
- SnapshotController::Client* client);
- // Creates a SnapshotController with document on load delay = 2s
- // and ignores document available signal.
- static std::unique_ptr<SnapshotController> CreateForBackgroundOfflining(
- const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
- SnapshotController::Client* client,
- bool renovations_enabled);
-
SnapshotController(
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
- SnapshotController::Client* client,
- int64_t delay_after_document_available_ms,
- int64_t delay_after_document_on_load_completed_ms,
- int64_t delay_after_renovations_completed_ms,
- bool document_available_triggers_snapshot,
- bool renovations_enabled);
+ SnapshotController::Client* client);
+
virtual ~SnapshotController();
// Resets the 'session', returning controller to initial state.
@@ -99,9 +78,6 @@
// now completed (so the next one can be started).
void PendingSnapshotCompleted();
- // The Client calls this when renovations have completed.
- void RenovationsCompleted();
-
// Invoked from WebContentObserver::DocumentAvailableInMainFrame
void DocumentAvailableInMainFrame();
@@ -110,7 +86,6 @@
int64_t GetDelayAfterDocumentAvailableForTest();
int64_t GetDelayAfterDocumentOnLoadCompletedForTest();
- int64_t GetDelayAfterRenovationsCompletedForTest();
PageQuality current_page_quality() const { return current_page_quality_; }
@@ -124,9 +99,6 @@
SnapshotController::State state_;
int64_t delay_after_document_available_ms_;
int64_t delay_after_document_on_load_completed_ms_;
- int64_t delay_after_renovations_completed_ms_;
- bool document_available_triggers_snapshot_;
- bool renovations_enabled_;
// The expected quality of a snapshot taken at the moment this value is
// queried.
diff --git a/components/offline_pages/core/snapshot_controller_unittest.cc b/components/offline_pages/core/snapshot_controller_unittest.cc
index fbd27e7..e0a74e0 100644
--- a/components/offline_pages/core/snapshot_controller_unittest.cc
+++ b/components/offline_pages/core/snapshot_controller_unittest.cc
@@ -7,9 +7,11 @@
#include "base/bind.h"
#include "base/run_loop.h"
#include "base/single_thread_task_runner.h"
+#include "base/test/scoped_feature_list.h"
#include "base/test/test_mock_time_task_runner.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
+#include "components/offline_pages/core/offline_page_feature.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace offline_pages {
@@ -29,7 +31,6 @@
// SnapshotController::Client
void StartSnapshot() override;
- void RunRenovations() override;
// Utility methods.
// Runs until all of the tasks that are not delayed are gone from the task
@@ -54,8 +55,7 @@
SnapshotControllerTest::~SnapshotControllerTest() {}
void SnapshotControllerTest::SetUp() {
- controller_ =
- SnapshotController::CreateForForegroundOfflining(task_runner_, this);
+ controller_ = std::make_unique<SnapshotController>(task_runner_, this);
snapshot_started_ = true;
}
@@ -67,10 +67,6 @@
snapshot_count_++;
}
-void SnapshotControllerTest::RunRenovations() {
- controller_->RenovationsCompleted();
-}
-
void SnapshotControllerTest::PumpLoop() {
task_runner_->RunUntilIdle();
}
