diff options
| -rw-r--r-- | chrome/browser/profiles/profile_downloader.cc | 143 | ||||
| -rw-r--r-- | chrome/browser/profiles/profile_downloader.h | 19 | ||||
| -rw-r--r-- | chrome/browser/profiles/profile_downloader_unittest.cc | 104 | ||||
| -rw-r--r-- | chrome/chrome_tests.gypi | 1 |
4 files changed, 201 insertions, 66 deletions
diff --git a/chrome/browser/profiles/profile_downloader.cc b/chrome/browser/profiles/profile_downloader.cc index 5ffc5f3..56483ba 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,11 +79,61 @@ const int kPhotoIdPathComponentIndex = 2; // Index of path component with photo version. const int kPhotoVersionPathComponentIndex = 3; +// Given an image URL this function builds a new URL set to |size|. +// For example, if |size| was set to 256 and |old_url| was either: +// https://example.com/--Abc/AAAAAAAAAAI/AAAAAAAAACQ/Efg/photo.jpg +// or +// https://example.com/--Abc/AAAAAAAAAAI/AAAAAAAAACQ/Efg/s64-c/photo.jpg +// then return value in |new_url| would be: +// https://example.com/--Abc/AAAAAAAAAAI/AAAAAAAAACQ/Efg/s256-c/photo.jpg +bool GetImageURLWithSize(const GURL& old_url, int size, GURL* new_url) { + DCHECK(new_url); + std::vector<std::string> components; + base::SplitString(old_url.path(), kURLPathSeparator, &components); + if (components.size() == 0) + return false; + + const std::string& old_spec = old_url.spec(); + std::string default_size_component( + base::StringPrintf(kThumbnailSizeFormat, kDefaultThumbnailSize)); + std::string new_size_component( + base::StringPrintf(kThumbnailSizeFormat, size)); + + size_t pos = old_spec.find(default_size_component); + size_t end = std::string::npos; + if (pos != std::string::npos) { + // The default size is already specified in the URL so it needs to be + // replaced with the new size. + end = pos + default_size_component.size(); + } else { + // The default size is not in the URL so try to insert it before the last + // component. + const std::string& file_name = old_url.ExtractFileName(); + if (!file_name.empty()) { + pos = old_spec.find(file_name); + end = pos - 1; + } + } + + if (pos != std::string::npos) { + std::string new_spec = old_spec.substr(0, pos) + new_size_component + + old_spec.substr(end); + *new_url = GURL(new_spec); + return new_url->is_valid(); + } + + // We can't set the image size, just use the default size. + *new_url = old_url; + return true; +} + } // namespace -bool ProfileDownloader::GetProfileNickNameAndImageURL(const std::string& data, - string16* nick_name, - std::string* url) const { +// static +bool ProfileDownloader::GetProfileNameAndImageURL(const std::string& data, + string16* nick_name, + std::string* url, + int image_size) { DCHECK(nick_name); DCHECK(url); *nick_name = string16(); @@ -94,7 +144,7 @@ bool ProfileDownloader::GetProfileNickNameAndImageURL(const std::string& data, scoped_ptr<base::Value> root_value(base::JSONReader::ReadAndReturnError( data, false, &error_code, &error_message)); if (!root_value.get()) { - LOG(ERROR) << "Error while parsing Picasa user entry response: " + LOG(ERROR) << "Error while parsing user entry response: " << error_message; return false; } @@ -106,55 +156,27 @@ bool ProfileDownloader::GetProfileNickNameAndImageURL(const std::string& data, base::DictionaryValue* root_dictionary = static_cast<base::DictionaryValue*>(root_value.get()); - if (!root_dictionary->GetString(kNickNamePath, nick_name)) { - LOG(ERROR) << "Can't find nick name path in JSON data: " - << data; - return false; - } - - std::string thumbnail_url_string; - if (!root_dictionary->GetString( - kPhotoThumbnailURLPath, &thumbnail_url_string)) { - LOG(ERROR) << "Can't find thumbnail path in JSON data: " - << data; - return false; - } + root_dictionary->GetString(kNickNamePath, nick_name); - // 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)); - 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. + std::string url_string; + if (root_dictionary->GetString(kPhotoThumbnailURLPath, &url_string)) { + GURL new_url; + if (!GetImageURLWithSize(GURL(url_string), image_size, &new_url)) { + LOG(ERROR) << "GetImageURLWithSize failed for url: " << url_string; + return false; + } + *url = new_url.spec(); } - GURL thumbnail_url(thumbnail_url_string); - if (!thumbnail_url.is_valid()) { - LOG(ERROR) << "Thumbnail URL is not valid: " << thumbnail_url_string; - return false; - } - *url = thumbnail_url.spec(); - return true; + // The profile data is considered valid as long as it has a name or a picture. + return !nick_name->empty() || !url->empty(); } -bool ProfileDownloader::IsDefaultProfileImageURL(const std::string& url) const { +// static +bool ProfileDownloader::IsDefaultProfileImageURL(const std::string& url) { + if (url.empty()) + return true; + GURL image_url_object(url); DCHECK(image_url_object.is_valid()); VLOG(1) << "URL to check for default image: " << image_url_object.spec(); @@ -163,7 +185,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 +265,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( @@ -269,7 +291,8 @@ void ProfileDownloader::OnURLFetchComplete(const content::URLFetcher* source) { if (source == user_entry_fetcher_.get()) { std::string image_url; - if (!GetProfileNickNameAndImageURL(data, &profile_full_name_, &image_url)) { + if (!GetProfileNameAndImageURL(data, &profile_full_name_, &image_url, + delegate_->GetDesiredImageSideLength())) { delegate_->OnDownloadComplete(this, false); return; } diff --git a/chrome/browser/profiles/profile_downloader.h b/chrome/browser/profiles/profile_downloader.h index 4e15a47..0395ab4 100644 --- a/chrome/browser/profiles/profile_downloader.h +++ b/chrome/browser/profiles/profile_downloader.h @@ -9,6 +9,7 @@ #include <string> #include "base/basictypes.h" +#include "base/gtest_prod_util.h" #include "base/memory/scoped_ptr.h" #include "base/string16.h" #include "chrome/browser/image_decoder.h" @@ -62,6 +63,10 @@ class ProfileDownloader : public content::URLFetcherDelegate, virtual std::string GetProfilePictureURL() const; private: + friend class ProfileDownloaderTest; + FRIEND_TEST_ALL_PREFIXES(ProfileDownloaderTest, ParseData); + FRIEND_TEST_ALL_PREFIXES(ProfileDownloaderTest, DefaultURL); + // Overriden from content::URLFetcherDelegate: virtual void OnURLFetchComplete(const content::URLFetcher* source) OVERRIDE; @@ -79,14 +84,16 @@ class ProfileDownloader : public content::URLFetcherDelegate, virtual void OnGetTokenSuccess(const std::string& access_token) OVERRIDE; virtual void OnGetTokenFailure(const GoogleServiceAuthError& error) OVERRIDE; - // Parses the entry response from Picasa and gets the nick name and - // and profile image URL. Returns false to indicate a parsing error. - bool GetProfileNickNameAndImageURL(const std::string& data, - string16* nick_name, - std::string* url) const; + // Parses the entry response and gets the name and and profile image URL. + // |data| should be the JSON formatted data return by the response. + // Returns false to indicate a parsing error. + static bool GetProfileNameAndImageURL(const std::string& data, + string16* nick_name, + std::string* url, + int image_size); // Returns true if the image url is url of the default profile picture. - bool IsDefaultProfileImageURL(const std::string& url) const; + static bool IsDefaultProfileImageURL(const std::string& url); // Issues the first request to get user profile image. void StartFetchingImage(); diff --git a/chrome/browser/profiles/profile_downloader_unittest.cc b/chrome/browser/profiles/profile_downloader_unittest.cc new file mode 100644 index 0000000..9b47c18 --- /dev/null +++ b/chrome/browser/profiles/profile_downloader_unittest.cc @@ -0,0 +1,104 @@ +// Copyright (c) 2011 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/profiles/profile_downloader.h" + +#include "base/utf_string_conversions.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +std::string GetJSonData(const std::string& name, const std::string& url) { + std::stringstream stream; + stream << "{ "; + if (!name.empty()) + stream << "\"name\": \"" << name << "\", "; + if (!url.empty()) + stream << "\"picture\": \"" << url << "\", "; + stream << "\"locale\": \"en-US\" }"; + return stream.str(); +} + +} // namespace + +class ProfileDownloaderTest : public testing::Test { + protected: + ProfileDownloaderTest() { + } + + virtual ~ProfileDownloaderTest() { + } + + void VerifyWithNameAndURL(const std::string& name, + const std::string& url, + const std::string& expected_url, + bool is_valid) { + string16 parsed_name; + std::string parsed_url; + bool result = ProfileDownloader::GetProfileNameAndImageURL( + GetJSonData(name, url), &parsed_name, &parsed_url, 32); + EXPECT_EQ(is_valid, result); + std::string parsed_name_utf8 = UTF16ToUTF8(parsed_name); + EXPECT_EQ(name, parsed_name_utf8); + EXPECT_EQ(expected_url, parsed_url); + } +}; + +TEST_F(ProfileDownloaderTest, ParseData) { + // URL without size specified. + VerifyWithNameAndURL( + "Pat Smith", + "https://example.com/--Abc/AAAAAAAAAAI/AAAAAAAAACQ/Efg/photo.jpg", + "https://example.com/--Abc/AAAAAAAAAAI/AAAAAAAAACQ/Efg/s32-c/photo.jpg", + true); + + // URL with size specified. + VerifyWithNameAndURL( + "Pat Smith", + "http://lh0.ggpht.com/-abcd1aBCDEf/AAAA/AAA_A/abc12/s64-c/1234567890.jpg", + "http://lh0.ggpht.com/-abcd1aBCDEf/AAAA/AAA_A/abc12/s32-c/1234567890.jpg", + true); + + // URL with unknown format. + VerifyWithNameAndURL( + "Pat Smith", + "http://lh0.ggpht.com/-abcd1aBCDEf/AAAA/AAA_A/", + "http://lh0.ggpht.com/-abcd1aBCDEf/AAAA/AAA_A/", + true); + + // Data with only name. + VerifyWithNameAndURL("Pat Smith", "", "", true); + + // Data with only URL. + VerifyWithNameAndURL( + "", + "https://example.com/--Abc/AAAAAAAAAAI/AAAAAAAAACQ/Efg/photo.jpg", + "https://example.com/--Abc/AAAAAAAAAAI/AAAAAAAAACQ/Efg/s32-c/photo.jpg", + true); + + // Data without name or URL. + VerifyWithNameAndURL("", "", "", false); + + // Data with an invalid URL. + VerifyWithNameAndURL( "", "invalid url", "", false); +} + +TEST_F(ProfileDownloaderTest, DefaultURL) { + // Empty URL should be default photo + EXPECT_TRUE(ProfileDownloader::IsDefaultProfileImageURL("")); + // Picasa default photo + EXPECT_TRUE(ProfileDownloader::IsDefaultProfileImageURL( + "https://example.com/-4/AAAAAAAAAAA/AAAAAAAAAAE/G/s64-c/photo.jpg")); + // G+ default photo + EXPECT_TRUE(ProfileDownloader::IsDefaultProfileImageURL( + "https://example.com/-4/AAAAAAAAAAI/AAAAAAAAAAA/G/photo.jpg")); + // Not default with 6 components + EXPECT_FALSE(ProfileDownloader::IsDefaultProfileImageURL( + "https://example.com/-4/AAAAAAAAAAI/AAAAAAAAACQ/Efg/photo.jpg")); + // Not default with 7 components + EXPECT_FALSE(ProfileDownloader::IsDefaultProfileImageURL( + "https://example.com/-4/AAAAAAAAAAI/AAAAAAAAACQ/Efg/s32-c/photo.jpg")); + // Not default with invalid URL + EXPECT_FALSE(ProfileDownloader::IsDefaultProfileImageURL("invalid url")); +} diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index b90de7cb..6597582 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1580,6 +1580,7 @@ 'browser/profiles/gaia_info_update_service_unittest.cc', 'browser/profiles/off_the_record_profile_impl_unittest.cc', 'browser/profiles/profile_dependency_manager_unittest.cc', + 'browser/profiles/profile_downloader_unittest.cc', 'browser/profiles/profile_info_cache_unittest.cc', 'browser/profiles/profile_info_cache_unittest.h', 'browser/profiles/profile_info_util_unittest.cc', |