Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(377)

Side by Side Diff: components/offline_pages/background/request_coordinator_unittest.cc

Issue 2104393002: Adds UMA for PrerenderingOffliner request processing result status. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Clarifies and fixes behavior wrt Offliner::Cancel() not calling the callback Created 4 years, 5 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 unified diff | Download patch
OLDNEW
1 // Copyright 2016 The Chromium Authors. All rights reserved. 1 // Copyright 2016 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/offline_pages/background/request_coordinator.h" 5 #include "components/offline_pages/background/request_coordinator.h"
6 6
7 #include <memory> 7 #include <memory>
8 #include <utility> 8 #include <utility>
9 #include <vector> 9 #include <vector>
10 10
(...skipping 50 matching lines...) Expand 10 before | Expand all | Expand 10 after
61 TriggerConditions const* conditions() const { return &conditions_; } 61 TriggerConditions const* conditions() const { return &conditions_; }
62 62
63 private: 63 private:
64 bool schedule_called_; 64 bool schedule_called_;
65 bool unschedule_called_; 65 bool unschedule_called_;
66 TriggerConditions conditions_; 66 TriggerConditions conditions_;
67 }; 67 };
68 68
69 class OfflinerStub : public Offliner { 69 class OfflinerStub : public Offliner {
70 public: 70 public:
71 OfflinerStub() : request_(kRequestId, kUrl, kClientId, base::Time::Now()), 71 OfflinerStub()
72 enable_callback_(false) {} 72 : request_(kRequestId, kUrl, kClientId, base::Time::Now()),
73 enable_callback_(false),
74 cancel_called_(false) {}
73 75
74 bool LoadAndSave(const SavePageRequest& request, 76 bool LoadAndSave(const SavePageRequest& request,
75 const CompletionCallback& callback) override { 77 const CompletionCallback& callback) override {
76 callback_ = callback; 78 callback_ = callback;
77 request_ = request; 79 request_ = request;
78 // Post the callback on the run loop. 80 // Post the callback on the run loop.
79 if (enable_callback_) { 81 if (enable_callback_) {
80 base::ThreadTaskRunnerHandle::Get()->PostTask( 82 base::ThreadTaskRunnerHandle::Get()->PostTask(
81 FROM_HERE, 83 FROM_HERE,
82 base::Bind(callback, request, Offliner::RequestStatus::SAVED)); 84 base::Bind(callback, request, Offliner::RequestStatus::SAVED));
83 } 85 }
84 return true; 86 return true;
85 } 87 }
86 88
87 // Clears the currently processing request, if any. Must have called 89 void Cancel() override { cancel_called_ = true; }
88 // LoadAndSave first to set the callback and request.
89 // Clears the currently processing request, if any.
90 void Cancel() override {
91 base::ThreadTaskRunnerHandle::Get()->PostTask(
92 FROM_HERE,
93 base::Bind(callback_, request_, Offliner::RequestStatus::CANCELED));
94 }
95 90
96 void enable_callback(bool enable) { 91 void enable_callback(bool enable) {
97 enable_callback_ = enable; 92 enable_callback_ = enable;
98 } 93 }
99 94
95 bool cancel_called() { return cancel_called_; }
96
100 private: 97 private:
101 CompletionCallback callback_; 98 CompletionCallback callback_;
102 SavePageRequest request_; 99 SavePageRequest request_;
103 bool enable_callback_; 100 bool enable_callback_;
101 bool cancel_called_;
104 }; 102 };
105 103
106 class OfflinerFactoryStub : public OfflinerFactory { 104 class OfflinerFactoryStub : public OfflinerFactory {
107 public: 105 public:
108
109 OfflinerFactoryStub() : offliner_(nullptr) {} 106 OfflinerFactoryStub() : offliner_(nullptr) {}
110 107
111 Offliner* GetOffliner(const OfflinerPolicy* policy) override { 108 Offliner* GetOffliner(const OfflinerPolicy* policy) override {
112 if (offliner_.get() == nullptr) { 109 if (offliner_.get() == nullptr) {
113 offliner_.reset(new OfflinerStub()); 110 offliner_.reset(new OfflinerStub());
114 } 111 }
115 return offliner_.get(); 112 return offliner_.get();
116 } 113 }
117 114
118 private: 115 private:
(...skipping 59 matching lines...) Expand 10 before | Expand all | Expand 10 after
178 } 175 }
179 176
180 void AdvanceClockBy(base::TimeDelta delta) { 177 void AdvanceClockBy(base::TimeDelta delta) {
181 task_runner_->FastForwardBy(delta); 178 task_runner_->FastForwardBy(delta);
182 } 179 }
183 180
184 Offliner::RequestStatus last_offlining_status() const { 181 Offliner::RequestStatus last_offlining_status() const {
185 return coordinator_->last_offlining_status_; 182 return coordinator_->last_offlining_status_;
186 } 183 }
187 184
185 bool OfflinerWasCanceled() const { return offliner_->cancel_called(); }
186
188 private: 187 private:
189 RequestQueue::GetRequestsResult last_get_requests_result_; 188 RequestQueue::GetRequestsResult last_get_requests_result_;
190 std::vector<SavePageRequest> last_requests_; 189 std::vector<SavePageRequest> last_requests_;
191 scoped_refptr<base::TestMockTimeTaskRunner> task_runner_; 190 scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
192 base::ThreadTaskRunnerHandle task_runner_handle_; 191 base::ThreadTaskRunnerHandle task_runner_handle_;
193 std::unique_ptr<RequestCoordinator> coordinator_; 192 std::unique_ptr<RequestCoordinator> coordinator_;
194 OfflinerStub* offliner_; 193 OfflinerStub* offliner_;
195 base::WaitableEvent waiter_; 194 base::WaitableEvent waiter_;
196 }; 195 };
197 196
(...skipping 151 matching lines...) Expand 10 before | Expand all | Expand 10 after
349 // We need to give a callback to the request. 348 // We need to give a callback to the request.
350 base::Callback<void(bool)> callback = 349 base::Callback<void(bool)> callback =
351 base::Bind( 350 base::Bind(
352 &RequestCoordinatorTest::EmptyCallbackFunction, 351 &RequestCoordinatorTest::EmptyCallbackFunction,
353 base::Unretained(this)); 352 base::Unretained(this));
354 coordinator()->SetProcessingCallbackForTest(callback); 353 coordinator()->SetProcessingCallbackForTest(callback);
355 354
356 // Call the OfflinerDoneCallback to simulate the request failed, wait 355 // Call the OfflinerDoneCallback to simulate the request failed, wait
357 // for callbacks. 356 // for callbacks.
358 EnableOfflinerCallback(true); 357 EnableOfflinerCallback(true);
359 SendOfflinerDoneCallback(request, Offliner::RequestStatus::FAILED); 358 SendOfflinerDoneCallback(request,
359 Offliner::RequestStatus::PRERENDERING_FAILED);
360 PumpLoop(); 360 PumpLoop();
361 361
362 // Verify the request is not removed from the queue, and wait for callbacks. 362 // Verify the request is not removed from the queue, and wait for callbacks.
363 coordinator()->queue()->GetRequests( 363 coordinator()->queue()->GetRequests(
364 base::Bind(&RequestCoordinatorTest::GetRequestsDone, 364 base::Bind(&RequestCoordinatorTest::GetRequestsDone,
365 base::Unretained(this))); 365 base::Unretained(this)));
366 PumpLoop(); 366 PumpLoop();
367 367
368 // Still one request in the queue. 368 // Still one request in the queue.
369 EXPECT_EQ(1UL, last_requests().size()); 369 EXPECT_EQ(1UL, last_requests().size());
(...skipping 20 matching lines...) Expand all
390 base::Unretained(this)); 390 base::Unretained(this));
391 EXPECT_TRUE(coordinator()->StartProcessing(device_conditions, callback)); 391 EXPECT_TRUE(coordinator()->StartProcessing(device_conditions, callback));
392 392
393 // Now, quick, before it can do much (we haven't called PumpLoop), cancel it. 393 // Now, quick, before it can do much (we haven't called PumpLoop), cancel it.
394 coordinator()->StopProcessing(); 394 coordinator()->StopProcessing();
395 395
396 // Let the async callbacks in the request coordinator run. 396 // Let the async callbacks in the request coordinator run.
397 PumpLoop(); 397 PumpLoop();
398 398
399 // OfflinerDoneCallback will not end up getting called with status SAVED, 399 // OfflinerDoneCallback will not end up getting called with status SAVED,
400 // Since we cancelled the event before it called offliner_->LoadAndSave(). 400 // since we cancelled the event before it called offliner_->LoadAndSave().
401 EXPECT_NE(Offliner::RequestStatus::SAVED, last_offlining_status()); 401 EXPECT_EQ(Offliner::RequestStatus::REQUEST_COORDINATOR_CANCELED,
402 last_offlining_status());
403
404 // Since offliner was not started, it will not have seen cancel call.
405 EXPECT_FALSE(OfflinerWasCanceled());
402 } 406 }
403 407
404 // This tests a StopProcessing call after the prerenderer has been started. 408 // This tests a StopProcessing call after the prerenderer has been started.
405 TEST_F(RequestCoordinatorTest, StartProcessingThenStopProcessingLater) { 409 TEST_F(RequestCoordinatorTest, StartProcessingThenStopProcessingLater) {
406 // Add a request to the queue, wait for callbacks to finish. 410 // Add a request to the queue, wait for callbacks to finish.
407 offline_pages::SavePageRequest request( 411 offline_pages::SavePageRequest request(
408 kRequestId, kUrl, kClientId, base::Time::Now()); 412 kRequestId, kUrl, kClientId, base::Time::Now());
409 coordinator()->queue()->AddRequest( 413 coordinator()->queue()->AddRequest(
410 request, 414 request,
411 base::Bind(&RequestCoordinatorTest::AddRequestDone, 415 base::Bind(&RequestCoordinatorTest::AddRequestDone,
(...skipping 14 matching lines...) Expand all
426 // Let all the async parts of the start processing pipeline run to completion. 430 // Let all the async parts of the start processing pipeline run to completion.
427 PumpLoop(); 431 PumpLoop();
428 432
429 // Now we cancel it while the prerenderer is busy. 433 // Now we cancel it while the prerenderer is busy.
430 coordinator()->StopProcessing(); 434 coordinator()->StopProcessing();
431 435
432 // Let the async callbacks in the cancel run. 436 // Let the async callbacks in the cancel run.
433 PumpLoop(); 437 PumpLoop();
434 438
435 // OfflinerDoneCallback will not end up getting called with status SAVED, 439 // OfflinerDoneCallback will not end up getting called with status SAVED,
436 // Since we cancelled the event before it called offliner_->LoadAndSave(). 440 // since we cancelled the event before the LoadAndSave completed.
437 EXPECT_EQ(Offliner::RequestStatus::CANCELED, last_offlining_status()); 441 EXPECT_EQ(Offliner::RequestStatus::REQUEST_COORDINATOR_CANCELED,
442 last_offlining_status());
443
444 // Since offliner was started, it will have seen cancel call.
445 EXPECT_TRUE(OfflinerWasCanceled());
438 } 446 }
439 447
440 TEST_F(RequestCoordinatorTest, PrerendererTimeout) { 448 TEST_F(RequestCoordinatorTest, PrerendererTimeout) {
441 // Build a request to use with the pre-renderer, and put it on the queue. 449 // Build a request to use with the pre-renderer, and put it on the queue.
442 offline_pages::SavePageRequest request( 450 offline_pages::SavePageRequest request(
443 kRequestId, kUrl, kClientId, base::Time::Now()); 451 kRequestId, kUrl, kClientId, base::Time::Now());
444 coordinator()->queue()->AddRequest( 452 coordinator()->queue()->AddRequest(
445 request, 453 request,
446 base::Bind(&RequestCoordinatorTest::AddRequestDone, 454 base::Bind(&RequestCoordinatorTest::AddRequestDone,
447 base::Unretained(this))); 455 base::Unretained(this)));
(...skipping 22 matching lines...) Expand all
470 // Advance the mock clock far enough to cause a watchdog timeout 478 // Advance the mock clock far enough to cause a watchdog timeout
471 AdvanceClockBy(base::TimeDelta::FromSeconds(kTestTimeoutSeconds + 1)); 479 AdvanceClockBy(base::TimeDelta::FromSeconds(kTestTimeoutSeconds + 1));
472 PumpLoop(); 480 PumpLoop();
473 481
474 // Wait for timeout to expire. Use a TaskRunner with a DelayedTaskRunner 482 // Wait for timeout to expire. Use a TaskRunner with a DelayedTaskRunner
475 // which won't time out immediately, so the watchdog thread doesn't kill valid 483 // which won't time out immediately, so the watchdog thread doesn't kill valid
476 // tasks too soon. 484 // tasks too soon.
477 WaitForCallback(); 485 WaitForCallback();
478 PumpLoop(); 486 PumpLoop();
479 487
480 // Now trying to start processing on another request should return false. 488 EXPECT_TRUE(OfflinerWasCanceled());
481 EXPECT_EQ(Offliner::RequestStatus::CANCELED, last_offlining_status()); 489 EXPECT_EQ(Offliner::RequestStatus::REQUEST_COORDINATOR_CANCELED,
490 last_offlining_status());
482 } 491 }
483 492
484 } // namespace offline_pages 493 } // namespace offline_pages
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698