Privet related test clean-up

Removing wait calls from the privet_http_unittest.cc by posting tasks
without delay in tests.
Also various other clean-ups.

Bug: 840390
Change-Id: Iac7d2cd92fcfba269e5edbf81d6df217b14016b6
Reviewed-on: https://chromium-review.googlesource.com/1146485
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577657}

chrome/browser/printing/cloud_print/privet_http_impl.cc

@@ -124,6 +124,21 @@ void PrivetInfoOperationImpl::OnParsedJson(int response_code,
   callback_.Run(&value);
 }
 
+// static
+bool PrivetRegisterOperationImpl::run_tasks_immediately_for_testing_ = false;
+
+PrivetRegisterOperationImpl::RunTasksImmediatelyForTesting::
+    RunTasksImmediatelyForTesting() {
+  DCHECK(!run_tasks_immediately_for_testing_);
+  run_tasks_immediately_for_testing_ = true;
+}
+
+PrivetRegisterOperationImpl::RunTasksImmediatelyForTesting::
+    ~RunTasksImmediatelyForTesting() {
+  DCHECK(run_tasks_immediately_for_testing_);
+  run_tasks_immediately_for_testing_ = false;
+}
+
 PrivetRegisterOperationImpl::PrivetRegisterOperationImpl(
     PrivetHTTPClient* privet_client,
     const std::string& user,
@@ -146,11 +161,14 @@ void PrivetRegisterOperationImpl::Cancel() {
   if (!ongoing_)
     return;
 
+  int delay_seconds =
+      run_tasks_immediately_for_testing_ ? 0 : kPrivetCancelationTimeoutSeconds;
+
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE,
       base::BindOnce(&PrivetRegisterOperationImpl::Cancelation::Cleanup,
                      base::Owned(new Cancelation(privet_client_, user_))),
-      base::TimeDelta::FromSeconds(kPrivetCancelationTimeoutSeconds));
+      base::TimeDelta::FromSeconds(delay_seconds));
   ongoing_ = false;
 }
 
@@ -227,12 +245,12 @@ void PrivetRegisterOperationImpl::StartResponse(
 
 void PrivetRegisterOperationImpl::GetClaimTokenResponse(
     const base::DictionaryValue& value) {
-  std::string claimUrl;
-  std::string claimToken;
-  bool got_url = value.GetString(kPrivetKeyClaimURL, &claimUrl);
-  bool got_token = value.GetString(kPrivetKeyClaimToken, &claimToken);
+  std::string claim_url;
+  std::string claim_token;
+  bool got_url = value.GetString(kPrivetKeyClaimURL, &claim_url);
+  bool got_token = value.GetString(kPrivetKeyClaimToken, &claim_token);
   if (got_url || got_token) {
-    delegate_->OnPrivetRegisterClaimToken(this, claimToken, GURL(claimUrl));
+    delegate_->OnPrivetRegisterClaimToken(this, claim_token, GURL(claim_url));
   } else {
     delegate_->OnPrivetRegisterError(this, current_action_,
                                      FAILURE_MALFORMED_RESPONSE, -1, nullptr);
@@ -359,6 +377,21 @@ void PrivetJSONOperationImpl::OnNeedPrivetToken(
 }
 
 #if BUILDFLAG(ENABLE_PRINT_PREVIEW)
+// static
+bool PrivetLocalPrintOperationImpl::run_tasks_immediately_for_testing_ = false;
+
+PrivetLocalPrintOperationImpl::RunTasksImmediatelyForTesting::
+    RunTasksImmediatelyForTesting() {
+  DCHECK(!run_tasks_immediately_for_testing_);
+  run_tasks_immediately_for_testing_ = true;
+}
+
+PrivetLocalPrintOperationImpl::RunTasksImmediatelyForTesting::
+    ~RunTasksImmediatelyForTesting() {
+  DCHECK(run_tasks_immediately_for_testing_);
+  run_tasks_immediately_for_testing_ = false;
+}
+
 PrivetLocalPrintOperationImpl::PrivetLocalPrintOperationImpl(
     PrivetHTTPClient* privet_client,
     PrivetLocalPrintOperation::Delegate* delegate)
@@ -531,7 +564,10 @@ void PrivetLocalPrintOperationImpl::OnSubmitdocResponse(
       double random_scaling_factor =
           1 + base::RandDouble() * kPrivetMaximumTimeRandomAddition;
 
-      timeout = std::max(static_cast<int>(timeout * random_scaling_factor),
+      timeout =
+          run_tasks_immediately_for_testing_
+              ? 0
+              : std::max(static_cast<int>(timeout * random_scaling_factor),
                          kPrivetMinimumTimeout);
       base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
           FROM_HERE,

chrome/browser/printing/cloud_print/privet_http_impl.h

@@ -73,6 +73,13 @@ class PrivetRegisterOperationImpl
                     bool has_error) override;
   void OnNeedPrivetToken(PrivetURLFetcher::TokenCallback callback) override;
 
+  // Used in test to skip delays when posting tasks for cancellation.
+  class RunTasksImmediatelyForTesting final {
+   public:
+    RunTasksImmediatelyForTesting();
+    ~RunTasksImmediatelyForTesting();
+  };
+
  private:
   class Cancelation : public PrivetURLFetcher::Delegate {
    public:
@@ -116,6 +123,8 @@ class PrivetRegisterOperationImpl
 
   std::unique_ptr<PrivetJSONOperation> info_operation_;
   std::string expected_id_;
+
+  static bool run_tasks_immediately_for_testing_;
 };
 
 class PrivetJSONOperationImpl : public PrivetJSONOperation,
@@ -176,6 +185,13 @@ class PrivetLocalPrintOperationImpl
                     bool has_error) override;
   void OnNeedPrivetToken(PrivetURLFetcher::TokenCallback callback) override;
 
+  // Used in test to skip delays when posting tasks for cancellation.
+  class RunTasksImmediatelyForTesting final {
+   public:
+    RunTasksImmediatelyForTesting();
+    ~RunTasksImmediatelyForTesting();
+  };
+
  private:
   using ResponseCallback =
       base::OnceCallback<void(/*has_error=*/bool,
@@ -222,6 +238,8 @@ class PrivetLocalPrintOperationImpl
   std::unique_ptr<printing::PwgRasterConverter> pwg_raster_converter_;
 
   base::WeakPtrFactory<PrivetLocalPrintOperationImpl> weak_factory_;
+
+  static bool run_tasks_immediately_for_testing_;
 };
 #endif  // ENABLE_PRINT_PREVIEW
 

chrome/browser/printing/cloud_print/privet_url_fetcher.cc

@@ -51,6 +51,19 @@ std::string MakeRangeHeader(int start, int end) {
 
 }  // namespace
 
+// static
+bool PrivetURLFetcher::skip_retry_timeouts_for_tests_ = false;
+
+PrivetURLFetcher::RetryImmediatelyForTest::RetryImmediatelyForTest() {
+  DCHECK(!skip_retry_timeouts_for_tests_);
+  skip_retry_timeouts_for_tests_ = true;
+}
+
+PrivetURLFetcher::RetryImmediatelyForTest::~RetryImmediatelyForTest() {
+  DCHECK(skip_retry_timeouts_for_tests_);
+  skip_retry_timeouts_for_tests_ = false;
+}
+
 void PrivetURLFetcher::Delegate::OnNeedPrivetToken(TokenCallback callback) {
   OnError(0, TOKEN_ERROR);
 }
@@ -315,6 +328,9 @@ void PrivetURLFetcher::ScheduleRetry(int timeout_seconds) {
   if (tries_ >= max_retries_)
     timeout_seconds_randomized = 0;
 
+  if (skip_retry_timeouts_for_tests_)
+    timeout_seconds_randomized = 0;
+
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE,
       base::BindOnce(&PrivetURLFetcher::Try, weak_factory_.GetWeakPtr()),

chrome/browser/printing/cloud_print/privet_url_fetcher.h

@@ -105,6 +105,17 @@ class PrivetURLFetcher : public net::URLFetcherDelegate {
     return url_fetcher_ ? url_fetcher_->GetResponseCode() : -1;
   }
 
+  // A class that can be used in tests that want to bypass the delay when
+  // retrying loading a URL. Create one of this object in your test, it will
+  // disable the delay for retry on construction and revert it back on
+  // destruction.
+  // Note that you should not have more than one of these allocated at a time.
+  class RetryImmediatelyForTest final {
+   public:
+    RetryImmediatelyForTest();
+    ~RetryImmediatelyForTest();
+  };
+
  private:
   void OnURLFetchCompleteParseData(const net::URLFetcher* source);
   bool OnURLFetchCompleteDoNotParseData(const net::URLFetcher* source);
@@ -131,6 +142,7 @@ class PrivetURLFetcher : public net::URLFetcherDelegate {
   bool send_empty_privet_token_ = false;
   bool has_byte_range_ = false;
   bool make_response_file_ = false;
+  static bool skip_retry_timeouts_for_tests_;
 
   int byte_range_start_ = 0;
   int byte_range_end_ = 0;

chrome/browser/ui/webui/local_discovery/local_discovery_ui_browsertest.cc

@@ -540,10 +540,7 @@ IN_PROC_BROWSER_TEST_F(LocalDiscoveryUITest, AddRowTest) {
   EXPECT_TRUE(WebUIBrowserTest::RunJavascriptTest("checkNoDevices"));
 }
 
-// Flaky: http://crbug.com/660669.
 IN_PROC_BROWSER_TEST_F(LocalDiscoveryUITest, RegisterTest) {
-  TestMessageLoopCondition condition_token_claimed;
-
   ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUIDevicesURL));
   condition_devices_listed().Wait();
 
@@ -557,20 +554,18 @@ IN_PROC_BROWSER_TEST_F(LocalDiscoveryUITest, RegisterTest) {
   EXPECT_TRUE(WebUIBrowserTest::RunJavascriptTest("registerShowOverlay"));
 
   {
+    base::RunLoop run_loop;
     InSequence s;
     EXPECT_CALL(fake_url_fetcher_creator(), OnCreateFakeURLFetcher(kURLInfo));
     EXPECT_CALL(fake_url_fetcher_creator(), OnCreateFakeURLFetcher(
         kURLRegisterStart));
-    EXPECT_CALL(fake_url_fetcher_creator(), OnCreateFakeURLFetcher(
-        kURLRegisterClaimToken))
-        .WillOnce(InvokeWithoutArgs(&condition_token_claimed,
-                                    &TestMessageLoopCondition::Signal));
+    EXPECT_CALL(fake_url_fetcher_creator(),
+                OnCreateFakeURLFetcher(kURLRegisterClaimToken))
+        .WillOnce(InvokeWithoutArgs([&]() { run_loop.Quit(); }));
+    EXPECT_TRUE(WebUIBrowserTest::RunJavascriptTest("registerBegin"));
+    run_loop.Run();
   }
 
-  EXPECT_TRUE(WebUIBrowserTest::RunJavascriptTest("registerBegin"));
-
-  condition_token_claimed.Wait();
-
   EXPECT_TRUE(WebUIBrowserTest::RunJavascriptTest("expectPageAdding1"));
 
   fake_fetcher_factory().SetFakeResponse(
@@ -586,6 +581,7 @@ IN_PROC_BROWSER_TEST_F(LocalDiscoveryUITest, RegisterTest) {
       net::URLRequestStatus::SUCCESS);
 
   {
+    base::RunLoop run_loop;
     InSequence s;
     EXPECT_CALL(fake_url_fetcher_creator(), OnCreateFakeURLFetcher(
         kURLRegisterClaimToken));
@@ -594,12 +590,10 @@ IN_PROC_BROWSER_TEST_F(LocalDiscoveryUITest, RegisterTest) {
     EXPECT_CALL(fake_url_fetcher_creator(), OnCreateFakeURLFetcher(
         kURLRegisterComplete));
     EXPECT_CALL(fake_url_fetcher_creator(), OnCreateFakeURLFetcher(kURLInfo))
-        .WillOnce(InvokeWithoutArgs(&condition_token_claimed,
-                                    &TestMessageLoopCondition::Signal));
+        .WillOnce(InvokeWithoutArgs([&]() { run_loop.Quit(); }));
+    run_loop.Run();
   }
 
-  condition_token_claimed.Wait();
-
   test_service_discovery_client()->SimulateReceive(
       kAnnouncePacketRegistered, sizeof(kAnnouncePacketRegistered));