contacts: Rate-limit GData photo requests and handle 404s.

chrome/browser/chromeos/gdata/gdata_contacts_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 "chrome/browser/chromeos/gdata/gdata_contacts_service.h"
 
 #include <cstring>
 #include <string>
 #include <map>
 #include <utility>
 
 #include "base/json/json_writer.h"
 #include "base/logging.h"
 #include "base/memory/weak_ptr.h"
 #include "base/stl_util.h"
+#include "base/time.h"
+#include "base/timer.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/contacts/contact.pb.h"
 #include "chrome/browser/chromeos/gdata/gdata_operation_registry.h"
 #include "chrome/browser/chromeos/gdata/gdata_operation_runner.h"
 #include "chrome/browser/chromeos/gdata/gdata_operations.h"
 #include "chrome/browser/chromeos/gdata/gdata_params.h"
 #include "chrome/browser/chromeos/gdata/gdata_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/browser_thread.h"
 
 using content::BrowserThread;
 
 namespace gdata {
 
 namespace {
 
-// Maximum number of profile photos that we'll download at once.
-const int kMaxSimultaneousPhotoDownloads = 10;
+// Maximum number of profile photos that we'll download per second.
+// At values above 10, Google starts returning 503 errors.
+const int kMaxPhotoDownloadsPerSecond = 10;
 
 // Field in the top-level object containing the contacts feed.
 const char kFeedField[] = "feed";
 
 // Field in the contacts feed containing a list of category information, along
 // with fields within the dictionaries contained in the list and expected
 // values.
 const char kCategoryField[] = "category";
 const char kCategorySchemeField[] = "scheme";
 const char kCategorySchemeValue[] = "http://schemas.google.com/g/2005#kind";
@@ skipping 271 matching lines @@
 // downloaded, the contacts are passed to the passed-in callback.
 class GDataContactsService::DownloadContactsRequest
     : public base::SupportsWeakPtr<DownloadContactsRequest> {
  public:
   DownloadContactsRequest(GDataContactsService* service,
                           Profile* profile,
                           GDataOperationRunner* runner,
                           SuccessCallback success_callback,
                           FailureCallback failure_callback,
                           const base::Time& min_update_time,
-                          int max_simultaneous_photo_downloads)
+                          int max_photo_downloads_per_second)
       : service_(service),
         profile_(profile),
         runner_(runner),
         success_callback_(success_callback),
         failure_callback_(failure_callback),
         min_update_time_(min_update_time),
         contacts_(new ScopedVector<contacts::Contact>),
-        max_simultaneous_photo_downloads_(max_simultaneous_photo_downloads),
+        max_photo_downloads_per_second_(max_photo_downloads_per_second),
         num_in_progress_photo_downloads_(0),
         photo_download_failed_(false) {
     DCHECK(service_);
     DCHECK(profile_);
     DCHECK(runner_);
   }
 
   ~DownloadContactsRequest() {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
     service_ = NULL;
@@ skipping 29 matching lines @@
     }
 
     VLOG(2) << "Got feed data:\n" << PrettyPrintValue(*(feed_data.get()));
     if (!ProcessFeedData(*feed_data.get())) {
       LOG(WARNING) << "Unable to process feed data";
       failure_callback_.Run();
       service_->OnRequestComplete(this);
       return;
     }
 
+    StartPhotoDownloads();
+    photo_download_timer_.Start(
+        FROM_HERE, base::TimeDelta::FromSeconds(1),
+        this, &DownloadContactsRequest::StartPhotoDownloads);
     CheckCompletion();
   }
 
   // Processes the raw contacts feed from |feed_data| and fills |contacts_|.
   // Returns true on success.
   bool ProcessFeedData(const base::Value& feed_data) {
     const DictionaryValue* toplevel_dict = NULL;
     if (!feed_data.GetAsDictionary(&toplevel_dict)) {
       LOG(WARNING) << "Top-level object is not a dictionary";
       return false;
@@ skipping 73 matching lines @@
   }
 
   // If we're done downloading photos, invokes a callback and deletes |this|.
   // Otherwise, starts one or more downloads of URLs from
   // |contacts_needing_photo_downloads_|.
   void CheckCompletion() {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
     if (contacts_needing_photo_downloads_.empty() &&
         num_in_progress_photo_downloads_ == 0) {
       VLOG(1) << "Done downloading photos; invoking callback";
+      photo_download_timer_.Stop();
       if (photo_download_failed_)
         failure_callback_.Run();
       else
         success_callback_.Run(contacts_.Pass());
       service_->OnRequestComplete(this);
       return;
     }
+  }
 
+  // Starts photo downloads for contacts in |contacts_needing_photo_downloads_|.
+  // Should be invoked only once per second.
+  void StartPhotoDownloads() {
     while (!contacts_needing_photo_downloads_.empty() &&
            (num_in_progress_photo_downloads_ <
-            max_simultaneous_photo_downloads_)) {
+            max_photo_downloads_per_second_)) {
       contacts::Contact* contact = contacts_needing_photo_downloads_.back();
       contacts_needing_photo_downloads_.pop_back();
       DCHECK(contact_photo_urls_.count(contact));
       std::string url = contact_photo_urls_[contact];
 
       VLOG(1) << "Starting download of photo " << url << " for "
               << contact->provider_id();
       runner_->StartOperationWithRetry(
           new GetContactPhotoOperation(
               runner_->operation_registry(),
               profile_,
               GURL(url),
               base::Bind(&DownloadContactsRequest::HandlePhotoData,
                          AsWeakPtr(), contact)));
       num_in_progress_photo_downloads_++;
     }
   }
 
   // Callback for GetContactPhotoOperation calls.  Updates the associated
   // Contact and checks for completion.
   void HandlePhotoData(contacts::Contact* contact,
                        GDataErrorCode error,
                        scoped_ptr<std::string> download_data) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
     VLOG(1) << "Got photo data for " << contact->provider_id()
             << " (error=" << error << " size=" << download_data->size() << ")";
     num_in_progress_photo_downloads_--;
 
+    if (error == HTTP_INTERNAL_SERVER_ERROR ||
+        error == HTTP_SERVICE_UNAVAILABLE) {
+      LOG(WARNING) << "Got error " << error << " while downloading photo "
+                   << "for " << contact->provider_id() << "; retrying";
+      contacts_needing_photo_downloads_.push_back(contact);
+      return;
+    }
+
+    if (error == HTTP_NOT_FOUND) {
+      LOG(WARNING) << "Got error " << error << " while downloading photo "
+                   << "for " << contact->provider_id() << "; skipping";
+      CheckCompletion();
+      return;
+    }
+
     if (error != HTTP_SUCCESS) {
       LOG(WARNING) << "Got error " << error << " while downloading photo "
-                   << "for " << contact->provider_id();
-      // TODO(derat): Retry several times for temporary failures?
+                   << "for " << contact->provider_id() << "; giving up";

[satorux1, 2012/08/06 20:57:08]

don't have to do it in the patch, but you might want to add some UMA for
failures like this.

[Daniel Erat, 2012/08/06 23:12:21]

Yep, I plan to add a lot of metrics in a later patch for temporary and permanent
failures (along with histograms for things like total number of contacts and
time taken to download them).

       photo_download_failed_ = true;
       // Make sure we don't start any more downloads.
       contacts_needing_photo_downloads_.clear();
       CheckCompletion();
       return;
     }
 
     contact->set_raw_untrusted_photo(*download_data);
     CheckCompletion();
   }
 
  private:
   typedef std::map<contacts::Contact*, std::string> ContactPhotoUrls;
 
   GDataContactsService* service_;  // not owned
   Profile* profile_;  // not owned
   GDataOperationRunner* runner_;  // not owned
 
   SuccessCallback success_callback_;
   FailureCallback failure_callback_;
 
   base::Time min_update_time_;
 
   scoped_ptr<ScopedVector<contacts::Contact> > contacts_;
 
   // Map from a contact to the URL at which its photo is located.
   // Contacts without photos do not appear in this map.
   ContactPhotoUrls contact_photo_urls_;
 
+  // Invokes StartPhotoDownloads() once per second.
+  base::RepeatingTimer<DownloadContactsRequest> photo_download_timer_;
+
   // Contacts that have photos that we still need to start downloading.
   // When we start a download, the contact is removed from this list.
   std::vector<contacts::Contact*> contacts_needing_photo_downloads_;
 
-  // Maximum number of photos we'll try to download at once.
-  int max_simultaneous_photo_downloads_;
+  // Maximum number of photos we'll try to download per second.
+  int max_photo_downloads_per_second_;
 
   // Number of in-progress photo downloads.
   int num_in_progress_photo_downloads_;
 
   // Did we encounter a fatal error while downloading a photo?
   bool photo_download_failed_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadContactsRequest);
 };
 
 GDataContactsService::GDataContactsService(Profile* profile)
     : profile_(profile),
       runner_(new GDataOperationRunner(profile)),
-      max_simultaneous_photo_downloads_(kMaxSimultaneousPhotoDownloads) {
+      max_photo_downloads_per_second_(kMaxPhotoDownloadsPerSecond) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(profile_);
 }
 
 GDataContactsService::~GDataContactsService() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   runner_->CancelAll();
   STLDeleteContainerPointers(requests_.begin(), requests_.end());
   requests_.clear();
 }
@@ skipping 11 matching lines @@
                                             FailureCallback failure_callback,
                                             const base::Time& min_update_time) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadContactsRequest* request =
       new DownloadContactsRequest(this,
                                   profile_,
                                   runner_.get(),
                                   success_callback,
                                   failure_callback,
                                   min_update_time,
-                                  max_simultaneous_photo_downloads_);
+                                  max_photo_downloads_per_second_);
   VLOG(1) << "Starting contacts download with request " << request;
   requests_.insert(request);
   request->Run();
 }
 
 void GDataContactsService::OnRequestComplete(DownloadContactsRequest* request) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(request);
   VLOG(1) << "Download request " << request << " complete";
   requests_.erase(request);
   delete request;
 }
 
 }  // namespace contacts