Don't retry fetching policies if it's a blocking operation,

components/policy/core/common/cloud/device_management_service.cc

 // Copyright (c) 2012 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/policy/core/common/cloud/device_management_service.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
@@ skipping 166 matching lines @@
   void ConfigureRequest(net::URLFetcher* fetcher);
 
   // Returns true if this job should be retried. |fetcher| has just completed,
   // and can be inspected to determine if the request failed and should be
   // retried.
   bool ShouldRetry(const net::URLFetcher* fetcher);
 
   // Invoked right before retrying this job.
   void PrepareRetry();
 
+  // Returns whether the retry should happen after a delay.
+  bool IsRetryWithDelay() { return retry_with_delay_; }
+
   // Number of times that this job has been retried due to connection errors.
   int retries_count() { return retries_count_; }
 
   // Get weak pointer
   base::WeakPtr<DeviceManagementRequestJobImpl> GetWeakPtr() {
     return weak_ptr_factory_.GetWeakPtr();
   }
 
  protected:
   // DeviceManagementRequestJob:
   void Run() override;
 
  private:
   // Invokes the callback with the given error code.
   void ReportError(DeviceManagementStatus code);
 
   // Pointer to the service this job is associated with.
   DeviceManagementService* service_;
 
   // Whether the BYPASS_PROXY flag should be set by ConfigureRequest().
   bool bypass_proxy_;
 
+  // Whether the retry should happen after a delay.
+  bool retry_with_delay_;
+
   // Number of times that this job has been retried due to connection errors.
   int retries_count_;
 
   // The request context to use for this job.
   scoped_refptr<net::URLRequestContextGetter> request_context_;
 
   // Used to get notified if the job has been canceled while waiting for retry.
   base::WeakPtrFactory<DeviceManagementRequestJobImpl> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DeviceManagementRequestJobImpl);
@@ skipping 123 matching lines @@
   CHECK(request_.SerializeToString(&payload));
   fetcher->SetUploadData(kPostContentType, payload);
   std::string extra_headers;
   if (!gaia_token_.empty())
     extra_headers += kServiceTokenAuthHeader + gaia_token_ + "\n";
   if (!dm_token_.empty())
     extra_headers += kDMTokenAuthHeader + dm_token_ + "\n";
   fetcher->SetExtraRequestHeaders(extra_headers);
 }
 
 bool DeviceManagementRequestJobImpl::ShouldRetry(

[Andrew T Wilson (Slow), 2016/08/23 13:46:27]

I think a better change would be to have ShouldRetry() return an enum: NO_RETRY,
RETRY_IMMEDIATELY, RETRY_WITH_DELAY. That way you don't need to store a
retry_with_delay_ variable and you don't need a second IsRetryWithDelay() API.

[Marton Hunyady, 2016/08/23 14:09:44]

Done.

     const net::URLFetcher* fetcher) {
   if (FailedWithProxy(fetcher) && !bypass_proxy_) {
     // Retry the job if it failed due to a broken proxy, by bypassing the
     // proxy on the next try.
     bypass_proxy_ = true;
+    retry_with_delay_ = false;
     return true;
   }
 
   // Early device policy fetches on ChromeOS and Auto-Enrollment checks are
   // often interrupted during ChromeOS startup when network is not yet ready.
   // Allowing the fetcher to retry once after that is enough to recover; allow
   // it to retry up to 3 times just in case.
-  if (IsConnectionError(fetcher->GetStatus()) && retries_count_ < kMaxRetries) {
+  if (IsConnectionError(fetcher->GetStatus()) && retries_count_ < kMaxRetries &&
+      type_ != DeviceManagementRequestJob::TYPE_POLICY_FETCH) {
     ++retries_count_;
+    retry_with_delay_ = true;
     return true;
   }
 
   // The request didn't fail, or the limit of retry attempts has been reached;
   // forward the result to the job owner.
   return false;
 }
 
 void DeviceManagementRequestJobImpl::PrepareRetry() {
   if (!retry_callback_.is_null())
@@ skipping 23 matching lines @@
   AddParameter(dm_protocol::kParamDeviceID, client_id);
 }
 
 em::DeviceManagementRequest* DeviceManagementRequestJob::GetRequest() {
   return &request_;
 }
 
 DeviceManagementRequestJob::DeviceManagementRequestJob(
     JobType type,
     const std::string& agent_parameter,
-    const std::string& platform_parameter) {
+    const std::string& platform_parameter)
+    : type_(type) {
   AddParameter(dm_protocol::kParamRequest, JobTypeToRequestType(type));
   AddParameter(dm_protocol::kParamDeviceType, dm_protocol::kValueDeviceType);
   AddParameter(dm_protocol::kParamAppType, dm_protocol::kValueAppType);
   AddParameter(dm_protocol::kParamAgent, agent_parameter);
   AddParameter(dm_protocol::kParamPlatform, platform_parameter);
 }
 
 void DeviceManagementRequestJob::SetRetryCallback(
     const RetryCallback& retry_callback) {
   retry_callback_ = retry_callback;
@@ skipping 116 matching lines @@
   if (entry == pending_jobs_.end()) {
     NOTREACHED() << "Callback from foreign URL fetcher";
     return;
   }
 
   DeviceManagementRequestJobImpl* job = entry->second;
   pending_jobs_.erase(entry);
 
   if (job->ShouldRetry(source)) {
     job->PrepareRetry();
-    int delay = g_retry_delay_ms << (job->retries_count() - 1);
+    int delay = job->IsRetryWithDelay()
+                    ? g_retry_delay_ms << (job->retries_count() - 1)
+                    : 0;
     LOG(WARNING) << "Dmserver request failed, retrying in " << delay / 1000
                  << "s.";
     task_runner_->PostDelayedTask(
         FROM_HERE,
         base::Bind(&DeviceManagementService::StartJobAfterDelay,
                    weak_ptr_factory_.GetWeakPtr(), job->GetWeakPtr()),
         base::TimeDelta::FromMilliseconds(delay));
   } else {
     std::string data;
     source->GetResponseAsString(&data);
@@ skipping 20 matching lines @@
     }
   }
 
   const JobQueue::iterator elem =
       std::find(queued_jobs_.begin(), queued_jobs_.end(), job);
   if (elem != queued_jobs_.end())
     queued_jobs_.erase(elem);
 }
 
 }  // namespace policy