components/offline_pages/core/background/request_coordinator_unittest.cc - Issue 2775223006: [Offline Pages] Improve RequestCoordinator state tracking.

Unified Diff: components/offline_pages/core/background/request_coordinator_unittest.cc

Issue 2775223006: [Offline Pages] Improve RequestCoordinator state tracking. (Closed)

Patch Set: comments. Created 3 years, 9 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

« components/offline_pages/core/background/request_coordinator.cc ('K') | « components/offline_pages/core/background/request_coordinator.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: components/offline_pages/core/background/request_coordinator_unittest.cc

diff --git a/components/offline_pages/core/background/request_coordinator_unittest.cc b/components/offline_pages/core/background/request_coordinator_unittest.cc

index 5e1b1dca3cf3046a76d3378467824ff533015eee..3f02bafb99a3da42af854393d0c81e8746107004 100644

--- a/components/offline_pages/core/background/request_coordinator_unittest.cc

+++ b/components/offline_pages/core/background/request_coordinator_unittest.cc

@@ -118,6 +118,8 @@ class ObserverStub : public RequestCoordinator::Observer {

class RequestCoordinatorTest : public testing::Test {

public:

+ using RequestCoordinatorState = RequestCoordinator::RequestCoordinatorState;

RequestCoordinatorTest();

~RequestCoordinatorTest() override;

@@ -129,9 +131,7 @@ class RequestCoordinatorTest : public testing::Test {

return coordinator_taco_->request_coordinator();

}

- bool is_busy() { return coordinator()->is_busy(); }

- bool is_starting() { return coordinator()->is_starting(); }

+ RequestCoordinatorState state() { return coordinator()->state(); }

Pete Williamson 2017/03/28 17:12:43 Looks good, thanks for doing this!

romax 2017/03/29 03:02:09 :)

// Test processing callback function.

void ProcessingCallbackFunction(bool result) {

@@ -492,7 +492,7 @@ TEST_F(RequestCoordinatorTest, StartScheduledProcessingWithRequestInProgress) {

processing_callback()));

PumpLoop();

- EXPECT_TRUE(is_busy());

+ EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);

// Since the offliner is disabled, this callback should not be called.

EXPECT_FALSE(processing_callback_called());

@@ -541,7 +541,7 @@ TEST_F(RequestCoordinatorTest, StartImmediateProcessingWithRequestInProgress) {

EXPECT_TRUE(coordinator()->StartImmediateProcessing(processing_callback()));

PumpLoop();

- EXPECT_TRUE(is_busy());

+ EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);

// Since the offliner is disabled, this callback should not be called.

EXPECT_FALSE(processing_callback_called());

@@ -696,13 +696,13 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestSucceededButLostNetwork) {

EXPECT_TRUE(processing_callback_called());

// Verify not busy with 2nd request (since no connection).

- EXPECT_FALSE(is_busy());

+ EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);

// Now connect network and verify processing starts.

SetNetworkConnected(true);

CallConnectionTypeObserver();

PumpLoop();

- EXPECT_TRUE(is_busy());

+ EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);

}

TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailed) {

@@ -728,7 +728,7 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailed) {

EXPECT_FALSE(processing_callback_called());

// Busy processing 2nd request.

- EXPECT_TRUE(is_busy());

+ EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);

coordinator()->queue()->GetRequests(base::Bind(

&RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));

@@ -770,7 +770,7 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailedNoRetryFailure) {

EXPECT_FALSE(processing_callback_called());

// Busy processing 2nd request.

- EXPECT_TRUE(is_busy());

+ EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);

coordinator()->queue()->GetRequests(base::Bind(

&RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));

@@ -807,7 +807,7 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailedNoNextFailure) {

EXPECT_TRUE(processing_callback_called());

// Not busy for NO_NEXT failure.

- EXPECT_FALSE(is_busy());

+ EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);

coordinator()->queue()->GetRequests(base::Bind(

&RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));

@@ -871,13 +871,13 @@ TEST_F(RequestCoordinatorTest, OfflinerDonePrerenderingCancel) {

TEST_F(RequestCoordinatorTest, RequestNotPickedDisabledItemsRemain) {

coordinator()->StartScheduledProcessing(device_conditions(),

processing_callback());

- EXPECT_TRUE(is_starting());

+ EXPECT_TRUE(state() == RequestCoordinatorState::PICKING);

// Call RequestNotPicked, simulating a request on the disabled list.

CallRequestNotPicked(false, true);

PumpLoop();

- EXPECT_FALSE(is_starting());

+ EXPECT_FALSE(state() == RequestCoordinatorState::PICKING);

// The scheduler should have been called to schedule the disabled task for

// 5 minutes from now.

@@ -892,14 +892,14 @@ TEST_F(RequestCoordinatorTest, RequestNotPickedDisabledItemsRemain) {

TEST_F(RequestCoordinatorTest, RequestNotPickedNonUserRequestedItemsRemain) {

coordinator()->StartScheduledProcessing(device_conditions(),

processing_callback());

- EXPECT_TRUE(is_starting());

+ EXPECT_TRUE(state() == RequestCoordinatorState::PICKING);

// Call RequestNotPicked, and make sure we pick schedule a task for non user

// requested conditions, with no tasks on the disabled list.

CallRequestNotPicked(true, false);

PumpLoop();

- EXPECT_FALSE(is_starting());

+ EXPECT_FALSE(state() == RequestCoordinatorState::PICKING);

EXPECT_TRUE(processing_callback_called());

// The scheduler should have been called to schedule the non-user requested

@@ -962,7 +962,7 @@ TEST_F(RequestCoordinatorTest, StartScheduledProcessingWithLoadingDisabled) {

PumpLoop();

EXPECT_TRUE(processing_callback_called());

- EXPECT_FALSE(is_starting());

+ EXPECT_FALSE(state() == RequestCoordinatorState::PICKING);

EXPECT_EQ(Offliner::LOADING_NOT_ACCEPTED, last_offlining_status());

}

@@ -978,7 +978,7 @@ TEST_F(RequestCoordinatorTest,

EXPECT_TRUE(coordinator()->StartScheduledProcessing(device_conditions(),

processing_callback()));

- EXPECT_TRUE(is_starting());

+ EXPECT_TRUE(state() == RequestCoordinatorState::PICKING);

// Now, quick, before it can do much (we haven't called PumpLoop), cancel it.

coordinator()->StopProcessing(Offliner::REQUEST_COORDINATOR_CANCELED);

@@ -987,7 +987,7 @@ TEST_F(RequestCoordinatorTest,

PumpLoop();

EXPECT_TRUE(processing_callback_called());

- EXPECT_FALSE(is_starting());

+ EXPECT_FALSE(state() == RequestCoordinatorState::PICKING);

// OfflinerDoneCallback will not end up getting called with status SAVED,

// since we cancelled the event before it called offliner_->LoadAndSave().

@@ -1010,7 +1010,7 @@ TEST_F(RequestCoordinatorTest,

EXPECT_TRUE(coordinator()->StartScheduledProcessing(device_conditions(),

processing_callback()));

- EXPECT_TRUE(is_starting());

+ EXPECT_TRUE(state() == RequestCoordinatorState::PICKING);

// Let all the async parts of the start processing pipeline run to completion.

PumpLoop();

@@ -1024,8 +1024,8 @@ TEST_F(RequestCoordinatorTest,

EXPECT_FALSE(processing_callback_called());

// Coordinator should now be busy.

- EXPECT_TRUE(is_busy());

- EXPECT_FALSE(is_starting());

+ EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);

+ EXPECT_FALSE(state() == RequestCoordinatorState::PICKING);

// Now we cancel it while the prerenderer is busy.

coordinator()->StopProcessing(Offliner::REQUEST_COORDINATOR_CANCELED);

@@ -1038,7 +1038,7 @@ TEST_F(RequestCoordinatorTest,

EXPECT_EQ(SavePageRequest::RequestState::AVAILABLE, observer().state());

observer().Clear();

- EXPECT_FALSE(is_busy());

+ EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);

// OfflinerDoneCallback will not end up getting called with status SAVED,

// since we cancelled the event before the LoadAndSave completed.

@@ -1170,8 +1170,8 @@ TEST_F(RequestCoordinatorTest,

WaitForCallback();

PumpLoop();

- EXPECT_FALSE(is_starting());

- EXPECT_FALSE(coordinator()->is_busy());

+ EXPECT_FALSE(state() == RequestCoordinatorState::PICKING);

+ EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);

EXPECT_TRUE(OfflinerWasCanceled());

}

@@ -1185,7 +1185,7 @@ TEST_F(RequestCoordinatorTest,

PumpLoop();

// Verify that immediate start from adding the request did happen.

- EXPECT_TRUE(coordinator()->is_busy());

+ EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);

// Advance the mock clock 1 second before the watchdog timeout.

AdvanceClockBy(base::TimeDelta::FromSeconds(

@@ -1196,7 +1196,7 @@ TEST_F(RequestCoordinatorTest,

PumpLoop();

// Verify still busy.

- EXPECT_TRUE(coordinator()->is_busy());

+ EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);

EXPECT_FALSE(OfflinerWasCanceled());

// Advance the mock clock past the watchdog timeout now.

@@ -1260,7 +1260,7 @@ TEST_F(RequestCoordinatorTest, TryNextRequestWithNoNetwork) {

EXPECT_TRUE(coordinator()->StartScheduledProcessing(device_conditions(),

waiting_callback()));

PumpLoop();

- EXPECT_TRUE(coordinator()->is_busy());

+ EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);

// Now lose the network connection.

SetNetworkConnected(false);

@@ -1271,8 +1271,8 @@ TEST_F(RequestCoordinatorTest, TryNextRequestWithNoNetwork) {

PumpLoop();

// Not starting nor busy with next request.

- EXPECT_FALSE(coordinator()->is_starting());

- EXPECT_FALSE(coordinator()->is_busy());

+ EXPECT_FALSE(state() == RequestCoordinatorState::PICKING);

+ EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);

// Get queued requests.

coordinator()->queue()->GetRequests(base::Bind(

@@ -1376,7 +1376,7 @@ TEST_F(RequestCoordinatorTest,

EXPECT_NE(0, SavePageLater());

PumpLoop();

- EXPECT_TRUE(is_busy());

+ EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);

}

TEST_F(RequestCoordinatorTest,

@@ -1390,7 +1390,7 @@ TEST_F(RequestCoordinatorTest,

PumpLoop();

// Verify not immediately busy (since low-end device).

- EXPECT_FALSE(is_busy());

+ EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);

// Set feature flag to allow concurrent loads.

base::test::ScopedFeatureList scoped_feature_list;

@@ -1411,7 +1411,7 @@ TEST_F(RequestCoordinatorTest,

PumpLoop();

// Verify immediate processing did start this time.

- EXPECT_TRUE(is_busy());

+ EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);

}

TEST_F(RequestCoordinatorTest, SavePageDoesntStartProcessingWhenDisconnected) {

@@ -1419,13 +1419,13 @@ TEST_F(RequestCoordinatorTest, SavePageDoesntStartProcessingWhenDisconnected) {

EnableOfflinerCallback(false);

EXPECT_NE(0, SavePageLater());

PumpLoop();

- EXPECT_FALSE(is_busy());

+ EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);

// Now connect network and verify processing starts.

SetNetworkConnected(true);

CallConnectionTypeObserver();

PumpLoop();

- EXPECT_TRUE(is_busy());

+ EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);

}

TEST_F(RequestCoordinatorTest,

@@ -1443,7 +1443,7 @@ TEST_F(RequestCoordinatorTest,

EXPECT_NE(0, SavePageLater());

PumpLoop();

- EXPECT_TRUE(is_busy());

+ EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);

}

TEST_F(RequestCoordinatorTest,

@@ -1458,7 +1458,7 @@ TEST_F(RequestCoordinatorTest,

// Add a request to the queue.

AddRequest1();

PumpLoop();

- EXPECT_FALSE(is_busy());

+ EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);

// Pause the request.

std::vector<int64_t> request_ids;

@@ -1469,7 +1469,7 @@ TEST_F(RequestCoordinatorTest,

// Resume the request while disconnected.

coordinator()->ResumeRequests(request_ids);

PumpLoop();

- EXPECT_FALSE(is_busy());

+ EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);

EXPECT_EQ(1UL, prioritized_requests().size());

// Pause the request again.

@@ -1482,11 +1482,11 @@ TEST_F(RequestCoordinatorTest,

// Resume the request while connected.

coordinator()->ResumeRequests(request_ids);

- EXPECT_FALSE(is_busy());

+ EXPECT_FALSE(state() == RequestCoordinatorState::OFFLINING);

PumpLoop();

EXPECT_EQ(1UL, prioritized_requests().size());

- EXPECT_TRUE(is_busy());

+ EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);

}

TEST_F(RequestCoordinatorTest, SnapshotOnLastTryForScheduledProcessing) {

@@ -1552,7 +1552,7 @@ TEST_F(RequestCoordinatorTest, SnapshotOnLastTryForImmediateProcessing) {

observer().Clear();

// Verify that the request is being processed.

- EXPECT_TRUE(coordinator()->is_busy());

+ EXPECT_TRUE(state() == RequestCoordinatorState::OFFLINING);

// Advance the mock clock 1 second more than the watchdog timeout.

AdvanceClockBy(base::TimeDelta::FromSeconds(

« components/offline_pages/core/background/request_coordinator.cc ('K') | « components/offline_pages/core/background/request_coordinator.cc ('k') | no next file » | no next file with comments »