Commit 6bb37957 authored by Dan Harrington's avatar Dan Harrington Committed by Commit Bot

Defer auto-fetch requests while the active tab matches.

Auto-fetch requests will be canceled if the page is loaded in the foreground.
This CL ensures that background loading doesn't take place while the foreground
active tab still points to the requested URL, since it has a good chance of
eventually loading.

Bug: 883486
Change-Id: I4cf8b039ed5dc92aea08c9e0f7b597893f880113
Reviewed-on: https://chromium-review.googlesource.com/c/1313567
Commit-Queue: Dan H <harringtond@google.com>
Reviewed-by: default avatarPeter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606186}
parent bc896707
......@@ -16,6 +16,8 @@
#include "chrome/browser/offline_pages/background_loader_offliner.h"
#include "chrome/browser/offline_pages/offline_page_model_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/android/tab_model/tab_model.h"
#include "chrome/browser/ui/android/tab_model/tab_model_list.h"
#include "chrome/common/chrome_constants.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/offline_pages/core/background/offliner.h"
......@@ -26,6 +28,7 @@
#include "components/offline_pages/core/background/scheduler.h"
#include "components/offline_pages/core/offline_page_feature.h"
#include "components/offline_pages/core/offline_pages_ukm_reporter.h"
#include "content/public/browser/web_contents.h"
namespace network {
class NetworkQualityTracker;
......@@ -33,6 +36,34 @@ class NetworkQualityTracker;
namespace offline_pages {
class ActiveTabInfo : public RequestCoordinator::ActiveTabInfo {
public:
explicit ActiveTabInfo(Profile* profile) : profile_(profile) {}
~ActiveTabInfo() override {}
bool DoesActiveTabMatch(const GURL& url) override {
// Loop through to find the active tab and report whether the URL matches.
for (auto iter = TabModelList::begin(); iter != TabModelList::end();
++iter) {
TabModel* model = *iter;
if (model->GetProfile() == profile_) {
content::WebContents* contents = model->GetActiveWebContents();
// Check visibility to make sure Chrome is in the foreground.
if (contents &&
contents->GetVisibility() == content::Visibility::VISIBLE) {
if (contents->GetVisibleURL() == url)
return true;
if (contents->GetLastCommittedURL() == url)
return true;
}
}
}
return false;
}
private:
Profile* profile_;
};
RequestCoordinatorFactory::RequestCoordinatorFactory()
: BrowserContextKeyedServiceFactory(
"OfflineRequestCoordinator",
......@@ -82,7 +113,8 @@ KeyedService* RequestCoordinatorFactory::BuildServiceInstanceFor(
new OfflinePagesUkmReporter());
RequestCoordinator* request_coordinator = new RequestCoordinator(
std::move(policy), std::move(offliner), std::move(queue),
std::move(scheduler), network_quality_tracker, std::move(ukm_reporter));
std::move(scheduler), network_quality_tracker, std::move(ukm_reporter),
std::make_unique<ActiveTabInfo>(profile));
CCTRequestObserver::AttachToRequestCoordinator(request_coordinator);
......
......@@ -4,6 +4,8 @@
#include "chrome/browser/offline_pages/offline_page_auto_fetcher_service.h"
#include <utility>
#include "base/time/time.h"
#include "chrome/browser/offline_pages/request_coordinator_factory.h"
#include "components/offline_pages/core/background/request_coordinator.h"
......@@ -40,7 +42,7 @@ class OfflinePageAutoFetcherService::TaskToken {
public:
// The static methods should only be called by StartOrEnqueue or TaskComplete.
static TaskToken NewToken() { return TaskToken(); }
static void Finalize(TaskToken& token) { token.alive_ = false; }
static void Finalize(TaskToken* token) { token->alive_ = false; }
TaskToken(TaskToken&& other) : alive_(other.alive_) {
DCHECK(other.alive_);
......@@ -195,7 +197,7 @@ void OfflinePageAutoFetcherService::StartOrEnqueue(TaskCallback task) {
}
void OfflinePageAutoFetcherService::TaskComplete(TaskToken token) {
TaskToken::Finalize(token);
TaskToken::Finalize(&token);
DCHECK(!task_queue_.empty());
DCHECK(!task_queue_.front());
task_queue_.pop();
......
......@@ -19,6 +19,31 @@ namespace offline_pages {
class RequestCoordinator;
class SavePageRequest;
// Implementation notes for the auto-fetch-on-net-error-page feature:
//
// The 'auto-fetch-on-net-error-page' (auto-fetch) feature kicks in when a
// 'dino' (offline error) page is reached. Chrome will schedule a request to
// save the page when the device gains connectivity. Users can cancel or
// explicitly request this behavior through UI on the dino page. Chrome attempts
// to avoid doing a background page save if the user ends up successfully
// navigating to the page. If the page is saved in the background, the a system
// notification is presented.
//
// Background page saves are implemented through |RequestCoordinator|. The
// |OfflinePageClientPolicy| for this feature is configured with the option
// |defer_background_fetch_while_page_is_active|. This instructs
// |RequestCoordinator| to first check if the page to be saved is currently
// active. If it is, the request is deferred. If a request is deferred 5 times,
// it is considered failed and removed. For this feature, we expect this
// condition to be rare because |RequestCoordinator| only processes requests
// when the device is connected, and the dino page automatically reloads when
// the device is connected.
//
// Additionally, save page requests are removed upon successful navigation
// commit. See |AutoFetchPageLoadWatcher|.
// A KeyedService that provides an interface to schedule and cancel auto-fetch
// requests.
class OfflinePageAutoFetcherService : public KeyedService {
public:
using OfflinePageAutoFetcherScheduleResult =
......
......@@ -19,6 +19,14 @@
namespace offline_pages {
namespace {
class ActiveTabInfo : public RequestCoordinator::ActiveTabInfo {
public:
~ActiveTabInfo() override {}
bool DoesActiveTabMatch(const GURL&) override { return false; }
};
} // namespace
std::unique_ptr<KeyedService> BuildTestRequestCoordinator(
content::BrowserContext* context) {
// Use original policy.
......@@ -38,7 +46,7 @@ std::unique_ptr<KeyedService> BuildTestRequestCoordinator(
return std::unique_ptr<RequestCoordinator>(new RequestCoordinator(
std::move(policy), std::move(offliner), std::move(queue),
std::move(scheduler_stub), g_browser_process->network_quality_tracker(),
std::move(ukm_reporter_stub)));
std::move(ukm_reporter_stub), std::make_unique<ActiveTabInfo>()));
}
} // namespace offline_pages
......@@ -28,6 +28,8 @@ static_library("background_offliner") {
"mark_attempt_aborted_task.h",
"mark_attempt_completed_task.cc",
"mark_attempt_completed_task.h",
"mark_attempt_deferred_task.cc",
"mark_attempt_deferred_task.h",
"mark_attempt_started_task.cc",
"mark_attempt_started_task.h",
"offliner.h",
......@@ -86,6 +88,7 @@ static_library("test_support") {
"scheduler_stub.h",
"test_request_queue_store.cc",
"test_request_queue_store.h",
"test_util.cc",
]
deps = [
......
// Copyright 2016 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/mark_attempt_deferred_task.h"
#include <utility>
#include "base/bind.h"
#include "base/time/time.h"
namespace offline_pages {
MarkAttemptDeferredTask::MarkAttemptDeferredTask(
RequestQueueStore* store,
int64_t request_id,
RequestQueueStore::UpdateCallback callback)
: UpdateRequestTask(store, request_id, std::move(callback)) {}
MarkAttemptDeferredTask::~MarkAttemptDeferredTask() {}
void MarkAttemptDeferredTask::UpdateRequestImpl(
UpdateRequestsResult read_result) {
if (!ValidateReadResult(read_result)) {
CompleteWithResult(std::move(read_result));
return;
}
// It is perfectly fine to reuse the read_result.updated_items collection, as
// it is owned by this callback and will be destroyed when out of scope.
read_result.updated_items[0].MarkAttemptDeferred(base::Time::Now());
store()->UpdateRequests(
read_result.updated_items,
base::BindOnce(&MarkAttemptDeferredTask::CompleteWithResult,
GetWeakPtr()));
}
} // namespace offline_pages
// 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_MARK_ATTEMPT_DEFERRED_TASK_H_
#define COMPONENTS_OFFLINE_PAGES_CORE_BACKGROUND_MARK_ATTEMPT_DEFERRED_TASK_H_
#include <stdint.h>
#include <memory>
#include "components/offline_items_collection/core/fail_state.h"
#include "components/offline_pages/core/background/request_queue_results.h"
#include "components/offline_pages/core/background/update_request_task.h"
#include "components/offline_pages/task/task.h"
namespace offline_pages {
class RequestQueueStore;
class MarkAttemptDeferredTask : public UpdateRequestTask {
public:
MarkAttemptDeferredTask(RequestQueueStore* store,
int64_t request_id,
RequestQueueStore::UpdateCallback callback);
~MarkAttemptDeferredTask() override;
protected:
// UpdateRequestTask implementation:
void UpdateRequestImpl(UpdateRequestsResult result) override;
};
} // namespace offline_pages
#endif // COMPONENTS_OFFLINE_PAGES_CORE_BACKGROUND_MARK_ATTEMPT_DEFERRED_TASK_H_
......@@ -75,8 +75,10 @@ class Offliner {
LOADING_FAILED_NET_ERROR = 19,
// Loader failed to load page due to HTTP error.
LOADING_FAILED_HTTP_ERROR = 20,
// Loading was deferred because the active tab URL matches.
LOADING_DEFERRED = 21,
kMaxValue = LOADING_FAILED_HTTP_ERROR,
kMaxValue = LOADING_DEFERRED,
};
// Reports the load progress of a request.
......
......@@ -4,7 +4,10 @@
#include "components/offline_pages/core/background/pick_request_task.h"
#include <memory>
#include <unordered_set>
#include <utility>
#include <vector>
#include "base/bind.h"
#include "base/logging.h"
......@@ -16,6 +19,7 @@
#include "components/offline_pages/core/background/request_notifier.h"
#include "components/offline_pages/core/background/request_queue_store.h"
#include "components/offline_pages/core/background/save_page_request.h"
#include "components/offline_pages/core/client_policy_controller.h"
namespace {
template <typename T>
......@@ -31,9 +35,13 @@ bool kNonUserRequestsFound = true;
namespace offline_pages {
const base::TimeDelta PickRequestTask::kDeferInterval =
base::TimeDelta::FromMinutes(1);
PickRequestTask::PickRequestTask(
RequestQueueStore* store,
OfflinerPolicy* policy,
ClientPolicyController* policy_controller,
RequestPickedCallback picked_callback,
RequestNotPickedCallback not_picked_callback,
RequestCountCallback request_count_callback,
......@@ -42,6 +50,7 @@ PickRequestTask::PickRequestTask(
base::circular_deque<int64_t>* prioritized_requests)
: store_(store),
policy_(policy),
policy_controller_(policy_controller),
picked_callback_(std::move(picked_callback)),
not_picked_callback_(std::move(not_picked_callback)),
request_count_callback_(std::move(request_count_callback)),
......@@ -69,7 +78,7 @@ void PickRequestTask::Choose(
if (requests.empty()) {
std::move(request_count_callback_).Run(requests.size(), 0);
std::move(not_picked_callback_)
.Run(!kNonUserRequestsFound, !kCleanupNeeded);
.Run(!kNonUserRequestsFound, !kCleanupNeeded, base::Time());
TaskComplete();
return;
}
......@@ -95,7 +104,9 @@ void PickRequestTask::Choose(
size_t total_request_count = requests.size();
// Request ids which are available for picking.
std::unordered_set<int64_t> available_request_ids;
// If there was a deferred task, this records the earliest time a task will
// become available.
base::Time defer_available_time;
// Iterate through the requests, filter out unavailable requests and get other
// information (if cleanup is needed and number of non-user-requested
// requests).
......@@ -123,6 +134,14 @@ void PickRequestTask::Choose(
available_requests->push_back(*request);
if (!RequestConditionsSatisfied(*request))
continue;
if (policy_controller_->GetPolicy(request->client_id().name_space)
.defer_background_fetch_while_page_is_active) {
if (!request->last_attempt_time().is_null() &&
base::Time::Now() - request->last_attempt_time() < kDeferInterval) {
defer_available_time = request->last_attempt_time() + kDeferInterval;
continue;
}
}
available_request_ids.insert(request->request_id());
}
// Report the request queue counts.
......@@ -169,7 +188,8 @@ void PickRequestTask::Choose(
.Run(*picked_request, std::move(available_requests), cleanup_needed);
} else {
std::move(not_picked_callback_)
.Run(non_user_requested_tasks_remaining, cleanup_needed);
.Run(non_user_requested_tasks_remaining, cleanup_needed,
defer_available_time);
}
TaskComplete();
......
......@@ -5,7 +5,9 @@
#ifndef COMPONENTS_OFFLINE_PAGES_CORE_BACKGROUND_PICK_REQUEST_TASK_H_
#define COMPONENTS_OFFLINE_PAGES_CORE_BACKGROUND_PICK_REQUEST_TASK_H_
#include <memory>
#include <set>
#include <vector>
#include "base/containers/circular_deque.h"
#include "base/memory/weak_ptr.h"
......@@ -16,6 +18,7 @@
namespace offline_pages {
class ClientPolicyController;
class OfflinerPolicy;
class PickRequestTask;
class RequestQueueStore;
......@@ -26,6 +29,8 @@ typedef bool (PickRequestTask::*RequestCompareFunction)(
class PickRequestTask : public Task {
public:
static const base::TimeDelta kDeferInterval;
// Callback to report when a request was available.
typedef base::OnceCallback<void(
const SavePageRequest& request,
......@@ -34,7 +39,9 @@ class PickRequestTask : public Task {
RequestPickedCallback;
// Callback to report when no request was available.
typedef base::OnceCallback<void(bool non_user_requests, bool cleanup_needed)>
typedef base::OnceCallback<void(bool non_user_requests,
bool cleanup_needed,
base::Time available_time)>
RequestNotPickedCallback;
// Callback to report available total and available queued request counts.
......@@ -42,6 +49,7 @@ class PickRequestTask : public Task {
PickRequestTask(RequestQueueStore* store,
OfflinerPolicy* policy,
ClientPolicyController* policy_controller,
RequestPickedCallback picked_callback,
RequestNotPickedCallback not_picked_callback,
RequestCountCallback request_count_callback,
......@@ -93,6 +101,7 @@ class PickRequestTask : public Task {
// Member variables, all pointers are not owned here.
RequestQueueStore* store_;
OfflinerPolicy* policy_;
ClientPolicyController* policy_controller_;
RequestPickedCallback picked_callback_;
RequestNotPickedCallback not_picked_callback_;
RequestCountCallback request_count_callback_;
......
......@@ -20,6 +20,7 @@
#include "components/offline_pages/core/background/request_queue_task_test_base.h"
#include "components/offline_pages/core/background/save_page_request.h"
#include "components/offline_pages/core/background/test_request_queue_store.h"
#include "components/offline_pages/core/client_policy_controller.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace offline_pages {
......@@ -102,7 +103,8 @@ class PickRequestTaskTest : public RequestQueueTaskTestBase {
bool cleanup_needed);
void RequestNotPicked(const bool non_user_requested_tasks_remaining,
bool cleanup_needed);
bool cleanup_needed,
base::Time available_time);
void RequestCountCallback(size_t total_count, size_t available_count);
......@@ -122,6 +124,7 @@ class PickRequestTaskTest : public RequestQueueTaskTestBase {
std::unique_ptr<RequestNotifierStub> notifier_;
std::unique_ptr<SavePageRequest> last_picked_;
std::unique_ptr<OfflinerPolicy> policy_;
ClientPolicyController policy_controller_;
RequestCoordinatorEventLogger event_logger_;
std::set<int64_t> disabled_requests_;
base::circular_deque<int64_t> prioritized_requests_;
......@@ -165,7 +168,8 @@ void PickRequestTaskTest::RequestPicked(
void PickRequestTaskTest::RequestNotPicked(
const bool non_user_requested_tasks_remaining,
const bool cleanup_needed) {
const bool cleanup_needed,
base::Time available_time) {
request_queue_not_picked_called_ = true;
}
......@@ -195,7 +199,7 @@ void PickRequestTaskTest::QueueRequests(const SavePageRequest& request1,
void PickRequestTaskTest::MakePickRequestTask() {
DeviceConditions conditions;
task_.reset(new PickRequestTask(
&store_, policy_.get(),
&store_, policy_.get(), &policy_controller_,
base::BindOnce(&PickRequestTaskTest::RequestPicked,
base::Unretained(this)),
base::BindOnce(&PickRequestTaskTest::RequestNotPicked,
......
......@@ -192,14 +192,7 @@ RequestCoordinator::SavePageLaterParams::SavePageLaterParams()
availability(RequestAvailability::ENABLED_FOR_OFFLINER) {}
RequestCoordinator::SavePageLaterParams::SavePageLaterParams(
const SavePageLaterParams& other) {
url = other.url;
client_id = other.client_id;
user_requested = other.user_requested;
availability = other.availability;
original_url = other.original_url;
request_origin = other.request_origin;
}
const SavePageLaterParams& other) = default;
RequestCoordinator::SavePageLaterParams::~SavePageLaterParams() = default;
......@@ -209,7 +202,8 @@ RequestCoordinator::RequestCoordinator(
std::unique_ptr<RequestQueue> queue,
std::unique_ptr<Scheduler> scheduler,
network::NetworkQualityTracker* network_quality_tracker,
std::unique_ptr<OfflinePagesUkmReporter> ukm_reporter)
std::unique_ptr<OfflinePagesUkmReporter> ukm_reporter,
std::unique_ptr<ActiveTabInfo> active_tab_info)
: is_low_end_device_(base::SysInfo::IsLowEndDevice()),
state_(RequestCoordinatorState::IDLE),
processing_state_(ProcessingWindowState::STOPPED),
......@@ -227,6 +221,7 @@ RequestCoordinator::RequestCoordinator(
scheduler_callback_(base::DoNothing()),
internal_start_processing_callback_(base::DoNothing()),
pending_state_updater_(this),
active_tab_info_(std::move(active_tab_info)),
weak_ptr_factory_(this) {
DCHECK(policy_ != nullptr);
DCHECK(network_quality_tracker_);
......@@ -599,6 +594,14 @@ void RequestCoordinator::TryNextRequestCallback(int64_t offline_id) {
TryNextRequest(!kStartOfProcessing);
}
void RequestCoordinator::MarkDeferredAttemptCallback(
UpdateRequestsResult result) {
// This is called after the attempt has been marked as deferred in the
// database. StopProcessing() is called to resume request processing.
state_ = RequestCoordinatorState::IDLE;
StopProcessing(Offliner::RequestStatus::LOADING_DEFERRED);
}
void RequestCoordinator::ScheduleAsNeeded() {
// Get all requests from queue (there is no filtering mechanism).
queue_->GetRequests(
......@@ -790,7 +793,7 @@ void RequestCoordinator::TryNextRequest(bool is_start_of_processing) {
// Ask request queue to make a new PickRequestTask object, then put it on
// the task queue.
queue_->PickNextRequest(
policy_.get(),
policy_.get(), policy_controller_.get(),
base::BindOnce(&RequestCoordinator::RequestPicked,
weak_ptr_factory_.GetWeakPtr()),
base::BindOnce(&RequestCoordinator::RequestNotPicked,
......@@ -826,7 +829,8 @@ void RequestCoordinator::RequestPicked(
void RequestCoordinator::RequestNotPicked(
bool non_user_requested_tasks_remaining,
bool cleanup_needed) {
bool cleanup_needed,
base::Time available_time) {
DVLOG(2) << __func__;
state_ = RequestCoordinatorState::IDLE;
......@@ -840,6 +844,11 @@ void RequestCoordinator::RequestNotPicked(
} else if (non_user_requested_tasks_remaining) {
// If we don't have any of those, check for non-user-requested tasks.
scheduler_->Schedule(GetTriggerConditions(!kUserRequest));
} else if (!available_time.is_null()) {
scheduler_->BackupSchedule(
GetTriggerConditions(kUserRequest),
(available_time - base::Time::Now()).InSeconds() +
1 /*Add an extra second to avoid rounding down.*/);
}
// Schedule a queue cleanup if needed.
......@@ -903,6 +912,16 @@ void RequestCoordinator::SendRequestToOffliner(const SavePageRequest& request) {
if (request.started_attempt_count() == 0) {
RecordStartTimeUMA(request);
}
const OfflinePageClientPolicy& policy =
policy_controller_->GetPolicy(request.client_id().name_space);
if (policy.defer_background_fetch_while_page_is_active &&
active_tab_info_->DoesActiveTabMatch(request.url())) {
queue_->MarkAttemptDeferred(
request.request_id(),
base::BindOnce(&RequestCoordinator::MarkDeferredAttemptCallback,
weak_ptr_factory_.GetWeakPtr()));
return;
}
// Mark attempt started in the database and start offliner when completed.
queue_->MarkAttemptStarted(
......@@ -992,7 +1011,7 @@ void RequestCoordinator::OfflinerProgressCallback(
const SavePageRequest& request,
int64_t received_bytes) {
DVLOG(2) << "offliner progress, received bytes: " << received_bytes;
DCHECK(received_bytes >= 0);
DCHECK_GE(received_bytes, 0);
NotifyNetworkProgress(request, received_bytes);
}
......@@ -1099,7 +1118,6 @@ void RequestCoordinator::MarkRequestCompleted(int64_t request_id) {
// calls for a particular request_id.
if (disabled_requests_.find(request_id) == disabled_requests_.end())
return;
// Clear from disabled list.
disabled_requests_.erase(request_id);
......
......@@ -64,6 +64,14 @@ class RequestCoordinator : public KeyedService,
int64_t received_bytes) = 0;
};
class ActiveTabInfo {
public:
virtual ~ActiveTabInfo() {}
// Returns true if the active tab's URL matches |url|. If Chrome is in the
// background, this should return false.
virtual bool DoesActiveTabMatch(const GURL& url) = 0;
};
enum class RequestAvailability {
ENABLED_FOR_OFFLINER,
DISABLED_FOR_OFFLINER,
......@@ -121,7 +129,8 @@ class RequestCoordinator : public KeyedService,
std::unique_ptr<RequestQueue> queue,
std::unique_ptr<Scheduler> scheduler,
network::NetworkQualityTracker* network_quality_tracker,
std::unique_ptr<OfflinePagesUkmReporter> ukm_reporter);
std::unique_ptr<OfflinePagesUkmReporter> ukm_reporter,
std::unique_ptr<ActiveTabInfo> active_tab_info);
~RequestCoordinator() override;
......@@ -313,6 +322,7 @@ class RequestCoordinator : public KeyedService,
void ResetActiveRequestCallback(int64_t offline_id);
void StartSchedulerCallback(int64_t offline_id);
void TryNextRequestCallback(int64_t offline_id);
void MarkDeferredAttemptCallback(UpdateRequestsResult result);
bool StartProcessingInternal(
const ProcessingWindowState processing_state,
......@@ -348,7 +358,8 @@ class RequestCoordinator : public KeyedService,
// The parameter is a signal for what (if any) conditions to schedule future
// processing for.
void RequestNotPicked(bool non_user_requested_tasks_remaining,
bool cleanup_needed);
bool cleanup_needed,
base::Time available_time);
// Callback from request picker that receives the current available queued
// request count as well as the total queued request count (which may be
......@@ -503,6 +514,8 @@ class RequestCoordinator : public KeyedService,
base::circular_deque<int64_t> prioritized_requests_;
// Updates a request's PendingState.
PendingStateUpdater pending_state_updater_;
std::unique_ptr<ActiveTabInfo> active_tab_info_;
// Allows us to pass a weak pointer to callbacks.
base::WeakPtrFactory<RequestCoordinator> weak_ptr_factory_;
......
......@@ -54,6 +54,8 @@ static std::string OfflinerRequestStatusToString(
return "LOADING_FAILED_NET_ERROR";
case Offliner::RequestStatus::LOADING_FAILED_HTTP_ERROR:
return "LOADING_FAILED_HTTP_ERROR";
case Offliner::RequestStatus::LOADING_DEFERRED:
return "LOADING_DEFERRED";
}
return "UNKNOWN";
}
......
......@@ -4,6 +4,8 @@
#include "components/offline_pages/core/background/request_coordinator_stub_taco.h"
#include <utility>
#include "components/offline_pages/core/background/offliner_stub.h"
#include "components/offline_pages/core/background/request_queue.h"
#include "components/offline_pages/core/background/request_queue_store.h"
......@@ -16,6 +18,12 @@
namespace offline_pages {
class ActiveTabInfo : public RequestCoordinator::ActiveTabInfo {
public:
~ActiveTabInfo() override {}
bool DoesActiveTabMatch(const GURL&) override { return false; }
};
RequestCoordinatorStubTaco::RequestCoordinatorStubTaco() {
policy_ = std::make_unique<OfflinerPolicy>();
queue_ =
......@@ -25,6 +33,7 @@ RequestCoordinatorStubTaco::RequestCoordinatorStubTaco() {
network_quality_tracker_ =
std::make_unique<network::TestNetworkQualityTracker>();
ukm_reporter_ = std::make_unique<OfflinePagesUkmReporterStub>();
active_tab_info_ = std::make_unique<ActiveTabInfo>();
}
RequestCoordinatorStubTaco::~RequestCoordinatorStubTaco() {
......@@ -73,15 +82,20 @@ void RequestCoordinatorStubTaco::SetOfflinePagesUkmReporter(
ukm_reporter_ = std::move(ukm_reporter);
}
void RequestCoordinatorStubTaco::SetRequestCoordinatorDelegate(
std::unique_ptr<RequestCoordinator::ActiveTabInfo> active_tab_info) {
active_tab_info_ = std::move(active_tab_info);
}
void RequestCoordinatorStubTaco::CreateRequestCoordinator() {
request_coordinator_ = std::make_unique<RequestCoordinator>(
std::move(policy_), std::move(offliner_), std::move(queue_),
std::move(scheduler_), network_quality_tracker_.get(),
std::move(ukm_reporter_));
std::move(ukm_reporter_), std::move(active_tab_info_));
}
RequestCoordinator* RequestCoordinatorStubTaco::request_coordinator() {
CHECK(request_coordinator_);
return request_coordinator_.get();
}
} // namespace offline_page
} // namespace offline_pages
......@@ -44,6 +44,8 @@ class RequestCoordinatorStubTaco {
std::unique_ptr<network::NetworkQualityTracker> network_quality_tracker);
void SetOfflinePagesUkmReporter(
std::unique_ptr<OfflinePagesUkmReporter> ukm_reporter);
void SetRequestCoordinatorDelegate(
std::unique_ptr<RequestCoordinator::ActiveTabInfo> delegate);
// Creates and caches an instance of RequestCoordinator, using default or
// overridden stub dependencies.
......@@ -65,6 +67,7 @@ class RequestCoordinatorStubTaco {
std::unique_ptr<Scheduler> scheduler_;
std::unique_ptr<network::NetworkQualityTracker> network_quality_tracker_;
std::unique_ptr<OfflinePagesUkmReporter> ukm_reporter_;
std::unique_ptr<RequestCoordinator::ActiveTabInfo> active_tab_info_;
std::unique_ptr<RequestCoordinator> request_coordinator_;
};
......
......@@ -31,6 +31,7 @@
#include "components/offline_pages/core/background/save_page_request.h"
#include "components/offline_pages/core/background/scheduler.h"
#include "components/offline_pages/core/background/scheduler_stub.h"
#include "components/offline_pages/core/client_namespace_constants.h"
#include "components/offline_pages/core/offline_page_feature.h"
#include "services/network/test/test_network_quality_tracker.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -124,6 +125,18 @@ class ObserverStub : public RequestCoordinator::Observer {
PendingState pending_state_;
};
class ActiveTabInfoStub : public RequestCoordinator::ActiveTabInfo {
public:
~ActiveTabInfoStub() override {}
bool DoesActiveTabMatch(const GURL&) override {
return does_active_tab_match_;
}
void set_does_active_tab_match(bool match) { does_active_tab_match_ = match; }
private:
bool does_active_tab_match_ = false;
};
} // namespace
// This class is a friend of RequestCoordinator, and can't be in the anonymous
......@@ -147,6 +160,9 @@ class RequestCoordinatorTest : public testing::Test {
RequestCoordinator* coordinator() const {
return coordinator_taco_->request_coordinator();
}
SchedulerStub* scheduler_stub() const {
return reinterpret_cast<SchedulerStub*>(coordinator()->scheduler());
}
RequestQueue* queue() {
return coordinator_taco_->request_coordinator()->queue_for_testing();
}
......@@ -258,7 +274,8 @@ class RequestCoordinatorTest : public testing::Test {
else
coordinator()->disabled_requests_.clear();
coordinator()->RequestNotPicked(non_user_requested_tasks_remaining, false);
coordinator()->RequestNotPicked(non_user_requested_tasks_remaining, false,
base::Time());
}
void SetDeviceConditionsForTest(DeviceConditions device_conditions) {
......@@ -348,6 +365,9 @@ class RequestCoordinatorTest : public testing::Test {
bool add_request_callback_called() { return add_request_callback_called_; }
protected:
ActiveTabInfoStub* active_tab_info_ = nullptr;
private:
GetRequestsResult last_get_requests_result_;
MultipleItemStatuses last_remove_results_;
......@@ -400,6 +420,9 @@ void RequestCoordinatorTest::SetUp() {
network_quality_tracker_ = test_network_quality_tracker.get();
coordinator_taco_->SetNetworkQualityProvider(
std::move(test_network_quality_tracker));
auto delegate = std::make_unique<ActiveTabInfoStub>();
active_tab_info_ = delegate.get();
coordinator_taco_->SetRequestCoordinatorDelegate(std::move(delegate));
coordinator_taco_->CreateRequestCoordinator();
......@@ -629,13 +652,11 @@ TEST_F(RequestCoordinatorTest, SavePageLater) {
EXPECT_EQ(kRequestOrigin, last_requests().at(0)->request_origin());
// Expect that the scheduler got notified.
SchedulerStub* scheduler_stub =
reinterpret_cast<SchedulerStub*>(coordinator()->scheduler());
EXPECT_TRUE(scheduler_stub->schedule_called());
EXPECT_TRUE(scheduler_stub()->schedule_called());
EXPECT_EQ(coordinator()
->GetTriggerConditions(last_requests()[0]->user_requested())
.minimum_battery_percentage,
scheduler_stub->trigger_conditions()->minimum_battery_percentage);
scheduler_stub()->trigger_conditions()->minimum_battery_percentage);
// Check that the observer got the notification that a page is available
EXPECT_TRUE(observer().added_called());
......@@ -679,13 +700,11 @@ TEST_F(RequestCoordinatorTest, SavePageLaterFailed) {
EXPECT_EQ(kClientId1, last_requests().at(0)->client_id());
// Expect that the scheduler got notified.
SchedulerStub* scheduler_stub =
reinterpret_cast<SchedulerStub*>(coordinator()->scheduler());
EXPECT_TRUE(scheduler_stub->schedule_called());
EXPECT_TRUE(scheduler_stub()->schedule_called());
EXPECT_EQ(coordinator()
->GetTriggerConditions(last_requests()[0]->user_requested())
.minimum_battery_percentage,
scheduler_stub->trigger_conditions()->minimum_battery_percentage);
scheduler_stub()->trigger_conditions()->minimum_battery_percentage);
// Check that the observer got the notification that a page is available
EXPECT_TRUE(observer().added_called());
......@@ -893,17 +912,16 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneForegroundCancel) {
EXPECT_EQ(0L, last_requests().at(0)->completed_attempt_count());
}
TEST_F(RequestCoordinatorTest, OfflinerDoneOffliningCancel) {
// Add a request to the queue, wait for callbacks to finish.
offline_pages::SavePageRequest request(kRequestId1, kUrl1, kClientId1,
base::Time::Now(), kUserRequested);
SetupForOfflinerDoneCallbackTest(&request);
// Call the OfflinerDoneCallback to simulate the request failed, wait
// for callbacks.
SendOfflinerDoneCallback(request, Offliner::RequestStatus::LOADING_CANCELED);
TEST_F(RequestCoordinatorTest, RequestDeferred) {
// Test handling of requests that can be deferred due to
// defer_while_page_is_active.
active_tab_info_->set_does_active_tab_match(true);
RequestCoordinator::SavePageLaterParams params;
params.url = kUrl1;
// Auto-async uses defer_background_fetch_while_page_is_active.
params.client_id = ClientId(kAutoAsyncNamespace, "1");
coordinator()->SavePageLater(params, base::DoNothing());
PumpLoop();
EXPECT_TRUE(processing_callback_called());
// Verify the request is not removed from the queue, and wait for callbacks.
queue()->GetRequests(base::BindOnce(&RequestCoordinatorTest::GetRequestsDone,
......@@ -911,11 +929,42 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneOffliningCancel) {
PumpLoop();
// Request still in the queue.
EXPECT_EQ(1UL, last_requests().size());
// Verify offlining cancel not counted as an attempt after all.
const std::unique_ptr<SavePageRequest>& found_request =
last_requests().front();
EXPECT_EQ(0L, found_request->completed_attempt_count());
ASSERT_EQ(1UL, last_requests().size());
EXPECT_EQ(1L, last_requests()[0]->started_attempt_count());
EXPECT_EQ(1L, last_requests()[0]->completed_attempt_count());
// The scheduler is called. Simulate the scheduler calling us back.
// This time, the request was tried recently, and will not be retried again.
// Since there are no requests this time, backup_schedule is called with a
// delay that matches the deferral interval.
ASSERT_TRUE(scheduler_stub()->schedule_called());
coordinator()->StartScheduledProcessing(device_conditions(),
processing_callback());
PumpLoop();
EXPECT_TRUE(scheduler_stub()->backup_schedule_called());
// Add plenty of tolerance to avoid flakes.
EXPECT_LT(PickRequestTask::kDeferInterval.InSeconds() - 10,
scheduler_stub()->schedule_delay());
}
TEST_F(RequestCoordinatorTest, RequestNotDeferred) {
// Test defer_while_page_is_active=true, but the DoesActiveTabMatch returns
// false. The page should be offlined.
active_tab_info_->set_does_active_tab_match(false);
RequestCoordinator::SavePageLaterParams params;
params.url = kUrl1;
// Auto-async uses defer_background_fetch_while_page_is_active.
params.client_id = ClientId(kAutoAsyncNamespace, "1");
coordinator()->SavePageLater(params, base::DoNothing());
PumpLoop();
queue()->GetRequests(base::BindOnce(&RequestCoordinatorTest::GetRequestsDone,
base::Unretained(this)));
PumpLoop();
// Request was completed.
ASSERT_EQ(0UL, last_requests().size());
}
// If one item completes, and there are no more user requeted items left,
......@@ -933,10 +982,8 @@ TEST_F(RequestCoordinatorTest, RequestNotPickedDisabledItemsRemain) {
// The scheduler should have been called to schedule the disabled task for
// 5 minutes from now.
SchedulerStub* scheduler_stub =
reinterpret_cast<SchedulerStub*>(coordinator()->scheduler());
EXPECT_TRUE(scheduler_stub->backup_schedule_called());
EXPECT_TRUE(scheduler_stub->unschedule_called());
EXPECT_TRUE(scheduler_stub()->backup_schedule_called());
EXPECT_TRUE(scheduler_stub()->unschedule_called());
}
// If one item completes, and there are no more user requeted items left,
......@@ -956,12 +1003,10 @@ TEST_F(RequestCoordinatorTest, RequestNotPickedNonUserRequestedItemsRemain) {
// The scheduler should have been called to schedule the non-user requested
// task.
SchedulerStub* scheduler_stub =
reinterpret_cast<SchedulerStub*>(coordinator()->scheduler());
EXPECT_TRUE(scheduler_stub->schedule_called());
EXPECT_TRUE(scheduler_stub->unschedule_called());
EXPECT_TRUE(scheduler_stub()->schedule_called());
EXPECT_TRUE(scheduler_stub()->unschedule_called());
const Scheduler::TriggerConditions* conditions =
scheduler_stub->trigger_conditions();
scheduler_stub()->trigger_conditions();
EXPECT_EQ(conditions->require_power_connected,
coordinator()->policy()->PowerRequired(!kUserRequested));
EXPECT_EQ(
......@@ -988,11 +1033,9 @@ TEST_F(RequestCoordinatorTest, SchedulerGetsLeastRestrictiveConditions) {
// Expect that the scheduler got notified, and it is at user_requested
// priority.
SchedulerStub* scheduler_stub =
reinterpret_cast<SchedulerStub*>(coordinator()->scheduler());
const Scheduler::TriggerConditions* conditions =
scheduler_stub->trigger_conditions();
EXPECT_TRUE(scheduler_stub->schedule_called());
scheduler_stub()->trigger_conditions();
EXPECT_TRUE(scheduler_stub()->schedule_called());
EXPECT_EQ(conditions->require_power_connected,
coordinator()->policy()->PowerRequired(kUserRequested));
EXPECT_EQ(conditions->minimum_battery_percentage,
......
......@@ -15,6 +15,7 @@
#include "components/offline_pages/core/background/initialize_store_task.h"
#include "components/offline_pages/core/background/mark_attempt_aborted_task.h"
#include "components/offline_pages/core/background/mark_attempt_completed_task.h"
#include "components/offline_pages/core/background/mark_attempt_deferred_task.h"
#include "components/offline_pages/core/background/mark_attempt_started_task.h"
#include "components/offline_pages/core/background/pick_request_task.h"
#include "components/offline_pages/core/background/reconcile_task.h"
......@@ -120,8 +121,16 @@ void RequestQueue::MarkAttemptCompleted(int64_t request_id,
task_queue_.AddTask(std::move(task));
}
void RequestQueue::MarkAttemptDeferred(int64_t request_id,
UpdateCallback callback) {
std::unique_ptr<Task> task(new MarkAttemptDeferredTask(
store_.get(), request_id, std::move(callback)));
task_queue_.AddTask(std::move(task));
}
void RequestQueue::PickNextRequest(
OfflinerPolicy* policy,
ClientPolicyController* policy_controller,
PickRequestTask::RequestPickedCallback picked_callback,
PickRequestTask::RequestNotPickedCallback not_picked_callback,
PickRequestTask::RequestCountCallback request_count_callback,
......@@ -130,7 +139,7 @@ void RequestQueue::PickNextRequest(
base::circular_deque<int64_t>* prioritized_requests) {
// Using the PickerContext, create a picker task.
std::unique_ptr<Task> task(new PickRequestTask(
store_.get(), policy, std::move(picked_callback),
store_.get(), policy, policy_controller, std::move(picked_callback),
std::move(not_picked_callback), std::move(request_count_callback),
std::move(conditions), disabled_requests, prioritized_requests));
......
......@@ -10,6 +10,7 @@
#include <memory>
#include <set>
#include <string>
#include <utility>
#include <vector>
#include "base/callback.h"
......@@ -29,6 +30,7 @@
namespace offline_pages {
class CleanupTaskFactory;
class ClientPolicyController;
class RequestQueueStore;
// Class responsible for managing save page requests.
......@@ -86,6 +88,10 @@ class RequestQueue : public TaskQueue::Delegate {
// |callback|.
void MarkAttemptAborted(int64_t request_id, UpdateCallback callback);
// Marks attempt with |request_id| as deferred. Results are returned through
// |callback|.
void MarkAttemptDeferred(int64_t request_id, UpdateCallback callback);
// Marks attempt with |request_id| as completed. The attempt may have
// completed with either success or failure (stored in FailState). Results are
// returned through |callback|.
......@@ -97,6 +103,7 @@ class RequestQueue : public TaskQueue::Delegate {
// callbacks.
void PickNextRequest(
OfflinerPolicy* policy,
ClientPolicyController* policy_controller,
PickRequestTask::RequestPickedCallback picked_callback,
PickRequestTask::RequestNotPickedCallback not_picked_callback,
PickRequestTask::RequestCountCallback request_count_callback,
......
......@@ -4,6 +4,7 @@
#include "components/offline_pages/core/background/request_queue_store.h"
#include <string>
#include <unordered_set>
#include <utility>
......@@ -254,6 +255,7 @@ ItemActionStatus Update(sql::Database* db, const SavePageRequest& request) {
" WHERE request_id = ?";
sql::Statement statement(db->GetCachedStatement(SQL_FROM_HERE, kSql));
// SET columns:
statement.BindInt64(0, store_utils::ToDatabaseTime(request.creation_time()));
statement.BindInt64(1, 0);
statement.BindInt64(2,
......@@ -267,6 +269,7 @@ ItemActionStatus Update(sql::Database* db, const SavePageRequest& request) {
statement.BindString(9, request.original_url().spec());
statement.BindString(10, request.request_origin());
statement.BindInt64(11, static_cast<int64_t>(request.fail_state()));
// WHERE:
statement.BindInt64(12, request.request_id());
if (!statement.Run())
......
......@@ -5,6 +5,8 @@
#include "components/offline_pages/core/background/request_queue_store.h"
#include <memory>
#include <string>
#include <utility>
#include "base/bind.h"
#include "base/files/file_path.h"
......@@ -32,7 +34,7 @@ const GURL kUrl2("http://another-example.com");
const ClientId kClientId("bookmark", "1234");
const ClientId kClientId2("async", "5678");
const bool kUserRequested = true;
const std::string kRequestOrigin = "abc.xyz";
const char kRequestOrigin[] = "abc.xyz";
enum class LastResult {
RESULT_NONE,
......@@ -40,6 +42,23 @@ enum class LastResult {
RESULT_TRUE,
};
SavePageRequest GetTestRequest() {
SavePageRequest request(kRequestId, kUrl, kClientId,
base::Time::FromDeltaSinceWindowsEpoch(
base::TimeDelta::FromSeconds(1000)),
kUserRequested);
// Set fields to non-default values.
request.set_fail_state(offline_items_collection::FailState::FILE_NO_SPACE);
request.set_started_attempt_count(2);
request.set_completed_attempt_count(3);
request.set_last_attempt_time(base::Time::FromDeltaSinceWindowsEpoch(
base::TimeDelta::FromSeconds(400)));
request.set_request_origin("http://www.origin.com");
// Note: pending_state is not stored.
request.set_original_url(kUrl2);
return request;
}
void BuildTestStoreWithSchemaFromM57(const base::FilePath& file) {
sql::Database connection;
ASSERT_TRUE(
......@@ -492,6 +511,23 @@ TEST_F(RequestQueueStoreTest, AddRequest) {
ASSERT_EQ(1ul, this->last_requests().size());
}
TEST_F(RequestQueueStoreTest, AddAndGetRequestsMatch) {
std::unique_ptr<RequestQueueStore> store(this->BuildStore());
this->InitializeStore(store.get());
const SavePageRequest request = GetTestRequest();
store->AddRequest(request,
base::BindOnce(&RequestQueueStoreTestBase::AddRequestDone,
base::Unretained(this)));
store->GetRequests(base::BindOnce(&RequestQueueStoreTestBase::GetRequestsDone,
base::Unretained(this)));
this->PumpLoop();
ASSERT_EQ(ItemActionStatus::SUCCESS, this->last_add_status());
ASSERT_EQ(LastResult::RESULT_TRUE, this->last_result());
ASSERT_EQ(1ul, this->last_requests().size());
EXPECT_EQ(request.ToString(), this->last_requests()[0]->ToString());
}
TEST_F(RequestQueueStoreTest, UpdateRequest) {
std::unique_ptr<RequestQueueStore> store(this->BuildStore());
this->InitializeStore(store.get());
......@@ -532,6 +568,8 @@ TEST_F(RequestQueueStoreTest, UpdateRequest) {
EXPECT_EQ(ItemActionStatus::NOT_FOUND,
this->last_update_result()->item_statuses[1].second);
EXPECT_EQ(1UL, this->last_update_result()->updated_items.size());
EXPECT_EQ(updated_request.ToString(),
this->last_update_result()->updated_items.begin()->ToString());
EXPECT_EQ(updated_request,
*(this->last_update_result()->updated_items.begin()));
......
......@@ -4,6 +4,8 @@
#include "components/offline_pages/core/background/save_page_request.h"
#include <string>
namespace offline_pages {
SavePageRequest::SavePageRequest(int64_t request_id,
......@@ -63,6 +65,13 @@ void SavePageRequest::MarkAttemptPaused() {
state_ = RequestState::PAUSED;
}
void SavePageRequest::MarkAttemptDeferred(const base::Time& attempt_time) {
++started_attempt_count_;
++completed_attempt_count_;
last_attempt_time_ = attempt_time;
state_ = RequestState::AVAILABLE;
}
void SavePageRequest::UpdateFailState(FailState fail_state) {
// The order of precedence for failure errors related to offline page
// downloads is as follows: NO_FAILURE, Failures that are not recoverable and
......
......@@ -53,6 +53,10 @@ class SavePageRequest {
// loading until it has been explicitly unpaused.
void MarkAttemptPaused();
// Mark the attempt as deferred. This counts as a failed attempt so that
// deferred attempts are not unlimited.
void MarkAttemptDeferred(const base::Time& attempt_time);
int64_t request_id() const { return request_id_; }
const GURL& url() const { return url_; }
......@@ -100,6 +104,9 @@ class SavePageRequest {
request_origin_ = request_origin;
}
// Implemented in test_util.cc.
std::string ToString() const;
private:
// ID of this request.
int64_t request_id_;
......
......@@ -39,6 +39,8 @@ class SchedulerStub : public Scheduler {
return &trigger_conditions_;
}
int64_t schedule_delay() const { return schedule_delay_; }
private:
bool schedule_called_;
bool backup_schedule_called_;
......
// 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 "base/json/json_writer.h"
#include "base/values.h"
#include "components/offline_pages/core/background/save_page_request.h"
namespace offline_pages {
std::string SavePageRequest::ToString() const {
base::DictionaryValue result;
result.SetInteger("request_id", request_id_);
result.SetString("url", url_.spec());
result.SetString("client_id", client_id_.ToString());
result.SetInteger("creation_time",
creation_time_.ToDeltaSinceWindowsEpoch().InSeconds());
result.SetInteger("started_attempt_count", started_attempt_count_);
result.SetInteger("completed_attempt_count", completed_attempt_count_);
result.SetInteger("last_attempt_time",
last_attempt_time_.ToDeltaSinceWindowsEpoch().InSeconds());
result.SetBoolean("user_requested", user_requested_);
result.SetInteger("state", static_cast<int>(state_));
result.SetInteger("fail_state", static_cast<int>(fail_state_));
result.SetInteger("pending_state", static_cast<int>(pending_state_));
result.SetString("original_url", original_url_.spec());
result.SetString("request_origin", request_origin_);
std::string result_string;
base::JSONWriter::Write(result, &result_string);
return result_string;
}
} // namespace offline_pages
......@@ -95,6 +95,7 @@ ClientPolicyController::ClientPolicyController() {
.SetIsRemovedOnCacheReset(true)
.SetExpirePeriod(base::TimeDelta::FromDays(30))
.SetIsUserRequestedDownload(false)
.SetDeferBackgroundFetchWhilePageIsActive(true)
.Build());
// Fallback policy.
......
......@@ -94,6 +94,10 @@ struct OfflinePageClientPolicy {
FeaturePolicy feature_policy;
// Whether background fetches are deferred while the active tab matches the
// SavePageRequestURL.
bool defer_background_fetch_while_page_is_active = false;
OfflinePageClientPolicy(std::string namespace_val,
LifetimePolicy lifetime_policy_val,
size_t pages_allowed_per_url_val,
......@@ -184,6 +188,12 @@ class OfflinePageClientPolicyBuilder {
return *this;
}
OfflinePageClientPolicyBuilder& SetDeferBackgroundFetchWhilePageIsActive(
bool defer) {
policy_.defer_background_fetch_while_page_is_active = defer;
return *this;
}
private:
OfflinePageClientPolicy policy_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment