Use Google Plus API to get profile information

chrome/browser/profiles/profile_downloader.cc

Index: chrome/browser/profiles/profile_downloader.cc
diff --git a/chrome/browser/profiles/profile_downloader.cc b/chrome/browser/profiles/profile_downloader.cc
index 5ffc5f3c9faa7d224da154cf9329d1695108d605..5faceadf560aa1bd94513639a243323d3f1085bd 100644
--- a/chrome/browser/profiles/profile_downloader.cc
+++ b/chrome/browser/profiles/profile_downloader.cc
@@ -38,21 +38,21 @@ namespace {
 const char kAuthorizationHeader[] =
     "Authorization: Bearer %s";
 
-// URL requesting Picasa API for user info.
+// URL requesting user info.
 const char kUserEntryURL[] =
-    "http://picasaweb.google.com/data/entry/api/user/default?alt=json";
+    "https://www.googleapis.com/oauth2/v1/userinfo?alt=json";
 
-// OAuth scope for the Picasa API.
-const char kPicasaScope[] = "http://picasaweb.google.com/data/";
+// OAuth scope for the user info API.
+const char kAPIScope[] = "https://www.googleapis.com/auth/userinfo.profile";
 
 // Path in JSON dictionary to user's photo thumbnail URL.
-const char kPhotoThumbnailURLPath[] = "entry.gphoto$thumbnail.$t";
+const char kPhotoThumbnailURLPath[] = "picture";
 
-const char kNickNamePath[] = "entry.gphoto$nickname.$t";
+const char kNickNamePath[] = "name";
 
 // Path format for specifying thumbnail's size.
 const char kThumbnailSizeFormat[] = "s%d-c";
-// Default Picasa thumbnail size.
+// Default thumbnail size.
 const int kDefaultThumbnailSize = 64;
 
 // Separator of URL path components.
@@ -70,8 +70,8 @@ const char kGooglePlusPhotoId[] = "AAAAAAAAAAI";
 // Photo version of the default Google+ profile picture (base64 of 0).
 const char kDefaultGooglePlusPhotoVersion[] = "AAAAAAAAAAA";
 
-// Number of path components in profile picture URL.
-const size_t kProfileImageURLPathComponentsCount = 7;
+// The minimum number of path components in profile picture URL.
+const size_t kProfileImageURLPathComponentsCount = 6;
 
 // Index of path component with photo ID.
 const int kPhotoIdPathComponentIndex = 2;
@@ -79,6 +79,46 @@ const int kPhotoIdPathComponentIndex = 2;
 // Index of path component with photo version.
 const int kPhotoVersionPathComponentIndex = 3;
 
+bool GetImageURLWithSize(const GURL& old_url, int size, GURL* new_url) {

[Munjal (Google), 2011/12/03 06:02:33]

Would be nice to add a comment about what this function does? May be with an
example old url and new url.

[sail, 2011/12/04 01:48:53]

Done.

+  DCHECK(new_url);
+  std::vector<std::string> components;
+  base::SplitString(old_url.path(), kURLPathSeparator, &components);
+  if (components.size() == 0)
+    return false;
+
+  std::string old_spec = old_url.spec();

[Munjal (Google), 2011/12/03 06:02:33]

Nit: const std::string& old_spec

[sail, 2011/12/04 01:48:53]

Done.

+  std::string default_size_component(
+      base::StringPrintf(kThumbnailSizeFormat, kDefaultThumbnailSize));
+  std::string new_size_component(
+      base::StringPrintf(kThumbnailSizeFormat, size));
+
+  // Check if a thumbnail size is already specified in the URL, e.g.,
+  // http://lh0.ggpht.com/-abcd1aBCDEf/AAAA/AAA_A/abc12/s64-c/1234567890.jpg
+  size_t pos = old_spec.find(default_size_component);
+  if (pos != std::string::npos) {
+    size_t end = pos + default_size_component.size();
+    std::string new_spec = old_spec.substr(0, pos) + new_size_component +
+                           old_spec.substr(end, old_spec.size() - end);

[Munjal (Google), 2011/12/03 06:02:33]

Nit: You can skip the second parameter to substr since it defaults to the end of
the string.

[sail, 2011/12/04 01:48:53]

Done.

+    *new_url = GURL(new_spec);
+    return new_url->is_valid();
+  }
+
+  // A thubmnail size is not already embedded in the URL so insert it before
+  // the last component.
+  std::string file_name = old_url.ExtractFileName();

[Munjal (Google), 2011/12/03 06:02:33]

Nit: const std::string& file_name

[sail, 2011/12/04 01:48:53]

Done.

+  pos = old_spec.find(file_name);
+  if (pos != std::string::npos) {

[Munjal (Google), 2011/12/03 06:02:33]

Would it ever happen that pos is npos given that file_name was extracted from
old_url so it has to exist right?

[sail, 2011/12/04 01:48:53]

Good point. The file name can be empty so I replace this with a check for empty
instead.

+    std::string new_spec = old_spec.substr(0, pos) + new_size_component +
+                           old_spec.substr(pos - 1, old_spec.size() - pos + 1);

[Munjal (Google), 2011/12/03 06:02:33]

Nit: Same comment about skipping second arg to substr here.

[sail, 2011/12/04 01:48:53]

Done.

+    *new_url = GURL(new_spec);
+    return new_url->is_valid();
+  }

[Munjal (Google), 2011/12/03 06:02:33]

Looking at the two code paths here, it seems like you can just compute prefix
length and suffix start in two code paths and then just have a common code path.

[sail, 2011/12/04 01:48:53]

Done.

+
+  // We can't set the image size, just use the default size.
+  *new_url = old_url;
+  return true;
+}

[Munjal (Google), 2011/12/03 06:02:33]

It will be nice to add some unit tests for this method since the code is
non-trivial.

[sail, 2011/12/04 01:48:53]

Done.

+
 }  // namespace
 
 bool ProfileDownloader::GetProfileNickNameAndImageURL(const std::string& data,
@@ -112,45 +152,21 @@ bool ProfileDownloader::GetProfileNickNameAndImageURL(const std::string& data,
     return false;
   }
 
-  std::string thumbnail_url_string;
-  if (!root_dictionary->GetString(
-          kPhotoThumbnailURLPath, &thumbnail_url_string)) {
+  std::string url_string;
+  if (!root_dictionary->GetString(kPhotoThumbnailURLPath, &url_string)) {
     LOG(ERROR) << "Can't find thumbnail path in JSON data: "
                << data;
     return false;
   }
 
-  // Try to change the size of thumbnail we are going to get.
-  // Typical URL looks like this:
-  // http://lh0.ggpht.com/-abcd1aBCDEf/AAAA/AAA_A/abc12/s64-c/1234567890.jpg
-  std::string default_thumbnail_size_path_component(
-      base::StringPrintf(kThumbnailSizeFormat, kDefaultThumbnailSize));
+  GURL new_url;
   int image_size = delegate_->GetDesiredImageSideLength();
-  std::string new_thumbnail_size_path_component(
-      base::StringPrintf(kThumbnailSizeFormat, image_size));
-  size_t thumbnail_size_pos =
-      thumbnail_url_string.find(default_thumbnail_size_path_component);
-  if (thumbnail_size_pos != std::string::npos) {
-    size_t thumbnail_size_end =
-        thumbnail_size_pos + default_thumbnail_size_path_component.size();
-   thumbnail_url_string =
-        thumbnail_url_string.substr(0, thumbnail_size_pos) +
-        new_thumbnail_size_path_component +
-        thumbnail_url_string.substr(
-            thumbnail_size_end,
-            thumbnail_url_string.size() - thumbnail_size_end);
-  } else {
-    LOG(WARNING) << "Hasn't found thumbnail size part in image URL: "
-                 << thumbnail_url_string;
-    // Use the thumbnail URL we have.
-  }
-
-  GURL thumbnail_url(thumbnail_url_string);
-  if (!thumbnail_url.is_valid()) {
-    LOG(ERROR) << "Thumbnail URL is not valid: " << thumbnail_url_string;
+  if (!GetImageURLWithSize(GURL(url_string), image_size, &new_url)) {
+    LOG(ERROR) << "GetImageURLWithSize failed for url: " << url_string;
     return false;
   }
-  *url = thumbnail_url.spec();
+
+  *url = new_url.spec();
   return true;
 }
 
@@ -163,7 +179,7 @@ bool ProfileDownloader::IsDefaultProfileImageURL(const std::string& url) const {
                     kURLPathSeparator,
                     &path_components);
 
-  if (path_components.size() != kProfileImageURLPathComponentsCount)
+  if (path_components.size() < kProfileImageURLPathComponentsCount)
     return false;
 
   const std::string& photo_id = path_components[kPhotoIdPathComponentIndex];
@@ -243,7 +259,7 @@ void ProfileDownloader::StartFetchingOAuth2AccessToken() {
   DCHECK(!service->GetOAuth2LoginRefreshToken().empty());
 
   std::vector<std::string> scopes;
-  scopes.push_back(kPicasaScope);
+  scopes.push_back(kAPIScope);
   oauth2_access_token_fetcher_.reset(new OAuth2AccessTokenFetcher(
       this, delegate_->GetBrowserProfile()->GetRequestContext()));
   oauth2_access_token_fetcher_->Start(