contacts: Add GDataContactsService.

chrome/browser/chromeos/gdata/gdata_contacts_service.cc

Index: chrome/browser/chromeos/gdata/gdata_contacts_service.cc
diff --git a/chrome/browser/chromeos/gdata/gdata_contacts_service.cc b/chrome/browser/chromeos/gdata/gdata_contacts_service.cc
new file mode 100644
index 0000000000000000000000000000000000000000..33d1e978778e810c746bf76e253f5ec41f1412ca
--- /dev/null
+++ b/chrome/browser/chromeos/gdata/gdata_contacts_service.cc
@@ -0,0 +1,657 @@
+// 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/stl_util.h"
+#include "base/values.h"
+#include "chrome/browser/chromeos/contacts/contact.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/image_decoder.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;
+
+// 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";
+const char kCategoryTermField[] = "term";
+const char kCategoryTermValue[] =
+    "http://schemas.google.com/contact/2008#contact";
+
+// Field in the contacts feed containing a list of contact entries.
+const char kEntryField[] = "entry";
+
+// Top-level fields in contact entries.
+const char kIdField[] = "id.$t";
+const char kDeletedField[] = "gd$deleted";
+const char kFullNameField[] = "gd$name.gd$fullName.$t";
+const char kGivenNameField[] = "gd$name.gd$givenName.$t";
+const char kAdditionalNameField[] = "gd$name.gd$additionalName.$t";
+const char kFamilyNameField[] = "gd$name.gd$familyName.$t";
+const char kNamePrefixField[] = "gd$name.gd$namePrefix.$t";
+const char kNameSuffixField[] = "gd$name.gd$nameSuffix.$t";
+const char kEmailField[] = "gd$email";
+const char kPhoneField[] = "gd$phoneNumber";
+const char kPostalAddressField[] = "gd$structuredPostalAddress";
+const char kInstantMessagingField[] = "gd$im";
+const char kLinkField[] = "link";
+const char kUpdatedField[] = "updated.$t";
+
+// Fields in entries in the |kEmailField| list.
+const char kEmailAddressField[] = "address";
+
+// Fields in entries in the |kPhoneField| list.
+const char kPhoneNumberField[] = "$t";
+
+// Fields in entries in the |kPostalAddressField| list.
+const char kPostalAddressFormattedField[] = "gd$formattedAddress.$t";
+
+// Fields in entries in the |kInstantMessagingField| list.
+const char kInstantMessagingAddressField[] = "address";
+const char kInstantMessagingProtocolField[] = "protocol";
+const char kInstantMessagingProtocolAimValue[] =
+    "http://schemas.google.com/g/2005#AIM";
+const char kInstantMessagingProtocolMsnValue[] =
+    "http://schemas.google.com/g/2005#MSN";
+const char kInstantMessagingProtocolYahooValue[] =
+    "http://schemas.google.com/g/2005#YAHOO";
+const char kInstantMessagingProtocolSkypeValue[] =
+    "http://schemas.google.com/g/2005#SKYPE";
+const char kInstantMessagingProtocolQqValue[] =
+    "http://schemas.google.com/g/2005#QQ";
+const char kInstantMessagingProtocolGoogleTalkValue[] =
+    "http://schemas.google.com/g/2005#GOOGLE_TALK";
+const char kInstantMessagingProtocolIcqValue[] =
+    "http://schemas.google.com/g/2005#ICQ";
+const char kInstantMessagingProtocolJabberValue[] =
+    "http://schemas.google.com/g/2005#JABBER";
+
+// Generic fields shared between address-like items (email, postal, etc.).
+const char kAddressPrimaryField[] = "primary";
+const char kAddressPrimaryTrueValue[] = "true";
+const char kAddressRelField[] = "rel";
+const char kAddressRelHomeValue[] = "http://schemas.google.com/g/2005#home";
+const char kAddressRelWorkValue[] = "http://schemas.google.com/g/2005#work";
+const char kAddressRelMobileValue[] = "http://schemas.google.com/g/2005#mobile";
+const char kAddressLabelField[] = "label";
+
+// Fields in entries in the |kLinkField| list.
+const char kLinkHrefField[] = "href";
+const char kLinkRelField[] = "rel";
+const char kLinkETagField[] = "gd$etag";
+const char kLinkRelPhotoValue[] =
+    "http://schemas.google.com/contacts/2008/rel#photo";
+
+// Returns a string containing a pretty-printed JSON representation of |value|.
+std::string PrettyPrintValue(const base::Value& value) {
+  std::string out;
+  base::JSONWriter::WriteWithOptions(
+      &value, base::JSONWriter::OPTIONS_PRETTY_PRINT, &out);
+  return out;
+}
+
+// Returns whether an address is primary, given a dictionary representing a
+// single address.
+bool IsAddressPrimary(const DictionaryValue& address_dict) {
+  std::string primary;
+  address_dict.GetString(kAddressPrimaryField, &primary);
+  return primary == kAddressPrimaryTrueValue;
+}
+
+// Initializes a AddressType struct given a dictionary representing a single

[satorux1, 2012/07/24 21:34:29]

nit: an AddressType

[Daniel Erat, 2012/07/27 16:54:19]

Done.

+// address.
+void InitAddressType(const DictionaryValue& address_dict,
+                     contacts::Contact::AddressType* type) {
+  std::string rel;
+  address_dict.GetString(kAddressRelField, &rel);
+  if (rel == kAddressRelHomeValue)
+    type->relation = contacts::Contact::AddressType::RELATION_HOME;
+  else if (rel == kAddressRelWorkValue)
+    type->relation = contacts::Contact::AddressType::RELATION_WORK;
+  else if (rel == kAddressRelMobileValue)
+    type->relation = contacts::Contact::AddressType::RELATION_MOBILE;
+  else
+    type->relation = contacts::Contact::AddressType::RELATION_OTHER;
+
+  address_dict.GetString(kAddressLabelField, &(type->label));
+}
+
+// Maps the protocol from a dictionary representing a contact's IM address to a
+// contacts::Contact::InstantMessagingAddress::Protocol value.
+contacts::Contact::InstantMessagingAddress::Protocol
+GetInstantMessagingProtocol(const DictionaryValue& im_dict) {
+  std::string protocol;
+  im_dict.GetString(kInstantMessagingProtocolField, &protocol);
+  if (protocol == kInstantMessagingProtocolAimValue)
+    return contacts::Contact::InstantMessagingAddress::PROTOCOL_AIM;
+  else if (protocol == kInstantMessagingProtocolMsnValue)
+    return contacts::Contact::InstantMessagingAddress::PROTOCOL_MSN;
+  else if (protocol == kInstantMessagingProtocolYahooValue)
+    return contacts::Contact::InstantMessagingAddress::PROTOCOL_YAHOO;
+  else if (protocol == kInstantMessagingProtocolSkypeValue)
+    return contacts::Contact::InstantMessagingAddress::PROTOCOL_SKYPE;
+  else if (protocol == kInstantMessagingProtocolQqValue)
+    return contacts::Contact::InstantMessagingAddress::PROTOCOL_QQ;
+  else if (protocol == kInstantMessagingProtocolGoogleTalkValue)
+    return contacts::Contact::InstantMessagingAddress::PROTOCOL_GOOGLE_TALK;
+  else if (protocol == kInstantMessagingProtocolIcqValue)
+    return contacts::Contact::InstantMessagingAddress::PROTOCOL_ICQ;
+  else if (protocol == kInstantMessagingProtocolJabberValue)
+    return contacts::Contact::InstantMessagingAddress::PROTOCOL_JABBER;
+  else
+    return contacts::Contact::InstantMessagingAddress::PROTOCOL_OTHER;
+}
+
+// Gets the photo URL from a contact's dictionary (within the "entry" list).
+// Returns an empty string if no photo was found.
+std::string GetPhotoUrl(const DictionaryValue& dict) {
+  ListValue* link_list = NULL;
+  if (!dict.GetList(kLinkField, &link_list))
+    return std::string();
+
+  for (size_t i = 0; i < link_list->GetSize(); ++i) {
+    DictionaryValue* link_dict = NULL;
+    if (!link_list->GetDictionary(i, &link_dict))
+      continue;
+
+    std::string rel;
+    if (!link_dict->GetString(kLinkRelField, &rel))
+      continue;
+    if (rel != kLinkRelPhotoValue)
+      continue;
+
+    // From https://goo.gl/7T6Od: "If a contact does not have a photo, then the
+    // photo link element has no gd:etag attribute."
+    std::string etag;
+    if (!link_dict->GetString(kLinkETagField, &etag))
+      continue;
+
+    std::string url;
+    if (link_dict->GetString(kLinkHrefField, &url))
+      return url;
+  }
+  return std::string();
+}
+
+// Fills a Contact's fields using an entry from a GData feed.
+bool FillContactFromDictionary(const base::DictionaryValue& dict,
+                               contacts::Contact* contact) {
+  DCHECK(contact);
+  if (!dict.GetString(kIdField, &(contact->provider_id)))
+    return false;
+
+  std::string updated;
+  if (dict.GetString(kUpdatedField, &updated)) {
+    if (!util::GetTimeFromString(updated, &(contact->update_time))) {
+      LOG(WARNING) << "Unable to parse time \"" << updated << "\"";
+      return false;
+    }
+  }
+
+  base::Value* deleted_value = NULL;
+  contact->deleted = dict.Get(kDeletedField, &deleted_value);
+  if (contact->deleted)
+    return true;
+
+  dict.GetString(kFullNameField, &(contact->full_name));
+  dict.GetString(kGivenNameField, &(contact->given_name));
+  dict.GetString(kAdditionalNameField, &(contact->additional_name));
+  dict.GetString(kFamilyNameField, &(contact->family_name));
+  dict.GetString(kNamePrefixField, &(contact->name_prefix));
+  dict.GetString(kNameSuffixField, &(contact->name_suffix));
+
+  ListValue* email_list = NULL;
+  if (dict.GetList(kEmailField, &email_list)) {
+    for (size_t i = 0; i < email_list->GetSize(); ++i) {
+      DictionaryValue* email_dict = NULL;
+      if (!email_list->GetDictionary(i, &email_dict))
+        return false;
+
+      contacts::Contact::EmailAddress email;
+      if (!email_dict->GetString(kEmailAddressField, &email.address))
+        return false;
+
+      email.primary = IsAddressPrimary(*email_dict);
+      InitAddressType(*email_dict, &email.type);
+      contact->email_addresses.push_back(email);
+    }
+  }
+
+  ListValue* phone_list = NULL;
+  if (dict.GetList(kPhoneField, &phone_list)) {
+    for (size_t i = 0; i < phone_list->GetSize(); ++i) {
+      DictionaryValue* phone_dict = NULL;
+      if (!phone_list->GetDictionary(i, &phone_dict))
+        return false;
+
+      contacts::Contact::PhoneNumber phone;
+      if (!phone_dict->GetString(kPhoneNumberField, &phone.number))
+        return false;
+
+      phone.primary = IsAddressPrimary(*phone_dict);
+      InitAddressType(*phone_dict, &phone.type);
+      contact->phone_numbers.push_back(phone);
+    }
+  }
+
+  ListValue* address_list = NULL;
+  if (dict.GetList(kPostalAddressField, &address_list)) {
+    for (size_t i = 0; i < address_list->GetSize(); ++i) {
+      DictionaryValue* address_dict = NULL;
+      if (!address_list->GetDictionary(i, &address_dict))
+        return false;
+
+      contacts::Contact::PostalAddress address;
+      if (!address_dict->GetString(kPostalAddressFormattedField,
+                                   &address.address)) {
+        return false;
+      }
+      address.primary = IsAddressPrimary(*address_dict);
+      InitAddressType(*address_dict, &address.type);
+      contact->postal_addresses.push_back(address);
+    }
+  }
+
+  ListValue* im_list = NULL;
+  if (dict.GetList(kInstantMessagingField, &im_list)) {
+    for (size_t i = 0; i < im_list->GetSize(); ++i) {
+      DictionaryValue* im_dict = NULL;
+      if (!im_list->GetDictionary(i, &im_dict))
+        return false;
+
+      contacts::Contact::InstantMessagingAddress im;
+      if (!im_dict->GetString(kInstantMessagingAddressField, &im.address))
+        return false;
+      im.primary = IsAddressPrimary(*im_dict);
+      InitAddressType(*im_dict, &im.type);
+      im.protocol = GetInstantMessagingProtocol(*im_dict);
+      contact->instant_messaging_addresses.push_back(im);
+    }
+  }
+
+  return true;
+}
+
+}  // namespace
+
+// This class handles a single request to download all of a user's contacts.
+//
+// First, the contacts feed is downloaded via GetContactsOperation and parsed.
+// Individual contacts::Contact objects are created using the data from the
+// feed.
+//
+// Next, GetContactPhotoOperations are created and used to start downloading
+// contacts' photos in parallel.  As the operations finish, sandboxed
+// ImageDecoders are created to convert the image data to SkBitmaps and
+// additional GetContactPhotoOperations are created.
+//
+// When all photos have been downloaded and decoded, the contacts are passed to
+// the passed-in callback.
+class GDataContactsService::DownloadContactsRequest
+    : public ImageDecoder::Delegate {
+ 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)
+      : 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),
+        num_in_progress_photo_downloads_(0),
+        photo_download_failed_(false) {
+    DCHECK(service_);
+    DCHECK(profile_);
+    DCHECK(runner_);
+  }
+
+  ~DownloadContactsRequest() {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    // Abandon any outstanding image decode requests.
+    for (ImageDecoderContactMap::iterator it =
+             in_progress_photo_decodes_.begin();
+         it != in_progress_photo_decodes_.end(); ++it) {
+      it->first->set_delegate(NULL);
+    }
+    service_ = NULL;
+    profile_ = NULL;
+    runner_ = NULL;
+  }
+
+  // Issues the initial request to download the contact feed.
+  void Run() {
+    GetContactsOperation* operation =
+        new GetContactsOperation(
+            runner_->operation_registry(),
+            profile_,
+            min_update_time_,
+            base::Bind(&DownloadContactsRequest::HandleFeedData,
+                       base::Unretained(this)));
+    if (!service_->feed_url_for_testing_.is_empty())
+      operation->set_feed_url_for_testing(service_->feed_url_for_testing_);
+
+    runner_->StartOperationWithRetry(operation);
+  }
+
+  // ImageDecoder::Delegate overrides:
+  virtual void OnImageDecoded(const ImageDecoder* decoder,
+                              const SkBitmap& decoded_image) OVERRIDE {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    ImageDecoderContactMap::iterator it =
+        in_progress_photo_decodes_.find(const_cast<ImageDecoder*>(decoder));
+    CHECK(it != in_progress_photo_decodes_.end());
+    contacts::Contact* contact = it->second;
+    VLOG(1) << "Downloaded and decoded " << decoded_image.width() << "x"
+            << decoded_image.height() << " photo for " << contact->provider_id;
+    contact->photo = decoded_image;

[satorux1, 2012/07/24 21:34:29]

just curious but how big is the decoded image in bytes? If the photo is
150x150xRGBA, then it'll be about 90KB. If I have 1000 friends, which I don't
have but you might have, it'll be 90MB in memory. This will also consume a
considerable amount of space on the disk, if cached locally.

Would it make sense to keep the image in the compressed form (JPEG?), and decode
them only when we show them?

[Daniel Erat, 2012/07/27 16:54:19]

I've switched this to re-encode the image as a PNG and store that instead. 
Storing it in the original compressed form seems like a non-option; we don't
necessarily trust this data, so I think that we'd always need to decode it
within a sandbox before displaying it.

+    in_progress_photo_decodes_.erase(it);
+    CheckCompletion();
+  }
+
+  virtual void OnDecodeImageFailed(const ImageDecoder* decoder) OVERRIDE {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    ImageDecoderContactMap::iterator it =
+        in_progress_photo_decodes_.find(const_cast<ImageDecoder*>(decoder));
+    CHECK(it != in_progress_photo_decodes_.end());
+    contacts::Contact* contact = it->second;
+    LOG(WARNING) << "Failed to decode image for contact "
+                 << contact->provider_id;
+    // Leave the photo blank but don't abort the entire download request.
+    in_progress_photo_decodes_.erase(it);
+    CheckCompletion();
+  }
+
+ private:
+  // Callback for GetContactsOperation calls.
+  void HandleFeedData(GDataErrorCode error,
+                      scoped_ptr<base::Value> feed_data) {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    if (error != HTTP_SUCCESS) {
+      LOG(WARNING) << "Got error " << error << " while downloading contacts";
+      failure_callback_.Run();
+      service_->OnRequestComplete(this);
+      return;
+    }
+
+    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;
+    }
+
+    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;
+    }
+
+    DictionaryValue* feed_dict = NULL;
+    if (!toplevel_dict->GetDictionary(kFeedField, &feed_dict)) {
+      LOG(WARNING) << "Feed dictionary missing";
+      return false;
+    }
+
+    // Check the category field to confirm that this is actually a contact feed.
+    ListValue* category_list = NULL;
+    if (!feed_dict->GetList(kCategoryField, &category_list)) {
+      LOG(WARNING) << "Category list missing";
+      return false;
+    }
+    DictionaryValue* category_dict = NULL;
+    if (!category_list->GetSize() == 1 ||
+        !category_list->GetDictionary(0, &category_dict)) {
+      LOG(WARNING) << "Unable to get dictionary from category list of size "
+                   << category_list->GetSize();
+      return false;
+    }
+    std::string category_scheme, category_term;
+    if (!category_dict->GetString(kCategorySchemeField, &category_scheme) ||
+        !category_dict->GetString(kCategoryTermField, &category_term) ||
+        category_scheme != kCategorySchemeValue ||
+        category_term != kCategoryTermValue) {
+      LOG(WARNING) << "Unexpected category (scheme was \"" << category_scheme
+                   << "\", term was \"" << category_term << "\")";
+      return false;
+    }
+
+    // A missing entry list means no entries (maybe we're doing an incremental
+    // update and nothing has changed).
+    ListValue* entry_list = NULL;
+    if (!feed_dict->GetList(kEntryField, &entry_list))
+      return true;
+
+    contacts_needing_photo_downloads_.reserve(entry_list->GetSize());
+
+    for (ListValue::const_iterator entry_it = entry_list->begin();
+         entry_it != entry_list->end(); ++entry_it) {
+      const size_t index = (entry_it - entry_list->begin());
+      const DictionaryValue* contact_dict = NULL;
+      if (!(*entry_it)->GetAsDictionary(&contact_dict)) {
+        LOG(WARNING) << "Entry " << index << " isn't a dictionary";
+        return false;
+      }
+
+      scoped_ptr<contacts::Contact> contact(new contacts::Contact);
+      if (!FillContactFromDictionary(*contact_dict, contact.get())) {
+        LOG(WARNING) << "Unable to fill entry " << index;
+        return false;
+      }
+
+      VLOG(1) << "Got contact " << index << ":"
+              << " id=" << contact->provider_id
+              << " full_name=\"" << contact->full_name << "\""
+              << " update_time=" << contact->update_time.ToDoubleT();
+
+      std::string photo_url = GetPhotoUrl(*contact_dict);
+      if (!photo_url.empty()) {
+        if (!service_->feed_url_for_testing_.is_empty()) {

[satorux1, 2012/07/24 21:34:29]

did you mean rewrite_photo_url_callback_for_testing_.is_empty()?

[Daniel Erat, 2012/07/27 16:54:19]

No.  Callbacks can't be empty, so I was using this as a proxy.  I agree that
this is ugly, so I've added another has_rewrite_photo_url_callback_for_testing_
member.

[satorux1, 2012/07/28 00:38:23]

I meant rewrite_photo_url_callback_for_testing_.is_null(). you probably don't
need has_rewrite_photo_url_callback_for_testing_

[Daniel Erat, 2012/07/29 19:54:25]

Ah, thanks.  I'd looked for is_null() in base/callback.h earlier but didn't find
it.  Wasn't aware of callback_internal.h. :-P

+          photo_url =
+              service_->rewrite_photo_url_callback_for_testing_.Run(photo_url);

[satorux1, 2012/07/24 21:34:29]

This is tricky. Might want to comment why we need to rewrite the URL for
testing.

[Daniel Erat, 2012/07/27 16:54:19]

Added a comment to the header.

+        }
+        contact_photo_urls_[contact.get()] = photo_url;
+        contacts_needing_photo_downloads_.push_back(contact.get());
+      }
+
+      contacts_->push_back(contact.release());
+    }
+
+    return true;
+  }
+
+  // If we're done downloading and decoding 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 &&
+        in_progress_photo_decodes_.empty()) {
+      VLOG(1) << "Done downloading and decoding photos; invoking callback";
+      if (photo_download_failed_)
+        failure_callback_.Run();
+      else
+        success_callback_.Run(contacts_.Pass());
+      service_->OnRequestComplete(this);
+      return;
+    }
+
+    while (!contacts_needing_photo_downloads_.empty() &&
+           (num_in_progress_photo_downloads_ <
+            max_simultaneous_photo_downloads_)) {
+      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,
+                         base::Unretained(this),

[satorux1, 2012/07/24 21:34:29]

It it safe to use base::Unretained() here? I guess you can easily use WeakPtr
here.

[Daniel Erat, 2012/07/27 16:54:19]

Is your concern that the GetContactPhotoOperation can outlive the
DownloadContactsRequest?  I thought that this wasn't a risk, since
GDataContactsService's d'tor cancels all operations along with deleting all
requests.  Additionally using WeakPtr seems fine to me, though, so added.

+                         base::Unretained(contact))));

[satorux1, 2012/07/24 21:34:29]

hmm, using base::Unretained() for the 2nd parameter of base::Bind()? It's new to
me. Is it necessary?

[Daniel Erat, 2012/07/27 16:54:19]

I don't think so; deleted.

+      num_in_progress_photo_downloads_++;
+    }
+  }
+
+  // Callback for GetContactPhotoOperation calls.  Decodes the image data,
+  // 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_SUCCESS) {
+      LOG(WARNING) << "Got error " << error << " while downloading photo "
+                   << "for " << contact->provider_id;
+      // TODO(derat): Retry several times for temporary failures?
+      photo_download_failed_ = true;
+      // Make sure we don't start any more downloads.
+      contacts_needing_photo_downloads_.clear();
+      CheckCompletion();
+      return;
+    }
+
+    scoped_refptr<ImageDecoder> image_decoder =
+        new ImageDecoder(this, *(download_data.get()));
+    in_progress_photo_decodes_.insert(
+        std::make_pair(image_decoder.get(), contact));
+    image_decoder->Start();
+  }
+
+ private:
+  typedef std::map<contacts::Contact*, std::string> ContactPhotoUrls;
+  typedef std::map<ImageDecoder*, contacts::Contact*> ImageDecoderContactMap;
+
+  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_;
+
+  // 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_;
+
+  // 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_;
+
+  // Maps from an ImageDecoder to the contact whose photo it's decoding.
+  ImageDecoderContactMap in_progress_photo_decodes_;
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadContactsRequest);
+};
+
+GDataContactsService::GDataContactsService(Profile* profile)
+    : profile_(profile),
+      runner_(new GDataOperationRunner(profile)),
+      max_simultaneous_photo_downloads_(kMaxSimultaneousPhotoDownloads) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(profile_);
+}
+
+GDataContactsService::~GDataContactsService() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  STLDeleteContainerPointers(requests_.begin(), requests_.end());
+  requests_.clear();
+  runner_->CancelAll();
+}
+
+GDataAuthService* GDataContactsService::auth_service_for_testing() {
+  return runner_->auth_service();
+}
+
+void GDataContactsService::Initialize() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  runner_->Initialize();
+}
+
+void GDataContactsService::DownloadContacts(SuccessCallback success_callback,
+                                            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_);
+  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