diff options
| author | Allan Sandfeld Jensen <allan.jensen@theqtcompany.com> | 2015-10-13 13:24:50 +0200 |
|---|---|---|
| committer | Allan Sandfeld Jensen <allan.jensen@theqtcompany.com> | 2015-10-14 10:57:25 +0000 |
| commit | af3d4809763ef308f08ced947a73b624729ac7ea (patch) | |
| tree | 4402b911e30383f6c6dace1e8cf3b8e85355db3a /chromium/google_apis | |
| parent | 0e8ff63a407fe323e215bb1a2c423c09a4747c8a (diff) | |
| download | qtwebengine-chromium-af3d4809763ef308f08ced947a73b624729ac7ea.tar.gz | |
BASELINE: Update Chromium to 47.0.2526.14
Also adding in sources needed for spellchecking.
Change-Id: Idd44170fa1616f26315188970a8d5ba7d472b18a
Reviewed-by: Michael Brüning <michael.bruning@theqtcompany.com>
Diffstat (limited to 'chromium/google_apis')
70 files changed, 719 insertions, 866 deletions
diff --git a/chromium/google_apis/BUILD.gn b/chromium/google_apis/BUILD.gn index 58cfa1d1357..53a41cebd69 100644 --- a/chromium/google_apis/BUILD.gn +++ b/chromium/google_apis/BUILD.gn @@ -176,10 +176,10 @@ source_set("test_support") { "gaia/fake_gaia.h", "gaia/fake_identity_provider.cc", "gaia/fake_identity_provider.h", - "gaia/fake_oauth2_token_service_delegate.cc", - "gaia/fake_oauth2_token_service_delegate.h", "gaia/fake_oauth2_token_service.cc", "gaia/fake_oauth2_token_service.h", + "gaia/fake_oauth2_token_service_delegate.cc", + "gaia/fake_oauth2_token_service_delegate.h", "gaia/mock_url_fetcher_factory.h", "gaia/oauth2_token_service_test_util.cc", "gaia/oauth2_token_service_test_util.h", @@ -205,6 +205,15 @@ source_set("test_support") { } } +# TODO(GYP): Delete this after we've converted everything to GN. +# The _run targets exist only for compatibility w/ GYP. +group("google_apis_unittests_run") { + testonly = true + deps = [ + ":google_apis_unittests", + ] +} + test("google_apis_unittests") { sources = [ "gaia/gaia_auth_fetcher_unittest.cc", @@ -220,6 +229,10 @@ test("google_apis_unittests") { "google_api_keys_unittest.cc", ] + data = [ + "test/", + ] + configs += [ ":key_defines" ] deps = [ diff --git a/chromium/google_apis/drive/base_requests.cc b/chromium/google_apis/drive/base_requests.cc index e526012eecc..18846cba9a5 100644 --- a/chromium/google_apis/drive/base_requests.cc +++ b/chromium/google_apis/drive/base_requests.cc @@ -193,8 +193,8 @@ namespace google_apis { scoped_ptr<base::Value> ParseJson(const std::string& json) { int error_code = -1; std::string error_message; - scoped_ptr<base::Value> value(base::JSONReader::DeprecatedReadAndReturnError( - json, base::JSON_PARSE_RFC, &error_code, &error_message)); + scoped_ptr<base::Value> value = base::JSONReader::ReadAndReturnError( + json, base::JSON_PARSE_RFC, &error_code, &error_message); if (!value.get()) { std::string trimmed_json; diff --git a/chromium/google_apis/drive/base_requests_unittest.cc b/chromium/google_apis/drive/base_requests_unittest.cc index 2bd0e03e886..42fa64039a5 100644 --- a/chromium/google_apis/drive/base_requests_unittest.cc +++ b/chromium/google_apis/drive/base_requests_unittest.cc @@ -193,7 +193,7 @@ TEST_F(MultipartUploadRequestBaseTest, Basic) { DriveApiErrorCode error = DRIVE_OTHER_ERROR; base::RunLoop run_loop; const base::FilePath source_path = - google_apis::test_util::GetTestFilePath("chromeos/file_manager/text.txt"); + google_apis::test_util::GetTestFilePath("drive/text.txt"); std::string upload_content_type; std::string upload_content_data; FakeMultipartUploadRequest* const multipart_request = diff --git a/chromium/google_apis/drive/drive_api_requests.cc b/chromium/google_apis/drive/drive_api_requests.cc index d9089bbe437..68b12def7fd 100644 --- a/chromium/google_apis/drive/drive_api_requests.cc +++ b/chromium/google_apis/drive/drive_api_requests.cc @@ -385,7 +385,8 @@ FilesInsertRequest::FilesInsertRequest( const DriveApiUrlGenerator& url_generator, const FileResourceCallback& callback) : DriveApiDataRequest<FileResource>(sender, callback), - url_generator_(url_generator) { + url_generator_(url_generator), + visibility_(FILE_VISIBILITY_DEFAULT) { DCHECK(!callback.is_null()); } @@ -434,7 +435,8 @@ bool FilesInsertRequest::GetContentData(std::string* upload_content_type, } GURL FilesInsertRequest::GetURLInternal() const { - return url_generator_.GetFilesInsertUrl(); + return url_generator_.GetFilesInsertUrl( + visibility_ == FILE_VISIBILITY_PRIVATE ? "PRIVATE" : ""); } //============================== FilesPatchRequest ============================ @@ -514,7 +516,8 @@ FilesCopyRequest::FilesCopyRequest( const DriveApiUrlGenerator& url_generator, const FileResourceCallback& callback) : DriveApiDataRequest<FileResource>(sender, callback), - url_generator_(url_generator) { + url_generator_(url_generator), + visibility_(FILE_VISIBILITY_DEFAULT) { DCHECK(!callback.is_null()); } @@ -526,7 +529,8 @@ net::URLFetcher::RequestType FilesCopyRequest::GetRequestType() const { } GURL FilesCopyRequest::GetURLInternal() const { - return url_generator_.GetFilesCopyUrl(file_id_); + return url_generator_.GetFilesCopyUrl( + file_id_, visibility_ == FILE_VISIBILITY_PRIVATE ? "PRIVATE" : ""); } bool FilesCopyRequest::GetContentData(std::string* upload_content_type, @@ -1400,7 +1404,12 @@ std::vector<std::string> BatchUploadRequest::GetExtraRequestHeaders() const { } void BatchUploadRequest::ProcessURLFetchResults(const net::URLFetcher* source) { - UMA_HISTOGRAM_SPARSE_SLOWLY(kUMADriveBatchUploadResponseCode, GetErrorCode()); + // Return the detailed raw HTTP code if the error code is abstracted + // DRIVE_OTHER_ERROR. + UMA_HISTOGRAM_SPARSE_SLOWLY(kUMADriveBatchUploadResponseCode, + GetErrorCode() != DRIVE_OTHER_ERROR + ? GetErrorCode() + : source->GetResponseCode()); if (!IsSuccessfulDriveApiErrorCode(GetErrorCode())) { RunCallbackOnPrematureFailure(GetErrorCode()); diff --git a/chromium/google_apis/drive/drive_api_requests.h b/chromium/google_apis/drive/drive_api_requests.h index ebef7fd1f95..702414477d8 100644 --- a/chromium/google_apis/drive/drive_api_requests.h +++ b/chromium/google_apis/drive/drive_api_requests.h @@ -249,6 +249,12 @@ class FilesAuthorizeRequest : public DriveApiDataRequest<FileResource> { //============================ FilesInsertRequest ============================= +// Enumeration type for specifying visibility of files. +enum FileVisibility { + FILE_VISIBILITY_DEFAULT, + FILE_VISIBILITY_PRIVATE, +}; + // This class performs the request for creating a resource. // This request is mapped to // https://developers.google.com/drive/v2/reference/files/insert @@ -261,6 +267,11 @@ class FilesInsertRequest : public DriveApiDataRequest<FileResource> { const FileResourceCallback& callback); ~FilesInsertRequest() override; + // Optional parameter + void set_visibility(FileVisibility visibility) { + visibility_ = visibility; + } + // Optional request body. const base::Time& last_viewed_by_me_date() const { return last_viewed_by_me_date_; @@ -302,6 +313,7 @@ class FilesInsertRequest : public DriveApiDataRequest<FileResource> { private: const DriveApiUrlGenerator url_generator_; + FileVisibility visibility_; base::Time last_viewed_by_me_date_; std::string mime_type_; base::Time modified_date_; @@ -409,6 +421,11 @@ class FilesCopyRequest : public DriveApiDataRequest<FileResource> { const std::string& file_id() const { return file_id_; } void set_file_id(const std::string& file_id) { file_id_ = file_id; } + // Optional parameter + void set_visibility(FileVisibility visibility) { + visibility_ = visibility; + } + // Optional request body. const std::vector<std::string>& parents() const { return parents_; } void add_parent(const std::string& parent) { parents_.push_back(parent); } @@ -434,6 +451,7 @@ class FilesCopyRequest : public DriveApiDataRequest<FileResource> { const DriveApiUrlGenerator url_generator_; std::string file_id_; + FileVisibility visibility_; base::Time modified_date_; std::vector<std::string> parents_; std::string title_; diff --git a/chromium/google_apis/drive/drive_api_requests_unittest.cc b/chromium/google_apis/drive/drive_api_requests_unittest.cc index 8ca6e1718a3..15cb7cceb88 100644 --- a/chromium/google_apis/drive/drive_api_requests_unittest.cc +++ b/chromium/google_apis/drive/drive_api_requests_unittest.cc @@ -558,6 +558,7 @@ TEST_F(DriveApiRequestsTest, FilesInsertRequest) { test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error, &file_resource))); + request->set_visibility(drive::FILE_VISIBILITY_PRIVATE); request->set_last_viewed_by_me_date( base::Time::FromUTCExploded(kLastViewedByMeDate)); request->set_mime_type("application/vnd.google-apps.folder"); @@ -571,7 +572,7 @@ TEST_F(DriveApiRequestsTest, FilesInsertRequest) { EXPECT_EQ(HTTP_SUCCESS, error); EXPECT_EQ(net::test_server::METHOD_POST, http_request_.method); - EXPECT_EQ("/drive/v2/files", http_request_.relative_url); + EXPECT_EQ("/drive/v2/files?visibility=PRIVATE", http_request_.relative_url); EXPECT_EQ("application/json", http_request_.headers["Content-Type"]); EXPECT_TRUE(http_request_.has_content); @@ -819,6 +820,7 @@ TEST_F(DriveApiRequestsTest, FilesCopyRequest) { test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error, &file_resource))); + request->set_visibility(drive::FILE_VISIBILITY_PRIVATE); request->set_file_id("resource_id"); request->set_modified_date(base::Time::FromUTCExploded(kModifiedDate)); request->add_parent("parent_resource_id"); @@ -829,7 +831,8 @@ TEST_F(DriveApiRequestsTest, FilesCopyRequest) { EXPECT_EQ(HTTP_SUCCESS, error); EXPECT_EQ(net::test_server::METHOD_POST, http_request_.method); - EXPECT_EQ("/drive/v2/files/resource_id/copy", http_request_.relative_url); + EXPECT_EQ("/drive/v2/files/resource_id/copy?visibility=PRIVATE", + http_request_.relative_url); EXPECT_EQ("application/json", http_request_.headers["Content-Type"]); EXPECT_TRUE(http_request_.has_content); @@ -1078,7 +1081,7 @@ TEST_F(DriveApiRequestsTest, UploadNewFileRequest) { EXPECT_EQ(HTTP_SUCCESS, error); EXPECT_EQ(kTestUploadNewFilePath, upload_url.path()); EXPECT_EQ(kTestContentType, http_request_.headers["X-Upload-Content-Type"]); - EXPECT_EQ(base::Int64ToString(kTestContent.size()), + EXPECT_EQ(base::SizeTToString(kTestContent.size()), http_request_.headers["X-Upload-Content-Length"]); EXPECT_EQ(net::test_server::METHOD_POST, http_request_.method); @@ -1126,8 +1129,8 @@ TEST_F(DriveApiRequestsTest, UploadNewFileRequest) { EXPECT_EQ(upload_url.path(), http_request_.relative_url); // Content-Range header should be added. EXPECT_EQ("bytes 0-" + - base::Int64ToString(kTestContent.size() - 1) + "/" + - base::Int64ToString(kTestContent.size()), + base::SizeTToString(kTestContent.size() - 1) + "/" + + base::SizeTToString(kTestContent.size()), http_request_.headers["Content-Range"]); // The upload content should be set in the HTTP request. EXPECT_TRUE(http_request_.has_content); @@ -1263,7 +1266,7 @@ TEST_F(DriveApiRequestsTest, UploadNewLargeFileRequest) { EXPECT_EQ(HTTP_SUCCESS, error); EXPECT_EQ(kTestUploadNewFilePath, upload_url.path()); EXPECT_EQ(kTestContentType, http_request_.headers["X-Upload-Content-Type"]); - EXPECT_EQ(base::Int64ToString(kTestContent.size()), + EXPECT_EQ(base::SizeTToString(kTestContent.size()), http_request_.headers["X-Upload-Content-Length"]); EXPECT_EQ(net::test_server::METHOD_POST, http_request_.method); @@ -1304,7 +1307,7 @@ TEST_F(DriveApiRequestsTest, UploadNewLargeFileRequest) { // Request should go to the upload URL. EXPECT_EQ(upload_url.path(), http_request_.relative_url); // Content-Range header should be added. - EXPECT_EQ("bytes */" + base::Int64ToString(kTestContent.size()), + EXPECT_EQ("bytes */" + base::SizeTToString(kTestContent.size()), http_request_.headers["Content-Range"]); EXPECT_TRUE(http_request_.has_content); EXPECT_TRUE(http_request_.content.empty()); @@ -1351,9 +1354,9 @@ TEST_F(DriveApiRequestsTest, UploadNewLargeFileRequest) { EXPECT_EQ(upload_url.path(), http_request_.relative_url); // Content-Range header should be added. EXPECT_EQ("bytes " + - base::Int64ToString(start_position) + "-" + - base::Int64ToString(end_position - 1) + "/" + - base::Int64ToString(kTestContent.size()), + base::SizeTToString(start_position) + "-" + + base::SizeTToString(end_position - 1) + "/" + + base::SizeTToString(kTestContent.size()), http_request_.headers["Content-Range"]); // The upload content should be set in the HTTP request. EXPECT_TRUE(http_request_.has_content); @@ -1394,7 +1397,7 @@ TEST_F(DriveApiRequestsTest, UploadNewLargeFileRequest) { // Request should go to the upload URL. EXPECT_EQ(upload_url.path(), http_request_.relative_url); // Content-Range header should be added. - EXPECT_EQ("bytes */" + base::Int64ToString(kTestContent.size()), + EXPECT_EQ("bytes */" + base::SizeTToString(kTestContent.size()), http_request_.headers["Content-Range"]); EXPECT_TRUE(http_request_.has_content); EXPECT_TRUE(http_request_.content.empty()); @@ -1445,7 +1448,7 @@ TEST_F(DriveApiRequestsTest, UploadNewFileWithMetadataRequest) { EXPECT_EQ(HTTP_SUCCESS, error); EXPECT_EQ(kTestUploadNewFilePath, upload_url.path()); EXPECT_EQ(kTestContentType, http_request_.headers["X-Upload-Content-Type"]); - EXPECT_EQ(base::Int64ToString(kTestContent.size()), + EXPECT_EQ(base::SizeTToString(kTestContent.size()), http_request_.headers["X-Upload-Content-Length"]); EXPECT_EQ(net::test_server::METHOD_POST, http_request_.method); @@ -1496,7 +1499,7 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequest) { EXPECT_EQ(HTTP_SUCCESS, error); EXPECT_EQ(kTestUploadExistingFilePath, upload_url.path()); EXPECT_EQ(kTestContentType, http_request_.headers["X-Upload-Content-Type"]); - EXPECT_EQ(base::Int64ToString(kTestContent.size()), + EXPECT_EQ(base::SizeTToString(kTestContent.size()), http_request_.headers["X-Upload-Content-Length"]); EXPECT_EQ("*", http_request_.headers["If-Match"]); @@ -1539,8 +1542,8 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequest) { EXPECT_EQ(upload_url.path(), http_request_.relative_url); // Content-Range header should be added. EXPECT_EQ("bytes 0-" + - base::Int64ToString(kTestContent.size() - 1) + "/" + - base::Int64ToString(kTestContent.size()), + base::SizeTToString(kTestContent.size() - 1) + "/" + + base::SizeTToString(kTestContent.size()), http_request_.headers["Content-Range"]); // The upload content should be set in the HTTP request. EXPECT_TRUE(http_request_.has_content); @@ -1587,7 +1590,7 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequestWithETag) { EXPECT_EQ(HTTP_SUCCESS, error); EXPECT_EQ(kTestUploadExistingFilePath, upload_url.path()); EXPECT_EQ(kTestContentType, http_request_.headers["X-Upload-Content-Type"]); - EXPECT_EQ(base::Int64ToString(kTestContent.size()), + EXPECT_EQ(base::SizeTToString(kTestContent.size()), http_request_.headers["X-Upload-Content-Length"]); EXPECT_EQ(kTestETag, http_request_.headers["If-Match"]); @@ -1626,8 +1629,8 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequestWithETag) { EXPECT_EQ(upload_url.path(), http_request_.relative_url); // Content-Range header should be added. EXPECT_EQ("bytes 0-" + - base::Int64ToString(kTestContent.size() - 1) + "/" + - base::Int64ToString(kTestContent.size()), + base::SizeTToString(kTestContent.size() - 1) + "/" + + base::SizeTToString(kTestContent.size()), http_request_.headers["Content-Range"]); // The upload content should be set in the HTTP request. EXPECT_TRUE(http_request_.has_content); @@ -1675,7 +1678,7 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequestWithETagConflicting) { EXPECT_EQ(HTTP_PRECONDITION, error); EXPECT_EQ(kTestContentType, http_request_.headers["X-Upload-Content-Type"]); - EXPECT_EQ(base::Int64ToString(kTestContent.size()), + EXPECT_EQ(base::SizeTToString(kTestContent.size()), http_request_.headers["X-Upload-Content-Length"]); EXPECT_EQ("Conflicting-etag", http_request_.headers["If-Match"]); @@ -1721,7 +1724,7 @@ TEST_F(DriveApiRequestsTest, EXPECT_EQ(HTTP_SUCCESS, error); EXPECT_EQ(kTestUploadExistingFilePath, upload_url.path()); EXPECT_EQ(kTestContentType, http_request_.headers["X-Upload-Content-Type"]); - EXPECT_EQ(base::Int64ToString(kTestContent.size()), + EXPECT_EQ(base::SizeTToString(kTestContent.size()), http_request_.headers["X-Upload-Content-Length"]); EXPECT_EQ(kTestETag, http_request_.headers["If-Match"]); @@ -1765,8 +1768,8 @@ TEST_F(DriveApiRequestsTest, EXPECT_EQ(upload_url.path(), http_request_.relative_url); // Content-Range header should be added. EXPECT_EQ("bytes 0-" + - base::Int64ToString(kTestContent.size() - 1) + "/" + - base::Int64ToString(kTestContent.size()), + base::SizeTToString(kTestContent.size() - 1) + "/" + + base::SizeTToString(kTestContent.size()), http_request_.headers["Content-Range"]); // The upload content should be set in the HTTP request. EXPECT_TRUE(http_request_.has_content); @@ -1823,7 +1826,7 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileWithMetadataRequest) { EXPECT_EQ(HTTP_SUCCESS, error); EXPECT_EQ(kTestUploadExistingFilePath, upload_url.path()); EXPECT_EQ(kTestContentType, http_request_.headers["X-Upload-Content-Type"]); - EXPECT_EQ(base::Int64ToString(kTestContent.size()), + EXPECT_EQ(base::SizeTToString(kTestContent.size()), http_request_.headers["X-Upload-Content-Length"]); EXPECT_EQ(kTestETag, http_request_.headers["If-Match"]); @@ -1942,9 +1945,9 @@ TEST_F(DriveApiRequestsTest, PermissionsInsertRequest) { http_request_.relative_url); EXPECT_EQ("application/json", http_request_.headers["Content-Type"]); - scoped_ptr<base::Value> expected(base::JSONReader::DeprecatedRead( + scoped_ptr<base::Value> expected = base::JSONReader::Read( "{\"additionalRoles\":[\"commenter\"], \"role\":\"reader\", " - "\"type\":\"user\",\"value\":\"user@example.com\"}")); + "\"type\":\"user\",\"value\":\"user@example.com\"}"); ASSERT_TRUE(expected); scoped_ptr<base::Value> result = @@ -1977,11 +1980,11 @@ TEST_F(DriveApiRequestsTest, PermissionsInsertRequest) { http_request_.relative_url); EXPECT_EQ("application/json", http_request_.headers["Content-Type"]); - expected.reset(base::JSONReader::DeprecatedRead( - "{\"role\":\"writer\", \"type\":\"domain\",\"value\":\"example.com\"}")); + expected = base::JSONReader::Read( + "{\"role\":\"writer\", \"type\":\"domain\",\"value\":\"example.com\"}"); ASSERT_TRUE(expected); - result.reset(base::JSONReader::DeprecatedRead(http_request_.content)); + result = base::JSONReader::Read(http_request_.content); EXPECT_TRUE(http_request_.has_content); EXPECT_TRUE(base::Value::Equals(expected.get(), result.get())); } diff --git a/chromium/google_apis/drive/drive_api_url_generator.cc b/chromium/google_apis/drive/drive_api_url_generator.cc index b6632d77eaf..14da8a3ae7f 100644 --- a/chromium/google_apis/drive/drive_api_url_generator.cc +++ b/chromium/google_apis/drive/drive_api_url_generator.cc @@ -116,8 +116,14 @@ GURL DriveApiUrlGenerator::GetFilesAuthorizeUrl( net::EscapePath(app_id).c_str())); } -GURL DriveApiUrlGenerator::GetFilesInsertUrl() const { - return base_url_.Resolve(kDriveV2FilesUrl); +GURL DriveApiUrlGenerator::GetFilesInsertUrl( + const std::string& visibility) const { + GURL url = base_url_.Resolve(kDriveV2FilesUrl); + + if (!visibility.empty()) + url = net::AppendOrReplaceQueryParameter(url, "visibility", visibility); + + return url; } GURL DriveApiUrlGenerator::GetFilesPatchUrl(const std::string& file_id, @@ -137,9 +143,16 @@ GURL DriveApiUrlGenerator::GetFilesPatchUrl(const std::string& file_id, return url; } -GURL DriveApiUrlGenerator::GetFilesCopyUrl(const std::string& file_id) const { - return base_url_.Resolve(base::StringPrintf( +GURL DriveApiUrlGenerator::GetFilesCopyUrl( + const std::string& file_id, + const std::string& visibility) const { + GURL url = base_url_.Resolve(base::StringPrintf( kDriveV2FileCopyUrlFormat, net::EscapePath(file_id).c_str())); + + if (!visibility.empty()) + url = net::AppendOrReplaceQueryParameter(url, "visibility", visibility); + + return url; } GURL DriveApiUrlGenerator::GetFilesListUrl(int max_results, diff --git a/chromium/google_apis/drive/drive_api_url_generator.h b/chromium/google_apis/drive/drive_api_url_generator.h index a1ec3c1439b..7e498fb6404 100644 --- a/chromium/google_apis/drive/drive_api_url_generator.h +++ b/chromium/google_apis/drive/drive_api_url_generator.h @@ -47,7 +47,7 @@ class DriveApiUrlGenerator { const std::string& app_id) const; // Returns a URL to create a resource. - GURL GetFilesInsertUrl() const; + GURL GetFilesInsertUrl(const std::string& visibility) const; // Returns a URL to patch file metadata. GURL GetFilesPatchUrl(const std::string& file_id, @@ -55,7 +55,8 @@ class DriveApiUrlGenerator { bool update_viewed_date) const; // Returns a URL to copy a resource specified by |file_id|. - GURL GetFilesCopyUrl(const std::string& file_id) const; + GURL GetFilesCopyUrl(const std::string& file_id, + const std::string& visibility) const; // Returns a URL to fetch file list. GURL GetFilesListUrl(int max_results, diff --git a/chromium/google_apis/drive/drive_api_url_generator_unittest.cc b/chromium/google_apis/drive/drive_api_url_generator_unittest.cc index 1ccae2fc57e..28e79c45460 100644 --- a/chromium/google_apis/drive/drive_api_url_generator_unittest.cc +++ b/chromium/google_apis/drive/drive_api_url_generator_unittest.cc @@ -62,7 +62,7 @@ TEST_F(DriveApiUrlGeneratorTest, GetFilesGetUrl) { url_generator_.GetFilesGetUrl("0ADK06pfg", true, GURL()).spec()); // If |embed_origin| is not empty, it should be added as a query parameter. - url::AddStandardScheme("chrome-extension"); + url::AddStandardScheme("chrome-extension", url::SCHEME_WITHOUT_PORT); EXPECT_EQ( "https://www.example.com/drive/v2/files/0ADK06pfg" "?embedOrigin=chrome-extension%3A%2F%2Ftest", @@ -83,7 +83,11 @@ TEST_F(DriveApiUrlGeneratorTest, GetFilesAuthorizeUrl) { TEST_F(DriveApiUrlGeneratorTest, GetFilesInsertUrl) { EXPECT_EQ("https://www.example.com/drive/v2/files", - url_generator_.GetFilesInsertUrl().spec()); + url_generator_.GetFilesInsertUrl("").spec()); + EXPECT_EQ("https://www.example.com/drive/v2/files?visibility=DEFAULT", + url_generator_.GetFilesInsertUrl("DEFAULT").spec()); + EXPECT_EQ("https://www.example.com/drive/v2/files?visibility=PRIVATE", + url_generator_.GetFilesInsertUrl("PRIVATE").spec()); } TEST_F(DriveApiUrlGeneratorTest, GetFilePatchUrl) { @@ -126,11 +130,17 @@ TEST_F(DriveApiUrlGeneratorTest, GetFilePatchUrl) { TEST_F(DriveApiUrlGeneratorTest, GetFilesCopyUrl) { // |file_id| should be embedded into the url. EXPECT_EQ("https://www.example.com/drive/v2/files/0ADK06pfg/copy", - url_generator_.GetFilesCopyUrl("0ADK06pfg").spec()); + url_generator_.GetFilesCopyUrl("0ADK06pfg", "").spec()); EXPECT_EQ("https://www.example.com/drive/v2/files/0Bz0bd074/copy", - url_generator_.GetFilesCopyUrl("0Bz0bd074").spec()); + url_generator_.GetFilesCopyUrl("0Bz0bd074", "").spec()); EXPECT_EQ("https://www.example.com/drive/v2/files/file%3Afile_id/copy", - url_generator_.GetFilesCopyUrl("file:file_id").spec()); + url_generator_.GetFilesCopyUrl("file:file_id", "").spec()); + EXPECT_EQ("https://www.example.com/drive/v2/files/0Bz0bd074/copy" + "?visibility=DEFAULT", + url_generator_.GetFilesCopyUrl("0Bz0bd074", "DEFAULT").spec()); + EXPECT_EQ("https://www.example.com/drive/v2/files/file%3Afile_id/copy" + "?visibility=PRIVATE", + url_generator_.GetFilesCopyUrl("file:file_id", "PRIVATE").spec()); } TEST_F(DriveApiUrlGeneratorTest, GetFilesListUrl) { diff --git a/chromium/google_apis/drive/test_util.cc b/chromium/google_apis/drive/test_util.cc index 6a22ca0ff2b..e77aedfdeb0 100644 --- a/chromium/google_apis/drive/test_util.cc +++ b/chromium/google_apis/drive/test_util.cc @@ -26,7 +26,7 @@ namespace test_util { bool RemovePrefix(const std::string& input, const std::string& prefix, std::string* output) { - if (!base::StartsWithASCII(input, prefix, true /* case sensitive */)) + if (!base::StartsWith(input, prefix, base::CompareCase::SENSITIVE)) return false; *output = input.substr(prefix.size()); @@ -37,7 +37,7 @@ base::FilePath GetTestFilePath(const std::string& relative_path) { base::FilePath path; if (!PathService::Get(base::DIR_SOURCE_ROOT, &path)) return base::FilePath(); - path = path.AppendASCII("chrome") + path = path.AppendASCII("google_apis") .AppendASCII("test") .AppendASCII("data") .Append(base::FilePath::FromUTF8Unsafe(relative_path)); @@ -97,7 +97,7 @@ scoped_ptr<net::test_server::BasicHttpResponse> CreateHttpResponseFromFile( std::string content_type = "text/plain"; if (base::EndsWith(file_path.AsUTF8Unsafe(), ".json", - true /* case sensitive */)) + base::CompareCase::SENSITIVE)) content_type = "application/json"; scoped_ptr<net::test_server::BasicHttpResponse> http_response( @@ -133,17 +133,16 @@ bool ParseContentRangeHeader(const std::string& value, if (!RemovePrefix(value, "bytes ", &remaining)) return false; - std::vector<std::string> parts; - base::SplitString(remaining, '/', &parts); + std::vector<base::StringPiece> parts = base::SplitStringPiece( + remaining, "/", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL); if (parts.size() != 2U) return false; - const std::string range = parts[0]; if (!base::StringToInt64(parts[1], length)) return false; - parts.clear(); - base::SplitString(range, '-', &parts); + parts = base::SplitStringPiece(parts[0], "-", base::TRIM_WHITESPACE, + base::SPLIT_WANT_ALL); if (parts.size() != 2U) return false; diff --git a/chromium/google_apis/gaia/OWNERS b/chromium/google_apis/gaia/OWNERS index 654a183990f..cba33772c4c 100644 --- a/chromium/google_apis/gaia/OWNERS +++ b/chromium/google_apis/gaia/OWNERS @@ -1,3 +1,2 @@ rogerta@chromium.org -tim@chromium.org zelidrag@chromium.org diff --git a/chromium/google_apis/gaia/account_tracker.cc b/chromium/google_apis/gaia/account_tracker.cc index f424863c9e8..83aed2fb2d8 100644 --- a/chromium/google_apis/gaia/account_tracker.cc +++ b/chromium/google_apis/gaia/account_tracker.cc @@ -177,7 +177,7 @@ void AccountTracker::NotifySignInChanged(const AccountState& account) { OnAccountSignInChanged(account.ids, account.is_signed_in)); } -void AccountTracker::UpdateSignInState(const std::string account_key, +void AccountTracker::UpdateSignInState(const std::string& account_key, bool is_signed_in) { // TODO(robliao): Remove ScopedTracker below once https://crbug.com/422460 is // fixed. @@ -198,7 +198,7 @@ void AccountTracker::UpdateSignInState(const std::string account_key, NotifySignInChanged(account); } -void AccountTracker::StartTrackingAccount(const std::string account_key) { +void AccountTracker::StartTrackingAccount(const std::string& account_key) { // TODO(robliao): Remove ScopedTracker below once https://crbug.com/422460 is // fixed. tracked_objects::ScopedTracker tracking_profile( @@ -235,7 +235,7 @@ void AccountTracker::StopTrackingAllAccounts() { StopTrackingAccount(accounts_.begin()->first); } -void AccountTracker::StartFetchingUserInfo(const std::string account_key) { +void AccountTracker::StartFetchingUserInfo(const std::string& account_key) { // TODO(robliao): Remove ScopedTracker below once https://crbug.com/422460 is // fixed. tracked_objects::ScopedTracker tracking_profile( diff --git a/chromium/google_apis/gaia/account_tracker.h b/chromium/google_apis/gaia/account_tracker.h index 61d3748868b..c0b65ff48f5 100644 --- a/chromium/google_apis/gaia/account_tracker.h +++ b/chromium/google_apis/gaia/account_tracker.h @@ -97,12 +97,17 @@ class AccountTracker : public OAuth2TokenService::Observer, void NotifyAccountRemoved(const AccountState& account); void NotifySignInChanged(const AccountState& account); - void UpdateSignInState(const std::string account_key, bool is_signed_in); + void UpdateSignInState(const std::string& account_key, bool is_signed_in); - void StartTrackingAccount(const std::string account_key); + void StartTrackingAccount(const std::string& account_key); + + // Note: |account_key| is passed by value here, because the original + // object may be stored in |accounts_| and if so, it will be destroyed + // after erasing the key from the map. void StopTrackingAccount(const std::string account_key); + void StopTrackingAllAccounts(); - void StartFetchingUserInfo(const std::string account_key); + void StartFetchingUserInfo(const std::string& account_key); void DeleteFetcher(AccountIdFetcher* fetcher); IdentityProvider* identity_provider_; // Not owned. diff --git a/chromium/google_apis/gaia/account_tracker_unittest.cc b/chromium/google_apis/gaia/account_tracker_unittest.cc index 61fded91a1a..e846bb14dab 100644 --- a/chromium/google_apis/gaia/account_tracker_unittest.cc +++ b/chromium/google_apis/gaia/account_tracker_unittest.cc @@ -29,7 +29,7 @@ enum TrackingEventType { SIGN_OUT }; -std::string AccountKeyToObfuscatedId(const std::string email) { +std::string AccountKeyToObfuscatedId(const std::string& email) { return "obfid-" + email; } @@ -294,7 +294,7 @@ class IdentityAccountTrackerTest : public testing::Test { // Helpers to pass fake events to the tracker. - void NotifyLogin(const std::string account_key) { + void NotifyLogin(const std::string& account_key) { identity_provider()->LogIn(account_key); } @@ -314,7 +314,7 @@ class IdentityAccountTrackerTest : public testing::Test { username, "access_token-" + username, base::Time::Max()); } - std::string GetValidTokenInfoResponse(const std::string account_key) { + std::string GetValidTokenInfoResponse(const std::string& account_key) { return std::string("{ \"id\": \"") + AccountKeyToObfuscatedId(account_key) + "\" }"; } diff --git a/chromium/google_apis/gaia/fake_gaia.cc b/chromium/google_apis/gaia/fake_gaia.cc index bbf52161d0e..699c2c680c9 100644 --- a/chromium/google_apis/gaia/fake_gaia.cc +++ b/chromium/google_apis/gaia/fake_gaia.cc @@ -71,17 +71,13 @@ typedef std::map<std::string, std::string> CookieMap; // Parses cookie name-value map our of |request|. CookieMap GetRequestCookies(const HttpRequest& request) { CookieMap result; - std::map<std::string, std::string>::const_iterator iter = - request.headers.find("Cookie"); + auto iter = request.headers.find("Cookie"); if (iter != request.headers.end()) { - std::vector<std::string> cookie_nv_pairs; - base::SplitString(iter->second, ' ', &cookie_nv_pairs); - for(std::vector<std::string>::const_iterator cookie_line = - cookie_nv_pairs.begin(); - cookie_line != cookie_nv_pairs.end(); - ++cookie_line) { - std::vector<std::string> name_value; - base::SplitString(*cookie_line, '=', &name_value); + for (const std::string& cookie_line : + base::SplitString(iter->second, " ", base::TRIM_WHITESPACE, + base::SPLIT_WANT_ALL)) { + std::vector<std::string> name_value = base::SplitString( + cookie_line, "=", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL); if (name_value.size() != 2) continue; @@ -102,8 +98,8 @@ bool GetAccessToken(const HttpRequest& request, std::map<std::string, std::string>::const_iterator auth_header_entry = request.headers.find("Authorization"); if (auth_header_entry != request.headers.end()) { - if (base::StartsWithASCII(auth_header_entry->second, auth_token_prefix, - true)) { + if (base::StartsWith(auth_header_entry->second, auth_token_prefix, + base::CompareCase::SENSITIVE)) { *access_token = auth_header_entry->second.substr( strlen(auth_token_prefix)); return true; @@ -441,8 +437,8 @@ const FakeGaia::AccessTokenInfo* FakeGaia::FindAccessTokenInfo( if (auth_token.empty() || client_id.empty()) return NULL; - std::vector<std::string> scope_list; - base::SplitString(scope_string, ' ', &scope_list); + std::vector<std::string> scope_list = base::SplitString( + scope_string, " ", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL); ScopeSet scopes(scope_list.begin(), scope_list.end()); for (AccessTokenInfoMap::const_iterator entry( @@ -480,6 +476,15 @@ void FakeGaia::HandleServiceLogin(const HttpRequest& request, void FakeGaia::HandleEmbeddedSetupChromeos(const HttpRequest& request, BasicHttpResponse* http_response) { + GURL request_url = GURL("http://localhost").Resolve(request.relative_url); + std::string client_id; + if (!GetQueryParameter(request_url.query(), "client_id", &client_id) || + GaiaUrls::GetInstance()->oauth2_chrome_client_id() != client_id) { + LOG(ERROR) << "Missing or invalid param 'client_id' in " + "/embedded/setup/chromeos call"; + return; + } + http_response->set_code(net::HTTP_OK); http_response->set_content(embedded_setup_chromeos_response_); http_response->set_content_type("text/html"); @@ -577,7 +582,7 @@ void FakeGaia::HandleEmbeddedLookupAccountLookup( url = net::AppendQueryParameter(url, "SAMLRequest", "fake_request"); url = net::AppendQueryParameter( url, "RelayState", - "chrome-extension://mfffpogegjflfpflabcdkioaeobkgjik/success.html"); + GaiaUrls::GetInstance()->signin_completed_continue_url().spec()); std::string redirect_url = url.spec(); http_response->AddCustomHeader("Google-Accounts-SAML", "Start"); @@ -708,7 +713,7 @@ void FakeGaia::HandleTokenInfo(const HttpRequest& request, response_dict.SetString("user_id", token_info->user_id); std::vector<std::string> scope_vector(token_info->scopes.begin(), token_info->scopes.end()); - response_dict.SetString("scope", JoinString(scope_vector, " ")); + response_dict.SetString("scope", base::JoinString(scope_vector, " ")); response_dict.SetInteger("expires_in", token_info->expires_in); response_dict.SetString("email", token_info->email); FormatJSONResponse(response_dict, http_response); diff --git a/chromium/google_apis/gaia/gaia_auth_fetcher.cc b/chromium/google_apis/gaia/gaia_auth_fetcher.cc index 9175bd2fc09..1423359b4ae 100644 --- a/chromium/google_apis/gaia/gaia_auth_fetcher.cc +++ b/chromium/google_apis/gaia/gaia_auth_fetcher.cc @@ -76,25 +76,6 @@ const char kGetTokenResponseRequested[] = "get_token"; } // namespace -// TODO(chron): Add sourceless version of this formatter. -// static -const char GaiaAuthFetcher::kClientLoginFormat[] = - "Email=%s&" - "Passwd=%s&" - "PersistentCookie=%s&" - "accountType=%s&" - "source=%s&" - "service=%s"; -// static -const char GaiaAuthFetcher::kClientLoginCaptchaFormat[] = - "Email=%s&" - "Passwd=%s&" - "PersistentCookie=%s&" - "accountType=%s&" - "source=%s&" - "service=%s&" - "logintoken=%s&" - "logincaptcha=%s"; // static const char GaiaAuthFetcher::kIssueAuthTokenFormat[] = "SID=%s&" @@ -153,16 +134,6 @@ const char GaiaAuthFetcher::kCaptchaUrlParam[] = "CaptchaUrl"; const char GaiaAuthFetcher::kCaptchaTokenParam[] = "CaptchaToken"; // static -const char GaiaAuthFetcher::kCookiePersistence[] = "true"; -// static -// TODO(johnnyg): When hosted accounts are supported by sync, -// we can always use "HOSTED_OR_GOOGLE" -const char GaiaAuthFetcher::kAccountTypeHostedOrGoogle[] = - "HOSTED_OR_GOOGLE"; -const char GaiaAuthFetcher::kAccountTypeGoogle[] = - "GOOGLE"; - -// static const char GaiaAuthFetcher::kSecondFactor[] = "Info=InvalidSecondFactor"; // static const char GaiaAuthFetcher::kWebLoginRequired[] = "Info=WebLoginRequired"; @@ -195,7 +166,6 @@ GaiaAuthFetcher::GaiaAuthFetcher(GaiaAuthConsumer* consumer, : consumer_(consumer), getter_(getter), source_(source), - client_login_gurl_(GaiaUrls::GetInstance()->client_login_url()), issue_auth_token_gurl_(GaiaUrls::GetInstance()->issue_auth_token_url()), oauth2_token_gurl_(GaiaUrls::GetInstance()->oauth2_token_url()), oauth2_revoke_gurl_(GaiaUrls::GetInstance()->oauth2_revoke_url()), @@ -266,47 +236,6 @@ void GaiaAuthFetcher::CreateAndStartGaiaFetcher(const std::string& body, } // static -std::string GaiaAuthFetcher::MakeClientLoginBody( - const std::string& username, - const std::string& password, - const std::string& source, - const char* service, - const std::string& login_token, - const std::string& login_captcha, - HostedAccountsSetting allow_hosted_accounts) { - std::string encoded_username = net::EscapeUrlEncodedData(username, true); - std::string encoded_password = net::EscapeUrlEncodedData(password, true); - std::string encoded_login_token = net::EscapeUrlEncodedData(login_token, - true); - std::string encoded_login_captcha = net::EscapeUrlEncodedData(login_captcha, - true); - - const char* account_type = allow_hosted_accounts == HostedAccountsAllowed ? - kAccountTypeHostedOrGoogle : - kAccountTypeGoogle; - - if (login_token.empty() || login_captcha.empty()) { - return base::StringPrintf(kClientLoginFormat, - encoded_username.c_str(), - encoded_password.c_str(), - kCookiePersistence, - account_type, - source.c_str(), - service); - } - - return base::StringPrintf(kClientLoginCaptchaFormat, - encoded_username.c_str(), - encoded_password.c_str(), - kCookiePersistence, - account_type, - source.c_str(), - service, - encoded_login_token.c_str(), - encoded_login_captcha.c_str()); -} - -// static std::string GaiaAuthFetcher::MakeIssueAuthTokenBody( const std::string& sid, const std::string& lsid, @@ -508,8 +437,8 @@ bool GaiaAuthFetcher::ParseClientLoginToOAuth2Response( // static bool GaiaAuthFetcher::ParseClientLoginToOAuth2Cookie(const std::string& cookie, std::string* auth_code) { - std::vector<std::string> parts; - base::SplitString(cookie, ';', &parts); + std::vector<std::string> parts = base::SplitString( + cookie, ";", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL); // Per documentation, the cookie should have Secure and HttpOnly. if (!CookiePartsContains(parts, kClientLoginToOAuth2CookiePartSecure) || !CookiePartsContains(parts, kClientLoginToOAuth2CookiePartHttpOnly)) { @@ -519,8 +448,8 @@ bool GaiaAuthFetcher::ParseClientLoginToOAuth2Cookie(const std::string& cookie, std::vector<std::string>::const_iterator iter; for (iter = parts.begin(); iter != parts.end(); ++iter) { const std::string& part = *iter; - if (base::StartsWithASCII(part, kClientLoginToOAuth2CookiePartCodePrefix, - false)) { + if (base::StartsWith(part, kClientLoginToOAuth2CookiePartCodePrefix, + base::CompareCase::INSENSITIVE_ASCII)) { auth_code->assign(part.substr( kClientLoginToOAuth2CookiePartCodePrefixLength)); return true; @@ -562,32 +491,6 @@ bool GaiaAuthFetcher::ParseListIdpSessionsResponse(const std::string& data, return true; } -void GaiaAuthFetcher::StartClientLogin( - const std::string& username, - const std::string& password, - const char* const service, - const std::string& login_token, - const std::string& login_captcha, - HostedAccountsSetting allow_hosted_accounts) { - - DCHECK(!fetch_pending_) << "Tried to fetch two things at once!"; - - // This class is thread agnostic, so be sure to call this only on the - // same thread each time. - DVLOG(1) << "Starting new ClientLogin fetch for:" << username; - - // Must outlive fetcher_. - request_body_ = MakeClientLoginBody(username, - password, - source_, - service, - login_token, - login_captcha, - allow_hosted_accounts); - CreateAndStartGaiaFetcher(request_body_, std::string(), client_login_gurl_, - kLoadFlagsIgnoreCookies); -} - void GaiaAuthFetcher::StartIssueAuthToken(const std::string& sid, const std::string& lsid, const char* const service) { @@ -807,22 +710,6 @@ GoogleServiceAuthError GaiaAuthFetcher::GenerateAuthError( return GoogleServiceAuthError(GoogleServiceAuthError::SERVICE_UNAVAILABLE); } -void GaiaAuthFetcher::OnClientLoginFetched(const std::string& data, - const net::URLRequestStatus& status, - int response_code) { - if (status.is_success() && response_code == net::HTTP_OK) { - DVLOG(1) << "ClientLogin successful!"; - std::string sid; - std::string lsid; - std::string token; - ParseClientLoginResponse(data, &sid, &lsid, &token); - consumer_->OnClientLoginSuccess( - GaiaAuthConsumer::ClientLoginResult(sid, lsid, token, data)); - } else { - consumer_->OnClientLoginFailure(GenerateAuthError(data, status)); - } -} - void GaiaAuthFetcher::OnIssueAuthTokenFetched( const std::string& data, const net::URLRequestStatus& status, @@ -1047,12 +934,11 @@ void GaiaAuthFetcher::DispatchFetchedRequest( const net::ResponseCookies& cookies, const net::URLRequestStatus& status, int response_code) { - if (url == client_login_gurl_) { - OnClientLoginFetched(data, status, response_code); - } else if (url == issue_auth_token_gurl_) { + if (url == issue_auth_token_gurl_) { OnIssueAuthTokenFetched(data, status, response_code); - } else if (base::StartsWithASCII(url.spec(), - client_login_to_oauth2_gurl_.spec(), true)) { + } else if (base::StartsWith(url.spec(), + client_login_to_oauth2_gurl_.spec(), + base::CompareCase::SENSITIVE)) { OnClientLoginToOAuth2Fetched(data, cookies, status, response_code); } else if (url == oauth2_token_gurl_) { OnOAuth2TokenPairFetched(data, status, response_code); diff --git a/chromium/google_apis/gaia/gaia_auth_fetcher.h b/chromium/google_apis/gaia/gaia_auth_fetcher.h index 4a9838f54e5..f6e4aa86154 100644 --- a/chromium/google_apis/gaia/gaia_auth_fetcher.h +++ b/chromium/google_apis/gaia/gaia_auth_fetcher.h @@ -35,11 +35,6 @@ class URLRequestStatus; class GaiaAuthFetcher : public net::URLFetcherDelegate { public: - enum HostedAccountsSetting { - HostedAccountsAllowed, - HostedAccountsNotAllowed - }; - // Magic string indicating that, while a second factor is still // needed to complete authentication, the user provided the right password. static const char kSecondFactor[]; @@ -55,23 +50,6 @@ class GaiaAuthFetcher : public net::URLFetcherDelegate { net::URLRequestContextGetter* getter); ~GaiaAuthFetcher() override; - // Start a request to obtain the SID and LSID cookies for the the account - // identified by |username| and |password|. If |service| is not null or - // empty, then also obtains a service token for specified service. - // - // If this is a second call because of captcha challenge, then the - // |login_token| and |login_captcha| arugment should correspond to the - // solution of the challenge. - // - // Either OnClientLoginSuccess or OnClientLoginFailure will be - // called on the consumer on the original thread. - void StartClientLogin(const std::string& username, - const std::string& password, - const char* const service, - const std::string& login_token, - const std::string& login_captcha, - HostedAccountsSetting allow_hosted_accounts); - // Start a request to obtain service token for the the account identified by // |sid| and |lsid| and the |service|. // @@ -251,15 +229,6 @@ class GaiaAuthFetcher : public net::URLFetcherDelegate { void SetPendingFetch(bool pending_fetch); private: - // ClientLogin body constants that don't change - static const char kCookiePersistence[]; - static const char kAccountTypeHostedOrGoogle[]; - static const char kAccountTypeGoogle[]; - - // The format of the POST body for ClientLogin. - static const char kClientLoginFormat[]; - // The format of said POST body when CAPTCHA token & answer are specified. - static const char kClientLoginCaptchaFormat[]; // The format of the POST body for IssueAuthToken. static const char kIssueAuthTokenFormat[]; // The format of the query string to get OAuth2 auth code from auth token. @@ -397,15 +366,6 @@ class GaiaAuthFetcher : public net::URLFetcherDelegate { // Is this a special case Gaia error for Less Secure Apps? static bool IsWebLoginRequiredSuccess(const std::string& alleged_error); - // Given parameters, create a ClientLogin request body. - static std::string MakeClientLoginBody( - const std::string& username, - const std::string& password, - const std::string& source, - const char* const service, - const std::string& login_token, - const std::string& login_captcha, - HostedAccountsSetting allow_hosted_accounts); // Supply the sid / lsid returned from ClientLogin in order to // request a long lived auth token for a service. static std::string MakeIssueAuthTokenBody(const std::string& sid, @@ -450,7 +410,6 @@ class GaiaAuthFetcher : public net::URLFetcherDelegate { GaiaAuthConsumer* const consumer_; net::URLRequestContextGetter* const getter_; std::string source_; - const GURL client_login_gurl_; const GURL issue_auth_token_gurl_; const GURL oauth2_token_gurl_; const GURL oauth2_revoke_gurl_; diff --git a/chromium/google_apis/gaia/gaia_auth_fetcher_unittest.cc b/chromium/google_apis/gaia/gaia_auth_fetcher_unittest.cc index 28e5aff83c6..307a2bcec2b 100644 --- a/chromium/google_apis/gaia/gaia_auth_fetcher_unittest.cc +++ b/chromium/google_apis/gaia/gaia_auth_fetcher_unittest.cc @@ -53,17 +53,16 @@ MockFetcher::MockFetcher(bool success, net::URLFetcherDelegate* d) : TestURLFetcher(0, url, d) { set_url(url); - net::URLRequestStatus::Status code; + net::Error error; if (success) { + error = net::OK; set_response_code(net::HTTP_OK); - code = net::URLRequestStatus::SUCCESS; } else { - set_response_code(net::HTTP_FORBIDDEN); - code = net::URLRequestStatus::FAILED; + error = net::ERR_FAILED; } - set_status(net::URLRequestStatus(code, 0)); + set_status(net::URLRequestStatus::FromError(error)); SetResponseString(results); } @@ -91,17 +90,16 @@ void MockFetcher::Start() { class GaiaAuthFetcherTest : public testing::Test { protected: GaiaAuthFetcherTest() - : client_login_source_(GaiaUrls::GetInstance()->client_login_url()), - issue_auth_token_source_( - GaiaUrls::GetInstance()->issue_auth_token_url()), - client_login_to_oauth2_source_( - GaiaUrls::GetInstance()->client_login_to_oauth2_url()), + : issue_auth_token_source_(GaiaUrls::GetInstance() + ->issue_auth_token_url()), + client_login_to_oauth2_source_(GaiaUrls::GetInstance() + ->client_login_to_oauth2_url()), oauth2_token_source_(GaiaUrls::GetInstance()->oauth2_token_url()), token_auth_source_(GaiaUrls::GetInstance()->token_auth_url()), merge_session_source_(GaiaUrls::GetInstance()->merge_session_url()), - uberauth_token_source_( - GaiaUrls::GetInstance()->oauth1_login_url().Resolve( - "?source=&issueuberauth=1")), + uberauth_token_source_(GaiaUrls::GetInstance() + ->oauth1_login_url() + .Resolve("?source=&issueuberauth=1")), oauth_login_gurl_(GaiaUrls::GetInstance()->oauth1_login_url()) {} void RunParsingTest(const std::string& data, @@ -143,7 +141,6 @@ class GaiaAuthFetcherTest : public testing::Test { } net::ResponseCookies cookies_; - GURL client_login_source_; GURL issue_auth_token_source_; GURL client_login_to_oauth2_source_; GURL oauth2_token_source_; @@ -224,25 +221,6 @@ TEST_F(GaiaAuthFetcherTest, MAYBE_ErrorComparator) { EXPECT_TRUE(expected_error == matching_error); } -TEST_F(GaiaAuthFetcherTest, LoginNetFailure) { - int error_no = net::ERR_CONNECTION_RESET; - net::URLRequestStatus status(net::URLRequestStatus::FAILED, error_no); - - GoogleServiceAuthError expected_error = - GoogleServiceAuthError::FromConnectionError(error_no); - - MockGaiaConsumer consumer; - EXPECT_CALL(consumer, OnClientLoginFailure(expected_error)) - .Times(1); - - GaiaAuthFetcher auth(&consumer, std::string(), GetRequestContext()); - - MockFetcher mock_fetcher( - client_login_source_, status, 0, net::ResponseCookies(), std::string(), - net::URLFetcher::GET, &auth); - auth.OnURLFetchComplete(&mock_fetcher); -} - TEST_F(GaiaAuthFetcherTest, TokenNetFailure) { int error_no = net::ERR_CONNECTION_RESET; net::URLRequestStatus status(net::URLRequestStatus::FAILED, error_no); @@ -263,25 +241,6 @@ TEST_F(GaiaAuthFetcherTest, TokenNetFailure) { } -TEST_F(GaiaAuthFetcherTest, LoginDenied) { - std::string data("Error=BadAuthentication"); - net::URLRequestStatus status(net::URLRequestStatus::SUCCESS, 0); - - GoogleServiceAuthError expected_error( - GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS); - - MockGaiaConsumer consumer; - EXPECT_CALL(consumer, OnClientLoginFailure(expected_error)) - .Times(1); - - GaiaAuthFetcher auth(&consumer, std::string(), GetRequestContext()); - - MockFetcher mock_fetcher( - client_login_source_, status, net::HTTP_FORBIDDEN, cookies_, data, - net::URLFetcher::GET, &auth); - auth.OnURLFetchComplete(&mock_fetcher); -} - TEST_F(GaiaAuthFetcherTest, ParseRequest) { RunParsingTest("SID=sid\nLSID=lsid\nAuth=auth\n", "sid", "lsid", "auth"); RunParsingTest("LSID=lsid\nSID=sid\nAuth=auth\n", "sid", "lsid", "auth"); @@ -307,28 +266,6 @@ TEST_F(GaiaAuthFetcherTest, ParseErrorRequest) { "CaptchaUrl=C\n", "E", "U", "C", "T"); } - -TEST_F(GaiaAuthFetcherTest, OnlineLogin) { - std::string data("SID=sid\nLSID=lsid\nAuth=auth\n"); - - GaiaAuthConsumer::ClientLoginResult result; - result.lsid = "lsid"; - result.sid = "sid"; - result.token = "auth"; - result.data = data; - - MockGaiaConsumer consumer; - EXPECT_CALL(consumer, OnClientLoginSuccess(result)) - .Times(1); - - GaiaAuthFetcher auth(&consumer, std::string(), GetRequestContext()); - net::URLRequestStatus status(net::URLRequestStatus::SUCCESS, 0); - MockFetcher mock_fetcher( - client_login_source_, status, net::HTTP_OK, cookies_, data, - net::URLFetcher::GET, &auth); - auth.OnURLFetchComplete(&mock_fetcher); -} - TEST_F(GaiaAuthFetcherTest, WorkingIssueAuthToken) { MockGaiaConsumer consumer; EXPECT_CALL(consumer, OnIssueAuthTokenSuccess(_, "token")) @@ -354,44 +291,6 @@ TEST_F(GaiaAuthFetcherTest, CheckNormalErrorCode) { EXPECT_FALSE(GaiaAuthFetcher::IsSecondFactorSuccess(response)); } -TEST_F(GaiaAuthFetcherTest, TwoFactorLogin) { - std::string response = base::StringPrintf("Error=BadAuthentication\n%s\n", - GaiaAuthFetcher::kSecondFactor); - - GoogleServiceAuthError error = - GoogleServiceAuthError(GoogleServiceAuthError::TWO_FACTOR); - - MockGaiaConsumer consumer; - EXPECT_CALL(consumer, OnClientLoginFailure(error)) - .Times(1); - - GaiaAuthFetcher auth(&consumer, std::string(), GetRequestContext()); - net::URLRequestStatus status(net::URLRequestStatus::SUCCESS, 0); - MockFetcher mock_fetcher( - client_login_source_, status, net::HTTP_FORBIDDEN, cookies_, response, - net::URLFetcher::GET, &auth); - auth.OnURLFetchComplete(&mock_fetcher); -} - -TEST_F(GaiaAuthFetcherTest, WebLoginRequired) { - std::string response = base::StringPrintf("Error=BadAuthentication\n%s\n", - GaiaAuthFetcher::kWebLoginRequired); - - GoogleServiceAuthError error = - GoogleServiceAuthError(GoogleServiceAuthError::WEB_LOGIN_REQUIRED); - - MockGaiaConsumer consumer; - EXPECT_CALL(consumer, OnClientLoginFailure(error)) - .Times(1); - - GaiaAuthFetcher auth(&consumer, std::string(), GetRequestContext()); - net::URLRequestStatus status(net::URLRequestStatus::SUCCESS, 0); - MockFetcher mock_fetcher( - client_login_source_, status, net::HTTP_FORBIDDEN, cookies_, response, - net::URLFetcher::GET, &auth); - auth.OnURLFetchComplete(&mock_fetcher); -} - TEST_F(GaiaAuthFetcherTest, CaptchaParse) { net::URLRequestStatus status(net::URLRequestStatus::SUCCESS, 0); std::string data = "Url=http://www.google.com/login/captcha\n" @@ -451,64 +350,6 @@ TEST_F(GaiaAuthFetcherTest, ServiceUnavailableError) { EXPECT_EQ(error.state(), GoogleServiceAuthError::SERVICE_UNAVAILABLE); } -TEST_F(GaiaAuthFetcherTest, FullLogin) { - MockGaiaConsumer consumer; - EXPECT_CALL(consumer, OnClientLoginSuccess(_)) - .Times(1); - - MockURLFetcherFactory<MockFetcher> factory; - - GaiaAuthFetcher auth(&consumer, std::string(), GetRequestContext()); - auth.StartClientLogin("username", - "password", - "service", - std::string(), - std::string(), - GaiaAuthFetcher::HostedAccountsAllowed); -} - -TEST_F(GaiaAuthFetcherTest, FullLoginFailure) { - MockGaiaConsumer consumer; - EXPECT_CALL(consumer, OnClientLoginFailure(_)) - .Times(1); - - MockURLFetcherFactory<MockFetcher> factory; - factory.set_success(false); - - GaiaAuthFetcher auth(&consumer, std::string(), GetRequestContext()); - auth.StartClientLogin("username", - "password", - "service", - std::string(), - std::string(), - GaiaAuthFetcher::HostedAccountsAllowed); -} - -TEST_F(GaiaAuthFetcherTest, ClientFetchPending) { - MockGaiaConsumer consumer; - EXPECT_CALL(consumer, OnClientLoginSuccess(_)) - .Times(1); - - net::TestURLFetcherFactory factory; - - GaiaAuthFetcher auth(&consumer, std::string(), GetRequestContext()); - auth.StartClientLogin("username", - "password", - "service", - std::string(), - std::string(), - GaiaAuthFetcher::HostedAccountsAllowed); - - EXPECT_TRUE(auth.HasPendingFetch()); - MockFetcher mock_fetcher( - client_login_source_, - net::URLRequestStatus(net::URLRequestStatus::SUCCESS, 0), - net::HTTP_OK, cookies_, "SID=sid\nLSID=lsid\nAuth=auth\n", - net::URLFetcher::GET, &auth); - auth.OnURLFetchComplete(&mock_fetcher); - EXPECT_FALSE(auth.HasPendingFetch()); -} - TEST_F(GaiaAuthFetcherTest, FullTokenSuccess) { MockGaiaConsumer consumer; EXPECT_CALL(consumer, OnIssueAuthTokenSuccess("service", "token")) diff --git a/chromium/google_apis/gaia/gaia_auth_util.cc b/chromium/google_apis/gaia/gaia_auth_util.cc index e9ec5454dda..caf802f9954 100644 --- a/chromium/google_apis/gaia/gaia_auth_util.cc +++ b/chromium/google_apis/gaia/gaia_auth_util.cc @@ -21,9 +21,8 @@ const char kGooglemailDomain[] = "googlemail.com"; std::string CanonicalizeEmailImpl(const std::string& email_address, bool change_googlemail_to_gmail) { - std::vector<std::string> parts; - char at = '@'; - base::SplitString(email_address, at, &parts); + std::vector<std::string> parts = base::SplitString( + email_address, "@", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL); if (parts.size() != 2U) { NOTREACHED() << "expecting exactly one @, but got " << (parts.empty() ? 0 : parts.size() - 1) @@ -36,7 +35,7 @@ std::string CanonicalizeEmailImpl(const std::string& email_address, base::RemoveChars(parts[0], ".", &parts[0]); } - std::string new_email = base::StringToLowerASCII(JoinString(parts, at)); + std::string new_email = base::ToLowerASCII(base::JoinString(parts, "@")); VLOG(1) << "Canonicalized " << email_address << " to " << new_email; return new_email; } @@ -72,7 +71,7 @@ std::string CanonicalizeEmail(const std::string& email_address) { std::string CanonicalizeDomain(const std::string& domain) { // Canonicalization of domain names means lower-casing them. Make sure to // update this function in sync with Canonicalize if this ever changes. - return base::StringToLowerASCII(domain); + return base::ToLowerASCII(domain); } std::string SanitizeEmail(const std::string& email_address) { diff --git a/chromium/google_apis/gaia/gaia_oauth_client.cc b/chromium/google_apis/gaia/gaia_oauth_client.cc index 14c3cc3016d..c021623b589 100644 --- a/chromium/google_apis/gaia/gaia_oauth_client.cc +++ b/chromium/google_apis/gaia/gaia_oauth_client.cc @@ -137,7 +137,7 @@ void GaiaOAuthClient::Core::RefreshToken( "&grant_type=refresh_token"; if (!scopes.empty()) { - std::string scopes_string = JoinString(scopes, ' '); + std::string scopes_string = base::JoinString(scopes, " "); post_body += "&scope=" + net::EscapeUrlEncodedData(scopes_string, true); } diff --git a/chromium/google_apis/gaia/gaia_oauth_client_unittest.cc b/chromium/google_apis/gaia/gaia_oauth_client_unittest.cc index 18bda59e297..b032b811dc4 100644 --- a/chromium/google_apis/gaia/gaia_oauth_client_unittest.cc +++ b/chromium/google_apis/gaia/gaia_oauth_client_unittest.cc @@ -56,12 +56,12 @@ class MockOAuthFetcher : public net::TestURLFetcher { set_response_code(net::HTTP_OK); } - net::URLRequestStatus::Status code = net::URLRequestStatus::SUCCESS; + net::Error error = net::OK; if (GetResponseCode() != net::HTTP_OK) { - code = net::URLRequestStatus::FAILED; + error = net::ERR_FAILED; current_failure_count_++; } - set_status(net::URLRequestStatus(code, 0)); + set_status(net::URLRequestStatus::FromError(error)); if (complete_immediately_) delegate()->OnURLFetchComplete(this); @@ -367,8 +367,8 @@ TEST_F(GaiaOAuthClientTest, GetUserInfo) { GaiaOAuthClient auth(GetRequestContext()); auth.GetUserInfo("access_token", 1, &delegate); - scoped_ptr<base::Value> value( - base::JSONReader::DeprecatedRead(kDummyFullUserInfoResult)); + scoped_ptr<base::Value> value = + base::JSONReader::Read(kDummyFullUserInfoResult); DCHECK(value); ASSERT_TRUE(value->IsType(base::Value::TYPE_DICTIONARY)); base::DictionaryValue* expected_result; diff --git a/chromium/google_apis/gaia/gaia_urls.cc b/chromium/google_apis/gaia/gaia_urls.cc index 0d1619e61c6..dae39b22c87 100644 --- a/chromium/google_apis/gaia/gaia_urls.cc +++ b/chromium/google_apis/gaia/gaia_urls.cc @@ -73,7 +73,7 @@ GURL GetURLSwitchValueWithDefault(const char* switch_value, } // namespace GaiaUrls* GaiaUrls::GetInstance() { - return Singleton<GaiaUrls>::get(); + return base::Singleton<GaiaUrls>::get(); } GaiaUrls::GaiaUrls() { @@ -283,3 +283,8 @@ GURL GaiaUrls::GetCheckConnectionInfoURLWithSource(const std::string& source) { : get_check_connection_info_url_.Resolve( base::StringPrintf("?source=%s", source.c_str())); } + +GURL GaiaUrls::signin_completed_continue_url() const { + return + GURL("chrome-extension://mfffpogegjflfpflabcdkioaeobkgjik/success.html"); +} diff --git a/chromium/google_apis/gaia/gaia_urls.h b/chromium/google_apis/gaia/gaia_urls.h index 4db875ac54f..b67b17716ce 100644 --- a/chromium/google_apis/gaia/gaia_urls.h +++ b/chromium/google_apis/gaia/gaia_urls.h @@ -53,11 +53,14 @@ class GaiaUrls { GURL LogOutURLWithSource(const std::string& source); GURL GetCheckConnectionInfoURLWithSource(const std::string& source); + // Continue URL used to signal the completion of the signin flow. + GURL signin_completed_continue_url() const; + private: GaiaUrls(); ~GaiaUrls(); - friend struct DefaultSingletonTraits<GaiaUrls>; + friend struct base::DefaultSingletonTraits<GaiaUrls>; GURL google_url_; GURL gaia_url_; diff --git a/chromium/google_apis/gaia/oauth2_access_token_fetcher_impl.cc b/chromium/google_apis/gaia/oauth2_access_token_fetcher_impl.cc index b513b78b5eb..1945c7946d8 100644 --- a/chromium/google_apis/gaia/oauth2_access_token_fetcher_impl.cc +++ b/chromium/google_apis/gaia/oauth2_access_token_fetcher_impl.cc @@ -283,7 +283,7 @@ std::string OAuth2AccessTokenFetcherImpl::MakeGetAccessTokenBody( enc_client_secret.c_str(), enc_refresh_token.c_str()); } else { - std::string scopes_string = JoinString(scopes, ' '); + std::string scopes_string = base::JoinString(scopes, " "); return base::StringPrintf( kGetAccessTokenBodyWithScopeFormat, enc_client_id.c_str(), diff --git a/chromium/google_apis/gaia/oauth2_access_token_fetcher_impl_unittest.cc b/chromium/google_apis/gaia/oauth2_access_token_fetcher_impl_unittest.cc index 55b50db05aa..f234b39d907 100644 --- a/chromium/google_apis/gaia/oauth2_access_token_fetcher_impl_unittest.cc +++ b/chromium/google_apis/gaia/oauth2_access_token_fetcher_impl_unittest.cc @@ -13,6 +13,7 @@ #include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/oauth2_access_token_consumer.h" #include "google_apis/gaia/oauth2_access_token_fetcher_impl.h" +#include "net/base/net_errors.h" #include "net/http/http_status_code.h" #include "net/url_request/test_url_fetcher_factory.h" #include "net/url_request/url_fetcher.h" @@ -106,9 +107,8 @@ class OAuth2AccessTokenFetcherImplTest : public testing::Test { const std::string& body) { GURL url(GaiaUrls::GetInstance()->oauth2_token_url()); TestURLFetcher* url_fetcher = new TestURLFetcher(0, url, &fetcher_); - URLRequestStatus::Status status = - fetch_succeeds ? URLRequestStatus::SUCCESS : URLRequestStatus::FAILED; - url_fetcher->set_status(URLRequestStatus(status, 0)); + net::Error error = fetch_succeeds ? net::OK : net::ERR_FAILED; + url_fetcher->set_status(URLRequestStatus::FromError(error)); if (response_code != 0) url_fetcher->set_response_code(response_code); diff --git a/chromium/google_apis/gaia/oauth2_api_call_flow.cc b/chromium/google_apis/gaia/oauth2_api_call_flow.cc index d02c8c6181d..19d5b06e77b 100644 --- a/chromium/google_apis/gaia/oauth2_api_call_flow.cc +++ b/chromium/google_apis/gaia/oauth2_api_call_flow.cc @@ -13,7 +13,6 @@ #include "net/base/escape.h" #include "net/base/load_flags.h" #include "net/http/http_status_code.h" -#include "net/url_request/url_fetcher.h" #include "net/url_request/url_request_context_getter.h" #include "net/url_request/url_request_status.h" @@ -65,6 +64,11 @@ std::string OAuth2ApiCallFlow::CreateApiCallBodyContentType() { return "application/x-www-form-urlencoded"; } +net::URLFetcher::RequestType OAuth2ApiCallFlow::GetRequestTypeForBody( + const std::string& body) { + return body.empty() ? URLFetcher::GET : URLFetcher::POST; +} + void OAuth2ApiCallFlow::OnURLFetchComplete(const net::URLFetcher* source) { CHECK(source); CHECK_EQ(API_CALL_STARTED, state_); @@ -75,10 +79,9 @@ scoped_ptr<URLFetcher> OAuth2ApiCallFlow::CreateURLFetcher( net::URLRequestContextGetter* context, const std::string& access_token) { std::string body = CreateApiCallBody(); - bool empty_body = body.empty(); - scoped_ptr<URLFetcher> result = net::URLFetcher::Create( - 0, CreateApiCallUrl(), empty_body ? URLFetcher::GET : URLFetcher::POST, - this); + net::URLFetcher::RequestType request_type = GetRequestTypeForBody(body); + scoped_ptr<URLFetcher> result = + net::URLFetcher::Create(0, CreateApiCallUrl(), request_type, this); result->SetRequestContext(context); result->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES | @@ -90,7 +93,10 @@ scoped_ptr<URLFetcher> OAuth2ApiCallFlow::CreateURLFetcher( // http://crbug.com/163710 result->SetAutomaticallyRetryOnNetworkChanges(3); - if (!empty_body) + // Even if the the body is empty, we still set the Content-Type because an + // empty string may be a meaningful value. For example, a Protocol Buffer + // message with only default values will be serialized as an empty string. + if (request_type != net::URLFetcher::GET) result->SetUploadData(CreateApiCallBodyContentType(), body); return result; diff --git a/chromium/google_apis/gaia/oauth2_api_call_flow.h b/chromium/google_apis/gaia/oauth2_api_call_flow.h index 5aa3399e1c1..cfb833fc21c 100644 --- a/chromium/google_apis/gaia/oauth2_api_call_flow.h +++ b/chromium/google_apis/gaia/oauth2_api_call_flow.h @@ -7,8 +7,8 @@ #include <string> -#include "base/gtest_prod_util.h" #include "base/memory/scoped_ptr.h" +#include "net/url_request/url_fetcher.h" #include "net/url_request/url_fetcher_delegate.h" #include "url/gurl.h" @@ -45,6 +45,11 @@ class OAuth2ApiCallFlow : public net::URLFetcherDelegate { virtual std::string CreateApiCallBody() = 0; virtual std::string CreateApiCallBodyContentType(); + // Returns the request type (e.g. GET, POST) for the |body| that will be sent + // with the request. + virtual net::URLFetcher::RequestType GetRequestTypeForBody( + const std::string& body); + // Sub-classes can expose an appropriate observer interface by implementing // these template methods. // Called when the API call finished successfully. diff --git a/chromium/google_apis/gaia/oauth2_api_call_flow_unittest.cc b/chromium/google_apis/gaia/oauth2_api_call_flow_unittest.cc index d537bb7f08b..bf4192c19f6 100644 --- a/chromium/google_apis/gaia/oauth2_api_call_flow_unittest.cc +++ b/chromium/google_apis/gaia/oauth2_api_call_flow_unittest.cc @@ -15,6 +15,7 @@ #include "google_apis/gaia/oauth2_access_token_consumer.h" #include "google_apis/gaia/oauth2_access_token_fetcher_impl.h" #include "google_apis/gaia/oauth2_api_call_flow.h" +#include "net/base/net_errors.h" #include "net/http/http_request_headers.h" #include "net/http/http_status_code.h" #include "net/url_request/test_url_fetcher_factory.h" @@ -105,9 +106,8 @@ class OAuth2ApiCallFlowTest : public testing::Test { const GURL& url, bool fetch_succeeds, int response_code, const std::string& body) { TestURLFetcher* url_fetcher = new TestURLFetcher(0, url, &flow_); - URLRequestStatus::Status status = - fetch_succeeds ? URLRequestStatus::SUCCESS : URLRequestStatus::FAILED; - url_fetcher->set_status(URLRequestStatus(status, 0)); + net::Error error = fetch_succeeds ? net::OK : net::ERR_FAILED; + url_fetcher->set_status(URLRequestStatus::FromError(error)); if (response_code != 0) url_fetcher->set_response_code(response_code); diff --git a/chromium/google_apis/gaia/oauth2_mint_token_flow.cc b/chromium/google_apis/gaia/oauth2_mint_token_flow.cc index 0316c6328ea..c2a6501b35c 100644 --- a/chromium/google_apis/gaia/oauth2_mint_token_flow.cc +++ b/chromium/google_apis/gaia/oauth2_mint_token_flow.cc @@ -167,7 +167,7 @@ std::string OAuth2MintTokenFlow::CreateApiCallBody() { net::EscapeUrlEncodedData(force_value, true).c_str(), net::EscapeUrlEncodedData(response_type_value, true).c_str(), net::EscapeUrlEncodedData( - JoinString(parameters_.scopes, ' '), true).c_str(), + base::JoinString(parameters_.scopes, " "), true).c_str(), net::EscapeUrlEncodedData(parameters_.client_id, true).c_str(), net::EscapeUrlEncodedData(parameters_.extension_id, true).c_str()); if (!parameters_.device_id.empty()) { diff --git a/chromium/google_apis/gaia/oauth2_mint_token_flow_unittest.cc b/chromium/google_apis/gaia/oauth2_mint_token_flow_unittest.cc index 913d5553547..e9eecbca43c 100644 --- a/chromium/google_apis/gaia/oauth2_mint_token_flow_unittest.cc +++ b/chromium/google_apis/gaia/oauth2_mint_token_flow_unittest.cc @@ -14,6 +14,7 @@ #include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/oauth2_access_token_fetcher.h" #include "google_apis/gaia/oauth2_mint_token_flow.h" +#include "net/base/net_errors.h" #include "net/url_request/test_url_fetcher_factory.h" #include "net/url_request/url_request_status.h" #include "testing/gmock/include/gmock/gmock.h" @@ -352,14 +353,14 @@ TEST_F(OAuth2MintTokenFlowTest, ProcessApiCallSuccess) { TEST_F(OAuth2MintTokenFlowTest, ProcessApiCallFailure) { { // Null delegate should work fine. TestURLFetcher url_fetcher(1, GURL("http://www.google.com"), NULL); - url_fetcher.set_status(URLRequestStatus(URLRequestStatus::FAILED, 101)); + url_fetcher.set_status(URLRequestStatus::FromError(net::ERR_FAILED)); CreateFlow(NULL, OAuth2MintTokenFlow::MODE_MINT_TOKEN_NO_FORCE, ""); flow_->ProcessApiCallFailure(&url_fetcher); } { // Non-null delegate. TestURLFetcher url_fetcher(1, GURL("http://www.google.com"), NULL); - url_fetcher.set_status(URLRequestStatus(URLRequestStatus::FAILED, 101)); + url_fetcher.set_status(URLRequestStatus::FromError(net::ERR_FAILED)); CreateFlow(OAuth2MintTokenFlow::MODE_MINT_TOKEN_NO_FORCE); EXPECT_CALL(delegate_, OnMintTokenFailure(_)); flow_->ProcessApiCallFailure(&url_fetcher); diff --git a/chromium/google_apis/gaia/oauth2_token_service.cc b/chromium/google_apis/gaia/oauth2_token_service.cc index 01e5d96a5b6..0c7519b57fd 100644 --- a/chromium/google_apis/gaia/oauth2_token_service.cc +++ b/chromium/google_apis/gaia/oauth2_token_service.cc @@ -172,7 +172,7 @@ class OAuth2TokenService::Fetcher : public OAuth2AccessTokenConsumer { std::vector<base::WeakPtr<RequestImpl> > waiting_requests_; int retry_number_; - base::OneShotTimer<Fetcher> retry_timer_; + base::OneShotTimer retry_timer_; scoped_ptr<OAuth2AccessTokenFetcher> fetcher_; // Variables that store fetch results. diff --git a/chromium/google_apis/gaia/oauth2_token_service_delegate.cc b/chromium/google_apis/gaia/oauth2_token_service_delegate.cc index aed5db8bc23..3c5f75ba9f3 100644 --- a/chromium/google_apis/gaia/oauth2_token_service_delegate.cc +++ b/chromium/google_apis/gaia/oauth2_token_service_delegate.cc @@ -24,16 +24,21 @@ OAuth2TokenServiceDelegate::OAuth2TokenServiceDelegate() OAuth2TokenServiceDelegate::~OAuth2TokenServiceDelegate() { } -void OAuth2TokenServiceDelegate::ValidateAccountId( +bool OAuth2TokenServiceDelegate::ValidateAccountId( const std::string& account_id) const { - DCHECK(!account_id.empty()); + bool valid = !account_id.empty(); // If the account is given as an email, make sure its a canonical email. // Note that some tests don't use email strings as account id, and after // the gaia id migration it won't be an email. So only check for // canonicalization if the account_id is suspected to be an email. - if (account_id.find('@') != std::string::npos) - DCHECK_EQ(gaia::CanonicalizeEmail(account_id), account_id); + if (account_id.find('@') != std::string::npos && + gaia::CanonicalizeEmail(account_id) != account_id) { + valid = false; + } + + DCHECK(valid); + return valid; } void OAuth2TokenServiceDelegate::AddObserver( diff --git a/chromium/google_apis/gaia/oauth2_token_service_delegate.h b/chromium/google_apis/gaia/oauth2_token_service_delegate.h index 3a2d39fb246..24bf063c059 100644 --- a/chromium/google_apis/gaia/oauth2_token_service_delegate.h +++ b/chromium/google_apis/gaia/oauth2_token_service_delegate.h @@ -48,7 +48,7 @@ class OAuth2TokenServiceDelegate { virtual void RevokeCredentials(const std::string& account_id) {} virtual net::URLRequestContextGetter* GetRequestContext() const; - void ValidateAccountId(const std::string& account_id) const; + bool ValidateAccountId(const std::string& account_id) const; // Add or remove observers of this token service. void AddObserver(OAuth2TokenService::Observer* observer); diff --git a/chromium/google_apis/gaia/oauth2_token_service_test_util.cc b/chromium/google_apis/gaia/oauth2_token_service_test_util.cc index 6dbed5f8974..597d61d6040 100644 --- a/chromium/google_apis/gaia/oauth2_token_service_test_util.cc +++ b/chromium/google_apis/gaia/oauth2_token_service_test_util.cc @@ -15,7 +15,7 @@ const char kValidTokenResponse[] = "}"; } -std::string GetValidTokenResponse(std::string token, int expiration) { +std::string GetValidTokenResponse(const std::string& token, int expiration) { return base::StringPrintf(kValidTokenResponse, token.c_str(), expiration); } diff --git a/chromium/google_apis/gaia/oauth2_token_service_test_util.h b/chromium/google_apis/gaia/oauth2_token_service_test_util.h index 6a14f139bed..e3881b7ef57 100644 --- a/chromium/google_apis/gaia/oauth2_token_service_test_util.h +++ b/chromium/google_apis/gaia/oauth2_token_service_test_util.h @@ -10,7 +10,7 @@ #include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/oauth2_token_service.h" -std::string GetValidTokenResponse(std::string token, int expiration); +std::string GetValidTokenResponse(const std::string& token, int expiration); // A simple testing consumer. class TestingOAuth2TokenServiceConsumer : public OAuth2TokenService::Consumer { diff --git a/chromium/google_apis/gaia/ubertoken_fetcher.h b/chromium/google_apis/gaia/ubertoken_fetcher.h index ade53cca267..56017461572 100644 --- a/chromium/google_apis/gaia/ubertoken_fetcher.h +++ b/chromium/google_apis/gaia/ubertoken_fetcher.h @@ -91,7 +91,7 @@ class UbertokenFetcher : public GaiaAuthConsumer, std::string account_id_; std::string access_token_; int retry_number_; - base::OneShotTimer<UbertokenFetcher> retry_timer_; + base::OneShotTimer retry_timer_; bool second_access_token_request_; DISALLOW_COPY_AND_ASSIGN(UbertokenFetcher); diff --git a/chromium/google_apis/gcm/BUILD.gn b/chromium/google_apis/gcm/BUILD.gn index 870a05d0ecb..30ab4a6b738 100644 --- a/chromium/google_apis/gcm/BUILD.gn +++ b/chromium/google_apis/gcm/BUILD.gn @@ -116,12 +116,22 @@ executable("mcs_probe") { ":gcm", ":test_support", "//base", + "//build/config/sanitizers:deps", "//net", "//net:test_support", "//third_party/protobuf:protobuf_lite", ] } +# TODO(GYP): Delete this after we've converted everything to GN. +# The _run targets exist only for compatibility w/ GYP. +group("gcm_unit_tests_run") { + testonly = true + deps = [ + ":gcm_unit_tests", + ] +} + test("gcm_unit_tests") { sources = [ "base/mcs_message_unittest.cc", @@ -131,6 +141,8 @@ test("gcm_unit_tests") { "engine/checkin_request_unittest.cc", "engine/connection_factory_impl_unittest.cc", "engine/connection_handler_impl_unittest.cc", + "engine/gcm_request_test_base.cc", + "engine/gcm_request_test_base.h", "engine/gcm_store_impl_unittest.cc", "engine/gservices_settings_unittest.cc", "engine/heartbeat_manager_unittest.cc", diff --git a/chromium/google_apis/gcm/OWNERS b/chromium/google_apis/gcm/OWNERS index 493c918a07c..efe9165db1b 100644 --- a/chromium/google_apis/gcm/OWNERS +++ b/chromium/google_apis/gcm/OWNERS @@ -1,3 +1,2 @@ zea@chromium.org dimich@chromium.org -tim@chromium.org diff --git a/chromium/google_apis/gcm/base/mcs_message_unittest.cc b/chromium/google_apis/gcm/base/mcs_message_unittest.cc index 3a0ba056bb6..4dd52d6adde 100644 --- a/chromium/google_apis/gcm/base/mcs_message_unittest.cc +++ b/chromium/google_apis/gcm/base/mcs_message_unittest.cc @@ -5,7 +5,8 @@ #include "google_apis/gcm/base/mcs_message.h" #include "base/logging.h" -#include "base/message_loop/message_loop.h" +#include "base/test/test_simple_task_runner.h" +#include "base/thread_task_runner_handle.h" #include "google_apis/gcm/base/mcs_util.h" #include "google_apis/gcm/protocol/mcs.pb.h" #include "testing/gtest/include/gtest/gtest.h" @@ -21,10 +22,13 @@ class MCSMessageTest : public testing::Test { ~MCSMessageTest() override; private: - base::MessageLoop message_loop_; + scoped_refptr<base::TestSimpleTaskRunner> task_runner_; + base::ThreadTaskRunnerHandle task_runner_handle_; }; -MCSMessageTest::MCSMessageTest() { +MCSMessageTest::MCSMessageTest() + : task_runner_(new base::TestSimpleTaskRunner()), + task_runner_handle_(task_runner_) { } MCSMessageTest::~MCSMessageTest() { diff --git a/chromium/google_apis/gcm/engine/checkin_request.cc b/chromium/google_apis/gcm/engine/checkin_request.cc index 8a9ef033d8c..c1c73e87e62 100644 --- a/chromium/google_apis/gcm/engine/checkin_request.cc +++ b/chromium/google_apis/gcm/engine/checkin_request.cc @@ -5,8 +5,9 @@ #include "google_apis/gcm/engine/checkin_request.h" #include "base/bind.h" -#include "base/message_loop/message_loop.h" +#include "base/location.h" #include "base/metrics/histogram.h" +#include "base/thread_task_runner_handle.h" #include "google_apis/gcm/monitoring/gcm_stats_recorder.h" #include "google_apis/gcm/protocol/checkin.pb.h" #include "net/base/load_flags.h" @@ -154,28 +155,20 @@ void CheckinRequest::Start() { url_fetcher_->Start(); } -void CheckinRequest::RetryWithBackoff(bool update_backoff) { - if (update_backoff) { - backoff_entry_.InformOfRequest(false); - url_fetcher_.reset(); - } - - if (backoff_entry_.ShouldRejectRequest()) { - DVLOG(1) << "Delay GCM checkin for: " - << backoff_entry_.GetTimeUntilRelease().InMilliseconds() - << " milliseconds."; - recorder_->RecordCheckinDelayedDueToBackoff( - backoff_entry_.GetTimeUntilRelease().InMilliseconds()); - base::MessageLoop::current()->PostDelayedTask( - FROM_HERE, - base::Bind(&CheckinRequest::RetryWithBackoff, - weak_ptr_factory_.GetWeakPtr(), - false), - backoff_entry_.GetTimeUntilRelease()); - return; - } - - Start(); +void CheckinRequest::RetryWithBackoff() { + backoff_entry_.InformOfRequest(false); + url_fetcher_.reset(); + + DVLOG(1) << "Delay GCM checkin for: " + << backoff_entry_.GetTimeUntilRelease().InMilliseconds() + << " milliseconds."; + recorder_->RecordCheckinDelayedDueToBackoff( + backoff_entry_.GetTimeUntilRelease().InMilliseconds()); + DCHECK(!weak_ptr_factory_.HasWeakPtrs()); + base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, + base::Bind(&CheckinRequest::Start, weak_ptr_factory_.GetWeakPtr()), + backoff_entry_.GetTimeUntilRelease()); } void CheckinRequest::OnURLFetchComplete(const net::URLFetcher* source) { @@ -184,7 +177,7 @@ void CheckinRequest::OnURLFetchComplete(const net::URLFetcher* source) { if (!source->GetStatus().is_success()) { LOG(ERROR) << "Failed to get checkin response. Fetcher failed. Retrying."; RecordCheckinStatusAndReportUMA(URL_FETCHING_FAILED, recorder_, true); - RetryWithBackoff(true); + RetryWithBackoff(); return; } @@ -211,7 +204,7 @@ void CheckinRequest::OnURLFetchComplete(const net::URLFetcher* source) { CheckinRequestStatus status = response_status != net::HTTP_OK ? HTTP_NOT_OK : RESPONSE_PARSING_FAILED; RecordCheckinStatusAndReportUMA(status, recorder_, true); - RetryWithBackoff(true); + RetryWithBackoff(); return; } @@ -221,7 +214,7 @@ void CheckinRequest::OnURLFetchComplete(const net::URLFetcher* source) { response_proto.security_token() == 0) { LOG(ERROR) << "Android ID or security token is 0. Retrying."; RecordCheckinStatusAndReportUMA(ZERO_ID_OR_TOKEN, recorder_, true); - RetryWithBackoff(true); + RetryWithBackoff(); return; } diff --git a/chromium/google_apis/gcm/engine/checkin_request.h b/chromium/google_apis/gcm/engine/checkin_request.h index e3ad8edd9a0..619a8f94458 100644 --- a/chromium/google_apis/gcm/engine/checkin_request.h +++ b/chromium/google_apis/gcm/engine/checkin_request.h @@ -72,9 +72,8 @@ class GCM_EXPORT CheckinRequest : public net::URLFetcherDelegate { void OnURLFetchComplete(const net::URLFetcher* source) override; private: - // Schedules a retry attempt, informs the backoff of a previous request's - // failure when |update_backoff| is true. - void RetryWithBackoff(bool update_backoff); + // Schedules a retry attempt with a backoff. + void RetryWithBackoff(); net::URLRequestContextGetter* request_context_getter_; CheckinRequestCallback callback_; diff --git a/chromium/google_apis/gcm/engine/checkin_request_unittest.cc b/chromium/google_apis/gcm/engine/checkin_request_unittest.cc index 1fa0efce197..9939c2736c5 100644 --- a/chromium/google_apis/gcm/engine/checkin_request_unittest.cc +++ b/chromium/google_apis/gcm/engine/checkin_request_unittest.cc @@ -5,47 +5,12 @@ #include <string> #include "google_apis/gcm/engine/checkin_request.h" +#include "google_apis/gcm/engine/gcm_request_test_base.h" #include "google_apis/gcm/monitoring/fake_gcm_stats_recorder.h" #include "google_apis/gcm/protocol/checkin.pb.h" -#include "net/base/backoff_entry.h" -#include "net/url_request/test_url_fetcher_factory.h" -#include "net/url_request/url_request_test_util.h" -#include "testing/gtest/include/gtest/gtest.h" namespace gcm { -namespace { - -const net::BackoffEntry::Policy kDefaultBackoffPolicy = { - // Number of initial errors (in sequence) to ignore before applying - // exponential back-off rules. - // Explicitly set to 1 to skip the delay of the first Retry, as we are not - // trying to test the backoff itself, but rather the fact that retry happens. - 1, - - // Initial delay for exponential back-off in ms. - 15000, // 15 seconds. - - // Factor by which the waiting time will be multiplied. - 2, - - // Fuzzing percentage. ex: 10% will spread requests randomly - // between 90%-100% of the calculated time. - 0.5, // 50%. - - // Maximum amount of time we are willing to delay our request in ms. - 1000 * 60 * 5, // 5 minutes. - - // Time to keep an entry from being discarded even when it - // has no significant state, -1 to never discard. - -1, - - // Don't use initial delay unless the last request was an error. - false, -}; - -} - const uint64 kAndroidId = 42UL; const uint64 kBlankAndroidId = 999999UL; const uint64 kBlankSecurityToken = 999999UL; @@ -56,7 +21,7 @@ const char kSettingsDigest[] = "settings_digest"; const char kEmailAddress[] = "test_user@gmail.com"; const char kTokenValue[] = "token_value"; -class CheckinRequestTest : public testing::Test { +class CheckinRequestTest : public GCMRequestTestBase { public: enum ResponseScenario { VALID_RESPONSE, // Both android_id and security_token set in response. @@ -74,22 +39,13 @@ class CheckinRequestTest : public testing::Test { void CreateRequest(uint64 android_id, uint64 security_token); - void SetResponseStatusAndString( - net::HttpStatusCode status_code, - const std::string& response_data); - - void CompleteFetch(); - - void SetResponse(ResponseScenario response_scenario); + void SetResponseScenario(ResponseScenario response_scenario); protected: bool callback_called_; uint64 android_id_; uint64 security_token_; int checkin_device_type_; - base::MessageLoop message_loop_; - net::TestURLFetcherFactory url_fetcher_factory_; - scoped_refptr<net::TestURLRequestContextGetter> url_request_context_getter_; checkin_proto::ChromeBuildProto chrome_build_proto_; scoped_ptr<CheckinRequest> request_; FakeGCMStatsRecorder recorder_; @@ -99,9 +55,7 @@ CheckinRequestTest::CheckinRequestTest() : callback_called_(false), android_id_(kBlankAndroidId), security_token_(kBlankSecurityToken), - checkin_device_type_(0), - url_request_context_getter_(new net::TestURLRequestContextGetter( - message_loop_.task_runner())) { + checkin_device_type_(0) { } CheckinRequestTest::~CheckinRequestTest() {} @@ -137,9 +91,9 @@ void CheckinRequestTest::CreateRequest(uint64 android_id, request_.reset(new CheckinRequest( GURL(kCheckinURL), request_info, - kDefaultBackoffPolicy, + GetBackoffPolicy(), base::Bind(&CheckinRequestTest::FetcherCallback, base::Unretained(this)), - url_request_context_getter_.get(), + url_request_context_getter(), &recorder_)); // Setting android_id_ and security_token_ to blank value, not used elsewhere @@ -149,24 +103,8 @@ void CheckinRequestTest::CreateRequest(uint64 android_id, security_token_ = kBlankSecurityToken; } -void CheckinRequestTest::SetResponseStatusAndString( - net::HttpStatusCode status_code, - const std::string& response_data) { - net::TestURLFetcher* fetcher = - url_fetcher_factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(status_code); - fetcher->SetResponseString(response_data); -} - -void CheckinRequestTest::CompleteFetch() { - net::TestURLFetcher* fetcher = - url_fetcher_factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->delegate()->OnURLFetchComplete(fetcher); -} - -void CheckinRequestTest::SetResponse(ResponseScenario response_scenario) { +void CheckinRequestTest::SetResponseScenario( + ResponseScenario response_scenario) { checkin_proto::AndroidCheckinResponse response; response.set_stats_ok(true); @@ -182,7 +120,7 @@ void CheckinRequestTest::SetResponse(ResponseScenario response_scenario) { std::string response_string; response.SerializeToString(&response_string); - SetResponseStatusAndString(net::HTTP_OK, response_string); + SetResponse(net::HTTP_OK, response_string); } TEST_F(CheckinRequestTest, FetcherDataAndURL) { @@ -190,7 +128,7 @@ TEST_F(CheckinRequestTest, FetcherDataAndURL) { request_->Start(); // Get data sent by request. - net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0); + net::TestURLFetcher* fetcher = GetFetcher(); ASSERT_TRUE(fetcher); EXPECT_EQ(GURL(kCheckinURL), fetcher->GetOriginalURL()); @@ -222,12 +160,12 @@ TEST_F(CheckinRequestTest, ResponseBodyEmpty) { CreateRequest(0u, 0u); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, std::string()); + SetResponse(net::HTTP_OK, std::string()); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponse(VALID_RESPONSE); + SetResponseScenario(VALID_RESPONSE); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -239,12 +177,12 @@ TEST_F(CheckinRequestTest, ResponseBodyCorrupted) { CreateRequest(0u, 0u); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, "Corrupted response body"); + SetResponse(net::HTTP_OK, "Corrupted response body"); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponse(VALID_RESPONSE); + SetResponseScenario(VALID_RESPONSE); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -256,7 +194,7 @@ TEST_F(CheckinRequestTest, ResponseHttpStatusUnauthorized) { CreateRequest(0u, 0u); request_->Start(); - SetResponseStatusAndString(net::HTTP_UNAUTHORIZED, std::string()); + SetResponse(net::HTTP_UNAUTHORIZED, std::string()); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -268,7 +206,7 @@ TEST_F(CheckinRequestTest, ResponseHttpStatusBadRequest) { CreateRequest(0u, 0u); request_->Start(); - SetResponseStatusAndString(net::HTTP_BAD_REQUEST, std::string()); + SetResponse(net::HTTP_BAD_REQUEST, std::string()); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -280,12 +218,12 @@ TEST_F(CheckinRequestTest, ResponseHttpStatusNotOK) { CreateRequest(0u, 0u); request_->Start(); - SetResponseStatusAndString(net::HTTP_INTERNAL_SERVER_ERROR, std::string()); + SetResponse(net::HTTP_INTERNAL_SERVER_ERROR, std::string()); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponse(VALID_RESPONSE); + SetResponseScenario(VALID_RESPONSE); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -297,12 +235,12 @@ TEST_F(CheckinRequestTest, ResponseMissingAndroidId) { CreateRequest(0u, 0u); request_->Start(); - SetResponse(MISSING_ANDROID_ID); + SetResponseScenario(MISSING_ANDROID_ID); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponse(VALID_RESPONSE); + SetResponseScenario(VALID_RESPONSE); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -314,12 +252,12 @@ TEST_F(CheckinRequestTest, ResponseMissingSecurityToken) { CreateRequest(0u, 0u); request_->Start(); - SetResponse(MISSING_SECURITY_TOKEN); + SetResponseScenario(MISSING_SECURITY_TOKEN); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponse(VALID_RESPONSE); + SetResponseScenario(VALID_RESPONSE); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -331,12 +269,12 @@ TEST_F(CheckinRequestTest, AndroidIdEqualsZeroInResponse) { CreateRequest(0u, 0u); request_->Start(); - SetResponse(ANDROID_ID_IS_ZER0); + SetResponseScenario(ANDROID_ID_IS_ZER0); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponse(VALID_RESPONSE); + SetResponseScenario(VALID_RESPONSE); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -348,12 +286,12 @@ TEST_F(CheckinRequestTest, SecurityTokenEqualsZeroInResponse) { CreateRequest(0u, 0u); request_->Start(); - SetResponse(SECURITY_TOKEN_IS_ZERO); + SetResponseScenario(SECURITY_TOKEN_IS_ZERO); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponse(VALID_RESPONSE); + SetResponseScenario(VALID_RESPONSE); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -365,7 +303,7 @@ TEST_F(CheckinRequestTest, SuccessfulFirstTimeCheckin) { CreateRequest(0u, 0u); request_->Start(); - SetResponse(VALID_RESPONSE); + SetResponseScenario(VALID_RESPONSE); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -377,7 +315,7 @@ TEST_F(CheckinRequestTest, SuccessfulSubsequentCheckin) { CreateRequest(kAndroidId, kSecurityToken); request_->Start(); - SetResponse(VALID_RESPONSE); + SetResponseScenario(VALID_RESPONSE); CompleteFetch(); EXPECT_TRUE(callback_called_); diff --git a/chromium/google_apis/gcm/engine/connection_factory_impl.cc b/chromium/google_apis/gcm/engine/connection_factory_impl.cc index b708edf869f..4c4f279bb1a 100644 --- a/chromium/google_apis/gcm/engine/connection_factory_impl.cc +++ b/chromium/google_apis/gcm/engine/connection_factory_impl.cc @@ -4,10 +4,11 @@ #include "google_apis/gcm/engine/connection_factory_impl.h" -#include "base/message_loop/message_loop.h" +#include "base/location.h" #include "base/metrics/histogram.h" #include "base/metrics/sparse_histogram.h" #include "base/profiler/scoped_tracker.h" +#include "base/thread_task_runner_handle.h" #include "google_apis/gcm/engine/connection_handler_impl.h" #include "google_apis/gcm/monitoring/gcm_stats_recorder.h" #include "google_apis/gcm/protocol/mcs.pb.h" @@ -136,7 +137,7 @@ void ConnectionFactoryImpl::ConnectWithBackoff() { waiting_for_backoff_ = true; recorder_->RecordConnectionDelayedDueToBackoff( backoff_entry_->GetTimeUntilRelease().InMilliseconds()); - base::MessageLoop::current()->PostDelayedTask( + base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( FROM_HERE, base::Bind(&ConnectionFactoryImpl::ConnectWithBackoff, weak_ptr_factory_.GetWeakPtr()), @@ -547,7 +548,7 @@ int ConnectionFactoryImpl::ReconsiderProxyAfterError(int error) { // If there is new proxy info, post OnProxyResolveDone to retry it. Otherwise, // if there was an error falling back, fail synchronously. if (status == net::OK) { - base::MessageLoop::current()->PostTask( + base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::Bind(&ConnectionFactoryImpl::OnProxyResolveDone, weak_ptr_factory_.GetWeakPtr(), status)); diff --git a/chromium/google_apis/gcm/engine/connection_factory_impl_unittest.cc b/chromium/google_apis/gcm/engine/connection_factory_impl_unittest.cc index 4c0e127bd63..31d907d3fac 100644 --- a/chromium/google_apis/gcm/engine/connection_factory_impl_unittest.cc +++ b/chromium/google_apis/gcm/engine/connection_factory_impl_unittest.cc @@ -272,9 +272,9 @@ class ConnectionFactoryImplTest }; ConnectionFactoryImplTest::ConnectionFactoryImplTest() - : factory_(base::Bind(&ConnectionFactoryImplTest::ConnectionsComplete, + : factory_(base::Bind(&ConnectionFactoryImplTest::ConnectionsComplete, base::Unretained(this))), - run_loop_(new base::RunLoop()) { + run_loop_(new base::RunLoop()) { factory()->SetConnectionListener(this); factory()->Initialize( ConnectionFactory::BuildLoginRequestCallback(), @@ -433,7 +433,7 @@ TEST_F(ConnectionFactoryImplTest, CanarySucceedsRetryDuringLogin) { EXPECT_FALSE(factory()->IsEndpointReachable()); // Pump the loop, to ensure the pending backoff retry has no effect. - base::MessageLoop::current()->PostDelayedTask( + base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( FROM_HERE, base::MessageLoop::QuitClosure(), base::TimeDelta::FromMilliseconds(1)); diff --git a/chromium/google_apis/gcm/engine/connection_handler_impl.cc b/chromium/google_apis/gcm/engine/connection_handler_impl.cc index 31a6748c76d..4d3fc82745c 100644 --- a/chromium/google_apis/gcm/engine/connection_handler_impl.cc +++ b/chromium/google_apis/gcm/engine/connection_handler_impl.cc @@ -4,7 +4,8 @@ #include "google_apis/gcm/engine/connection_handler_impl.h" -#include "base/message_loop/message_loop.h" +#include "base/location.h" +#include "base/thread_task_runner_handle.h" #include "google/protobuf/io/coded_stream.h" #include "google/protobuf/io/zero_copy_stream_impl_lite.h" #include "google_apis/gcm/base/mcs_util.h" @@ -129,7 +130,7 @@ void ConnectionHandlerImpl::Login( if (output_stream_->Flush( base::Bind(&ConnectionHandlerImpl::OnMessageSent, weak_ptr_factory_.GetWeakPtr())) != net::ERR_IO_PENDING) { - base::MessageLoop::current()->PostTask( + base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::Bind(&ConnectionHandlerImpl::OnMessageSent, weak_ptr_factory_.GetWeakPtr())); @@ -259,7 +260,7 @@ void ConnectionHandlerImpl::WaitForData(ProcessingState state) { DVLOG(1) << "Socket read finished prematurely. Waiting for " << min_bytes_needed - input_stream_->UnreadByteCount() << " more bytes."; - base::MessageLoop::current()->PostTask( + base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::Bind(&ConnectionHandlerImpl::WaitForData, weak_ptr_factory_.GetWeakPtr(), @@ -374,7 +375,7 @@ void ConnectionHandlerImpl::OnGotMessageBytes() { // Messages with no content are valid; just use the default protobuf for // that tag. if (protobuf.get() && message_size_ == 0) { - base::MessageLoop::current()->PostTask( + base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::Bind(&ConnectionHandlerImpl::GetNextMessage, weak_ptr_factory_.GetWeakPtr())); @@ -444,7 +445,7 @@ void ConnectionHandlerImpl::OnGotMessageBytes() { } input_stream_->RebuildBuffer(); - base::MessageLoop::current()->PostTask( + base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::Bind(&ConnectionHandlerImpl::GetNextMessage, weak_ptr_factory_.GetWeakPtr())); diff --git a/chromium/google_apis/gcm/engine/connection_handler_impl.h b/chromium/google_apis/gcm/engine/connection_handler_impl.h index 399b39723d0..e2702c0acb1 100644 --- a/chromium/google_apis/gcm/engine/connection_handler_impl.h +++ b/chromium/google_apis/gcm/engine/connection_handler_impl.h @@ -93,7 +93,7 @@ class GCM_EXPORT ConnectionHandlerImpl : public ConnectionHandler { // TODO(zea): consider enforcing a separate timeout when waiting for // a message to send. const base::TimeDelta read_timeout_; - base::OneShotTimer<ConnectionHandlerImpl> read_timeout_timer_; + base::OneShotTimer read_timeout_timer_; // This connection's socket and the input/output streams attached to it. net::StreamSocket* socket_; diff --git a/chromium/google_apis/gcm/engine/gcm_request_test_base.cc b/chromium/google_apis/gcm/engine/gcm_request_test_base.cc new file mode 100644 index 00000000000..5337f7e9819 --- /dev/null +++ b/chromium/google_apis/gcm/engine/gcm_request_test_base.cc @@ -0,0 +1,89 @@ +// Copyright 2015 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 <cmath> + +#include "google_apis/gcm/engine/gcm_request_test_base.h" +#include "net/url_request/url_fetcher_delegate.h" + +namespace { + +// Backoff policy for testing registration request. +const net::BackoffEntry::Policy kDefaultBackoffPolicy = { + // Number of initial errors (in sequence) to ignore before applying + // exponential back-off rules. + 0, + + // Initial delay for exponential back-off in ms. + 15000, // 15 seconds. + + // Factor by which the waiting time will be multiplied. + 2, + + // Fuzzing percentage. ex: 10% will spread requests randomly + // between 90%-100% of the calculated time. + 0.5, // 50%. + + // Maximum amount of time we are willing to delay our request in ms. + 1000 * 60 * 5, // 5 minutes. + + // Time to keep an entry from being discarded even when it + // has no significant state, -1 to never discard. + -1, + + // Don't use initial delay unless the last request was an error. + false, +}; + +} // namespace + +namespace gcm { + +GCMRequestTestBase::GCMRequestTestBase() + : task_runner_(new base::TestMockTimeTaskRunner), + task_runner_handle_(task_runner_), + url_request_context_getter_(new net::TestURLRequestContextGetter( + task_runner_)), + retry_count_(0) { +} + +GCMRequestTestBase::~GCMRequestTestBase() { +} + +const net::BackoffEntry::Policy& GCMRequestTestBase::GetBackoffPolicy() const { + return kDefaultBackoffPolicy; +} + +net::TestURLFetcher* GCMRequestTestBase::GetFetcher() const { + return url_fetcher_factory_.GetFetcherByID(0); +} + +void GCMRequestTestBase::SetResponse(net::HttpStatusCode status_code, + const std::string& response_body) { + if (retry_count_++) + FastForwardToTriggerNextRetry(); + + net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0); + ASSERT_TRUE(fetcher); + fetcher->set_response_code(status_code); + fetcher->SetResponseString(response_body); +} + +void GCMRequestTestBase::CompleteFetch() { + net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0); + ASSERT_TRUE(fetcher); + fetcher->delegate()->OnURLFetchComplete(fetcher); +} + +void GCMRequestTestBase::FastForwardToTriggerNextRetry() { + // Here we compute the maximum delay time by skipping the jitter fluctuation + // that only affects in the negative way. + int next_retry_delay_ms = kDefaultBackoffPolicy.initial_delay_ms; + next_retry_delay_ms *= + pow(kDefaultBackoffPolicy.multiply_factor, retry_count_); + task_runner_->FastForwardBy( + base::TimeDelta::FromMilliseconds(next_retry_delay_ms)); +} + +} // namespace gcm diff --git a/chromium/google_apis/gcm/engine/gcm_request_test_base.h b/chromium/google_apis/gcm/engine/gcm_request_test_base.h new file mode 100644 index 00000000000..2af7b0ae7bc --- /dev/null +++ b/chromium/google_apis/gcm/engine/gcm_request_test_base.h @@ -0,0 +1,53 @@ +// Copyright 2015 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 "base/memory/ref_counted.h" +#include "base/test/test_mock_time_task_runner.h" +#include "base/thread_task_runner_handle.h" +#include "net/base/backoff_entry.h" +#include "net/url_request/test_url_fetcher_factory.h" +#include "net/url_request/url_request_test_util.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace gcm { + +// The base class for testing various GCM requests that contains all the common +// logic to set up a task runner and complete a request with retries. +class GCMRequestTestBase : public testing::Test { + public: + GCMRequestTestBase(); + ~GCMRequestTestBase() override; + + const net::BackoffEntry::Policy& GetBackoffPolicy() const; + net::TestURLFetcher* GetFetcher() const; + + // Set the response status and body that will be returned by the URL fetch. + void SetResponse(net::HttpStatusCode status_code, + const std::string& response_body); + + // Completes the URL fetch. + // It can be overridden by the test class to add additional logic. + virtual void CompleteFetch(); + + net::URLRequestContextGetter* url_request_context_getter() const { + return url_request_context_getter_.get(); + } + + private: + // Fast forward the timer used in the test to retry the request immediately. + void FastForwardToTriggerNextRetry(); + + scoped_refptr<base::TestMockTimeTaskRunner> task_runner_; + base::ThreadTaskRunnerHandle task_runner_handle_; + scoped_refptr<net::TestURLRequestContextGetter> url_request_context_getter_; + + net::TestURLFetcherFactory url_fetcher_factory_; + + // Tracks the number of retries so far. + int retry_count_; + + DISALLOW_COPY_AND_ASSIGN(GCMRequestTestBase); +}; + +} // namespace gcm diff --git a/chromium/google_apis/gcm/engine/gcm_store_impl.cc b/chromium/google_apis/gcm/engine/gcm_store_impl.cc index 3b9dd0ad88e..3ad9b99ed90 100644 --- a/chromium/google_apis/gcm/engine/gcm_store_impl.cc +++ b/chromium/google_apis/gcm/engine/gcm_store_impl.cc @@ -294,9 +294,13 @@ LoadStatus GCMStoreImpl::Backend::OpenStoreAndLoadData(StoreOpenMode open_mode, leveldb::Options options; options.create_if_missing = open_mode == CREATE_IF_MISSING; options.reuse_logs = leveldb_env::kDefaultLogReuseOptionValue; + options.paranoid_checks = true; leveldb::DB* db; leveldb::Status status = leveldb::DB::Open(options, path_.AsUTF8Unsafe(), &db); + UMA_HISTOGRAM_ENUMERATION("GCM.Database.Open", + leveldb_env::GetLevelDBStatusUMAValue(status), + leveldb_env::LEVELDB_STATUS_MAX); if (!status.ok()) { LOG(ERROR) << "Failed to open database " << path_.value() << ": " << status.ToString(); @@ -353,7 +357,8 @@ void GCMStoreImpl::Backend::Load(StoreOpenMode open_mode, int gcm_registration_count = 0; int instance_id_token_count = 0; for (const auto& registration : result->registrations) { - if (base::StartsWithASCII(registration.first, "iid-", true)) + if (base::StartsWith(registration.first, "iid-", + base::CompareCase::SENSITIVE)) instance_id_token_count++; else gcm_registration_count++; @@ -997,7 +1002,7 @@ bool GCMStoreImpl::Backend::LoadOutgoingMessages( return false; } DVLOG(1) << "Found outgoing message with id " << id << " of type " - << base::IntToString(tag); + << base::UintToString(tag); (*outgoing_messages)[id] = make_linked_ptr(message.release()); } diff --git a/chromium/google_apis/gcm/engine/gcm_store_impl_unittest.cc b/chromium/google_apis/gcm/engine/gcm_store_impl_unittest.cc index 442f898f1d8..9b748552faf 100644 --- a/chromium/google_apis/gcm/engine/gcm_store_impl_unittest.cc +++ b/chromium/google_apis/gcm/engine/gcm_store_impl_unittest.cc @@ -12,9 +12,9 @@ #include "base/files/file_path.h" #include "base/files/scoped_temp_dir.h" #include "base/memory/scoped_ptr.h" -#include "base/message_loop/message_loop.h" -#include "base/run_loop.h" #include "base/strings/string_number_conversions.h" +#include "base/test/test_simple_task_runner.h" +#include "base/thread_task_runner_handle.h" #include "google_apis/gcm/base/fake_encryptor.h" #include "google_apis/gcm/base/mcs_message.h" #include "google_apis/gcm/base/mcs_util.h" @@ -61,18 +61,19 @@ class GCMStoreImplTest : public testing::Test { void UpdateCallback(bool success); protected: - base::MessageLoop message_loop_; + scoped_refptr<base::TestSimpleTaskRunner> task_runner_; + base::ThreadTaskRunnerHandle task_runner_handle_; base::ScopedTempDir temp_directory_; bool expected_success_; uint64 next_persistent_id_; - scoped_ptr<base::RunLoop> run_loop_; }; GCMStoreImplTest::GCMStoreImplTest() - : expected_success_(true), + : task_runner_(new base::TestSimpleTaskRunner()), + task_runner_handle_(task_runner_), + expected_success_(true), next_persistent_id_(base::Time::Now().ToInternalValue()) { EXPECT_TRUE(temp_directory_.CreateUniqueTempDir()); - run_loop_.reset(new base::RunLoop()); } GCMStoreImplTest::~GCMStoreImplTest() {} @@ -83,7 +84,7 @@ scoped_ptr<GCMStoreImpl> GCMStoreImplTest::BuildGCMStore() { // behavior in the production code. Currently GCMStoreImpl checks if // the directory exist or not to determine the store existence. temp_directory_.path().Append(FILE_PATH_LITERAL("GCM Store")), - message_loop_.task_runner(), + task_runner_, make_scoped_ptr<Encryptor>(new FakeEncryptor))); } @@ -101,7 +102,7 @@ std::string GCMStoreImplTest::GetNextPersistentId() { return base::Uint64ToString(next_persistent_id_++); } -void GCMStoreImplTest::PumpLoop() { message_loop_.RunUntilIdle(); } +void GCMStoreImplTest::PumpLoop() { task_runner_->RunUntilIdle(); } void GCMStoreImplTest::LoadCallback( scoped_ptr<GCMStore::LoadResult>* result_dst, @@ -114,8 +115,6 @@ void GCMStoreImplTest::LoadWithoutCheckCallback( scoped_ptr<GCMStore::LoadResult>* result_dst, scoped_ptr<GCMStore::LoadResult> result) { *result_dst = result.Pass(); - run_loop_->Quit(); - run_loop_.reset(new base::RunLoop()); } void GCMStoreImplTest::UpdateCallback(bool success) { diff --git a/chromium/google_apis/gcm/engine/gservices_settings.cc b/chromium/google_apis/gcm/engine/gservices_settings.cc index c50f3e89a33..6621ff88cc3 100644 --- a/chromium/google_apis/gcm/engine/gservices_settings.cc +++ b/chromium/google_apis/gcm/engine/gservices_settings.cc @@ -198,7 +198,7 @@ std::string GServicesSettings::CalculateDigest(const SettingsMap& settings) { reinterpret_cast<const unsigned char*>(&data[0]), data.size(), hash); std::string digest = kDigestVersionPrefix + base::HexEncode(hash, base::kSHA1Length); - digest = base::StringToLowerASCII(digest); + digest = base::ToLowerASCII(digest); return digest; } diff --git a/chromium/google_apis/gcm/engine/heartbeat_manager.cc b/chromium/google_apis/gcm/engine/heartbeat_manager.cc index 31c2876a60a..0e78b028079 100644 --- a/chromium/google_apis/gcm/engine/heartbeat_manager.cc +++ b/chromium/google_apis/gcm/engine/heartbeat_manager.cc @@ -5,8 +5,10 @@ #include "google_apis/gcm/engine/heartbeat_manager.h" #include "base/callback.h" +#include "base/location.h" #include "base/metrics/histogram.h" #include "base/power_monitor/power_monitor.h" +#include "base/thread_task_runner_handle.h" #include "base/time/time.h" #include "base/timer/timer.h" #include "google_apis/gcm/protocol/mcs.pb.h" @@ -23,6 +25,8 @@ const int kWifiHeartbeatDefaultMs = 1000 * 60 * 15; // 15 minutes. const int kHeartbeatAckDefaultMs = 1000 * 60 * 1; // 1 minute. // Minimum allowed client default heartbeat interval. const int kMinClientHeartbeatIntervalMs = 1000 * 60 * 2; // 2 minutes. +// Minimum time spent sleeping before we force a new heartbeat. +const int kMinSuspendTimeMs = 1000 * 10; // 10 seconds. #if defined(OS_LINUX) && !defined(OS_CHROMEOS) // The period at which to check if the heartbeat time has passed. Used to @@ -42,10 +46,6 @@ HeartbeatManager::HeartbeatManager() heartbeat_timer_(new base::Timer(true /* retain_user_task */, false /* is_repeating */)), weak_ptr_factory_(this) { - // Listen for system suspend and resume events. - base::PowerMonitor* monitor = base::PowerMonitor::Get(); - if (monitor) - monitor->AddObserver(this); } HeartbeatManager::~HeartbeatManager() { @@ -63,6 +63,11 @@ void HeartbeatManager::Start( send_heartbeat_callback_ = send_heartbeat_callback; trigger_reconnect_callback_ = trigger_reconnect_callback; + // Listen for system suspend and resume events. + base::PowerMonitor* monitor = base::PowerMonitor::Get(); + if (monitor) + monitor->AddObserver(this); + // Kicks off the timer. waiting_for_ack_ = false; RestartTimer(); @@ -73,6 +78,10 @@ void HeartbeatManager::Stop() { heartbeat_interval_ms_ = 0; heartbeat_timer_->Stop(); waiting_for_ack_ = false; + + base::PowerMonitor* monitor = base::PowerMonitor::Get(); + if (monitor) + monitor->RemoveObserver(this); } void HeartbeatManager::OnHeartbeatAcked() { @@ -116,8 +125,26 @@ void HeartbeatManager::UpdateHeartbeatTimer(scoped_ptr<base::Timer> timer) { heartbeat_timer_->Start(FROM_HERE, remaining_delay, timer_task); } +void HeartbeatManager::OnSuspend() { + // The system is going to sleep. Record the time, so on resume we know how + // much time the machine was suspended. + suspend_time_ = base::Time::Now(); +} + void HeartbeatManager::OnResume() { - CheckForMissedHeartbeat(); + // The system just resumed from sleep. It's likely that the connection to + // MCS was silently lost during that time, even if a heartbeat is not yet + // due. Force a heartbeat to detect if the connection is still good. + base::TimeDelta elapsed = base::Time::Now() - suspend_time_; + UMA_HISTOGRAM_LONG_TIMES("GCM.SuspendTime", elapsed); + + // Make sure a minimum amount of time has passed before forcing a heartbeat to + // avoid any tight loop scenarios. + // If the |send_heartbeat_callback_| is null, it means the heartbeat manager + // hasn't been started, so do nothing. + if (elapsed > base::TimeDelta::FromMilliseconds(kMinSuspendTimeMs) && + !send_heartbeat_callback_.is_null()) + OnHeartbeatTriggered(); } void HeartbeatManager::OnHeartbeatTriggered() { @@ -174,7 +201,7 @@ void HeartbeatManager::RestartTimer() { // Windows, Mac, Android, iOS, and Chrome OS all provide a way to be notified // when the system is suspending or resuming. The only one that does not is // Linux so we need to poll to check for missed heartbeats. - base::MessageLoop::current()->PostDelayedTask( + base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( FROM_HERE, base::Bind(&HeartbeatManager::CheckForMissedHeartbeat, weak_ptr_factory_.GetWeakPtr()), @@ -197,7 +224,7 @@ void HeartbeatManager::CheckForMissedHeartbeat() { #if defined(OS_LINUX) && !defined(OS_CHROMEOS) // Otherwise check again later. - base::MessageLoop::current()->PostDelayedTask( + base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( FROM_HERE, base::Bind(&HeartbeatManager::CheckForMissedHeartbeat, weak_ptr_factory_.GetWeakPtr()), diff --git a/chromium/google_apis/gcm/engine/heartbeat_manager.h b/chromium/google_apis/gcm/engine/heartbeat_manager.h index a6d47ea241f..83a25cfda52 100644 --- a/chromium/google_apis/gcm/engine/heartbeat_manager.h +++ b/chromium/google_apis/gcm/engine/heartbeat_manager.h @@ -61,6 +61,7 @@ class GCM_EXPORT HeartbeatManager : public base::PowerObserver { void UpdateHeartbeatTimer(scoped_ptr<base::Timer> timer); // base::PowerObserver override. + void OnSuspend() override; void OnResume() override; // Maximum and minimum of the custom client interval that can be requested, @@ -114,6 +115,9 @@ class GCM_EXPORT HeartbeatManager : public base::PowerObserver { // Timer for triggering heartbeats. scoped_ptr<base::Timer> heartbeat_timer_; + // Time at which the machine was last suspended. + base::Time suspend_time_; + // Callbacks for interacting with the the connection. base::Closure send_heartbeat_callback_; ReconnectCallback trigger_reconnect_callback_; diff --git a/chromium/google_apis/gcm/engine/heartbeat_manager_unittest.cc b/chromium/google_apis/gcm/engine/heartbeat_manager_unittest.cc index 6e378c2311a..b7ee684734f 100644 --- a/chromium/google_apis/gcm/engine/heartbeat_manager_unittest.cc +++ b/chromium/google_apis/gcm/engine/heartbeat_manager_unittest.cc @@ -7,7 +7,8 @@ #include "base/bind.h" #include "base/bind_helpers.h" #include "base/memory/scoped_ptr.h" -#include "base/message_loop/message_loop.h" +#include "base/test/test_simple_task_runner.h" +#include "base/thread_task_runner_handle.h" #include "base/time/time.h" #include "base/timer/timer.h" #include "google_apis/gcm/protocol/mcs.pb.h" @@ -65,13 +66,16 @@ class HeartbeatManagerTest : public testing::Test { int heartbeats_sent_; int reconnects_triggered_; - base::MessageLoop message_loop_; + scoped_refptr<base::TestSimpleTaskRunner> task_runner_; + base::ThreadTaskRunnerHandle task_runner_handle_; }; HeartbeatManagerTest::HeartbeatManagerTest() : manager_(new TestHeartbeatManager()), heartbeats_sent_(0), - reconnects_triggered_(0) { + reconnects_triggered_(0), + task_runner_(new base::TestSimpleTaskRunner()), + task_runner_handle_(task_runner_) { } void HeartbeatManagerTest::StartManager() { diff --git a/chromium/google_apis/gcm/engine/mcs_client.cc b/chromium/google_apis/gcm/engine/mcs_client.cc index 9b4607d7b67..f20d9377736 100644 --- a/chromium/google_apis/gcm/engine/mcs_client.cc +++ b/chromium/google_apis/gcm/engine/mcs_client.cc @@ -8,9 +8,10 @@ #include "base/basictypes.h" #include "base/bind.h" -#include "base/message_loop/message_loop.h" +#include "base/location.h" #include "base/metrics/histogram.h" #include "base/strings/string_number_conversions.h" +#include "base/thread_task_runner_handle.h" #include "base/time/clock.h" #include "base/time/time.h" #include "base/timer/timer.h" @@ -544,7 +545,7 @@ void MCSClient::MaybeSendMessage() { packet->persistent_id, base::Bind(&MCSClient::OnGCMUpdateFinished, weak_ptr_factory_.GetWeakPtr())); - base::MessageLoop::current()->PostTask( + base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::Bind(&MCSClient::MaybeSendMessage, weak_ptr_factory_.GetWeakPtr())); @@ -717,14 +718,14 @@ void MCSClient::HandlePacketFromWire( DCHECK_EQ(1U, stream_id_out_); // Pass the login response on up. - base::MessageLoop::current()->PostTask( + base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::Bind(message_received_callback_, MCSMessage(tag, protobuf.Pass()))); // If there are pending messages, attempt to send one. if (!to_send_.empty()) { - base::MessageLoop::current()->PostTask( + base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::Bind(&MCSClient::MaybeSendMessage, weak_ptr_factory_.GetWeakPtr())); @@ -788,7 +789,7 @@ void MCSClient::HandlePacketFromWire( } DCHECK(protobuf.get()); - base::MessageLoop::current()->PostTask( + base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::Bind(message_received_callback_, MCSMessage(tag, protobuf.Pass()))); @@ -896,7 +897,7 @@ void MCSClient::HandleSelectiveAck(const PersistentIdList& id_list) { to_send_.push_front(to_resend_.back()); to_resend_.pop_back(); } - base::MessageLoop::current()->PostTask( + base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::Bind(&MCSClient::MaybeSendMessage, weak_ptr_factory_.GetWeakPtr())); @@ -923,7 +924,7 @@ void MCSClient::HandleServerConfirmedReceipt(StreamId device_stream_id) { } MCSClient::PersistentId MCSClient::GetNextPersistentId() { - return base::Uint64ToString(base::TimeTicks::Now().ToInternalValue()); + return base::Int64ToString(base::TimeTicks::Now().ToInternalValue()); } void MCSClient::OnConnectionResetByHeartbeat( diff --git a/chromium/google_apis/gcm/engine/registration_request.cc b/chromium/google_apis/gcm/engine/registration_request.cc index a593e2bf833..fa66a7777dc 100644 --- a/chromium/google_apis/gcm/engine/registration_request.cc +++ b/chromium/google_apis/gcm/engine/registration_request.cc @@ -5,9 +5,11 @@ #include "google_apis/gcm/engine/registration_request.h" #include "base/bind.h" +#include "base/location.h" #include "base/message_loop/message_loop.h" #include "base/metrics/histogram.h" #include "base/strings/string_number_conversions.h" +#include "base/thread_task_runner_handle.h" #include "base/values.h" #include "google_apis/gcm/base/gcm_util.h" #include "google_apis/gcm/monitoring/gcm_stats_recorder.h" @@ -154,34 +156,26 @@ void RegistrationRequest::BuildRequestBody(std::string* body) { custom_request_handler_->BuildRequestBody(body); } -void RegistrationRequest::RetryWithBackoff(bool update_backoff) { - if (update_backoff) { - DCHECK_GT(retries_left_, 0); - --retries_left_; - url_fetcher_.reset(); - backoff_entry_.InformOfRequest(false); - } - - if (backoff_entry_.ShouldRejectRequest()) { - DVLOG(1) << "Delaying GCM registration of app: " - << request_info_.app_id << ", for " - << backoff_entry_.GetTimeUntilRelease().InMilliseconds() - << " milliseconds."; - recorder_->RecordRegistrationRetryDelayed( - request_info_.app_id, - source_to_record_, - backoff_entry_.GetTimeUntilRelease().InMilliseconds(), - retries_left_ + 1); - base::MessageLoop::current()->PostDelayedTask( - FROM_HERE, - base::Bind(&RegistrationRequest::RetryWithBackoff, - weak_ptr_factory_.GetWeakPtr(), - false), - backoff_entry_.GetTimeUntilRelease()); - return; - } - - Start(); +void RegistrationRequest::RetryWithBackoff() { + DCHECK_GT(retries_left_, 0); + --retries_left_; + url_fetcher_.reset(); + backoff_entry_.InformOfRequest(false); + + DVLOG(1) << "Delaying GCM registration of app: " + << request_info_.app_id << ", for " + << backoff_entry_.GetTimeUntilRelease().InMilliseconds() + << " milliseconds."; + recorder_->RecordRegistrationRetryDelayed( + request_info_.app_id, + source_to_record_, + backoff_entry_.GetTimeUntilRelease().InMilliseconds(), + retries_left_ + 1); + DCHECK(!weak_ptr_factory_.HasWeakPtrs()); + base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, + base::Bind(&RegistrationRequest::Start, weak_ptr_factory_.GetWeakPtr()), + backoff_entry_.GetTimeUntilRelease()); } RegistrationRequest::Status RegistrationRequest::ParseResponse( @@ -241,7 +235,7 @@ void RegistrationRequest::OnURLFetchComplete(const net::URLFetcher* source) { if (ShouldRetryWithStatus(status)) { if (retries_left_ > 0) { - RetryWithBackoff(true); + RetryWithBackoff(); return; } diff --git a/chromium/google_apis/gcm/engine/registration_request.h b/chromium/google_apis/gcm/engine/registration_request.h index 96d77fd36cf..bcc3ee04fe2 100644 --- a/chromium/google_apis/gcm/engine/registration_request.h +++ b/chromium/google_apis/gcm/engine/registration_request.h @@ -112,9 +112,8 @@ class GCM_EXPORT RegistrationRequest : public net::URLFetcherDelegate { void OnURLFetchComplete(const net::URLFetcher* source) override; private: - // Schedules a retry attempt, informs the backoff of a previous request's - // failure, when |update_backoff| is true. - void RetryWithBackoff(bool update_backoff); + // Schedules a retry attempt with a backoff. + void RetryWithBackoff(); void BuildRequestHeaders(std::string* extra_headers); void BuildRequestBody(std::string* body); diff --git a/chromium/google_apis/gcm/engine/registration_request_unittest.cc b/chromium/google_apis/gcm/engine/registration_request_unittest.cc index 98133de0e7f..683e1d8ae4a 100644 --- a/chromium/google_apis/gcm/engine/registration_request_unittest.cc +++ b/chromium/google_apis/gcm/engine/registration_request_unittest.cc @@ -9,13 +9,12 @@ #include "base/strings/string_number_conversions.h" #include "base/strings/string_tokenizer.h" #include "google_apis/gcm/engine/gcm_registration_request_handler.h" +#include "google_apis/gcm/engine/gcm_request_test_base.h" #include "google_apis/gcm/engine/instance_id_get_token_request_handler.h" #include "google_apis/gcm/monitoring/fake_gcm_stats_recorder.h" #include "net/base/load_flags.h" -#include "net/url_request/test_url_fetcher_factory.h" +#include "net/base/net_errors.h" #include "net/url_request/url_request_status.h" -#include "net/url_request/url_request_test_util.h" -#include "testing/gtest/include/gtest/gtest.h" namespace gcm { @@ -30,38 +29,9 @@ const int kGCMVersion = 40; const char kInstanceId[] = "IID1"; const char kScope[] = "GCM"; -// Backoff policy for testing registration request. -const net::BackoffEntry::Policy kDefaultBackoffPolicy = { - // Number of initial errors (in sequence) to ignore before applying - // exponential back-off rules. - // Explicitly set to 2 to skip the delay on the first retry, as we are not - // trying to test the backoff itself, but rather the fact that retry happens. - 2, - - // Initial delay for exponential back-off in ms. - 15000, // 15 seconds. - - // Factor by which the waiting time will be multiplied. - 2, - - // Fuzzing percentage. ex: 10% will spread requests randomly - // between 90%-100% of the calculated time. - 0.5, // 50%. - - // Maximum amount of time we are willing to delay our request in ms. - 1000 * 60 * 5, // 5 minutes. - - // Time to keep an entry from being discarded even when it - // has no significant state, -1 to never discard. - -1, - - // Don't use initial delay unless the last request was an error. - false, -}; - } // namespace -class RegistrationRequestTest : public testing::Test { +class RegistrationRequestTest : public GCMRequestTestBase { public: RegistrationRequestTest(); ~RegistrationRequestTest() override; @@ -69,9 +39,8 @@ class RegistrationRequestTest : public testing::Test { void RegistrationCallback(RegistrationRequest::Status status, const std::string& registration_id); - void SetResponseStatusAndString(net::HttpStatusCode status_code, - const std::string& response_body); - void CompleteFetch(); + void CompleteFetch() override; + void set_max_retry_count(int max_retry_count) { max_retry_count_ = max_retry_count; } @@ -83,18 +52,13 @@ class RegistrationRequestTest : public testing::Test { bool callback_called_; std::map<std::string, std::string> extras_; scoped_ptr<RegistrationRequest> request_; - base::MessageLoop message_loop_; - net::TestURLFetcherFactory url_fetcher_factory_; - scoped_refptr<net::TestURLRequestContextGetter> url_request_context_getter_; FakeGCMStatsRecorder recorder_; }; RegistrationRequestTest::RegistrationRequestTest() : max_retry_count_(2), status_(RegistrationRequest::SUCCESS), - callback_called_(false), - url_request_context_getter_(new net::TestURLRequestContextGetter( - message_loop_.task_runner())) {} + callback_called_(false) {} RegistrationRequestTest::~RegistrationRequestTest() {} @@ -106,23 +70,12 @@ void RegistrationRequestTest::RegistrationCallback( callback_called_ = true; } -void RegistrationRequestTest::SetResponseStatusAndString( - net::HttpStatusCode status_code, - const std::string& response_body) { - net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(status_code); - fetcher->SetResponseString(response_body); -} - void RegistrationRequestTest::CompleteFetch() { registration_id_.clear(); status_ = RegistrationRequest::SUCCESS; callback_called_ = false; - net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->delegate()->OnURLFetchComplete(fetcher); + GCMRequestTestBase::CompleteFetch(); } class GCMRegistrationRequestTest : public RegistrationRequestTest { @@ -148,11 +101,11 @@ void GCMRegistrationRequestTest::CreateRequest(const std::string& sender_ids) { GURL(kRegistrationURL), request_info, request_handler.Pass(), - kDefaultBackoffPolicy, + GetBackoffPolicy(), base::Bind(&RegistrationRequestTest::RegistrationCallback, base::Unretained(this)), max_retry_count_, - url_request_context_getter_.get(), + url_request_context_getter(), &recorder_, sender_ids)); } @@ -162,7 +115,7 @@ TEST_F(GCMRegistrationRequestTest, RequestSuccessful) { CreateRequest("sender1,sender2"); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, "token=2501"); + SetResponse(net::HTTP_OK, "token=2501"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -175,7 +128,7 @@ TEST_F(GCMRegistrationRequestTest, RequestDataAndURL) { request_->Start(); // Get data sent by request. - net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0); + net::TestURLFetcher* fetcher = GetFetcher(); ASSERT_TRUE(fetcher); EXPECT_EQ(GURL(kRegistrationURL), fetcher->GetOriginalURL()); @@ -218,7 +171,7 @@ TEST_F(GCMRegistrationRequestTest, RequestRegistrationWithMultipleSenderIds) { CreateRequest("sender1,sender2@gmail.com"); request_->Start(); - net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0); + net::TestURLFetcher* fetcher = GetFetcher(); ASSERT_TRUE(fetcher); // Verify data was formatted properly. @@ -243,7 +196,7 @@ TEST_F(GCMRegistrationRequestTest, ResponseParsing) { CreateRequest("sender1,sender2"); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, "token=2501"); + SetResponse(net::HTTP_OK, "token=2501"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -255,12 +208,12 @@ TEST_F(GCMRegistrationRequestTest, ResponseHttpStatusNotOK) { CreateRequest("sender1,sender2"); request_->Start(); - SetResponseStatusAndString(net::HTTP_UNAUTHORIZED, "token=2501"); + SetResponse(net::HTTP_UNAUTHORIZED, "token=2501"); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponseStatusAndString(net::HTTP_OK, "token=2501"); + SetResponse(net::HTTP_OK, "token=2501"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -272,18 +225,18 @@ TEST_F(GCMRegistrationRequestTest, ResponseMissingRegistrationId) { CreateRequest("sender1,sender2"); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, ""); + SetResponse(net::HTTP_OK, ""); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponseStatusAndString(net::HTTP_OK, "some error in response"); + SetResponse(net::HTTP_OK, "some error in response"); CompleteFetch(); EXPECT_FALSE(callback_called_); // Ensuring a retry happened and succeeds. - SetResponseStatusAndString(net::HTTP_OK, "token=2501"); + SetResponse(net::HTTP_OK, "token=2501"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -295,13 +248,13 @@ TEST_F(GCMRegistrationRequestTest, ResponseDeviceRegistrationError) { CreateRequest("sender1,sender2"); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, "Error=PHONE_REGISTRATION_ERROR"); + SetResponse(net::HTTP_OK, "Error=PHONE_REGISTRATION_ERROR"); CompleteFetch(); EXPECT_FALSE(callback_called_); // Ensuring a retry happened and succeeds. - SetResponseStatusAndString(net::HTTP_OK, "token=2501"); + SetResponse(net::HTTP_OK, "token=2501"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -313,14 +266,14 @@ TEST_F(GCMRegistrationRequestTest, ResponseAuthenticationError) { CreateRequest("sender1,sender2"); request_->Start(); - SetResponseStatusAndString(net::HTTP_UNAUTHORIZED, + SetResponse(net::HTTP_UNAUTHORIZED, "Error=AUTHENTICATION_FAILED"); CompleteFetch(); EXPECT_FALSE(callback_called_); // Ensuring a retry happened and succeeds. - SetResponseStatusAndString(net::HTTP_OK, "token=2501"); + SetResponse(net::HTTP_OK, "token=2501"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -332,7 +285,7 @@ TEST_F(GCMRegistrationRequestTest, ResponseInvalidParameters) { CreateRequest("sender1,sender2"); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, "Error=INVALID_PARAMETERS"); + SetResponse(net::HTTP_OK, "Error=INVALID_PARAMETERS"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -344,7 +297,7 @@ TEST_F(GCMRegistrationRequestTest, ResponseInvalidSender) { CreateRequest("sender1,sender2"); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, "Error=INVALID_SENDER"); + SetResponse(net::HTTP_OK, "Error=INVALID_SENDER"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -356,7 +309,7 @@ TEST_F(GCMRegistrationRequestTest, ResponseInvalidSenderBadRequest) { CreateRequest("sender1"); request_->Start(); - SetResponseStatusAndString(net::HTTP_BAD_REQUEST, "Error=INVALID_SENDER"); + SetResponse(net::HTTP_BAD_REQUEST, "Error=INVALID_SENDER"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -368,18 +321,18 @@ TEST_F(GCMRegistrationRequestTest, RequestNotSuccessful) { CreateRequest("sender1,sender2"); request_->Start(); - net::URLRequestStatus request_status(net::URLRequestStatus::FAILED, 1); - SetResponseStatusAndString(net::HTTP_OK, "token=2501"); - net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0); + SetResponse(net::HTTP_OK, "token=2501"); + + net::TestURLFetcher* fetcher = GetFetcher(); ASSERT_TRUE(fetcher); - fetcher->set_status(request_status); + GetFetcher()->set_status(net::URLRequestStatus::FromError(net::ERR_FAILED)); CompleteFetch(); EXPECT_FALSE(callback_called_); // Ensuring a retry happened and succeeded. - SetResponseStatusAndString(net::HTTP_OK, "token=2501"); + SetResponse(net::HTTP_OK, "token=2501"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -391,13 +344,13 @@ TEST_F(GCMRegistrationRequestTest, ResponseHttpNotOk) { CreateRequest("sender1,sender2"); request_->Start(); - SetResponseStatusAndString(net::HTTP_GATEWAY_TIMEOUT, "token=2501"); + SetResponse(net::HTTP_GATEWAY_TIMEOUT, "token=2501"); CompleteFetch(); EXPECT_FALSE(callback_called_); // Ensuring a retry happened and succeeded. - SetResponseStatusAndString(net::HTTP_OK, "token=2501"); + SetResponse(net::HTTP_OK, "token=2501"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -410,7 +363,7 @@ TEST_F(GCMRegistrationRequestTest, MaximumAttemptsReachedWithZeroRetries) { CreateRequest("sender1,sender2"); request_->Start(); - SetResponseStatusAndString(net::HTTP_GATEWAY_TIMEOUT, "token=2501"); + SetResponse(net::HTTP_GATEWAY_TIMEOUT, "token=2501"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -422,17 +375,17 @@ TEST_F(GCMRegistrationRequestTest, MaximumAttemptsReached) { CreateRequest("sender1,sender2"); request_->Start(); - SetResponseStatusAndString(net::HTTP_GATEWAY_TIMEOUT, "token=2501"); + SetResponse(net::HTTP_GATEWAY_TIMEOUT, "token=2501"); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponseStatusAndString(net::HTTP_GATEWAY_TIMEOUT, "token=2501"); + SetResponse(net::HTTP_GATEWAY_TIMEOUT, "token=2501"); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponseStatusAndString(net::HTTP_GATEWAY_TIMEOUT, "token=2501"); + SetResponse(net::HTTP_GATEWAY_TIMEOUT, "token=2501"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -471,11 +424,11 @@ void InstanceIDGetTokenRequestTest::CreateRequest( GURL(kRegistrationURL), request_info, request_handler.Pass(), - kDefaultBackoffPolicy, + GetBackoffPolicy(), base::Bind(&RegistrationRequestTest::RegistrationCallback, base::Unretained(this)), max_retry_count_, - url_request_context_getter_.get(), + url_request_context_getter(), &recorder_, authorized_entity)); } @@ -488,7 +441,7 @@ TEST_F(InstanceIDGetTokenRequestTest, RequestSuccessful) { CreateRequest(kInstanceId, kDeveloperId, kScope, options); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, "token=2501"); + SetResponse(net::HTTP_OK, "token=2501"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -503,7 +456,7 @@ TEST_F(InstanceIDGetTokenRequestTest, RequestDataAndURL) { request_->Start(); // Get data sent by request. - net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0); + net::TestURLFetcher* fetcher = GetFetcher(); ASSERT_TRUE(fetcher); EXPECT_EQ(GURL(kRegistrationURL), fetcher->GetOriginalURL()); @@ -558,12 +511,12 @@ TEST_F(InstanceIDGetTokenRequestTest, ResponseHttpStatusNotOK) { CreateRequest(kInstanceId, kDeveloperId, kScope, options); request_->Start(); - SetResponseStatusAndString(net::HTTP_UNAUTHORIZED, "token=2501"); + SetResponse(net::HTTP_UNAUTHORIZED, "token=2501"); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponseStatusAndString(net::HTTP_OK, "token=2501"); + SetResponse(net::HTTP_OK, "token=2501"); CompleteFetch(); EXPECT_TRUE(callback_called_); diff --git a/chromium/google_apis/gcm/engine/unregistration_request.cc b/chromium/google_apis/gcm/engine/unregistration_request.cc index e9fab18f9c6..6cde5819045 100644 --- a/chromium/google_apis/gcm/engine/unregistration_request.cc +++ b/chromium/google_apis/gcm/engine/unregistration_request.cc @@ -5,9 +5,10 @@ #include "google_apis/gcm/engine/unregistration_request.h" #include "base/bind.h" -#include "base/message_loop/message_loop.h" +#include "base/location.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_piece.h" +#include "base/thread_task_runner_handle.h" #include "base/values.h" #include "google_apis/gcm/base/gcm_util.h" #include "google_apis/gcm/monitoring/gcm_stats_recorder.h" @@ -146,34 +147,26 @@ UnregistrationRequest::Status UnregistrationRequest::ParseResponse( return custom_request_handler_->ParseResponse(source); } -void UnregistrationRequest::RetryWithBackoff(bool update_backoff) { - if (update_backoff) { - DCHECK_GT(retries_left_, 0); - --retries_left_; - url_fetcher_.reset(); - backoff_entry_.InformOfRequest(false); - } - - if (backoff_entry_.ShouldRejectRequest()) { - DVLOG(1) << "Delaying GCM unregistration of app: " - << request_info_.app_id << ", for " - << backoff_entry_.GetTimeUntilRelease().InMilliseconds() - << " milliseconds."; - recorder_->RecordUnregistrationRetryDelayed( - request_info_.app_id, - source_to_record_, - backoff_entry_.GetTimeUntilRelease().InMilliseconds(), - retries_left_ + 1); - base::MessageLoop::current()->PostDelayedTask( - FROM_HERE, - base::Bind(&UnregistrationRequest::RetryWithBackoff, - weak_ptr_factory_.GetWeakPtr(), - false), - backoff_entry_.GetTimeUntilRelease()); - return; - } - - Start(); +void UnregistrationRequest::RetryWithBackoff() { + DCHECK_GT(retries_left_, 0); + --retries_left_; + url_fetcher_.reset(); + backoff_entry_.InformOfRequest(false); + + DVLOG(1) << "Delaying GCM unregistration of app: " + << request_info_.app_id << ", for " + << backoff_entry_.GetTimeUntilRelease().InMilliseconds() + << " milliseconds."; + recorder_->RecordUnregistrationRetryDelayed( + request_info_.app_id, + source_to_record_, + backoff_entry_.GetTimeUntilRelease().InMilliseconds(), + retries_left_ + 1); + DCHECK(!weak_ptr_factory_.HasWeakPtrs()); + base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, + base::Bind(&UnregistrationRequest::Start, weak_ptr_factory_.GetWeakPtr()), + backoff_entry_.GetTimeUntilRelease()); } void UnregistrationRequest::OnURLFetchComplete(const net::URLFetcher* source) { @@ -198,7 +191,7 @@ void UnregistrationRequest::OnURLFetchComplete(const net::URLFetcher* source) { status == INCORRECT_APP_ID || status == RESPONSE_PARSING_FAILED) { if (retries_left_ > 0) { - RetryWithBackoff(true); + RetryWithBackoff(); return; } diff --git a/chromium/google_apis/gcm/engine/unregistration_request.h b/chromium/google_apis/gcm/engine/unregistration_request.h index 56854255622..b6b5eaccd7a 100644 --- a/chromium/google_apis/gcm/engine/unregistration_request.h +++ b/chromium/google_apis/gcm/engine/unregistration_request.h @@ -120,9 +120,8 @@ class GCM_EXPORT UnregistrationRequest : public net::URLFetcherDelegate { void BuildRequestBody(std::string* body); Status ParseResponse(const net::URLFetcher* source); - // Schedules a retry attempt and informs the backoff of previous request's - // failure, when |update_backoff| is true. - void RetryWithBackoff(bool update_backoff); + // Schedules a retry attempt with a backoff. + void RetryWithBackoff(); UnregistrationCallback callback_; RequestInfo request_info_; diff --git a/chromium/google_apis/gcm/engine/unregistration_request_unittest.cc b/chromium/google_apis/gcm/engine/unregistration_request_unittest.cc index e850de88c0e..e6ca23d2d6b 100644 --- a/chromium/google_apis/gcm/engine/unregistration_request_unittest.cc +++ b/chromium/google_apis/gcm/engine/unregistration_request_unittest.cc @@ -8,13 +8,11 @@ #include "base/strings/string_number_conversions.h" #include "base/strings/string_tokenizer.h" +#include "google_apis/gcm/engine/gcm_request_test_base.h" #include "google_apis/gcm/engine/gcm_unregistration_request_handler.h" #include "google_apis/gcm/engine/instance_id_delete_token_request_handler.h" #include "google_apis/gcm/monitoring/fake_gcm_stats_recorder.h" #include "net/base/load_flags.h" -#include "net/url_request/test_url_fetcher_factory.h" -#include "net/url_request/url_request_test_util.h" -#include "testing/gtest/include/gtest/gtest.h" namespace gcm { @@ -32,46 +30,16 @@ const char kInstanceId[] = "IID1"; const char kDeveloperId[] = "Project1"; const char kScope[] = "GCM"; -// Backoff policy for testing registration request. -const net::BackoffEntry::Policy kDefaultBackoffPolicy = { - // Number of initial errors (in sequence) to ignore before applying - // exponential back-off rules. - // Explicitly set to 2 to skip the delay on the first retry, as we are not - // trying to test the backoff itself, but rather the fact that retry happens. - 2, - - // Initial delay for exponential back-off in ms. - 15000, // 15 seconds. - - // Factor by which the waiting time will be multiplied. - 2, - - // Fuzzing percentage. ex: 10% will spread requests randomly - // between 90%-100% of the calculated time. - 0.5, // 50%. - - // Maximum amount of time we are willing to delay our request in ms. - 1000 * 60 * 5, // 5 minutes. - - // Time to keep an entry from being discarded even when it - // has no significant state, -1 to never discard. - -1, - - // Don't use initial delay unless the last request was an error. - false, -}; } // namespace -class UnregistrationRequestTest : public testing::Test { +class UnregistrationRequestTest : public GCMRequestTestBase { public: UnregistrationRequestTest(); ~UnregistrationRequestTest() override; void UnregistrationCallback(UnregistrationRequest::Status status); - void SetResponseStatusAndString(net::HttpStatusCode status_code, - const std::string& response_body); - void CompleteFetch(); + void CompleteFetch() override; int max_retry_count() const { return max_retry_count_; } void set_max_retry_count(int max_retry_count) { @@ -83,18 +51,13 @@ class UnregistrationRequestTest : public testing::Test { bool callback_called_; UnregistrationRequest::Status status_; scoped_ptr<UnregistrationRequest> request_; - base::MessageLoop message_loop_; - net::TestURLFetcherFactory url_fetcher_factory_; - scoped_refptr<net::TestURLRequestContextGetter> url_request_context_getter_; FakeGCMStatsRecorder recorder_; }; UnregistrationRequestTest::UnregistrationRequestTest() : max_retry_count_(kMaxRetries), callback_called_(false), - status_(UnregistrationRequest::UNREGISTRATION_STATUS_COUNT), - url_request_context_getter_(new net::TestURLRequestContextGetter( - message_loop_.task_runner())) {} + status_(UnregistrationRequest::UNREGISTRATION_STATUS_COUNT) {} UnregistrationRequestTest::~UnregistrationRequestTest() {} @@ -104,21 +67,11 @@ void UnregistrationRequestTest::UnregistrationCallback( status_ = status; } -void UnregistrationRequestTest::SetResponseStatusAndString( - net::HttpStatusCode status_code, - const std::string& response_body) { - net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->set_response_code(status_code); - fetcher->SetResponseString(response_body); -} - void UnregistrationRequestTest::CompleteFetch() { status_ = UnregistrationRequest::UNREGISTRATION_STATUS_COUNT; callback_called_ = false; - net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0); - ASSERT_TRUE(fetcher); - fetcher->delegate()->OnURLFetchComplete(fetcher); + + GCMRequestTestBase::CompleteFetch(); } class GCMUnregistrationRequestTest : public UnregistrationRequestTest { @@ -144,11 +97,11 @@ void GCMUnregistrationRequestTest::CreateRequest() { GURL(kRegistrationURL), request_info, request_handler.Pass(), - kDefaultBackoffPolicy, + GetBackoffPolicy(), base::Bind(&UnregistrationRequestTest::UnregistrationCallback, base::Unretained(this)), max_retry_count_, - url_request_context_getter_.get(), + url_request_context_getter(), &recorder_, std::string())); } @@ -158,7 +111,7 @@ TEST_F(GCMUnregistrationRequestTest, RequestDataPassedToFetcher) { request_->Start(); // Get data sent by request. - net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0); + net::TestURLFetcher* fetcher = GetFetcher(); ASSERT_TRUE(fetcher); EXPECT_EQ(GURL(kRegistrationURL), fetcher->GetOriginalURL()); @@ -210,7 +163,7 @@ TEST_F(GCMUnregistrationRequestTest, SuccessfulUnregistration) { CreateRequest(); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, kDeletedAppId); + SetResponse(net::HTTP_OK, kDeletedAppId); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -221,12 +174,12 @@ TEST_F(GCMUnregistrationRequestTest, ResponseHttpStatusNotOK) { CreateRequest(); request_->Start(); - SetResponseStatusAndString(net::HTTP_UNAUTHORIZED, ""); + SetResponse(net::HTTP_UNAUTHORIZED, ""); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponseStatusAndString(net::HTTP_OK, kDeletedAppId); + SetResponse(net::HTTP_OK, kDeletedAppId); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -237,12 +190,12 @@ TEST_F(GCMUnregistrationRequestTest, ResponseEmpty) { CreateRequest(); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, ""); + SetResponse(net::HTTP_OK, ""); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponseStatusAndString(net::HTTP_OK, kDeletedAppId); + SetResponse(net::HTTP_OK, kDeletedAppId); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -253,7 +206,7 @@ TEST_F(GCMUnregistrationRequestTest, InvalidParametersError) { CreateRequest(); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, "Error=INVALID_PARAMETERS"); + SetResponse(net::HTTP_OK, "Error=INVALID_PARAMETERS"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -264,7 +217,7 @@ TEST_F(GCMUnregistrationRequestTest, UnkwnownError) { CreateRequest(); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, "Error=XXX"); + SetResponse(net::HTTP_OK, "Error=XXX"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -275,12 +228,12 @@ TEST_F(GCMUnregistrationRequestTest, ServiceUnavailable) { CreateRequest(); request_->Start(); - SetResponseStatusAndString(net::HTTP_SERVICE_UNAVAILABLE, ""); + SetResponse(net::HTTP_SERVICE_UNAVAILABLE, ""); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponseStatusAndString(net::HTTP_OK, kDeletedAppId); + SetResponse(net::HTTP_OK, kDeletedAppId); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -291,12 +244,12 @@ TEST_F(GCMUnregistrationRequestTest, InternalServerError) { CreateRequest(); request_->Start(); - SetResponseStatusAndString(net::HTTP_INTERNAL_SERVER_ERROR, ""); + SetResponse(net::HTTP_INTERNAL_SERVER_ERROR, ""); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponseStatusAndString(net::HTTP_OK, kDeletedAppId); + SetResponse(net::HTTP_OK, kDeletedAppId); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -307,12 +260,12 @@ TEST_F(GCMUnregistrationRequestTest, IncorrectAppId) { CreateRequest(); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, "deleted=OtherTestAppId"); + SetResponse(net::HTTP_OK, "deleted=OtherTestAppId"); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponseStatusAndString(net::HTTP_OK, kDeletedAppId); + SetResponse(net::HTTP_OK, kDeletedAppId); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -323,12 +276,12 @@ TEST_F(GCMUnregistrationRequestTest, ResponseParsingFailed) { CreateRequest(); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, "some malformed response"); + SetResponse(net::HTTP_OK, "some malformed response"); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponseStatusAndString(net::HTTP_OK, kDeletedAppId); + SetResponse(net::HTTP_OK, kDeletedAppId); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -340,7 +293,7 @@ TEST_F(GCMUnregistrationRequestTest, MaximumAttemptsReachedWithZeroRetries) { CreateRequest(); request_->Start(); - SetResponseStatusAndString(net::HTTP_GATEWAY_TIMEOUT, "bad response"); + SetResponse(net::HTTP_GATEWAY_TIMEOUT, "bad response"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -351,17 +304,17 @@ TEST_F(GCMUnregistrationRequestTest, MaximumAttemptsReached) { CreateRequest(); request_->Start(); - SetResponseStatusAndString(net::HTTP_GATEWAY_TIMEOUT, "bad response"); + SetResponse(net::HTTP_GATEWAY_TIMEOUT, "bad response"); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponseStatusAndString(net::HTTP_GATEWAY_TIMEOUT, "bad response"); + SetResponse(net::HTTP_GATEWAY_TIMEOUT, "bad response"); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponseStatusAndString(net::HTTP_GATEWAY_TIMEOUT, "bad response"); + SetResponse(net::HTTP_GATEWAY_TIMEOUT, "bad response"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -397,11 +350,11 @@ void InstaceIDDeleteTokenRequestTest::CreateRequest( GURL(kRegistrationURL), request_info, request_handler.Pass(), - kDefaultBackoffPolicy, + GetBackoffPolicy(), base::Bind(&UnregistrationRequestTest::UnregistrationCallback, base::Unretained(this)), max_retry_count(), - url_request_context_getter_.get(), + url_request_context_getter(), &recorder_, std::string())); } @@ -411,7 +364,7 @@ TEST_F(InstaceIDDeleteTokenRequestTest, RequestDataPassedToFetcher) { request_->Start(); // Get data sent by request. - net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0); + net::TestURLFetcher* fetcher = GetFetcher(); ASSERT_TRUE(fetcher); EXPECT_EQ(GURL(kRegistrationURL), fetcher->GetOriginalURL()); @@ -463,7 +416,7 @@ TEST_F(InstaceIDDeleteTokenRequestTest, SuccessfulUnregistration) { CreateRequest(kInstanceId, kDeveloperId, kScope); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, kDeletedToken); + SetResponse(net::HTTP_OK, kDeletedToken); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -474,12 +427,12 @@ TEST_F(InstaceIDDeleteTokenRequestTest, ResponseHttpStatusNotOK) { CreateRequest(kInstanceId, kDeveloperId, kScope); request_->Start(); - SetResponseStatusAndString(net::HTTP_UNAUTHORIZED, ""); + SetResponse(net::HTTP_UNAUTHORIZED, ""); CompleteFetch(); EXPECT_FALSE(callback_called_); - SetResponseStatusAndString(net::HTTP_OK, kDeletedToken); + SetResponse(net::HTTP_OK, kDeletedToken); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -490,7 +443,7 @@ TEST_F(InstaceIDDeleteTokenRequestTest, InvalidParametersError) { CreateRequest(kInstanceId, kDeveloperId, kScope); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, "Error=INVALID_PARAMETERS"); + SetResponse(net::HTTP_OK, "Error=INVALID_PARAMETERS"); CompleteFetch(); EXPECT_TRUE(callback_called_); @@ -501,7 +454,7 @@ TEST_F(InstaceIDDeleteTokenRequestTest, UnkwnownError) { CreateRequest(kInstanceId, kDeveloperId, kScope); request_->Start(); - SetResponseStatusAndString(net::HTTP_OK, "Error=XXX"); + SetResponse(net::HTTP_OK, "Error=XXX"); CompleteFetch(); EXPECT_TRUE(callback_called_); diff --git a/chromium/google_apis/gcm/gcm.gyp b/chromium/google_apis/gcm/gcm.gyp index 0d6a3cced70..b3b9df029cd 100644 --- a/chromium/google_apis/gcm/gcm.gyp +++ b/chromium/google_apis/gcm/gcm.gyp @@ -175,6 +175,8 @@ 'engine/checkin_request_unittest.cc', 'engine/connection_factory_impl_unittest.cc', 'engine/connection_handler_impl_unittest.cc', + 'engine/gcm_request_test_base.cc', + 'engine/gcm_request_test_base.h', 'engine/gcm_store_impl_unittest.cc', 'engine/gservices_settings_unittest.cc', 'engine/heartbeat_manager_unittest.cc', diff --git a/chromium/google_apis/gcm/gcm_unit_tests.isolate b/chromium/google_apis/gcm/gcm_unit_tests.isolate index cf5b3026680..ecd9dd2ff93 100644 --- a/chromium/google_apis/gcm/gcm_unit_tests.isolate +++ b/chromium/google_apis/gcm/gcm_unit_tests.isolate @@ -16,7 +16,6 @@ ], 'files': [ '../../testing/test_env.py', - '<(PRODUCT_DIR)/gcm_unit_tests<(EXECUTABLE_SUFFIX)', ], }, }], diff --git a/chromium/google_apis/gcm/monitoring/fake_gcm_stats_recorder.cc b/chromium/google_apis/gcm/monitoring/fake_gcm_stats_recorder.cc index f2934d45416..e61570ac806 100644 --- a/chromium/google_apis/gcm/monitoring/fake_gcm_stats_recorder.cc +++ b/chromium/google_apis/gcm/monitoring/fake_gcm_stats_recorder.cc @@ -21,9 +21,8 @@ void FakeGCMStatsRecorder::RecordCheckinDelayedDueToBackoff(int64 delay_msec) { void FakeGCMStatsRecorder::RecordCheckinSuccess() { } -void FakeGCMStatsRecorder::RecordCheckinFailure(std::string status, - bool will_retry) { -} +void FakeGCMStatsRecorder::RecordCheckinFailure(const std::string& status, + bool will_retry) {} void FakeGCMStatsRecorder::RecordConnectionInitiated(const std::string& host) { } diff --git a/chromium/google_apis/gcm/monitoring/fake_gcm_stats_recorder.h b/chromium/google_apis/gcm/monitoring/fake_gcm_stats_recorder.h index ba8acfdf8bf..e0350f4aab8 100644 --- a/chromium/google_apis/gcm/monitoring/fake_gcm_stats_recorder.h +++ b/chromium/google_apis/gcm/monitoring/fake_gcm_stats_recorder.h @@ -18,7 +18,8 @@ class FakeGCMStatsRecorder : public GCMStatsRecorder { void RecordCheckinInitiated(uint64 android_id) override; void RecordCheckinDelayedDueToBackoff(int64 delay_msec) override; void RecordCheckinSuccess() override; - void RecordCheckinFailure(std::string status, bool will_retry) override; + void RecordCheckinFailure(const std::string& status, + bool will_retry) override; void RecordConnectionInitiated(const std::string& host) override; void RecordConnectionDelayedDueToBackoff(int64 delay_msec) override; void RecordConnectionSuccess() override; diff --git a/chromium/google_apis/gcm/monitoring/gcm_stats_recorder.h b/chromium/google_apis/gcm/monitoring/gcm_stats_recorder.h index 8a093ebbb82..72c4446e26d 100644 --- a/chromium/google_apis/gcm/monitoring/gcm_stats_recorder.h +++ b/chromium/google_apis/gcm/monitoring/gcm_stats_recorder.h @@ -52,7 +52,8 @@ class GCM_EXPORT GCMStatsRecorder { // Records that a check-in request has failed. If a retry will be tempted then // will_retry should be true. - virtual void RecordCheckinFailure(std::string status, bool will_retry) = 0; + virtual void RecordCheckinFailure(const std::string& status, + bool will_retry) = 0; // Records that a connection to MCS has been initiated. virtual void RecordConnectionInitiated(const std::string& host) = 0; diff --git a/chromium/google_apis/gcm/protocol/mcs.proto b/chromium/google_apis/gcm/protocol/mcs.proto index 2926d1037f3..a1ea61418cc 100644 --- a/chromium/google_apis/gcm/protocol/mcs.proto +++ b/chromium/google_apis/gcm/protocol/mcs.proto @@ -250,6 +250,9 @@ message DataMessageStanza { optional int32 queued = 19; optional int64 status = 20; + + // Optional field containing the binary payload of the message. + optional bytes raw_data = 21; } /** diff --git a/chromium/google_apis/gcm/tools/mcs_probe.cc b/chromium/google_apis/gcm/tools/mcs_probe.cc index 9d0881b0f13..1063db9b587 100644 --- a/chromium/google_apis/gcm/tools/mcs_probe.cc +++ b/chromium/google_apis/gcm/tools/mcs_probe.cc @@ -137,7 +137,7 @@ class MyTestURLRequestContext : public net::TestURLRequestContext { context_storage_.set_host_resolver( net::HostResolver::CreateDefaultResolver(NULL)); context_storage_.set_transport_security_state( - new net::TransportSecurityState()); + make_scoped_ptr(new net::TransportSecurityState())); Init(); } @@ -368,7 +368,7 @@ void MCSProbe::InitializeNetworkState() { if (command_line_.HasSwitch(kIgnoreCertSwitch)) { cert_verifier_.reset(new MyTestCertVerifier()); } else { - cert_verifier_.reset(net::CertVerifier::CreateDefault()); + cert_verifier_ = net::CertVerifier::CreateDefault(); } system_channel_id_service_.reset( new net::ChannelIDService( @@ -382,7 +382,7 @@ void MCSProbe::InitializeNetworkState() { host_resolver_.get(), std::string(), std::string(), false, false)); http_server_properties_.reset(new net::HttpServerPropertiesImpl()); host_mapping_rules_.reset(new net::HostMappingRules()); - proxy_service_.reset(net::ProxyService::CreateDirectWithNetLog(&net_log_)); + proxy_service_ = net::ProxyService::CreateDirectWithNetLog(&net_log_); } void MCSProbe::BuildNetworkSession() { diff --git a/chromium/google_apis/google_apis.gyp b/chromium/google_apis/google_apis.gyp index 4d409268127..659d9835248 100644 --- a/chromium/google_apis/google_apis.gyp +++ b/chromium/google_apis/google_apis.gyp @@ -207,4 +207,23 @@ ], }, ], + 'conditions': [ + ['test_isolation_mode != "noop"', { + 'targets': [ + { + 'target_name': 'google_apis_unittests_run', + 'type': 'none', + 'dependencies': [ + 'google_apis_unittests', + ], + 'includes': [ + '../build/isolate.gypi', + ], + 'sources': [ + 'google_apis_unittests.isolate', + ], + }, + ], + }], + ], } diff --git a/chromium/google_apis/google_apis_unittests.isolate b/chromium/google_apis/google_apis_unittests.isolate new file mode 100644 index 00000000000..5d9e22e8ba9 --- /dev/null +++ b/chromium/google_apis/google_apis_unittests.isolate @@ -0,0 +1,24 @@ +# Copyright 2015 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. +{ + 'includes': [ + '../base/base.isolate', + ], + 'conditions': [ + ['OS=="win" or OS=="mac" or OS=="linux"', { + 'variables': { + 'command': [ + '../testing/test_env.py', + '<(PRODUCT_DIR)/google_apis_unittests<(EXECUTABLE_SUFFIX)', + '--brave-new-test-launcher', + '--test-launcher-bot-mode', + ], + 'files': [ + '../testing/test_env.py', + 'test/', + ], + }, + }], + ], +} |