diff options
Diffstat (limited to 'chromium/google_apis')
70 files changed, 1207 insertions, 1395 deletions
diff --git a/chromium/google_apis/BUILD.gn b/chromium/google_apis/BUILD.gn index 064eca60952..7be06fd5547 100644 --- a/chromium/google_apis/BUILD.gn +++ b/chromium/google_apis/BUILD.gn @@ -173,6 +173,17 @@ template("google_apis_tmpl") { "drive/time_util.h", ] } + + if (is_mac || is_ios) { + set_sources_assignment_filter([]) + sources += [ + "google_api_keys_mac.h", + "google_api_keys_mac.mm", + ] + set_sources_assignment_filter(sources_assignment_filter) + + libs = [ "Foundation.framework" ] + } } } @@ -182,7 +193,7 @@ google_apis_tmpl("google_apis") { ] } -source_set("test_support") { +static_library("test_support") { testonly = true sources = [ "gaia/fake_gaia.cc", @@ -202,7 +213,6 @@ source_set("test_support") { ":google_apis", "//base", "//base/test:test_support", - "//net", "//net:test_support", ] @@ -231,6 +241,7 @@ test("google_apis_unittests") { "gaia/oauth_request_signer_unittest.cc", "gaia/ubertoken_fetcher_unittest.cc", "google_api_keys_unittest.cc", + "google_api_keys_unittest.h", ] data = [ @@ -261,4 +272,12 @@ test("google_apis_unittests") { "drive/time_util_unittest.cc", ] } + + if (is_mac || is_ios) { + set_sources_assignment_filter([]) + sources += [ "google_api_keys_mac_unittest.mm" ] + set_sources_assignment_filter(sources_assignment_filter) + + deps += [ "//third_party/ocmock" ] + } } diff --git a/chromium/google_apis/DEPS b/chromium/google_apis/DEPS index 367bfb8ac5c..54031e7ebea 100644 --- a/chromium/google_apis/DEPS +++ b/chromium/google_apis/DEPS @@ -3,4 +3,5 @@ include_rules = [ "-content", "+crypto", "+net", + "+third_party/ocmock", ] diff --git a/chromium/google_apis/determine_use_official_keys.gypi b/chromium/google_apis/determine_use_official_keys.gypi deleted file mode 100644 index d9f11164af7..00000000000 --- a/chromium/google_apis/determine_use_official_keys.gypi +++ /dev/null @@ -1,37 +0,0 @@ -# Copyright (c) 2013 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 this .gypi in your target to dynamically set the -# use_official_google_api_keys variable (unless it is already -# explicitly set) and the associated preprocessor define. - -{ - 'variables': { - 'variables': { - # See documentation of this variable in //build/common.gypi. - 'use_official_google_api_keys%': 2, - }, - - # Copy conditionally-set variables out one scope. - 'use_official_google_api_keys%': '<(use_official_google_api_keys)', - - 'conditions': [ - # If use_official_google_api_keys is already set (to 0 or 1), we - # do none of the implicit checking. If it is set to 1 and the - # internal keys file is missing, the build will fail at compile - # time. If it is set to 0 and keys are not provided by other - # means, a warning will be printed at compile time. - ['use_official_google_api_keys==2', { - 'use_official_google_api_keys%': - '<!(python <(DEPTH)/google_apis/build/check_internal.py <(DEPTH)/google_apis/internal/google_chrome_api_keys.h)', - }], - ] - }, - - 'conditions': [ - ['use_official_google_api_keys==1', { - 'defines': ['USE_OFFICIAL_GOOGLE_API_KEYS=1'], - }], - ], -} diff --git a/chromium/google_apis/drive/base_requests.cc b/chromium/google_apis/drive/base_requests.cc index 97bc82d43b1..d1aa60d9299 100644 --- a/chromium/google_apis/drive/base_requests.cc +++ b/chromium/google_apis/drive/base_requests.cc @@ -293,8 +293,8 @@ int ResponseWriter::Write(net::IOBuffer* buffer, int num_bytes, const net::CompletionCallback& callback) { if (!get_content_callback_.is_null()) { - get_content_callback_.Run(HTTP_SUCCESS, base::WrapUnique(new std::string( - buffer->data(), num_bytes))); + get_content_callback_.Run( + HTTP_SUCCESS, base::MakeUnique<std::string>(buffer->data(), num_bytes)); } if (file_writer_) { @@ -312,9 +312,10 @@ int ResponseWriter::Write(net::IOBuffer* buffer, return num_bytes; } -int ResponseWriter::Finish(const net::CompletionCallback& callback) { +int ResponseWriter::Finish(int net_error, + const net::CompletionCallback& callback) { if (file_writer_) - return file_writer_->Finish(callback); + return file_writer_->Finish(net_error, callback); return net::OK; } @@ -984,7 +985,8 @@ void DownloadFileRequestBase::GetOutputFilePath( void DownloadFileRequestBase::OnURLFetchDownloadProgress( const URLFetcher* source, int64_t current, - int64_t total) { + int64_t total, + int64_t current_network_bytes) { if (!progress_callback_.is_null()) progress_callback_.Run(current, total); } diff --git a/chromium/google_apis/drive/base_requests.h b/chromium/google_apis/drive/base_requests.h index e94f7d389de..c4e2a2eeadd 100644 --- a/chromium/google_apis/drive/base_requests.h +++ b/chromium/google_apis/drive/base_requests.h @@ -135,7 +135,7 @@ class ResponseWriter : public net::URLFetcherResponseWriter { int Write(net::IOBuffer* buffer, int num_bytes, const net::CompletionCallback& callback) override; - int Finish(const net::CompletionCallback& callback) override; + int Finish(int net_error, const net::CompletionCallback& callback) override; private: void DidWrite(scoped_refptr<net::IOBuffer> buffer, @@ -630,7 +630,8 @@ class DownloadFileRequestBase : public UrlFetchRequestBase { // net::URLFetcherDelegate overrides. void OnURLFetchDownloadProgress(const net::URLFetcher* source, int64_t current, - int64_t total) override; + int64_t total, + int64_t current_network_bytes) override; private: const DownloadActionCallback download_action_callback_; diff --git a/chromium/google_apis/drive/base_requests_server_unittest.cc b/chromium/google_apis/drive/base_requests_server_unittest.cc index 5aa76cf3fcf..822e5a631f9 100644 --- a/chromium/google_apis/drive/base_requests_server_unittest.cc +++ b/chromium/google_apis/drive/base_requests_server_unittest.cc @@ -9,6 +9,7 @@ #include "base/bind.h" #include "base/files/file_util.h" #include "base/files/scoped_temp_dir.h" +#include "base/memory/ptr_util.h" #include "base/message_loop/message_loop.h" #include "base/run_loop.h" #include "google_apis/drive/dummy_auth_service.h" @@ -55,7 +56,7 @@ class BaseRequestsServerTest : public testing::Test { // Returns a temporary file path suitable for storing the cache file. base::FilePath GetTestCachedFilePath(const base::FilePath& file_name) { - return temp_dir_.path().Append(file_name); + return temp_dir_.GetPath().Append(file_name); } base::MessageLoopForIO message_loop_; // Test server needs IO thread. @@ -75,17 +76,17 @@ TEST_F(BaseRequestsServerTest, DownloadFileRequest_ValidFile) { base::FilePath temp_file; { base::RunLoop run_loop; - DownloadFileRequestBase* request = new DownloadFileRequestBase( - request_sender_.get(), - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&result_code, &temp_file)), - GetContentCallback(), - ProgressCallback(), - test_server_.GetURL("/files/drive/testfile.txt"), - GetTestCachedFilePath( - base::FilePath::FromUTF8Unsafe("cached_testfile.txt"))); - request_sender_->StartRequestWithAuthRetry(request); + std::unique_ptr<DownloadFileRequestBase> request = + base::MakeUnique<DownloadFileRequestBase>( + request_sender_.get(), + test_util::CreateQuitCallback( + &run_loop, + test_util::CreateCopyResultCallback(&result_code, &temp_file)), + GetContentCallback(), ProgressCallback(), + test_server_.GetURL("/files/drive/testfile.txt"), + GetTestCachedFilePath( + base::FilePath::FromUTF8Unsafe("cached_testfile.txt"))); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -109,17 +110,17 @@ TEST_F(BaseRequestsServerTest, DownloadFileRequest_NonExistentFile) { base::FilePath temp_file; { base::RunLoop run_loop; - DownloadFileRequestBase* request = new DownloadFileRequestBase( - request_sender_.get(), - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&result_code, &temp_file)), - GetContentCallback(), - ProgressCallback(), - test_server_.GetURL("/files/gdata/no-such-file.txt"), - GetTestCachedFilePath( - base::FilePath::FromUTF8Unsafe("cache_no-such-file.txt"))); - request_sender_->StartRequestWithAuthRetry(request); + std::unique_ptr<DownloadFileRequestBase> request = + base::MakeUnique<DownloadFileRequestBase>( + request_sender_.get(), + test_util::CreateQuitCallback( + &run_loop, + test_util::CreateCopyResultCallback(&result_code, &temp_file)), + GetContentCallback(), ProgressCallback(), + test_server_.GetURL("/files/gdata/no-such-file.txt"), + GetTestCachedFilePath( + base::FilePath::FromUTF8Unsafe("cache_no-such-file.txt"))); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } EXPECT_EQ(HTTP_NOT_FOUND, result_code); diff --git a/chromium/google_apis/drive/base_requests_unittest.cc b/chromium/google_apis/drive/base_requests_unittest.cc index 0e9472c3c61..a6dcbe1ba53 100644 --- a/chromium/google_apis/drive/base_requests_unittest.cc +++ b/chromium/google_apis/drive/base_requests_unittest.cc @@ -10,6 +10,7 @@ #include <utility> #include "base/bind.h" +#include "base/memory/ptr_util.h" #include "base/message_loop/message_loop.h" #include "base/run_loop.h" #include "base/values.h" @@ -179,7 +180,7 @@ TEST_F(BaseRequestsTest, UrlFetchRequestBaseResponseCodeOverride) { DriveApiErrorCode error = DRIVE_OTHER_ERROR; base::RunLoop run_loop; - sender_->StartRequestWithAuthRetry(new FakeUrlFetchRequest( + sender_->StartRequestWithAuthRetry(base::MakeUnique<FakeUrlFetchRequest>( sender_.get(), test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error)), @@ -200,8 +201,8 @@ TEST_F(MultipartUploadRequestBaseTest, Basic) { google_apis::test_util::GetTestFilePath("drive/text.txt"); std::string upload_content_type; std::string upload_content_data; - FakeMultipartUploadRequest* const multipart_request = - new FakeMultipartUploadRequest( + std::unique_ptr<FakeMultipartUploadRequest> multipart_request = + base::MakeUnique<FakeMultipartUploadRequest>( sender_->blocking_task_runner(), "{json:\"test\"}", "text/plain", 10, source_path, test_util::CreateQuitCallback( @@ -209,10 +210,9 @@ TEST_F(MultipartUploadRequestBaseTest, Basic) { ProgressCallback(), test_server_.base_url(), &upload_content_type, &upload_content_data); multipart_request->SetBoundaryForTesting("TESTBOUNDARY"); - std::unique_ptr<drive::SingleBatchableDelegateRequest> request( - new drive::SingleBatchableDelegateRequest(sender_.get(), - multipart_request)); - sender_->StartRequestWithAuthRetry(request.release()); + sender_->StartRequestWithAuthRetry( + base::MakeUnique<drive::SingleBatchableDelegateRequest>( + sender_.get(), std::move(multipart_request))); run_loop.Run(); EXPECT_EQ("multipart/related; boundary=TESTBOUNDARY", upload_content_type); EXPECT_EQ( diff --git a/chromium/google_apis/drive/drive_api_parser.cc b/chromium/google_apis/drive/drive_api_parser.cc index 572b38b4147..5f511645672 100644 --- a/chromium/google_apis/drive/drive_api_parser.cc +++ b/chromium/google_apis/drive/drive_api_parser.cc @@ -32,7 +32,7 @@ bool CreateFileResourceFromValue(const base::Value* value, // for JSONValueConverter::RegisterCustomField method. // TODO(mukai): make it return false in case of invalid |url_string|. bool GetGURLFromString(const base::StringPiece& url_string, GURL* result) { - *result = GURL(url_string.as_string()); + *result = GURL(url_string); return true; } @@ -133,7 +133,6 @@ const char kAppListKind[] = "drive#appList"; // Parent Resource // https://developers.google.com/drive/v2/reference/parents const char kParentReferenceKind[] = "drive#parentReference"; -const char kParentLink[] = "parentLink"; // File Resource // https://developers.google.com/drive/v2/reference/files @@ -155,6 +154,7 @@ const char kImageMediaMetadata[] = "imageMediaMetadata"; const char kShared[] = "shared"; // These 5 flags are defined under |labels|. const char kLabelTrashed[] = "trashed"; +const char kLabelStarred[] = "starred"; // These 3 flags are defined under |imageMediaMetadata|. const char kImageMediaMetadataWidth[] = "width"; const char kImageMediaMetadataHeight[] = "height"; @@ -402,9 +402,6 @@ ParentReference::~ParentReference() {} void ParentReference::RegisterJSONConverter( base::JSONValueConverter<ParentReference>* converter) { converter->RegisterStringField(kId, &ParentReference::file_id_); - converter->RegisterCustomField<GURL>(kParentLink, - &ParentReference::parent_link_, - GetGURLFromString); } // static @@ -642,7 +639,9 @@ bool ChangeList::Parse(const base::Value& value) { //////////////////////////////////////////////////////////////////////////////// // FileLabels implementation -FileLabels::FileLabels() : trashed_(false) {} +FileLabels::FileLabels() + : trashed_(false), + starred_(false) {} FileLabels::~FileLabels() {} @@ -650,6 +649,7 @@ FileLabels::~FileLabels() {} void FileLabels::RegisterJSONConverter( base::JSONValueConverter<FileLabels>* converter) { converter->RegisterBoolField(kLabelTrashed, &FileLabels::trashed_); + converter->RegisterBoolField(kLabelStarred, &FileLabels::starred_); } // static diff --git a/chromium/google_apis/drive/drive_api_parser.h b/chromium/google_apis/drive/drive_api_parser.h index d3f84f04ca6..75ff0e7f047 100644 --- a/chromium/google_apis/drive/drive_api_parser.h +++ b/chromium/google_apis/drive/drive_api_parser.h @@ -338,13 +338,7 @@ class ParentReference { // Returns the file id of the reference. const std::string& file_id() const { return file_id_; } - // Returns the URL for the parent in Drive. - const GURL& parent_link() const { return parent_link_; } - void set_file_id(const std::string& file_id) { file_id_ = file_id; } - void set_parent_link(const GURL& parent_link) { - parent_link_ = parent_link; - } private: // Parses and initializes data members from content of |value|. @@ -352,7 +346,6 @@ class ParentReference { bool Parse(const base::Value& value); std::string file_id_; - GURL parent_link_; }; // FileLabels represents labels for file or folder. @@ -372,8 +365,11 @@ class FileLabels { // Whether this file has been trashed. bool is_trashed() const { return trashed_; } + // Whether this file is starred by the user. + bool is_starred() const { return starred_; } void set_trashed(bool trashed) { trashed_ = trashed; } + void set_starred(bool starred) { starred_ = starred; } private: friend class FileResource; @@ -383,6 +379,7 @@ class FileLabels { bool Parse(const base::Value& value); bool trashed_; + bool starred_; }; // ImageMediaMetadata represents image metadata for a file. diff --git a/chromium/google_apis/drive/drive_api_parser_unittest.cc b/chromium/google_apis/drive/drive_api_parser_unittest.cc index 70ae96e3a92..712e0ec6363 100644 --- a/chromium/google_apis/drive/drive_api_parser_unittest.cc +++ b/chromium/google_apis/drive/drive_api_parser_unittest.cc @@ -134,6 +134,7 @@ TEST(DriveAPIParserTest, FileListParser) { EXPECT_EQ("application/octet-stream", file1.mime_type()); EXPECT_FALSE(file1.labels().is_trashed()); + EXPECT_FALSE(file1.labels().is_starred()); EXPECT_FALSE(file1.shared()); EXPECT_EQ(640, file1.image_media_metadata().width()); @@ -152,9 +153,6 @@ TEST(DriveAPIParserTest, FileListParser) { ASSERT_EQ(1U, file1.parents().size()); EXPECT_EQ("0B4v7G8yEYAWHYW1OcExsUVZLABC", file1.parents()[0].file_id()); - EXPECT_EQ(GURL("https://www.googleapis.com/drive/v2/files/" - "0B4v7G8yEYAWHYW1OcExsUVZLABC"), - file1.parents()[0].parent_link()); EXPECT_EQ("d41d8cd98f00b204e9800998ecf8427e", file1.md5_checksum()); EXPECT_EQ(1000U, file1.file_size()); @@ -174,6 +172,7 @@ TEST(DriveAPIParserTest, FileListParser) { EXPECT_EQ("application/vnd.google-apps.document", file2.mime_type()); EXPECT_TRUE(file2.labels().is_trashed()); + EXPECT_TRUE(file2.labels().is_starred()); EXPECT_TRUE(file2.shared()); EXPECT_EQ(-1, file2.image_media_metadata().width()); diff --git a/chromium/google_apis/drive/drive_api_requests.cc b/chromium/google_apis/drive/drive_api_requests.cc index 665a2463118..607c657f453 100644 --- a/chromium/google_apis/drive/drive_api_requests.cc +++ b/chromium/google_apis/drive/drive_api_requests.cc @@ -15,6 +15,8 @@ #include "base/sequenced_task_runner.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_piece.h" +#include "base/strings/string_split.h" +#include "base/strings/string_util.h" #include "base/strings/stringprintf.h" #include "base/task_runner_util.h" #include "base/values.h" @@ -90,7 +92,8 @@ void AttachProperties(const Properties& properties, base::ListValue* const properties_value = new base::ListValue; for (const auto& property : properties) { - base::DictionaryValue* const property_value = new base::DictionaryValue; + std::unique_ptr<base::DictionaryValue> property_value( + new base::DictionaryValue); std::string visibility_as_string; switch (property.visibility()) { case Property::VISIBILITY_PRIVATE: @@ -103,7 +106,7 @@ void AttachProperties(const Properties& properties, property_value->SetString("visibility", visibility_as_string); property_value->SetString("key", property.key()); property_value->SetString("value", property.value()); - properties_value->Append(property_value); + properties_value->Append(std::move(property_value)); } request_body->Set("properties", properties_value); } @@ -144,37 +147,6 @@ std::string CreateMultipartUploadMetadataJson( return json_string; } -// Splits |string| into lines by |kHttpBr|. -// Each line does not include |kHttpBr|. -void SplitIntoLines(const std::string& string, - std::vector<base::StringPiece>* output) { - const size_t br_size = std::string(kHttpBr).size(); - std::string::const_iterator it = string.begin(); - std::vector<base::StringPiece> lines; - while (true) { - const std::string::const_iterator next_pos = - std::search(it, string.end(), kHttpBr, kHttpBr + br_size); - lines.push_back(base::StringPiece(it, next_pos)); - if (next_pos == string.end()) - break; - it = next_pos + br_size; - } - output->swap(lines); -} - -// Remove transport padding (spaces and tabs at the end of line) from |piece|. -base::StringPiece TrimTransportPadding(const base::StringPiece& piece) { - size_t trim_size = 0; - while (trim_size < piece.size() && - (piece[piece.size() - 1 - trim_size] == ' ' || - piece[piece.size() - 1 - trim_size] == '\t')) { - ++trim_size; - } - return piece.substr(0, piece.size() - trim_size); -} - -void EmptyClosure(std::unique_ptr<BatchableDelegate>) {} - } // namespace MultipartHttpResponse::MultipartHttpResponse() : code(HTTP_SUCCESS) { @@ -219,10 +191,9 @@ bool ParseMultipartResponse(const std::string& content_type, if (content_type_piece.empty()) return false; if (content_type_piece[0] == '"') { - if (content_type_piece.size() <= 2 || - content_type_piece[content_type_piece.size() - 1] != '"') { + if (content_type_piece.size() <= 2 || content_type_piece.back() != '"') return false; - } + content_type_piece = content_type_piece.substr(1, content_type_piece.size() - 2); } @@ -232,8 +203,8 @@ bool ParseMultipartResponse(const std::string& content_type, const std::string header = "--" + boundary; const std::string terminator = "--" + boundary + "--"; - std::vector<base::StringPiece> lines; - SplitIntoLines(response, &lines); + std::vector<base::StringPiece> lines = base::SplitStringPieceUsingSubstr( + response, kHttpBr, base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL); enum { STATE_START, @@ -275,7 +246,8 @@ bool ParseMultipartResponse(const std::string& content_type, body.clear(); continue; } - const base::StringPiece chopped_line = TrimTransportPadding(line); + const base::StringPiece chopped_line = + base::TrimString(line, " \t", base::TRIM_TRAILING); const bool is_new_part = chopped_line == header; const bool was_last_part = chopped_line == terminator; if (is_new_part || was_last_part) { @@ -1169,11 +1141,10 @@ bool PermissionsInsertRequest::GetContentData(std::string* upload_content_type, SingleBatchableDelegateRequest::SingleBatchableDelegateRequest( RequestSender* sender, - BatchableDelegate* delegate) + std::unique_ptr<BatchableDelegate> delegate) : UrlFetchRequestBase(sender), - delegate_(delegate), - weak_ptr_factory_(this) { -} + delegate_(std::move(delegate)), + weak_ptr_factory_(this) {} SingleBatchableDelegateRequest::~SingleBatchableDelegateRequest() { } @@ -1311,7 +1282,7 @@ ScopedVector<BatchUploadChildEntry>::iterator BatchUploadRequest::GetChildEntry( void BatchUploadRequest::MayCompletePrepare() { if (!committed_ || prepare_callback_.is_null()) return; - for (const auto& child : child_requests_) { + for (auto* child : child_requests_) { if (!child->prepared) return; } @@ -1319,7 +1290,7 @@ void BatchUploadRequest::MayCompletePrepare() { // Build multipart body here. int64_t total_size = 0; std::vector<ContentTypeAndData> parts; - for (auto& child : child_requests_) { + for (auto* child : child_requests_) { std::string type; std::string data; const bool result = child->request->GetContentData(&type, &data); @@ -1347,10 +1318,7 @@ void BatchUploadRequest::MayCompletePrepare() { child->data_size = data.size(); total_size += data.size(); - parts.push_back(ContentTypeAndData()); - parts.back().type = kHttpContentType; - parts.back().data = header; - parts.back().data.append(data); + parts.push_back(ContentTypeAndData({kHttpContentType, header + data})); } UMA_HISTOGRAM_COUNTS_100(kUMADriveTotalFileCountInBatchUpload, parts.size()); @@ -1428,12 +1396,12 @@ void BatchUploadRequest::ProcessURLFetchResults(const net::URLFetcher* source) { } for (size_t i = 0; i < parts.size(); ++i) { - BatchableDelegate* const delegate = child_requests_[i]->request.get(); - // Pass onwership of |delegate| so that child_requests_.clear() won't + BatchableDelegate* delegate = child_requests_[i]->request.get(); + // Pass ownership of |delegate| so that child_requests_.clear() won't // kill the delegate. It has to be deleted after the notification. - delegate->NotifyResult( - parts[i].code, parts[i].body, - base::Bind(&EmptyClosure, base::Passed(&child_requests_[i]->request))); + delegate->NotifyResult(parts[i].code, parts[i].body, + base::Bind(&base::DeletePointer<BatchableDelegate>, + child_requests_[i]->request.release())); } child_requests_.clear(); @@ -1441,16 +1409,15 @@ void BatchUploadRequest::ProcessURLFetchResults(const net::URLFetcher* source) { } void BatchUploadRequest::RunCallbackOnPrematureFailure(DriveApiErrorCode code) { - for (auto child : child_requests_) { + for (auto* child : child_requests_) child->request->NotifyError(code); - } child_requests_.clear(); } void BatchUploadRequest::OnURLFetchUploadProgress(const net::URLFetcher* source, int64_t current, int64_t total) { - for (auto child : child_requests_) { + for (auto* child : child_requests_) { if (child->data_offset <= current && current <= child->data_offset + child->data_size) { child->request->NotifyUploadProgress(source, current - child->data_offset, diff --git a/chromium/google_apis/drive/drive_api_requests.h b/chromium/google_apis/drive/drive_api_requests.h index 3c9ee58d19c..1802d0d66d0 100644 --- a/chromium/google_apis/drive/drive_api_requests.h +++ b/chromium/google_apis/drive/drive_api_requests.h @@ -1140,9 +1140,8 @@ class PermissionsInsertRequest : public EntryActionRequest { // Request that is operated by single BatchableDelegate. class SingleBatchableDelegateRequest : public UrlFetchRequestBase { public: - // The instance takes ownership of |delegate|. SingleBatchableDelegateRequest(RequestSender* sender, - BatchableDelegate* delegate); + std::unique_ptr<BatchableDelegate> delegate); ~SingleBatchableDelegateRequest() override; private: diff --git a/chromium/google_apis/drive/drive_api_requests_unittest.cc b/chromium/google_apis/drive/drive_api_requests_unittest.cc index 67a78e043d1..f249a6eb994 100644 --- a/chromium/google_apis/drive/drive_api_requests_unittest.cc +++ b/chromium/google_apis/drive/drive_api_requests_unittest.cc @@ -13,6 +13,7 @@ #include "base/files/file_util.h" #include "base/files/scoped_temp_dir.h" #include "base/json/json_reader.h" +#include "base/memory/ptr_util.h" #include "base/message_loop/message_loop.h" #include "base/run_loop.h" #include "base/strings/string_number_conversions.h" @@ -114,8 +115,9 @@ class TestBatchableDelegate : public BatchableDelegate { std::vector<int64_t> progress_values_; }; -void EmptyPreapreCallback(DriveApiErrorCode) { +void EmptyPrepareCallback(DriveApiErrorCode) { } + void EmptyClosure() { } @@ -514,15 +516,15 @@ TEST_F(DriveApiRequestsTest, DriveApiDataRequest_Fields) { { base::RunLoop run_loop; - drive::AboutGetRequest* request = new drive::AboutGetRequest( - request_sender_.get(), - *url_generator_, - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error, &about_resource))); + std::unique_ptr<drive::AboutGetRequest> request = + base::MakeUnique<drive::AboutGetRequest>( + request_sender_.get(), *url_generator_, + test_util::CreateQuitCallback( + &run_loop, + test_util::CreateCopyResultCallback(&error, &about_resource))); request->set_fields("kind,quotaBytesTotal,quotaBytesUsedAggregate," "largestChangeId,rootFolderId"); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -558,12 +560,12 @@ TEST_F(DriveApiRequestsTest, FilesInsertRequest) { // Create "new directory" in the root directory. { base::RunLoop run_loop; - drive::FilesInsertRequest* request = new drive::FilesInsertRequest( - request_sender_.get(), - *url_generator_, - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error, &file_resource))); + std::unique_ptr<drive::FilesInsertRequest> request = + base::MakeUnique<drive::FilesInsertRequest>( + request_sender_.get(), *url_generator_, + 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)); @@ -572,7 +574,7 @@ TEST_F(DriveApiRequestsTest, FilesInsertRequest) { request->add_parent("root"); request->set_title("new directory"); request->set_properties(testing_properties_); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -619,12 +621,12 @@ TEST_F(DriveApiRequestsTest, FilesPatchRequest) { { base::RunLoop run_loop; - drive::FilesPatchRequest* request = new drive::FilesPatchRequest( - request_sender_.get(), - *url_generator_, - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error, &file_resource))); + std::unique_ptr<drive::FilesPatchRequest> request = + base::MakeUnique<drive::FilesPatchRequest>( + request_sender_.get(), *url_generator_, + test_util::CreateQuitCallback( + &run_loop, + test_util::CreateCopyResultCallback(&error, &file_resource))); request->set_file_id("resource_id"); request->set_set_modified_date(true); request->set_update_viewed_date(false); @@ -636,7 +638,7 @@ TEST_F(DriveApiRequestsTest, FilesPatchRequest) { request->add_parent("parent_resource_id"); request->set_properties(testing_properties_); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -670,13 +672,13 @@ TEST_F(DriveApiRequestsTest, AboutGetRequest_ValidJson) { { base::RunLoop run_loop; - drive::AboutGetRequest* request = new drive::AboutGetRequest( - request_sender_.get(), - *url_generator_, - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error, &about_resource))); - request_sender_->StartRequestWithAuthRetry(request); + std::unique_ptr<drive::AboutGetRequest> request = + base::MakeUnique<drive::AboutGetRequest>( + request_sender_.get(), *url_generator_, + test_util::CreateQuitCallback( + &run_loop, + test_util::CreateCopyResultCallback(&error, &about_resource))); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -704,13 +706,13 @@ TEST_F(DriveApiRequestsTest, AboutGetRequest_InvalidJson) { { base::RunLoop run_loop; - drive::AboutGetRequest* request = new drive::AboutGetRequest( - request_sender_.get(), - *url_generator_, - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error, &about_resource))); - request_sender_->StartRequestWithAuthRetry(request); + std::unique_ptr<drive::AboutGetRequest> request = + base::MakeUnique<drive::AboutGetRequest>( + request_sender_.get(), *url_generator_, + test_util::CreateQuitCallback( + &run_loop, + test_util::CreateCopyResultCallback(&error, &about_resource))); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -731,14 +733,14 @@ TEST_F(DriveApiRequestsTest, AppsListRequest) { { base::RunLoop run_loop; - drive::AppsListRequest* request = new drive::AppsListRequest( - request_sender_.get(), - *url_generator_, - false, // use_internal_endpoint - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error, &app_list))); - request_sender_->StartRequestWithAuthRetry(request); + std::unique_ptr<drive::AppsListRequest> request = + base::MakeUnique<drive::AppsListRequest>( + request_sender_.get(), *url_generator_, + false, // use_internal_endpoint + test_util::CreateQuitCallback( + &run_loop, + test_util::CreateCopyResultCallback(&error, &app_list))); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -758,15 +760,16 @@ TEST_F(DriveApiRequestsTest, ChangesListRequest) { { base::RunLoop run_loop; - drive::ChangesListRequest* request = new drive::ChangesListRequest( - request_sender_.get(), *url_generator_, - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error, &result))); + std::unique_ptr<drive::ChangesListRequest> request = + base::MakeUnique<drive::ChangesListRequest>( + request_sender_.get(), *url_generator_, + test_util::CreateQuitCallback( + &run_loop, + test_util::CreateCopyResultCallback(&error, &result))); request->set_include_deleted(true); request->set_start_change_id(100); request->set_max_results(500); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -787,14 +790,14 @@ TEST_F(DriveApiRequestsTest, ChangesListNextPageRequest) { { base::RunLoop run_loop; - drive::ChangesListNextPageRequest* request = - new drive::ChangesListNextPageRequest( + std::unique_ptr<drive::ChangesListNextPageRequest> request = + base::MakeUnique<drive::ChangesListNextPageRequest>( request_sender_.get(), test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error, &result))); request->set_next_link(test_server_.GetURL("/continue/get/change/list")); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -818,18 +821,18 @@ TEST_F(DriveApiRequestsTest, FilesCopyRequest) { // Copy the file to a new file named "new title". { base::RunLoop run_loop; - drive::FilesCopyRequest* request = new drive::FilesCopyRequest( - request_sender_.get(), - *url_generator_, - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error, &file_resource))); + std::unique_ptr<drive::FilesCopyRequest> request = + base::MakeUnique<drive::FilesCopyRequest>( + request_sender_.get(), *url_generator_, + 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"); request->set_title("new title"); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -859,15 +862,15 @@ TEST_F(DriveApiRequestsTest, FilesCopyRequest_EmptyParentResourceId) { // Copy the file to a new file named "new title". { base::RunLoop run_loop; - drive::FilesCopyRequest* request = new drive::FilesCopyRequest( - request_sender_.get(), - *url_generator_, - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error, &file_resource))); + std::unique_ptr<drive::FilesCopyRequest> request = + base::MakeUnique<drive::FilesCopyRequest>( + request_sender_.get(), *url_generator_, + test_util::CreateQuitCallback( + &run_loop, + test_util::CreateCopyResultCallback(&error, &file_resource))); request->set_file_id("resource_id"); request->set_title("new title"); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -891,14 +894,15 @@ TEST_F(DriveApiRequestsTest, FilesListRequest) { { base::RunLoop run_loop; - drive::FilesListRequest* request = new drive::FilesListRequest( - request_sender_.get(), *url_generator_, - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error, &result))); + std::unique_ptr<drive::FilesListRequest> request = + base::MakeUnique<drive::FilesListRequest>( + request_sender_.get(), *url_generator_, + test_util::CreateQuitCallback( + &run_loop, + test_util::CreateCopyResultCallback(&error, &result))); request->set_max_results(50); request->set_q("\"abcde\" in parents"); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -919,14 +923,14 @@ TEST_F(DriveApiRequestsTest, FilesListNextPageRequest) { { base::RunLoop run_loop; - drive::FilesListNextPageRequest* request = - new drive::FilesListNextPageRequest( + std::unique_ptr<drive::FilesListNextPageRequest> request = + base::MakeUnique<drive::FilesListNextPageRequest>( request_sender_.get(), test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error, &result))); request->set_next_link(test_server_.GetURL("/continue/get/file/list")); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -942,14 +946,14 @@ TEST_F(DriveApiRequestsTest, FilesDeleteRequest) { // Delete a resource with the given resource id. { base::RunLoop run_loop; - drive::FilesDeleteRequest* request = new drive::FilesDeleteRequest( - request_sender_.get(), - *url_generator_, - test_util::CreateQuitCallback( - &run_loop, test_util::CreateCopyResultCallback(&error))); + std::unique_ptr<drive::FilesDeleteRequest> request = + base::MakeUnique<drive::FilesDeleteRequest>( + request_sender_.get(), *url_generator_, + test_util::CreateQuitCallback( + &run_loop, test_util::CreateCopyResultCallback(&error))); request->set_file_id("resource_id"); request->set_etag(kTestETag); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -972,14 +976,14 @@ TEST_F(DriveApiRequestsTest, FilesTrashRequest) { // Trash a resource with the given resource id. { base::RunLoop run_loop; - drive::FilesTrashRequest* request = new drive::FilesTrashRequest( - request_sender_.get(), - *url_generator_, - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error, &file_resource))); + std::unique_ptr<drive::FilesTrashRequest> request = + base::MakeUnique<drive::FilesTrashRequest>( + request_sender_.get(), *url_generator_, + test_util::CreateQuitCallback( + &run_loop, + test_util::CreateCopyResultCallback(&error, &file_resource))); request->set_file_id("resource_id"); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1001,15 +1005,14 @@ TEST_F(DriveApiRequestsTest, ChildrenInsertRequest) { // "parent_resource_id". { base::RunLoop run_loop; - drive::ChildrenInsertRequest* request = new drive::ChildrenInsertRequest( - request_sender_.get(), - *url_generator_, - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error))); + std::unique_ptr<drive::ChildrenInsertRequest> request = + base::MakeUnique<drive::ChildrenInsertRequest>( + request_sender_.get(), *url_generator_, + test_util::CreateQuitCallback( + &run_loop, test_util::CreateCopyResultCallback(&error))); request->set_folder_id("parent_resource_id"); request->set_id("resource_id"); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1030,15 +1033,14 @@ TEST_F(DriveApiRequestsTest, ChildrenDeleteRequest) { // "parent_resource_id". { base::RunLoop run_loop; - drive::ChildrenDeleteRequest* request = new drive::ChildrenDeleteRequest( - request_sender_.get(), - *url_generator_, - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error))); + std::unique_ptr<drive::ChildrenDeleteRequest> request = + base::MakeUnique<drive::ChildrenDeleteRequest>( + request_sender_.get(), *url_generator_, + test_util::CreateQuitCallback( + &run_loop, test_util::CreateCopyResultCallback(&error))); request->set_child_id("resource_id"); request->set_folder_id("parent_resource_id"); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1056,7 +1058,7 @@ TEST_F(DriveApiRequestsTest, UploadNewFileRequest) { const char kTestContentType[] = "text/plain"; const std::string kTestContent(100, 'a'); const base::FilePath kTestFilePath = - temp_dir_.path().AppendASCII("upload_file.txt"); + temp_dir_.GetPath().AppendASCII("upload_file.txt"); ASSERT_TRUE(test_util::WriteStringToFile(kTestFilePath, kTestContent)); DriveApiErrorCode error = DRIVE_OTHER_ERROR; @@ -1066,19 +1068,17 @@ TEST_F(DriveApiRequestsTest, UploadNewFileRequest) { // "parent_resource_id". { base::RunLoop run_loop; - drive::InitiateUploadNewFileRequest* request = - new drive::InitiateUploadNewFileRequest( - request_sender_.get(), - *url_generator_, - kTestContentType, + std::unique_ptr<drive::InitiateUploadNewFileRequest> request = + base::MakeUnique<drive::InitiateUploadNewFileRequest>( + request_sender_.get(), *url_generator_, kTestContentType, kTestContent.size(), "parent_resource_id", // The resource id of the parent directory. - "new file title", // The title of the file being uploaded. + "new file title", // The title of the file being uploaded. test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error, &upload_url))); request->set_properties(testing_properties_); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1110,20 +1110,18 @@ TEST_F(DriveApiRequestsTest, UploadNewFileRequest) { { base::RunLoop run_loop; - drive::ResumeUploadRequest* resume_request = - new drive::ResumeUploadRequest( - request_sender_.get(), - upload_url, - 0, // start_position + std::unique_ptr<drive::ResumeUploadRequest> request = + base::MakeUnique<drive::ResumeUploadRequest>( + request_sender_.get(), upload_url, + 0, // start_position kTestContent.size(), // end_position (exclusive) kTestContent.size(), // content_length, - kTestContentType, - kTestFilePath, + kTestContentType, kTestFilePath, test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&response, &new_entry)), ProgressCallback()); - request_sender_->StartRequestWithAuthRetry(resume_request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1154,7 +1152,7 @@ TEST_F(DriveApiRequestsTest, UploadNewEmptyFileRequest) { const char kTestContentType[] = "text/plain"; const char kTestContent[] = ""; const base::FilePath kTestFilePath = - temp_dir_.path().AppendASCII("empty_file.txt"); + temp_dir_.GetPath().AppendASCII("empty_file.txt"); ASSERT_TRUE(test_util::WriteStringToFile(kTestFilePath, kTestContent)); DriveApiErrorCode error = DRIVE_OTHER_ERROR; @@ -1163,18 +1161,15 @@ TEST_F(DriveApiRequestsTest, UploadNewEmptyFileRequest) { // Initiate uploading a new file to the directory with "parent_resource_id". { base::RunLoop run_loop; - drive::InitiateUploadNewFileRequest* request = - new drive::InitiateUploadNewFileRequest( - request_sender_.get(), - *url_generator_, - kTestContentType, - 0, + std::unique_ptr<drive::InitiateUploadNewFileRequest> request = + base::MakeUnique<drive::InitiateUploadNewFileRequest>( + request_sender_.get(), *url_generator_, kTestContentType, 0, "parent_resource_id", // The resource id of the parent directory. - "new file title", // The title of the file being uploaded. + "new file title", // The title of the file being uploaded. test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error, &upload_url))); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1201,20 +1196,18 @@ TEST_F(DriveApiRequestsTest, UploadNewEmptyFileRequest) { { base::RunLoop run_loop; - drive::ResumeUploadRequest* resume_request = - new drive::ResumeUploadRequest( - request_sender_.get(), - upload_url, + std::unique_ptr<drive::ResumeUploadRequest> request = + base::MakeUnique<drive::ResumeUploadRequest>( + request_sender_.get(), upload_url, 0, // start_position 0, // end_position (exclusive) 0, // content_length, - kTestContentType, - kTestFilePath, + kTestContentType, kTestFilePath, test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&response, &new_entry)), ProgressCallback()); - request_sender_->StartRequestWithAuthRetry(resume_request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1243,7 +1236,7 @@ TEST_F(DriveApiRequestsTest, UploadNewLargeFileRequest) { const size_t kNumChunkBytes = 10; // Num bytes in a chunk. const std::string kTestContent(100, 'a'); const base::FilePath kTestFilePath = - temp_dir_.path().AppendASCII("upload_file.txt"); + temp_dir_.GetPath().AppendASCII("upload_file.txt"); ASSERT_TRUE(test_util::WriteStringToFile(kTestFilePath, kTestContent)); DriveApiErrorCode error = DRIVE_OTHER_ERROR; @@ -1252,18 +1245,16 @@ TEST_F(DriveApiRequestsTest, UploadNewLargeFileRequest) { // Initiate uploading a new file to the directory with "parent_resource_id". { base::RunLoop run_loop; - drive::InitiateUploadNewFileRequest* request = - new drive::InitiateUploadNewFileRequest( - request_sender_.get(), - *url_generator_, - kTestContentType, + std::unique_ptr<drive::InitiateUploadNewFileRequest> request = + base::MakeUnique<drive::InitiateUploadNewFileRequest>( + request_sender_.get(), *url_generator_, kTestContentType, kTestContent.size(), "parent_resource_id", // The resource id of the parent directory. - "new file title", // The title of the file being uploaded. + "new file title", // The title of the file being uploaded. test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error, &upload_url))); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1294,15 +1285,13 @@ TEST_F(DriveApiRequestsTest, UploadNewLargeFileRequest) { // Check the response by GetUploadStatusRequest. { base::RunLoop run_loop; - drive::GetUploadStatusRequest* get_upload_status_request = - new drive::GetUploadStatusRequest( - request_sender_.get(), - upload_url, - kTestContent.size(), + std::unique_ptr<drive::GetUploadStatusRequest> request = + base::MakeUnique<drive::GetUploadStatusRequest>( + request_sender_.get(), upload_url, kTestContent.size(), test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&response, &new_entry))); - request_sender_->StartRequestWithAuthRetry(get_upload_status_request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1335,20 +1324,16 @@ TEST_F(DriveApiRequestsTest, UploadNewLargeFileRequest) { { base::RunLoop run_loop; - drive::ResumeUploadRequest* resume_request = - new drive::ResumeUploadRequest( - request_sender_.get(), - upload_url, - start_position, - end_position, + std::unique_ptr<drive::ResumeUploadRequest> request = + base::MakeUnique<drive::ResumeUploadRequest>( + request_sender_.get(), upload_url, start_position, end_position, kTestContent.size(), // content_length, - kTestContentType, - kTestFilePath, + kTestContentType, kTestFilePath, test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&response, &new_entry)), ProgressCallback()); - request_sender_->StartRequestWithAuthRetry(resume_request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1385,15 +1370,13 @@ TEST_F(DriveApiRequestsTest, UploadNewLargeFileRequest) { // Check the response by GetUploadStatusRequest. { base::RunLoop run_loop; - drive::GetUploadStatusRequest* get_upload_status_request = - new drive::GetUploadStatusRequest( - request_sender_.get(), - upload_url, - kTestContent.size(), + std::unique_ptr<drive::GetUploadStatusRequest> request = + base::MakeUnique<drive::GetUploadStatusRequest>( + request_sender_.get(), upload_url, kTestContent.size(), test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&response, &new_entry))); - request_sender_->StartRequestWithAuthRetry(get_upload_status_request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1432,21 +1415,19 @@ TEST_F(DriveApiRequestsTest, UploadNewFileWithMetadataRequest) { // Initiate uploading a new file to the directory with "parent_resource_id". { base::RunLoop run_loop; - drive::InitiateUploadNewFileRequest* request = - new drive::InitiateUploadNewFileRequest( - request_sender_.get(), - *url_generator_, - kTestContentType, + std::unique_ptr<drive::InitiateUploadNewFileRequest> request = + base::MakeUnique<drive::InitiateUploadNewFileRequest>( + request_sender_.get(), *url_generator_, kTestContentType, kTestContent.size(), "parent_resource_id", // The resource id of the parent directory. - "new file title", // The title of the file being uploaded. + "new file title", // The title of the file being uploaded. test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error, &upload_url))); request->set_modified_date(base::Time::FromUTCExploded(kModifiedDate)); request->set_last_viewed_by_me_date( base::Time::FromUTCExploded(kLastViewedByMeDate)); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1476,7 +1457,7 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequest) { const char kTestContentType[] = "text/plain"; const std::string kTestContent(100, 'a'); const base::FilePath kTestFilePath = - temp_dir_.path().AppendASCII("upload_file.txt"); + temp_dir_.GetPath().AppendASCII("upload_file.txt"); ASSERT_TRUE(test_util::WriteStringToFile(kTestFilePath, kTestContent)); DriveApiErrorCode error = DRIVE_OTHER_ERROR; @@ -1485,11 +1466,9 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequest) { // Initiate uploading a new file to the directory with "parent_resource_id". { base::RunLoop run_loop; - drive::InitiateUploadExistingFileRequest* request = - new drive::InitiateUploadExistingFileRequest( - request_sender_.get(), - *url_generator_, - kTestContentType, + std::unique_ptr<drive::InitiateUploadExistingFileRequest> request = + base::MakeUnique<drive::InitiateUploadExistingFileRequest>( + request_sender_.get(), *url_generator_, kTestContentType, kTestContent.size(), "resource_id", // The resource id of the file to be overwritten. std::string(), // No etag. @@ -1497,7 +1476,7 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequest) { &run_loop, test_util::CreateCopyResultCallback(&error, &upload_url))); request->set_properties(testing_properties_); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1524,20 +1503,18 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequest) { { base::RunLoop run_loop; - drive::ResumeUploadRequest* resume_request = - new drive::ResumeUploadRequest( - request_sender_.get(), - upload_url, - 0, // start_position + std::unique_ptr<drive::ResumeUploadRequest> request = + base::MakeUnique<drive::ResumeUploadRequest>( + request_sender_.get(), upload_url, + 0, // start_position kTestContent.size(), // end_position (exclusive) kTestContent.size(), // content_length, - kTestContentType, - kTestFilePath, + kTestContentType, kTestFilePath, test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&response, &new_entry)), ProgressCallback()); - request_sender_->StartRequestWithAuthRetry(resume_request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1568,7 +1545,7 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequestWithETag) { const char kTestContentType[] = "text/plain"; const std::string kTestContent(100, 'a'); const base::FilePath kTestFilePath = - temp_dir_.path().AppendASCII("upload_file.txt"); + temp_dir_.GetPath().AppendASCII("upload_file.txt"); ASSERT_TRUE(test_util::WriteStringToFile(kTestFilePath, kTestContent)); DriveApiErrorCode error = DRIVE_OTHER_ERROR; @@ -1577,18 +1554,15 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequestWithETag) { // Initiate uploading a new file to the directory with "parent_resource_id". { base::RunLoop run_loop; - drive::InitiateUploadExistingFileRequest* request = - new drive::InitiateUploadExistingFileRequest( - request_sender_.get(), - *url_generator_, - kTestContentType, + std::unique_ptr<drive::InitiateUploadExistingFileRequest> request = + base::MakeUnique<drive::InitiateUploadExistingFileRequest>( + request_sender_.get(), *url_generator_, kTestContentType, kTestContent.size(), "resource_id", // The resource id of the file to be overwritten. - kTestETag, - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error, &upload_url))); - request_sender_->StartRequestWithAuthRetry(request); + kTestETag, test_util::CreateQuitCallback( + &run_loop, test_util::CreateCopyResultCallback( + &error, &upload_url))); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1611,20 +1585,18 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequestWithETag) { { base::RunLoop run_loop; - drive::ResumeUploadRequest* resume_request = - new drive::ResumeUploadRequest( - request_sender_.get(), - upload_url, - 0, // start_position + std::unique_ptr<drive::ResumeUploadRequest> request = + base::MakeUnique<drive::ResumeUploadRequest>( + request_sender_.get(), upload_url, + 0, // start_position kTestContent.size(), // end_position (exclusive) kTestContent.size(), // content_length, - kTestContentType, - kTestFilePath, + kTestContentType, kTestFilePath, test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&response, &new_entry)), ProgressCallback()); - request_sender_->StartRequestWithAuthRetry(resume_request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1666,18 +1638,16 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileRequestWithETagConflicting) { // Initiate uploading a new file to the directory with "parent_resource_id". { base::RunLoop run_loop; - drive::InitiateUploadExistingFileRequest* request = - new drive::InitiateUploadExistingFileRequest( - request_sender_.get(), - *url_generator_, - kTestContentType, + std::unique_ptr<drive::InitiateUploadExistingFileRequest> request = + base::MakeUnique<drive::InitiateUploadExistingFileRequest>( + request_sender_.get(), *url_generator_, kTestContentType, kTestContent.size(), "resource_id", // The resource id of the file to be overwritten. "Conflicting-etag", test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&error, &upload_url))); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1702,7 +1672,7 @@ TEST_F(DriveApiRequestsTest, const char kTestContentType[] = "text/plain"; const std::string kTestContent(100, 'a'); const base::FilePath kTestFilePath = - temp_dir_.path().AppendASCII("upload_file.txt"); + temp_dir_.GetPath().AppendASCII("upload_file.txt"); ASSERT_TRUE(test_util::WriteStringToFile(kTestFilePath, kTestContent)); DriveApiErrorCode error = DRIVE_OTHER_ERROR; @@ -1711,18 +1681,15 @@ TEST_F(DriveApiRequestsTest, // Initiate uploading a new file to the directory with "parent_resource_id". { base::RunLoop run_loop; - drive::InitiateUploadExistingFileRequest* request = - new drive::InitiateUploadExistingFileRequest( - request_sender_.get(), - *url_generator_, - kTestContentType, + std::unique_ptr<drive::InitiateUploadExistingFileRequest> request = + base::MakeUnique<drive::InitiateUploadExistingFileRequest>( + request_sender_.get(), *url_generator_, kTestContentType, kTestContent.size(), "resource_id", // The resource id of the file to be overwritten. - kTestETag, - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error, &upload_url))); - request_sender_->StartRequestWithAuthRetry(request); + kTestETag, test_util::CreateQuitCallback( + &run_loop, test_util::CreateCopyResultCallback( + &error, &upload_url))); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1750,20 +1717,18 @@ TEST_F(DriveApiRequestsTest, { base::RunLoop run_loop; - drive::ResumeUploadRequest* resume_request = - new drive::ResumeUploadRequest( - request_sender_.get(), - upload_url, - 0, // start_position + std::unique_ptr<drive::ResumeUploadRequest> resume_request = + base::MakeUnique<drive::ResumeUploadRequest>( + request_sender_.get(), upload_url, + 0, // start_position kTestContent.size(), // end_position (exclusive) kTestContent.size(), // content_length, - kTestContentType, - kTestFilePath, + kTestContentType, kTestFilePath, test_util::CreateQuitCallback( &run_loop, test_util::CreateCopyResultCallback(&response, &new_entry)), ProgressCallback()); - request_sender_->StartRequestWithAuthRetry(resume_request); + request_sender_->StartRequestWithAuthRetry(std::move(resume_request)); run_loop.Run(); } @@ -1807,24 +1772,21 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileWithMetadataRequest) { // Initiate uploading a new file to the directory with "parent_resource_id". { base::RunLoop run_loop; - drive::InitiateUploadExistingFileRequest* request = - new drive::InitiateUploadExistingFileRequest( - request_sender_.get(), - *url_generator_, - kTestContentType, + std::unique_ptr<drive::InitiateUploadExistingFileRequest> request = + base::MakeUnique<drive::InitiateUploadExistingFileRequest>( + request_sender_.get(), *url_generator_, kTestContentType, kTestContent.size(), "resource_id", // The resource id of the file to be overwritten. - kTestETag, - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error, &upload_url))); + kTestETag, test_util::CreateQuitCallback( + &run_loop, test_util::CreateCopyResultCallback( + &error, &upload_url))); request->set_parent_resource_id("new_parent_resource_id"); request->set_title("new file title"); request->set_modified_date(base::Time::FromUTCExploded(kModifiedDate)); request->set_last_viewed_by_me_date( base::Time::FromUTCExploded(kLastViewedByMeDate)); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1851,24 +1813,22 @@ TEST_F(DriveApiRequestsTest, UploadExistingFileWithMetadataRequest) { TEST_F(DriveApiRequestsTest, DownloadFileRequest) { const base::FilePath kDownloadedFilePath = - temp_dir_.path().AppendASCII("cache_file"); + temp_dir_.GetPath().AppendASCII("cache_file"); const std::string kTestId("dummyId"); DriveApiErrorCode result_code = DRIVE_OTHER_ERROR; base::FilePath temp_file; { base::RunLoop run_loop; - drive::DownloadFileRequest* request = new drive::DownloadFileRequest( - request_sender_.get(), - *url_generator_, - kTestId, - kDownloadedFilePath, - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&result_code, &temp_file)), - GetContentCallback(), - ProgressCallback()); - request_sender_->StartRequestWithAuthRetry(request); + std::unique_ptr<drive::DownloadFileRequest> request = + base::MakeUnique<drive::DownloadFileRequest>( + request_sender_.get(), *url_generator_, kTestId, + kDownloadedFilePath, + test_util::CreateQuitCallback( + &run_loop, + test_util::CreateCopyResultCallback(&result_code, &temp_file)), + GetContentCallback(), ProgressCallback()); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1888,7 +1848,7 @@ TEST_F(DriveApiRequestsTest, DownloadFileRequest) { TEST_F(DriveApiRequestsTest, DownloadFileRequest_GetContentCallback) { const base::FilePath kDownloadedFilePath = - temp_dir_.path().AppendASCII("cache_file"); + temp_dir_.GetPath().AppendASCII("cache_file"); const std::string kTestId("dummyId"); DriveApiErrorCode result_code = DRIVE_OTHER_ERROR; @@ -1896,17 +1856,15 @@ TEST_F(DriveApiRequestsTest, DownloadFileRequest_GetContentCallback) { std::string contents; { base::RunLoop run_loop; - drive::DownloadFileRequest* request = new drive::DownloadFileRequest( - request_sender_.get(), - *url_generator_, - kTestId, - kDownloadedFilePath, - test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&result_code, &temp_file)), - base::Bind(&AppendContent, &contents), - ProgressCallback()); - request_sender_->StartRequestWithAuthRetry(request); + std::unique_ptr<drive::DownloadFileRequest> request = + base::MakeUnique<drive::DownloadFileRequest>( + request_sender_.get(), *url_generator_, kTestId, + kDownloadedFilePath, + test_util::CreateQuitCallback( + &run_loop, + test_util::CreateCopyResultCallback(&result_code, &temp_file)), + base::Bind(&AppendContent, &contents), ProgressCallback()); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1931,18 +1889,16 @@ TEST_F(DriveApiRequestsTest, PermissionsInsertRequest) { // Add comment permission to the user "user@example.com". { base::RunLoop run_loop; - drive::PermissionsInsertRequest* request = - new drive::PermissionsInsertRequest( - request_sender_.get(), - *url_generator_, + std::unique_ptr<drive::PermissionsInsertRequest> request = + base::MakeUnique<drive::PermissionsInsertRequest>( + request_sender_.get(), *url_generator_, test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error))); + &run_loop, test_util::CreateCopyResultCallback(&error))); request->set_id("resource_id"); request->set_role(drive::PERMISSION_ROLE_COMMENTER); request->set_type(drive::PERMISSION_TYPE_USER); request->set_value("user@example.com"); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -1966,18 +1922,16 @@ TEST_F(DriveApiRequestsTest, PermissionsInsertRequest) { error = DRIVE_OTHER_ERROR; { base::RunLoop run_loop; - drive::PermissionsInsertRequest* request = - new drive::PermissionsInsertRequest( - request_sender_.get(), - *url_generator_, + std::unique_ptr<drive::PermissionsInsertRequest> request = + base::MakeUnique<drive::PermissionsInsertRequest>( + request_sender_.get(), *url_generator_, test_util::CreateQuitCallback( - &run_loop, - test_util::CreateCopyResultCallback(&error))); + &run_loop, test_util::CreateCopyResultCallback(&error))); request->set_id("resource_id2"); request->set_role(drive::PERMISSION_ROLE_WRITER); request->set_type(drive::PERMISSION_TYPE_DOMAIN); request->set_value("example.com"); - request_sender_->StartRequestWithAuthRetry(request); + request_sender_->StartRequestWithAuthRetry(std::move(request)); run_loop.Run(); } @@ -2001,14 +1955,16 @@ TEST_F(DriveApiRequestsTest, BatchUploadRequest) { const char kTestContentType[] = "text/plain"; const std::string kTestContent(10, 'a'); const base::FilePath kTestFilePath = - temp_dir_.path().AppendASCII("upload_file.txt"); + temp_dir_.GetPath().AppendASCII("upload_file.txt"); ASSERT_TRUE(test_util::WriteStringToFile(kTestFilePath, kTestContent)); // Create batch request. - drive::BatchUploadRequest* const request = - new drive::BatchUploadRequest(request_sender_.get(), *url_generator_); - request->SetBoundaryForTesting("OUTERBOUNDARY"); - request_sender_->StartRequestWithAuthRetry(request); + std::unique_ptr<drive::BatchUploadRequest> request = + base::MakeUnique<drive::BatchUploadRequest>(request_sender_.get(), + *url_generator_); + drive::BatchUploadRequest* request_ptr = request.get(); + request_ptr->SetBoundaryForTesting("OUTERBOUNDARY"); + request_sender_->StartRequestWithAuthRetry(std::move(request)); // Create child request. DriveApiErrorCode errors[] = {DRIVE_OTHER_ERROR, DRIVE_OTHER_ERROR}; @@ -2026,9 +1982,9 @@ TEST_F(DriveApiRequestsTest, BatchUploadRequest) { base::Time(), base::Time(), kTestFilePath, drive::Properties(), *url_generator_, callback, ProgressCallback()); child_request->SetBoundaryForTesting("INNERBOUNDARY"); - request->AddRequest(child_request); + request_ptr->AddRequest(child_request); } - request->Commit(); + request_ptr->Commit(); run_loop[0].Run(); run_loop[1].Run(); @@ -2082,21 +2038,14 @@ TEST_F(DriveApiRequestsTest, BatchUploadRequest) { EXPECT_EQ(HTTP_SERVICE_UNAVAILABLE, errors[1]); } -TEST_F(DriveApiRequestsTest, EmptyBatchUploadRequest) { - drive::BatchUploadRequest* const request = - new drive::BatchUploadRequest(request_sender_.get(), *url_generator_); - base::WeakPtr<drive::BatchUploadRequest> weak_ptr = - request->GetWeakPtrAsBatchUploadRequest(); - request->Commit(); - ASSERT_FALSE(weak_ptr.get()); -} - TEST_F(DriveApiRequestsTest, BatchUploadRequestWithBodyIncludingZero) { // Create batch request. - drive::BatchUploadRequest* const request = - new drive::BatchUploadRequest(request_sender_.get(), *url_generator_); - request->SetBoundaryForTesting("OUTERBOUNDARY"); - request_sender_->StartRequestWithAuthRetry(request); + std::unique_ptr<drive::BatchUploadRequest> request = + base::MakeUnique<drive::BatchUploadRequest>(request_sender_.get(), + *url_generator_); + drive::BatchUploadRequest* request_ptr = request.get(); + request_ptr->SetBoundaryForTesting("OUTERBOUNDARY"); + request_sender_->StartRequestWithAuthRetry(std::move(request)); // Create child request. { @@ -2104,8 +2053,8 @@ TEST_F(DriveApiRequestsTest, BatchUploadRequestWithBodyIncludingZero) { TestBatchableDelegate* const child_request = new TestBatchableDelegate( GURL("http://example.com/test"), "application/binary", std::string("Apple\0Orange\0", 13), loop.QuitClosure()); - request->AddRequest(child_request); - request->Commit(); + request_ptr->AddRequest(child_request); + request_ptr->Commit(); loop.Run(); } @@ -2130,8 +2079,9 @@ TEST_F(DriveApiRequestsTest, BatchUploadRequestWithBodyIncludingZero) { TEST_F(DriveApiRequestsTest, BatchUploadRequestProgress) { // Create batch request. - drive::BatchUploadRequest* const request = - new drive::BatchUploadRequest(request_sender_.get(), *url_generator_); + std::unique_ptr<drive::BatchUploadRequest> request = + base::MakeUnique<drive::BatchUploadRequest>(request_sender_.get(), + *url_generator_); TestBatchableDelegate* requests[] = { new TestBatchableDelegate(GURL("http://example.com/test"), "application/binary", std::string(100, 'a'), @@ -2148,7 +2098,7 @@ TEST_F(DriveApiRequestsTest, BatchUploadRequestProgress) { request->AddRequest(requests[1]); request->AddRequest(requests[2]); request->Commit(); - request->Prepare(base::Bind(&EmptyPreapreCallback)); + request->Prepare(base::Bind(&EmptyPrepareCallback)); request->OnURLFetchUploadProgress(nullptr, 0, kExpectedUploadDataSize); request->OnURLFetchUploadProgress(nullptr, 150, kExpectedUploadDataSize); diff --git a/chromium/google_apis/drive/files_list_request_runner.cc b/chromium/google_apis/drive/files_list_request_runner.cc index 1872eea8f84..a7f29d6ea07 100644 --- a/chromium/google_apis/drive/files_list_request_runner.cc +++ b/chromium/google_apis/drive/files_list_request_runner.cc @@ -7,6 +7,7 @@ #include <utility> #include "base/bind.h" +#include "base/memory/ptr_util.h" #include "base/metrics/histogram_macros.h" #include "base/metrics/sparse_histogram.h" #include "google_apis/drive/drive_api_error_codes.h" @@ -34,15 +35,17 @@ CancelCallback FilesListRequestRunner::CreateAndStartWithSizeBackoff( UMA_HISTOGRAM_COUNTS_1000("Drive.FilesListRequestRunner.MaxResults", max_results); base::Closure* const cancel_callback = new base::Closure; - drive::FilesListRequest* const request = new drive::FilesListRequest( - request_sender_, url_generator_, - base::Bind(&FilesListRequestRunner::OnCompleted, - weak_ptr_factory_.GetWeakPtr(), max_results, q, fields, - callback, base::Owned(cancel_callback))); + std::unique_ptr<drive::FilesListRequest> request = + base::MakeUnique<drive::FilesListRequest>( + request_sender_, url_generator_, + base::Bind(&FilesListRequestRunner::OnCompleted, + weak_ptr_factory_.GetWeakPtr(), max_results, q, fields, + callback, base::Owned(cancel_callback))); request->set_max_results(max_results); request->set_q(q); request->set_fields(fields); - *cancel_callback = request_sender_->StartRequestWithAuthRetry(request); + *cancel_callback = + request_sender_->StartRequestWithAuthRetry(std::move(request)); // The cancellation callback is owned by the completion callback, so it must // not be used after |callback| is called. diff --git a/chromium/google_apis/drive/request_sender.cc b/chromium/google_apis/drive/request_sender.cc index 2f8b4c8bf88..dc946881c0d 100644 --- a/chromium/google_apis/drive/request_sender.cc +++ b/chromium/google_apis/drive/request_sender.cc @@ -4,9 +4,10 @@ #include "google_apis/drive/request_sender.h" +#include <algorithm> + #include "base/bind.h" #include "base/sequenced_task_runner.h" -#include "base/stl_util.h" #include "google_apis/drive/auth_service.h" #include "google_apis/drive/base_requests.h" #include "net/url_request/url_request_context_getter.h" @@ -27,32 +28,33 @@ RequestSender::RequestSender( RequestSender::~RequestSender() { DCHECK(thread_checker_.CalledOnValidThread()); - STLDeleteContainerPointers(in_flight_requests_.begin(), - in_flight_requests_.end()); } base::Closure RequestSender::StartRequestWithAuthRetry( - AuthenticatedRequestInterface* request) { + std::unique_ptr<AuthenticatedRequestInterface> request) { DCHECK(thread_checker_.CalledOnValidThread()); - in_flight_requests_.insert(request); + AuthenticatedRequestInterface* request_ptr = request.get(); + in_flight_requests_.insert(std::move(request)); + + return StartRequestWithAuthRetryInternal(request_ptr); +} +base::Closure RequestSender::StartRequestWithAuthRetryInternal( + AuthenticatedRequestInterface* request) { // TODO(kinaba): Stop relying on weak pointers. Move lifetime management // of the requests to request sender. base::Closure cancel_closure = - base::Bind(&RequestSender::CancelRequest, - weak_ptr_factory_.GetWeakPtr(), + base::Bind(&RequestSender::CancelRequest, weak_ptr_factory_.GetWeakPtr(), request->GetWeakPtr()); if (!auth_service_->HasAccessToken()) { // Fetch OAuth2 access token from the refresh token first. auth_service_->StartAuthentication( base::Bind(&RequestSender::OnAccessTokenFetched, - weak_ptr_factory_.GetWeakPtr(), - request->GetWeakPtr())); + weak_ptr_factory_.GetWeakPtr(), request->GetWeakPtr())); } else { - request->Start(auth_service_->access_token(), - custom_user_agent_, + request->Start(auth_service_->access_token(), custom_user_agent_, base::Bind(&RequestSender::RetryRequest, weak_ptr_factory_.GetWeakPtr())); } @@ -72,7 +74,7 @@ void RequestSender::OnAccessTokenFetched( if (code == HTTP_SUCCESS) { DCHECK(auth_service_->HasAccessToken()); - StartRequestWithAuthRetry(request.get()); + StartRequestWithAuthRetryInternal(request.get()); } else { request->OnAuthFailed(code); } @@ -84,7 +86,7 @@ void RequestSender::RetryRequest(AuthenticatedRequestInterface* request) { auth_service_->ClearAccessToken(); // User authentication might have expired - rerun the request to force // auth token refresh. - StartRequestWithAuthRetry(request); + StartRequestWithAuthRetryInternal(request); } void RequestSender::CancelRequest( @@ -98,8 +100,19 @@ void RequestSender::CancelRequest( } void RequestSender::RequestFinished(AuthenticatedRequestInterface* request) { - in_flight_requests_.erase(request); - delete request; + auto it = std::find_if( + in_flight_requests_.begin(), in_flight_requests_.end(), + [request](const std::unique_ptr<AuthenticatedRequestInterface>& ptr) { + return ptr.get() == request; + }); + if (it == in_flight_requests_.end()) { + // Various BatchUpload tests in DriveApiRequestsTest will commit requests + // using this RequestSender without actually starting them on it. In that + // case, there's nothing to be done, so just return. + return; + } + + in_flight_requests_.erase(it); } } // namespace google_apis diff --git a/chromium/google_apis/drive/request_sender.h b/chromium/google_apis/drive/request_sender.h index 9925e09236d..6fb7437a006 100644 --- a/chromium/google_apis/drive/request_sender.h +++ b/chromium/google_apis/drive/request_sender.h @@ -63,20 +63,21 @@ class RequestSender { // Starts a request implementing the AuthenticatedRequestInterface // interface, and makes the request retry upon authentication failures by - // calling back to RetryRequest. The |request| object is owned by this - // RequestSender. It will be deleted in RequestSender's destructor or - // in RequestFinished(). + // calling back to RetryRequest. // // Returns a closure to cancel the request. The closure cancels the request // if it is in-flight, and does nothing if it is already terminated. base::Closure StartRequestWithAuthRetry( - AuthenticatedRequestInterface* request); + std::unique_ptr<AuthenticatedRequestInterface> request); // Notifies to this RequestSender that |request| has finished. // TODO(kinaba): refactor the life time management and make this at private. void RequestFinished(AuthenticatedRequestInterface* request); private: + base::Closure StartRequestWithAuthRetryInternal( + AuthenticatedRequestInterface* request); + // Called when the access token is fetched. void OnAccessTokenFetched( const base::WeakPtr<AuthenticatedRequestInterface>& request, @@ -96,7 +97,7 @@ class RequestSender { scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_; scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_; - std::set<AuthenticatedRequestInterface*> in_flight_requests_; + std::set<std::unique_ptr<AuthenticatedRequestInterface>> in_flight_requests_; const std::string custom_user_agent_; base::ThreadChecker thread_checker_; diff --git a/chromium/google_apis/drive/request_sender_unittest.cc b/chromium/google_apis/drive/request_sender_unittest.cc index fb4a1569be8..97839008296 100644 --- a/chromium/google_apis/drive/request_sender_unittest.cc +++ b/chromium/google_apis/drive/request_sender_unittest.cc @@ -4,6 +4,7 @@ #include "google_apis/drive/request_sender.h" +#include "base/memory/ptr_util.h" #include "base/sequenced_task_runner.h" #include "base/strings/string_number_conversions.h" #include "google_apis/drive/base_requests.h" @@ -134,19 +135,20 @@ class TestRequest : public AuthenticatedRequestInterface { TEST_F(RequestSenderTest, StartAndFinishRequest) { bool start_called = false; FinishReason finish_reason = NONE; - TestRequest* request = new TestRequest(&request_sender_, - &start_called, - &finish_reason); - base::WeakPtr<AuthenticatedRequestInterface> weak_ptr = request->GetWeakPtr(); + std::unique_ptr<TestRequest> request = base::MakeUnique<TestRequest>( + &request_sender_, &start_called, &finish_reason); + TestRequest* request_ptr = request.get(); + base::WeakPtr<AuthenticatedRequestInterface> weak_ptr = + request_ptr->GetWeakPtr(); base::Closure cancel_closure = - request_sender_.StartRequestWithAuthRetry(request); + request_sender_.StartRequestWithAuthRetry(std::move(request)); EXPECT_TRUE(!cancel_closure.is_null()); // Start is called with the specified access token. Let it succeed. EXPECT_TRUE(start_called); - EXPECT_EQ(kTestAccessToken, request->passed_access_token()); - request->FinishRequestWithSuccess(); + EXPECT_EQ(kTestAccessToken, request_ptr->passed_access_token()); + request_ptr->FinishRequestWithSuccess(); EXPECT_FALSE(weak_ptr); // The request object is deleted. // It is safe to run the cancel closure even after the request is finished. @@ -158,13 +160,12 @@ TEST_F(RequestSenderTest, StartAndFinishRequest) { TEST_F(RequestSenderTest, StartAndCancelRequest) { bool start_called = false; FinishReason finish_reason = NONE; - TestRequest* request = new TestRequest(&request_sender_, - &start_called, - &finish_reason); + std::unique_ptr<TestRequest> request = base::MakeUnique<TestRequest>( + &request_sender_, &start_called, &finish_reason); base::WeakPtr<AuthenticatedRequestInterface> weak_ptr = request->GetWeakPtr(); base::Closure cancel_closure = - request_sender_.StartRequestWithAuthRetry(request); + request_sender_.StartRequestWithAuthRetry(std::move(request)); EXPECT_TRUE(!cancel_closure.is_null()); EXPECT_TRUE(start_called); @@ -179,13 +180,12 @@ TEST_F(RequestSenderTest, NoRefreshToken) { bool start_called = false; FinishReason finish_reason = NONE; - TestRequest* request = new TestRequest(&request_sender_, - &start_called, - &finish_reason); + std::unique_ptr<TestRequest> request = base::MakeUnique<TestRequest>( + &request_sender_, &start_called, &finish_reason); base::WeakPtr<AuthenticatedRequestInterface> weak_ptr = request->GetWeakPtr(); base::Closure cancel_closure = - request_sender_.StartRequestWithAuthRetry(request); + request_sender_.StartRequestWithAuthRetry(std::move(request)); EXPECT_TRUE(!cancel_closure.is_null()); // The request is not started at all because no access token is obtained. @@ -199,20 +199,21 @@ TEST_F(RequestSenderTest, ValidRefreshTokenAndNoAccessToken) { bool start_called = false; FinishReason finish_reason = NONE; - TestRequest* request = new TestRequest(&request_sender_, - &start_called, - &finish_reason); - base::WeakPtr<AuthenticatedRequestInterface> weak_ptr = request->GetWeakPtr(); + std::unique_ptr<TestRequest> request = base::MakeUnique<TestRequest>( + &request_sender_, &start_called, &finish_reason); + TestRequest* request_ptr = request.get(); + base::WeakPtr<AuthenticatedRequestInterface> weak_ptr = + request_ptr->GetWeakPtr(); base::Closure cancel_closure = - request_sender_.StartRequestWithAuthRetry(request); + request_sender_.StartRequestWithAuthRetry(std::move(request)); EXPECT_TRUE(!cancel_closure.is_null()); // Access token should indicate that this is the first retry. EXPECT_TRUE(start_called); EXPECT_EQ(kTestAccessToken + std::string("1"), - request->passed_access_token()); - request->FinishRequestWithSuccess(); + request_ptr->passed_access_token()); + request_ptr->FinishRequestWithSuccess(); EXPECT_EQ(SUCCESS, finish_reason); EXPECT_FALSE(weak_ptr); // The request object is deleted. } @@ -220,34 +221,35 @@ TEST_F(RequestSenderTest, ValidRefreshTokenAndNoAccessToken) { TEST_F(RequestSenderTest, AccessTokenRejectedSeveralTimes) { bool start_called = false; FinishReason finish_reason = NONE; - TestRequest* request = new TestRequest(&request_sender_, - &start_called, - &finish_reason); - base::WeakPtr<AuthenticatedRequestInterface> weak_ptr = request->GetWeakPtr(); + std::unique_ptr<TestRequest> request = base::MakeUnique<TestRequest>( + &request_sender_, &start_called, &finish_reason); + TestRequest* request_ptr = request.get(); + base::WeakPtr<AuthenticatedRequestInterface> weak_ptr = + request_ptr->GetWeakPtr(); base::Closure cancel_closure = - request_sender_.StartRequestWithAuthRetry(request); + request_sender_.StartRequestWithAuthRetry(std::move(request)); EXPECT_TRUE(!cancel_closure.is_null()); EXPECT_TRUE(start_called); - EXPECT_EQ(kTestAccessToken, request->passed_access_token()); + EXPECT_EQ(kTestAccessToken, request_ptr->passed_access_token()); // Emulate the case that the access token was rejected by the remote service. - request->passed_reauth_callback().Run(request); + request_ptr->passed_reauth_callback().Run(request_ptr); // New access token is fetched. Let it fail once again. EXPECT_EQ(kTestAccessToken + std::string("1"), - request->passed_access_token()); - request->passed_reauth_callback().Run(request); + request_ptr->passed_access_token()); + request_ptr->passed_reauth_callback().Run(request_ptr); // Once more. EXPECT_EQ(kTestAccessToken + std::string("2"), - request->passed_access_token()); - request->passed_reauth_callback().Run(request); + request_ptr->passed_access_token()); + request_ptr->passed_reauth_callback().Run(request_ptr); // Currently, limit for the retry is controlled in each request object, not // by the RequestSender. So with this TestRequest, RequestSender retries // infinitely. Let it succeed/ EXPECT_EQ(kTestAccessToken + std::string("3"), - request->passed_access_token()); - request->FinishRequestWithSuccess(); + request_ptr->passed_access_token()); + request_ptr->FinishRequestWithSuccess(); EXPECT_EQ(SUCCESS, finish_reason); EXPECT_FALSE(weak_ptr); } diff --git a/chromium/google_apis/drive/time_util.cc b/chromium/google_apis/drive/time_util.cc index cf2b453d38c..5a559cc08fe 100644 --- a/chromium/google_apis/drive/time_util.cc +++ b/chromium/google_apis/drive/time_util.cc @@ -64,7 +64,7 @@ bool GetTimeFromString(const base::StringPiece& raw_value, // Parses timezone suffix on the time part if available. { std::vector<base::StringPiece> parts; - if (time_and_tz[time_and_tz.size() - 1] == 'Z') { + if (time_and_tz.back() == 'Z') { // Timezone is 'Z' (UTC) has_timezone = true; offset_to_utc_in_minutes = 0; diff --git a/chromium/google_apis/gaia/account_tracker.cc b/chromium/google_apis/gaia/account_tracker.cc index 83aed2fb2d8..0a2cce054d3 100644 --- a/chromium/google_apis/gaia/account_tracker.cc +++ b/chromium/google_apis/gaia/account_tracker.cc @@ -28,7 +28,7 @@ AccountTracker::~AccountTracker() { void AccountTracker::Shutdown() { shutdown_called_ = true; - STLDeleteValues(&user_info_requests_); + base::STLDeleteValues(&user_info_requests_); identity_provider_->GetTokenService()->RemoveObserver(this); identity_provider_->RemoveObserver(this); } @@ -205,7 +205,7 @@ void AccountTracker::StartTrackingAccount(const std::string& account_key) { FROM_HERE_WITH_EXPLICIT_FUNCTION( "422460 AccountTracker::StartTrackingAccount")); - if (!ContainsKey(accounts_, account_key)) { + if (!base::ContainsKey(accounts_, account_key)) { DVLOG(1) << "StartTracking " << account_key; AccountState account_state; account_state.ids.account_key = account_key; @@ -217,7 +217,7 @@ void AccountTracker::StartTrackingAccount(const std::string& account_key) { void AccountTracker::StopTrackingAccount(const std::string account_key) { DVLOG(1) << "StopTracking " << account_key; - if (ContainsKey(accounts_, account_key)) { + if (base::ContainsKey(accounts_, account_key)) { AccountState& account = accounts_[account_key]; if (!account.ids.gaia.empty()) { UpdateSignInState(account_key, false); @@ -226,7 +226,7 @@ void AccountTracker::StopTrackingAccount(const std::string account_key) { accounts_.erase(account_key); } - if (ContainsKey(user_info_requests_, account_key)) + if (base::ContainsKey(user_info_requests_, account_key)) DeleteFetcher(user_info_requests_[account_key]); } @@ -242,7 +242,7 @@ void AccountTracker::StartFetchingUserInfo(const std::string& account_key) { FROM_HERE_WITH_EXPLICIT_FUNCTION( "422460 AccountTracker::StartFetchingUserInfo")); - if (ContainsKey(user_info_requests_, account_key)) { + if (base::ContainsKey(user_info_requests_, account_key)) { // TODO(robliao): Remove ScopedTracker below once https://crbug.com/422460 // is fixed. tracked_objects::ScopedTracker tracking_profile1( @@ -278,7 +278,7 @@ void AccountTracker::StartFetchingUserInfo(const std::string& account_key) { void AccountTracker::OnUserInfoFetchSuccess(AccountIdFetcher* fetcher, const std::string& gaia_id) { const std::string& account_key = fetcher->account_key(); - DCHECK(ContainsKey(accounts_, account_key)); + DCHECK(base::ContainsKey(accounts_, account_key)); AccountState& account = accounts_[account_key]; account.ids.gaia = gaia_id; @@ -300,7 +300,7 @@ void AccountTracker::OnUserInfoFetchFailure(AccountIdFetcher* fetcher) { void AccountTracker::DeleteFetcher(AccountIdFetcher* fetcher) { DVLOG(1) << "DeleteFetcher " << fetcher->account_key(); const std::string& account_key = fetcher->account_key(); - DCHECK(ContainsKey(user_info_requests_, account_key)); + DCHECK(base::ContainsKey(user_info_requests_, account_key)); DCHECK_EQ(fetcher, user_info_requests_[account_key]); user_info_requests_.erase(account_key); delete fetcher; diff --git a/chromium/google_apis/gaia/fake_gaia.cc b/chromium/google_apis/gaia/fake_gaia.cc index 583494b3f50..ce62f1f1703 100644 --- a/chromium/google_apis/gaia/fake_gaia.cc +++ b/chromium/google_apis/gaia/fake_gaia.cc @@ -85,7 +85,7 @@ CookieMap GetRequestCookies(const HttpRequest& request) { continue; std::string value = name_value[1]; - if (value.size() && value[value.size() - 1] == ';') + if (value.size() && value.back() == ';') value = value.substr(0, value.size() -1); result.insert(std::make_pair(name_value[0], value)); diff --git a/chromium/google_apis/gaia/gaia_auth_fetcher_unittest.cc b/chromium/google_apis/gaia/gaia_auth_fetcher_unittest.cc index 197597b131e..1e93b128d16 100644 --- a/chromium/google_apis/gaia/gaia_auth_fetcher_unittest.cc +++ b/chromium/google_apis/gaia/gaia_auth_fetcher_unittest.cc @@ -9,6 +9,7 @@ #include "base/json/json_reader.h" #include "base/memory/ref_counted.h" +#include "base/message_loop/message_loop.h" #include "base/strings/stringprintf.h" #include "base/values.h" #include "build/build_config.h" diff --git a/chromium/google_apis/gaia/gaia_auth_util.cc b/chromium/google_apis/gaia/gaia_auth_util.cc index f81b430b462..93b449690dd 100644 --- a/chromium/google_apis/gaia/gaia_auth_util.cc +++ b/chromium/google_apis/gaia/gaia_auth_util.cc @@ -168,8 +168,8 @@ bool ParseListAccountsData(const std::string& data, listed_account.valid = is_email_valid != 0; listed_account.signed_out = signed_out != 0; listed_account.raw_email = email; - auto list = listed_account.signed_out ? signed_out_accounts : - accounts; + auto* list = + listed_account.signed_out ? signed_out_accounts : accounts; if (list) list->push_back(listed_account); } diff --git a/chromium/google_apis/gaia/gaia_oauth_client_unittest.cc b/chromium/google_apis/gaia/gaia_oauth_client_unittest.cc index 695ed94e017..721291b4ebe 100644 --- a/chromium/google_apis/gaia/gaia_oauth_client_unittest.cc +++ b/chromium/google_apis/gaia/gaia_oauth_client_unittest.cc @@ -9,6 +9,7 @@ #include "base/json/json_reader.h" #include "base/macros.h" +#include "base/message_loop/message_loop.h" #include "base/strings/string_number_conversions.h" #include "base/values.h" #include "google_apis/gaia/gaia_oauth_client.h" diff --git a/chromium/google_apis/gaia/gaia_urls.cc b/chromium/google_apis/gaia/gaia_urls.cc index c7026bed254..a5dff93659f 100644 --- a/chromium/google_apis/gaia/gaia_urls.cc +++ b/chromium/google_apis/gaia/gaia_urls.cc @@ -13,7 +13,7 @@ namespace { // Gaia service constants -const char kDefaultGoogleUrl[] = "http://.google.com"; +const char kDefaultGoogleUrl[] = "http://google.com"; const char kDefaultGaiaUrl[] = "https://accounts.google.com"; const char kDefaultGoogleApisBaseUrl[] = "https://www.googleapis.com"; diff --git a/chromium/google_apis/gaia/google_service_auth_error.cc b/chromium/google_apis/gaia/google_service_auth_error.cc index 34c94c1073a..909cf163a2d 100644 --- a/chromium/google_apis/gaia/google_service_auth_error.cc +++ b/chromium/google_apis/gaia/google_service_auth_error.cc @@ -126,6 +126,11 @@ GoogleServiceAuthError GoogleServiceAuthError::AuthErrorNone() { return GoogleServiceAuthError(NONE); } +// static +bool GoogleServiceAuthError::IsDeprecated(State state) { + return state == HOSTED_NOT_ALLOWED_DEPRECATED; +} + GoogleServiceAuthError::State GoogleServiceAuthError::state() const { return state_; } @@ -176,7 +181,6 @@ base::DictionaryValue* GoogleServiceAuthError::ToValue() const { STATE_CASE(SERVICE_UNAVAILABLE); STATE_CASE(TWO_FACTOR); STATE_CASE(REQUEST_CANCELED); - STATE_CASE(HOSTED_NOT_ALLOWED); STATE_CASE(UNEXPECTED_SERVICE_RESPONSE); STATE_CASE(SERVICE_ERROR); STATE_CASE(WEB_LOGIN_REQUIRED); @@ -235,8 +239,6 @@ std::string GoogleServiceAuthError::ToString() const { second_factor_.token.c_str()); case REQUEST_CANCELED: return "Request canceled."; - case HOSTED_NOT_ALLOWED: - return "Google account required."; case UNEXPECTED_SERVICE_RESPONSE: return base::StringPrintf("Unexpected service response (%s)", error_message_.c_str()); diff --git a/chromium/google_apis/gaia/google_service_auth_error.h b/chromium/google_apis/gaia/google_service_auth_error.h index 9549ad78174..2736e5766d7 100644 --- a/chromium/google_apis/gaia/google_service_auth_error.h +++ b/chromium/google_apis/gaia/google_service_auth_error.h @@ -74,9 +74,9 @@ class GoogleServiceAuthError { // prior to completion. REQUEST_CANCELED = 9, - // The user has provided a HOSTED account, when this service requires - // a GOOGLE account. - HOSTED_NOT_ALLOWED = 10, + // HOSTED accounts are deprecated; left in enumeration to match + // GoogleServiceAuthError enum in histograms.xml. + HOSTED_NOT_ALLOWED_DEPRECATED = 10, // Indicates the service responded to a request, but we cannot // interpret the response. @@ -129,11 +129,11 @@ class GoogleServiceAuthError { // Globally identifies the specific second-factor challenge. std::string token; - // Localised prompt text, eg “Enter the verification code sent to your - // phone number ending in XXX”. + // Localized prompt text, e.g. "Enter the verification code sent to your + // phone number ending in XXX". std::string prompt_text; - // Localized text describing an alternate option, eg “Get a verification - // code in a text message”. + // Localized text describing an alternate option, e.g. "Get a verification + // code in a text message". std::string alternate_text; // Character length for the challenge field. int field_length; @@ -176,6 +176,8 @@ class GoogleServiceAuthError { // to explicit class and State enum relation. Note: shouldn't be inlined! static GoogleServiceAuthError AuthErrorNone(); + static bool IsDeprecated(State state); + // The error information. State state() const; const Captcha& captcha() const; diff --git a/chromium/google_apis/gaia/oauth2_token_service.cc b/chromium/google_apis/gaia/oauth2_token_service.cc index 8c703a14179..7e55a491d4e 100644 --- a/chromium/google_apis/gaia/oauth2_token_service.cc +++ b/chromium/google_apis/gaia/oauth2_token_service.cc @@ -11,12 +11,12 @@ #include "base/bind.h" #include "base/location.h" #include "base/macros.h" +#include "base/memory/ptr_util.h" #include "base/memory/weak_ptr.h" #include "base/metrics/histogram_macros.h" #include "base/profiler/scoped_tracker.h" #include "base/rand_util.h" #include "base/single_thread_task_runner.h" -#include "base/stl_util.h" #include "base/threading/thread_task_runner_handle.h" #include "base/time/time.h" #include "base/timer/timer.h" @@ -121,13 +121,14 @@ class OAuth2TokenService::Fetcher : public OAuth2AccessTokenConsumer { // Creates a Fetcher and starts fetching an OAuth2 access token for // |account_id| and |scopes| in the request context obtained by |getter|. // The given |oauth2_token_service| will be informed when fetching is done. - static Fetcher* CreateAndStart(OAuth2TokenService* oauth2_token_service, - const std::string& account_id, - net::URLRequestContextGetter* getter, - const std::string& client_id, - const std::string& client_secret, - const ScopeSet& scopes, - base::WeakPtr<RequestImpl> waiting_request); + static std::unique_ptr<OAuth2TokenService::Fetcher> CreateAndStart( + OAuth2TokenService* oauth2_token_service, + const std::string& account_id, + net::URLRequestContextGetter* getter, + const std::string& client_id, + const std::string& client_secret, + const ScopeSet& scopes, + base::WeakPtr<RequestImpl> waiting_request); ~Fetcher() override; // Add a request that is waiting for the result of this Fetcher. @@ -198,7 +199,8 @@ class OAuth2TokenService::Fetcher : public OAuth2AccessTokenConsumer { }; // static -OAuth2TokenService::Fetcher* OAuth2TokenService::Fetcher::CreateAndStart( +std::unique_ptr<OAuth2TokenService::Fetcher> +OAuth2TokenService::Fetcher::CreateAndStart( OAuth2TokenService* oauth2_token_service, const std::string& account_id, net::URLRequestContextGetter* getter, @@ -206,14 +208,9 @@ OAuth2TokenService::Fetcher* OAuth2TokenService::Fetcher::CreateAndStart( const std::string& client_secret, const OAuth2TokenService::ScopeSet& scopes, base::WeakPtr<RequestImpl> waiting_request) { - OAuth2TokenService::Fetcher* fetcher = new Fetcher( - oauth2_token_service, - account_id, - getter, - client_id, - client_secret, - scopes, - waiting_request); + std::unique_ptr<OAuth2TokenService::Fetcher> fetcher = base::WrapUnique( + new Fetcher(oauth2_token_service, account_id, getter, client_id, + client_secret, scopes, waiting_request)); // TODO(robliao): Remove ScopedTracker below once https://crbug.com/422460 is // fixed. @@ -398,8 +395,7 @@ OAuth2TokenService::OAuth2TokenService(OAuth2TokenServiceDelegate* delegate) OAuth2TokenService::~OAuth2TokenService() { // Release all the pending fetchers. - STLDeleteContainerPairSecondPointers( - pending_fetchers_.begin(), pending_fetchers_.end()); + pending_fetchers_.clear(); } OAuth2TokenServiceDelegate* OAuth2TokenService::GetDelegate() { @@ -553,8 +549,7 @@ void OAuth2TokenService::FetchOAuth2Token(RequestImpl* request, RequestParameters request_parameters = RequestParameters(client_id, account_id, scopes); - std::map<RequestParameters, Fetcher*>::iterator iter = - pending_fetchers_.find(request_parameters); + auto iter = pending_fetchers_.find(request_parameters); if (iter != pending_fetchers_.end()) { iter->second->AddWaitingRequest(request->AsWeakPtr()); return; @@ -694,10 +689,12 @@ void OAuth2TokenService::OnFetchComplete(Fetcher* fetcher) { } } - std::map<RequestParameters, Fetcher*>::iterator iter = - pending_fetchers_.find(request_param); + auto iter = pending_fetchers_.find(request_param); DCHECK(iter != pending_fetchers_.end()); - DCHECK_EQ(fetcher, iter->second); + DCHECK_EQ(fetcher, iter->second.get()); + + // The Fetcher deletes itself. + iter->second.release(); pending_fetchers_.erase(iter); } @@ -784,11 +781,9 @@ void OAuth2TokenService::ClearCacheForAccount(const std::string& account_id) { void OAuth2TokenService::CancelAllRequests() { std::vector<Fetcher*> fetchers_to_cancel; - for (std::map<RequestParameters, Fetcher*>::iterator iter = - pending_fetchers_.begin(); - iter != pending_fetchers_.end(); + for (auto iter = pending_fetchers_.begin(); iter != pending_fetchers_.end(); ++iter) { - fetchers_to_cancel.push_back(iter->second); + fetchers_to_cancel.push_back(iter->second.get()); } CancelFetchers(fetchers_to_cancel); } @@ -796,21 +791,17 @@ void OAuth2TokenService::CancelAllRequests() { void OAuth2TokenService::CancelRequestsForAccount( const std::string& account_id) { std::vector<Fetcher*> fetchers_to_cancel; - for (std::map<RequestParameters, Fetcher*>::iterator iter = - pending_fetchers_.begin(); - iter != pending_fetchers_.end(); + for (auto iter = pending_fetchers_.begin(); iter != pending_fetchers_.end(); ++iter) { if (iter->first.account_id == account_id) - fetchers_to_cancel.push_back(iter->second); + fetchers_to_cancel.push_back(iter->second.get()); } CancelFetchers(fetchers_to_cancel); } void OAuth2TokenService::CancelFetchers( std::vector<Fetcher*> fetchers_to_cancel) { - for (std::vector<OAuth2TokenService::Fetcher*>::iterator iter = - fetchers_to_cancel.begin(); - iter != fetchers_to_cancel.end(); + for (auto iter = fetchers_to_cancel.begin(); iter != fetchers_to_cancel.end(); ++iter) { (*iter)->Cancel(); } @@ -826,11 +817,8 @@ size_t OAuth2TokenService::GetNumPendingRequestsForTesting( const std::string& client_id, const std::string& account_id, const ScopeSet& scopes) const { - PendingFetcherMap::const_iterator iter = pending_fetchers_.find( - OAuth2TokenService::RequestParameters( - client_id, - account_id, - scopes)); + auto iter = pending_fetchers_.find( + OAuth2TokenService::RequestParameters(client_id, account_id, scopes)); return iter == pending_fetchers_.end() ? 0 : iter->second->GetWaitingRequestCount(); } diff --git a/chromium/google_apis/gaia/oauth2_token_service.h b/chromium/google_apis/gaia/oauth2_token_service.h index c52bb3b70ad..77ec6c4c962 100644 --- a/chromium/google_apis/gaia/oauth2_token_service.h +++ b/chromium/google_apis/gaia/oauth2_token_service.h @@ -310,8 +310,6 @@ class OAuth2TokenService : public base::NonThreadSafe { ScopeSet scopes; }; - typedef std::map<RequestParameters, Fetcher*> PendingFetcherMap; - // Provide a request context used for fetching access tokens with the // |StartRequest| method. net::URLRequestContextGetter* GetRequestContext() const; @@ -368,7 +366,7 @@ class OAuth2TokenService : public base::NonThreadSafe { // A map from fetch parameters to a fetcher that is fetching an OAuth2 access // token using these parameters. - PendingFetcherMap pending_fetchers_; + std::map<RequestParameters, std::unique_ptr<Fetcher>> pending_fetchers_; // List of observers to notify when access token status changes. base::ObserverList<DiagnosticsObserver, true> diagnostics_observer_list_; diff --git a/chromium/google_apis/gcm/BUILD.gn b/chromium/google_apis/gcm/BUILD.gn index 494c845933e..335d41c705c 100644 --- a/chromium/google_apis/gcm/BUILD.gn +++ b/chromium/google_apis/gcm/BUILD.gn @@ -85,7 +85,7 @@ proto_library("proto") { defines = [ "GCM_IMPLEMENTATION" ] } -source_set("test_support") { +static_library("test_support") { testonly = true sources = [ "base/fake_encryptor.cc", diff --git a/chromium/google_apis/gcm/base/socket_stream_unittest.cc b/chromium/google_apis/gcm/base/socket_stream_unittest.cc index 78299acf1e8..d82ebb05614 100644 --- a/chromium/google_apis/gcm/base/socket_stream_unittest.cc +++ b/chromium/google_apis/gcm/base/socket_stream_unittest.cc @@ -13,6 +13,7 @@ #include "base/run_loop.h" #include "base/strings/string_piece.h" #include "net/base/ip_address.h" +#include "net/log/net_log_source.h" #include "net/socket/socket_test_util.h" #include "testing/gtest/include/gtest/gtest.h" @@ -176,7 +177,7 @@ void GCMSocketStreamTest::WaitForData(int msg_size) { void GCMSocketStreamTest::OpenConnection() { socket_ = socket_factory_.CreateTransportClientSocket( - address_list_, NULL, NULL, net::NetLog::Source()); + address_list_, NULL, NULL, net::NetLogSource()); socket_->Connect( base::Bind(&GCMSocketStreamTest::ConnectCallback, base::Unretained(this))); diff --git a/chromium/google_apis/gcm/engine/account_mapping.cc b/chromium/google_apis/gcm/engine/account_mapping.cc index 411d177b761..12dbafa0842 100644 --- a/chromium/google_apis/gcm/engine/account_mapping.cc +++ b/chromium/google_apis/gcm/engine/account_mapping.cc @@ -37,9 +37,8 @@ std::string StatusToString(AccountMapping::MappingStatus status) { return kStatusMapped; case AccountMapping::REMOVING: return kStatusRemoving; - default: - NOTREACHED(); } + NOTREACHED(); return std::string(); } diff --git a/chromium/google_apis/gcm/engine/checkin_request.cc b/chromium/google_apis/gcm/engine/checkin_request.cc index a198fef119d..6e527ebe36c 100644 --- a/chromium/google_apis/gcm/engine/checkin_request.cc +++ b/chromium/google_apis/gcm/engine/checkin_request.cc @@ -6,7 +6,7 @@ #include "base/bind.h" #include "base/location.h" -#include "base/metrics/histogram.h" +#include "base/metrics/histogram_macros.h" #include "base/threading/thread_task_runner_handle.h" #include "google_apis/gcm/monitoring/gcm_stats_recorder.h" #include "google_apis/gcm/protocol/checkin.pb.h" @@ -58,10 +58,11 @@ std::string GetCheckinRequestStatusString(CheckinRequestStatus status) { return "RESPONSE_PARSING_FAILED"; case ZERO_ID_OR_TOKEN: return "ZERO_ID_OR_TOKEN"; - default: + case STATUS_COUNT: NOTREACHED(); - return "UNKNOWN_STATUS"; + break; } + return "UNKNOWN_STATUS"; } // Records checkin status to both stats recorder and reports to UMA. diff --git a/chromium/google_apis/gcm/engine/connection_factory_impl.cc b/chromium/google_apis/gcm/engine/connection_factory_impl.cc index 631cad9c483..3fec71ef749 100644 --- a/chromium/google_apis/gcm/engine/connection_factory_impl.cc +++ b/chromium/google_apis/gcm/engine/connection_factory_impl.cc @@ -15,11 +15,10 @@ #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" -#include "net/base/load_flags.h" #include "net/base/net_errors.h" #include "net/http/http_network_session.h" #include "net/http/http_request_headers.h" -#include "net/log/net_log.h" +#include "net/log/net_log_source_type.h" #include "net/proxy/proxy_info.h" #include "net/socket/client_socket_handle.h" #include "net/socket/client_socket_pool_manager.h" @@ -55,22 +54,22 @@ ConnectionFactoryImpl::ConnectionFactoryImpl( net::HttpNetworkSession* http_network_session, net::NetLog* net_log, GCMStatsRecorder* recorder) - : mcs_endpoints_(mcs_endpoints), - next_endpoint_(0), - last_successful_endpoint_(0), - backoff_policy_(backoff_policy), - gcm_network_session_(gcm_network_session), - http_network_session_(http_network_session), - bound_net_log_( - net::BoundNetLog::Make(net_log, net::NetLog::SOURCE_SOCKET)), - pac_request_(NULL), - connecting_(false), - waiting_for_backoff_(false), - waiting_for_network_online_(false), - logging_in_(false), - recorder_(recorder), - listener_(NULL), - weak_ptr_factory_(this) { + : mcs_endpoints_(mcs_endpoints), + next_endpoint_(0), + last_successful_endpoint_(0), + backoff_policy_(backoff_policy), + gcm_network_session_(gcm_network_session), + http_network_session_(http_network_session), + net_log_( + net::NetLogWithSource::Make(net_log, net::NetLogSourceType::SOCKET)), + pac_request_(NULL), + connecting_(false), + waiting_for_backoff_(false), + waiting_for_network_online_(false), + logging_in_(false), + recorder_(recorder), + listener_(NULL), + weak_ptr_factory_(this) { DCHECK_GE(mcs_endpoints_.size(), 1U); DCHECK(!http_network_session_ || (gcm_network_session_ != http_network_session_)); @@ -311,13 +310,12 @@ void ConnectionFactoryImpl::ConnectImpl() { int status = gcm_network_session_->proxy_service()->ResolveProxy( current_endpoint, std::string(), - net::LOAD_NORMAL, &proxy_info_, base::Bind(&ConnectionFactoryImpl::OnProxyResolveDone, weak_ptr_factory_.GetWeakPtr()), &pac_request_, NULL, - bound_net_log_); + net_log_); if (status != net::ERR_IO_PENDING) OnProxyResolveDone(status); } @@ -464,7 +462,7 @@ void ConnectionFactoryImpl::OnProxyResolveDone(int status) { ssl_config, ssl_config, net::PRIVACY_MODE_DISABLED, - bound_net_log_, + net_log_, &socket_handle_, base::Bind(&ConnectionFactoryImpl::OnConnectDone, weak_ptr_factory_.GetWeakPtr())); @@ -532,13 +530,10 @@ int ConnectionFactoryImpl::ReconsiderProxyAfterError(int error) { } int status = gcm_network_session_->proxy_service()->ReconsiderProxyAfterError( - GetCurrentEndpoint(), - std::string(), net::LOAD_NORMAL, error, &proxy_info_, + GetCurrentEndpoint(), std::string(), error, &proxy_info_, base::Bind(&ConnectionFactoryImpl::OnProxyResolveDone, weak_ptr_factory_.GetWeakPtr()), - &pac_request_, - NULL, - bound_net_log_); + &pac_request_, NULL, net_log_); if (status == net::OK || status == net::ERR_IO_PENDING) { CloseSocket(); } else { diff --git a/chromium/google_apis/gcm/engine/connection_factory_impl.h b/chromium/google_apis/gcm/engine/connection_factory_impl.h index 2815337b6d8..04c5e4f9c4b 100644 --- a/chromium/google_apis/gcm/engine/connection_factory_impl.h +++ b/chromium/google_apis/gcm/engine/connection_factory_impl.h @@ -16,6 +16,7 @@ #include "google_apis/gcm/protocol/mcs.pb.h" #include "net/base/backoff_entry.h" #include "net/base/network_change_notifier.h" +#include "net/log/net_log_with_source.h" #include "net/proxy/proxy_info.h" #include "net/proxy/proxy_service.h" #include "net/socket/client_socket_handle.h" @@ -146,7 +147,7 @@ class GCM_EXPORT ConnectionFactoryImpl : // credentials. If nullptr, is ignored. net::HttpNetworkSession* http_network_session_; // Net log to use in connection attempts. - net::BoundNetLog bound_net_log_; + net::NetLogWithSource net_log_; // The current PAC request, if one exists. Owned by the proxy service. net::ProxyService::PacRequest* pac_request_; // The current proxy info. 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 ad09a8bf479..a4cc8d72e86 100644 --- a/chromium/google_apis/gcm/engine/connection_factory_impl_unittest.cc +++ b/chromium/google_apis/gcm/engine/connection_factory_impl_unittest.cc @@ -192,8 +192,7 @@ void TestConnectionFactoryImpl::InitHandler() { std::unique_ptr<net::BackoffEntry> TestConnectionFactoryImpl::CreateBackoffEntry( const net::BackoffEntry::Policy* const policy) { - return base::WrapUnique( - new net::BackoffEntry(&kTestBackoffPolicy, &tick_clock_)); + return base::MakeUnique<net::BackoffEntry>(&kTestBackoffPolicy, &tick_clock_); } std::unique_ptr<ConnectionHandler> diff --git a/chromium/google_apis/gcm/engine/connection_handler_impl.cc b/chromium/google_apis/gcm/engine/connection_handler_impl.cc index 38b2ee4fbe2..d2c071ade81 100644 --- a/chromium/google_apis/gcm/engine/connection_handler_impl.cc +++ b/chromium/google_apis/gcm/engine/connection_handler_impl.cc @@ -229,8 +229,6 @@ void ConnectionHandlerImpl::WaitForData(ProcessingState state) { max_bytes_needed = bytes_left; } break; - default: - NOTREACHED(); } DCHECK_GE(max_bytes_needed, min_bytes_needed); @@ -285,8 +283,6 @@ void ConnectionHandlerImpl::WaitForData(ProcessingState state) { case MCS_PROTO_BYTES: OnGotMessageBytes(); break; - default: - NOTREACHED(); } } diff --git a/chromium/google_apis/gcm/engine/connection_handler_impl_unittest.cc b/chromium/google_apis/gcm/engine/connection_handler_impl_unittest.cc index 97a026849ca..40b6777f22b 100644 --- a/chromium/google_apis/gcm/engine/connection_handler_impl_unittest.cc +++ b/chromium/google_apis/gcm/engine/connection_handler_impl_unittest.cc @@ -21,6 +21,7 @@ #include "google_apis/gcm/base/socket_stream.h" #include "google_apis/gcm/protocol/mcs.pb.h" #include "net/base/ip_address.h" +#include "net/log/net_log_source.h" #include "net/socket/socket_test_util.h" #include "net/socket/stream_socket.h" #include "testing/gtest/include/gtest/gtest.h" @@ -210,7 +211,7 @@ net::StreamSocket* GCMConnectionHandlerImplTest::BuildSocket( socket_factory_.AddSocketDataProvider(data_provider_.get()); socket_ = socket_factory_.CreateTransportClientSocket( - address_list_, NULL, NULL, net::NetLog::Source()); + address_list_, NULL, NULL, net::NetLogSource()); socket_->Connect(net::CompletionCallback()); run_loop_.reset(new base::RunLoop()); diff --git a/chromium/google_apis/gcm/engine/gcm_request_test_base.cc b/chromium/google_apis/gcm/engine/gcm_request_test_base.cc index 5337f7e9819..4968790cdb8 100644 --- a/chromium/google_apis/gcm/engine/gcm_request_test_base.cc +++ b/chromium/google_apis/gcm/engine/gcm_request_test_base.cc @@ -76,6 +76,26 @@ void GCMRequestTestBase::CompleteFetch() { fetcher->delegate()->OnURLFetchComplete(fetcher); } +void GCMRequestTestBase::VerifyFetcherUploadData( + std::map<std::string, std::string>* expected_pairs) { + net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0); + ASSERT_TRUE(fetcher); + + // Verify data was formatted properly. + std::string upload_data = fetcher->upload_data(); + base::StringTokenizer data_tokenizer(upload_data, "&="); + while (data_tokenizer.GetNext()) { + auto iter = expected_pairs->find(data_tokenizer.token()); + ASSERT_TRUE(iter != expected_pairs->end()) << data_tokenizer.token(); + ASSERT_TRUE(data_tokenizer.GetNext()) << data_tokenizer.token(); + ASSERT_EQ(iter->second, data_tokenizer.token()); + // Ensure that none of the keys appears twice. + expected_pairs->erase(iter); + } + + ASSERT_EQ(0UL, expected_pairs->size()); +} + void GCMRequestTestBase::FastForwardToTriggerNextRetry() { // Here we compute the maximum delay time by skipping the jitter fluctuation // that only affects in the negative way. diff --git a/chromium/google_apis/gcm/engine/gcm_request_test_base.h b/chromium/google_apis/gcm/engine/gcm_request_test_base.h index 8781dda837a..3eb384d17af 100644 --- a/chromium/google_apis/gcm/engine/gcm_request_test_base.h +++ b/chromium/google_apis/gcm/engine/gcm_request_test_base.h @@ -31,6 +31,12 @@ class GCMRequestTestBase : public testing::Test { // It can be overridden by the test class to add additional logic. virtual void CompleteFetch(); + // Verifies that the Fetcher's upload_data exactly matches the given + // properties. The map will be cleared as a side-effect. Wrap calls to this + // with ASSERT_NO_FATAL_FAILURE. + void VerifyFetcherUploadData( + std::map<std::string, std::string>* expected_pairs); + net::URLRequestContextGetter* url_request_context_getter() const { return url_request_context_getter_.get(); } diff --git a/chromium/google_apis/gcm/engine/gcm_store_impl.cc b/chromium/google_apis/gcm/engine/gcm_store_impl.cc index 1ed91a9b8f0..59600a11524 100644 --- a/chromium/google_apis/gcm/engine/gcm_store_impl.cc +++ b/chromium/google_apis/gcm/engine/gcm_store_impl.cc @@ -178,6 +178,13 @@ leveldb::Slice MakeSlice(const base::StringPiece& s) { return leveldb::Slice(s.begin(), s.size()); } +bool DatabaseExists(const base::FilePath& path) { + // It's not enough to check that the directory exists, since DestroyDB + // sometimes leaves behind an empty directory + // (https://github.com/google/leveldb/issues/215). + return base::PathExists(path.Append(FILE_PATH_LITERAL("CURRENT"))); +} + } // namespace class GCMStoreImpl::Backend @@ -287,7 +294,7 @@ LoadStatus GCMStoreImpl::Backend::OpenStoreAndLoadData(StoreOpenMode open_mode, // Checks if the store exists or not. Calling DB::Open with create_if_missing // not set will still create a new directory if the store does not exist. - if (open_mode == DO_NOT_CREATE && !base::DirectoryExists(path_)) { + if (open_mode == DO_NOT_CREATE && !DatabaseExists(path_)) { DVLOG(2) << "Database " << path_.value() << " does not exist"; return STORE_DOES_NOT_EXIST; } 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 97ac211c8de..a21facfa242 100644 --- a/chromium/google_apis/gcm/engine/gcm_store_impl_unittest.cc +++ b/chromium/google_apis/gcm/engine/gcm_store_impl_unittest.cc @@ -14,6 +14,7 @@ #include "base/bind.h" #include "base/command_line.h" #include "base/files/file_path.h" +#include "base/files/file_util.h" #include "base/files/scoped_temp_dir.h" #include "base/memory/ptr_util.h" #include "base/strings/string_number_conversions.h" @@ -69,6 +70,7 @@ class GCMStoreImplTest : public testing::Test { scoped_refptr<base::TestSimpleTaskRunner> task_runner_; base::ThreadTaskRunnerHandle task_runner_handle_; base::ScopedTempDir temp_directory_; + base::FilePath store_path_; bool expected_success_; uint64_t next_persistent_id_; }; @@ -84,12 +86,14 @@ GCMStoreImplTest::GCMStoreImplTest() GCMStoreImplTest::~GCMStoreImplTest() {} std::unique_ptr<GCMStoreImpl> GCMStoreImplTest::BuildGCMStore() { - return std::unique_ptr<GCMStoreImpl>(new GCMStoreImpl( - // Pass an non-existent directory as store path to match the exact - // 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")), - task_runner_, base::WrapUnique<Encryptor>(new FakeEncryptor))); + // Pass an non-existent directory as store path to match the exact behavior in + // the production code. Currently GCMStoreImpl checks if the directory exists + // and contains a CURRENT file to determine the store existence. + store_path_ = + temp_directory_.GetPath().Append(FILE_PATH_LITERAL("GCM Store")); + return std::unique_ptr<GCMStoreImpl>( + new GCMStoreImpl(store_path_, task_runner_, + base::WrapUnique<Encryptor>(new FakeEncryptor))); } void GCMStoreImplTest::LoadGCMStore( @@ -140,15 +144,32 @@ TEST_F(GCMStoreImplTest, LoadNew) { EXPECT_EQ(base::Time::FromInternalValue(0LL), load_result->last_checkin_time); } -// Verify new database is not created when DO_NOT_CREATE_NEW_STORE is passed. +// Verify new database is not created when DO_NOT_CREATE is passed. TEST_F(GCMStoreImplTest, LoadWithoutCreatingNewStore) { std::unique_ptr<GCMStoreImpl> gcm_store(BuildGCMStore()); std::unique_ptr<GCMStore::LoadResult> load_result; - gcm_store->Load( - GCMStore::DO_NOT_CREATE, - base::Bind(&GCMStoreImplTest::LoadWithoutCheckCallback, - base::Unretained(this), - &load_result)); + gcm_store->Load(GCMStore::DO_NOT_CREATE, + base::Bind(&GCMStoreImplTest::LoadWithoutCheckCallback, + base::Unretained(this), &load_result)); + PumpLoop(); + + EXPECT_FALSE(load_result->success); + EXPECT_TRUE(load_result->store_does_not_exist); +} + +// Verifies that loads with DO_NOT_CREATE set store_does_not_exist to true when +// an empty directory was left behind after destroying the database. +TEST_F(GCMStoreImplTest, LoadWithEmptyDirectory) { + std::unique_ptr<GCMStoreImpl> gcm_store(BuildGCMStore()); + + // Create an empty directory at the store path, to simulate an empty directory + // being left behind after destroying a previous store. + ASSERT_TRUE(base::CreateDirectory(store_path_)); + + std::unique_ptr<GCMStore::LoadResult> load_result; + gcm_store->Load(GCMStore::DO_NOT_CREATE, + base::Bind(&GCMStoreImplTest::LoadWithoutCheckCallback, + base::Unretained(this), &load_result)); PumpLoop(); EXPECT_FALSE(load_result->success); diff --git a/chromium/google_apis/gcm/engine/gcm_unregistration_request_handler.cc b/chromium/google_apis/gcm/engine/gcm_unregistration_request_handler.cc index 2e1fde66bc6..d623bd19aa7 100644 --- a/chromium/google_apis/gcm/engine/gcm_unregistration_request_handler.cc +++ b/chromium/google_apis/gcm/engine/gcm_unregistration_request_handler.cc @@ -21,8 +21,6 @@ const char kUnregistrationCallerValue[] = "false"; // Response constants. const char kDeletedPrefix[] = "deleted="; -const char kErrorPrefix[] = "Error="; -const char kInvalidParameters[] = "INVALID_PARAMETERS"; } // namespace @@ -38,13 +36,7 @@ void GCMUnregistrationRequestHandler::BuildRequestBody(std::string* body){ } UnregistrationRequest::Status GCMUnregistrationRequestHandler::ParseResponse( - const net::URLFetcher* source) { - std::string response; - if (!source->GetResponseAsString(&response)) { - DVLOG(1) << "Failed to get response body."; - return UnregistrationRequest::NO_RESPONSE_BODY; - } - + const std::string& response) { DVLOG(1) << "Parsing unregistration response."; if (response.find(kDeletedPrefix) != std::string::npos) { std::string deleted_app_id = response.substr( @@ -54,14 +46,6 @@ UnregistrationRequest::Status GCMUnregistrationRequestHandler::ParseResponse( UnregistrationRequest::INCORRECT_APP_ID; } - if (response.find(kErrorPrefix) != std::string::npos) { - std::string error = response.substr( - response.find(kErrorPrefix) + arraysize(kErrorPrefix) - 1); - return error == kInvalidParameters ? - UnregistrationRequest::INVALID_PARAMETERS : - UnregistrationRequest::UNKNOWN_ERROR; - } - DVLOG(1) << "Not able to parse a meaningful output from response body." << response; return UnregistrationRequest::RESPONSE_PARSING_FAILED; diff --git a/chromium/google_apis/gcm/engine/gcm_unregistration_request_handler.h b/chromium/google_apis/gcm/engine/gcm_unregistration_request_handler.h index b5a3b944d32..06c963b9dad 100644 --- a/chromium/google_apis/gcm/engine/gcm_unregistration_request_handler.h +++ b/chromium/google_apis/gcm/engine/gcm_unregistration_request_handler.h @@ -21,7 +21,7 @@ class GCM_EXPORT GCMUnregistrationRequestHandler : // UnregistrationRequest::CustomRequestHandler overrides: void BuildRequestBody(std::string* body) override; UnregistrationRequest::Status ParseResponse( - const net::URLFetcher* source) override; + const std::string& response) override; void ReportUMAs(UnregistrationRequest::Status status, int retry_count, base::TimeDelta complete_time) override; diff --git a/chromium/google_apis/gcm/engine/gservices_settings.cc b/chromium/google_apis/gcm/engine/gservices_settings.cc index cde4d7fbb2b..d66e576c8c3 100644 --- a/chromium/google_apis/gcm/engine/gservices_settings.cc +++ b/chromium/google_apis/gcm/engine/gservices_settings.cc @@ -227,7 +227,8 @@ bool GServicesSettings::UpdateFromCheckinResponse( return false; } - if (settings_diff && name.find(kDeleteSettingPrefix) == 0) { + if (settings_diff && base::StartsWith(name, kDeleteSettingPrefix, + base::CompareCase::SENSITIVE)) { std::string setting_to_delete = name.substr(arraysize(kDeleteSettingPrefix) - 1); new_settings.erase(setting_to_delete); @@ -304,7 +305,7 @@ GURL GServicesSettings::GetMCSMainEndpoint() const { else mcs_hostname = kDefaultMCSHostname; - // Get alternative secure port or use defualt. + // Get alternative secure port or use default. int mcs_secure_port = 0; iter = settings_.find(kMCSSecurePortKey); if (iter == settings_.end() || iter->second.empty() || diff --git a/chromium/google_apis/gcm/engine/instance_id_delete_token_request_handler.cc b/chromium/google_apis/gcm/engine/instance_id_delete_token_request_handler.cc index 35dac620bc0..b3ef7a8cbea 100644 --- a/chromium/google_apis/gcm/engine/instance_id_delete_token_request_handler.cc +++ b/chromium/google_apis/gcm/engine/instance_id_delete_token_request_handler.cc @@ -19,14 +19,11 @@ namespace { const char kGMSVersionKey[] = "gmsv"; const char kInstanceIDKey[] = "appid"; const char kSenderKey[] = "sender"; -const char kSubtypeKey[] = "X-subtype"; const char kScopeKey[] = "scope"; const char kExtraScopeKey[] = "X-scope"; // Response constants. const char kTokenPrefix[] = "token="; -const char kErrorPrefix[] = "Error="; -const char kInvalidParameters[] = "INVALID_PARAMETERS"; } // namespace @@ -52,28 +49,11 @@ void InstanceIDDeleteTokenRequestHandler::BuildRequestBody(std::string* body){ BuildFormEncoding(kScopeKey, scope_, body); BuildFormEncoding(kExtraScopeKey, scope_, body); BuildFormEncoding(kGMSVersionKey, base::IntToString(gcm_version_), body); - // TODO(jianli): To work around server bug. To be removed when the server fix - // is deployed. - BuildFormEncoding(kSubtypeKey, authorized_entity_, body); } UnregistrationRequest::Status InstanceIDDeleteTokenRequestHandler::ParseResponse( - const net::URLFetcher* source) { - std::string response; - if (!source->GetResponseAsString(&response)) { - DVLOG(1) << "Failed to get response body."; - return UnregistrationRequest::NO_RESPONSE_BODY; - } - - if (response.find(kErrorPrefix) != std::string::npos) { - std::string error = response.substr( - response.find(kErrorPrefix) + arraysize(kErrorPrefix) - 1); - return error == kInvalidParameters ? - UnregistrationRequest::INVALID_PARAMETERS : - UnregistrationRequest::UNKNOWN_ERROR; - } - + const std::string& response) { if (response.find(kTokenPrefix) == std::string::npos) return UnregistrationRequest::RESPONSE_PARSING_FAILED; diff --git a/chromium/google_apis/gcm/engine/instance_id_delete_token_request_handler.h b/chromium/google_apis/gcm/engine/instance_id_delete_token_request_handler.h index cb1ed063fd9..4783526cb34 100644 --- a/chromium/google_apis/gcm/engine/instance_id_delete_token_request_handler.h +++ b/chromium/google_apis/gcm/engine/instance_id_delete_token_request_handler.h @@ -26,7 +26,7 @@ class GCM_EXPORT InstanceIDDeleteTokenRequestHandler : // UnregistrationRequest overrides: void BuildRequestBody(std::string* body) override; UnregistrationRequest::Status ParseResponse( - const net::URLFetcher* source) override; + const std::string& response) override; void ReportUMAs(UnregistrationRequest::Status status, int retry_count, base::TimeDelta complete_time) override; diff --git a/chromium/google_apis/gcm/engine/instance_id_get_token_request_handler.cc b/chromium/google_apis/gcm/engine/instance_id_get_token_request_handler.cc index 187db61df2c..3ebda2079b3 100644 --- a/chromium/google_apis/gcm/engine/instance_id_get_token_request_handler.cc +++ b/chromium/google_apis/gcm/engine/instance_id_get_token_request_handler.cc @@ -15,7 +15,6 @@ namespace { // Request constants. const char kAuthorizedEntityKey[] = "sender"; -const char kSubtypeKey[] = "X-subtype"; const char kGMSVersionKey[] = "gmsv"; const char kInstanceIDKey[] = "appid"; const char kScopeKey[] = "scope"; @@ -51,9 +50,6 @@ void InstanceIDGetTokenRequestHandler::BuildRequestBody(std::string* body){ BuildFormEncoding(kGMSVersionKey, base::IntToString(gcm_version_), body); BuildFormEncoding(kInstanceIDKey, instance_id_, body); BuildFormEncoding(kAuthorizedEntityKey, authorized_entity_, body); - // TODO(jianli): To work around server bug. To be removed when the server fix - // is deployed. - BuildFormEncoding(kSubtypeKey, authorized_entity_, body); } void InstanceIDGetTokenRequestHandler::ReportUMAs( diff --git a/chromium/google_apis/gcm/engine/mcs_client.cc b/chromium/google_apis/gcm/engine/mcs_client.cc index 63ec45a3ad8..b2a81fc57be 100644 --- a/chromium/google_apis/gcm/engine/mcs_client.cc +++ b/chromium/google_apis/gcm/engine/mcs_client.cc @@ -155,10 +155,9 @@ std::string MCSClient::GetStateString() const { return "CONNECTING"; case CONNECTED: return "CONNECTED"; - default: - NOTREACHED(); - return std::string(); } + NOTREACHED(); + return std::string(); } MCSClient::MCSClient(const std::string& version_string, diff --git a/chromium/google_apis/gcm/engine/mcs_client.h b/chromium/google_apis/gcm/engine/mcs_client.h index f7796509fea..cd99cad99c1 100644 --- a/chromium/google_apis/gcm/engine/mcs_client.h +++ b/chromium/google_apis/gcm/engine/mcs_client.h @@ -155,7 +155,7 @@ class GCM_EXPORT MCSClient { // Allows a caller to set a heartbeat interval (in milliseconds) with which // the MCS connection will be monitored on both ends, to detect device - // presence. In case the newly set interval is less then the current one, + // presence. In case the newly set interval is smaller than the current one, // connection will be restarted with new heartbeat interval. Valid values have // to be between GetMax/GetMinClientHeartbeatIntervalMs of HeartbeatManager, // otherwise the setting won't take effect. diff --git a/chromium/google_apis/gcm/engine/mcs_client_unittest.cc b/chromium/google_apis/gcm/engine/mcs_client_unittest.cc index c3f57d17a50..5835ed7f7c8 100644 --- a/chromium/google_apis/gcm/engine/mcs_client_unittest.cc +++ b/chromium/google_apis/gcm/engine/mcs_client_unittest.cc @@ -205,7 +205,7 @@ void MCSClientTest::SetUp() { void MCSClientTest::BuildMCSClient() { gcm_store_.reset( - new GCMStoreImpl(temp_directory_.path(), message_loop_.task_runner(), + new GCMStoreImpl(temp_directory_.GetPath(), message_loop_.task_runner(), base::WrapUnique<Encryptor>(new FakeEncryptor))); mcs_client_.reset(new TestMCSClient(&clock_, &connection_factory_, diff --git a/chromium/google_apis/gcm/engine/registration_request.cc b/chromium/google_apis/gcm/engine/registration_request.cc index 287732a7680..386af0e7eab 100644 --- a/chromium/google_apis/gcm/engine/registration_request.cc +++ b/chromium/google_apis/gcm/engine/registration_request.cc @@ -33,7 +33,8 @@ const char kRegistrationRequestContentType[] = "application/x-www-form-urlencoded"; // Request constants. -const char kAppIdKey[] = "app"; +const char kCategoryKey[] = "app"; +const char kSubtypeKey[] = "X-subtype"; const char kDeviceIdKey[] = "device"; const char kLoginHeader[] = "AidLogin"; @@ -44,11 +45,12 @@ const char kDeviceRegistrationError[] = "PHONE_REGISTRATION_ERROR"; const char kAuthenticationFailed[] = "AUTHENTICATION_FAILED"; const char kInvalidSender[] = "INVALID_SENDER"; const char kInvalidParameters[] = "INVALID_PARAMETERS"; +const char kInternalServerError[] = "InternalServerError"; +const char kQuotaExceeded[] = "QUOTA_EXCEEDED"; +const char kTooManyRegistrations[] = "TOO_MANY_REGISTRATIONS"; // Gets correct status from the error message. RegistrationRequest::Status GetStatusFromError(const std::string& error) { - // TODO(fgorski): Improve error parsing in case there is nore then just an - // Error=ERROR_STRING in response. if (error.find(kDeviceRegistrationError) != std::string::npos) return RegistrationRequest::DEVICE_REGISTRATION_ERROR; if (error.find(kAuthenticationFailed) != std::string::npos) @@ -57,28 +59,55 @@ RegistrationRequest::Status GetStatusFromError(const std::string& error) { return RegistrationRequest::INVALID_SENDER; if (error.find(kInvalidParameters) != std::string::npos) return RegistrationRequest::INVALID_PARAMETERS; + if (error.find(kInternalServerError) != std::string::npos) + return RegistrationRequest::INTERNAL_SERVER_ERROR; + if (error.find(kQuotaExceeded) != std::string::npos) + return RegistrationRequest::QUOTA_EXCEEDED; + if (error.find(kTooManyRegistrations) != std::string::npos) + return RegistrationRequest::TOO_MANY_REGISTRATIONS; + // Should not be reached, unless the server adds new error types. return RegistrationRequest::UNKNOWN_ERROR; } -// Indicates whether a retry attempt should be made based on the status of the -// last request. +// Determines whether to retry based on the status of the last request. bool ShouldRetryWithStatus(RegistrationRequest::Status status) { - return status == RegistrationRequest::UNKNOWN_ERROR || - status == RegistrationRequest::AUTHENTICATION_FAILED || - status == RegistrationRequest::DEVICE_REGISTRATION_ERROR || - status == RegistrationRequest::HTTP_NOT_OK || - status == RegistrationRequest::URL_FETCHING_FAILED || - status == RegistrationRequest::RESPONSE_PARSING_FAILED; + switch (status) { + case RegistrationRequest::AUTHENTICATION_FAILED: + case RegistrationRequest::DEVICE_REGISTRATION_ERROR: + case RegistrationRequest::UNKNOWN_ERROR: + case RegistrationRequest::URL_FETCHING_FAILED: + case RegistrationRequest::HTTP_NOT_OK: + case RegistrationRequest::NO_RESPONSE_BODY: + case RegistrationRequest::RESPONSE_PARSING_FAILED: + case RegistrationRequest::INTERNAL_SERVER_ERROR: + return true; + case RegistrationRequest::SUCCESS: + case RegistrationRequest::INVALID_PARAMETERS: + case RegistrationRequest::INVALID_SENDER: + case RegistrationRequest::QUOTA_EXCEEDED: + case RegistrationRequest::TOO_MANY_REGISTRATIONS: + case RegistrationRequest::REACHED_MAX_RETRIES: + return false; + case RegistrationRequest::STATUS_COUNT: + NOTREACHED(); + break; + } + return false; } } // namespace RegistrationRequest::RequestInfo::RequestInfo(uint64_t android_id, uint64_t security_token, - const std::string& app_id) - : android_id(android_id), security_token(security_token), app_id(app_id) { + const std::string& category, + const std::string& subtype) + : android_id(android_id), + security_token(security_token), + category(category), + subtype(subtype) { DCHECK(android_id != 0UL); DCHECK(security_token != 0UL); + DCHECK(!category.empty()); } RegistrationRequest::RequestInfo::~RequestInfo() {} @@ -129,10 +158,10 @@ void RegistrationRequest::Start() { std::string body; BuildRequestBody(&body); - DVLOG(1) << "Performing registration for: " << request_info_.app_id; + DVLOG(1) << "Performing registration for: " << request_info_.app_id(); DVLOG(1) << "Registration request: " << body; url_fetcher_->SetUploadData(kRegistrationRequestContentType, body); - recorder_->RecordRegistrationSent(request_info_.app_id, source_to_record_); + recorder_->RecordRegistrationSent(request_info_.app_id(), source_to_record_); request_start_time_ = base::TimeTicks::Now(); url_fetcher_->Start(); } @@ -148,7 +177,10 @@ void RegistrationRequest::BuildRequestHeaders(std::string* extra_headers) { } void RegistrationRequest::BuildRequestBody(std::string* body) { - BuildFormEncoding(kAppIdKey, request_info_.app_id, body); + BuildFormEncoding(kCategoryKey, request_info_.category, body); + if (!request_info_.subtype.empty()) + BuildFormEncoding(kSubtypeKey, request_info_.subtype, body); + BuildFormEncoding(kDeviceIdKey, base::Uint64ToString(request_info_.android_id), body); @@ -163,15 +195,12 @@ void RegistrationRequest::RetryWithBackoff() { url_fetcher_.reset(); backoff_entry_.InformOfRequest(false); - DVLOG(1) << "Delaying GCM registration of app: " - << request_info_.app_id << ", for " - << backoff_entry_.GetTimeUntilRelease().InMilliseconds() + 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); + 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, @@ -182,51 +211,48 @@ void RegistrationRequest::RetryWithBackoff() { RegistrationRequest::Status RegistrationRequest::ParseResponse( const net::URLFetcher* source, std::string* token) { if (!source->GetStatus().is_success()) { - LOG(ERROR) << "URL fetching failed."; + DVLOG(1) << "Registration URL fetching failed."; return URL_FETCHING_FAILED; } std::string response; if (!source->GetResponseAsString(&response)) { - LOG(ERROR) << "Failed to parse registration response as a string."; - return RESPONSE_PARSING_FAILED; + DVLOG(1) << "Failed to get registration response body."; + return NO_RESPONSE_BODY; } - if (source->GetResponseCode() == net::HTTP_OK) { - size_t token_pos = response.find(kTokenPrefix); - if (token_pos != std::string::npos) { - *token = response.substr(token_pos + arraysize(kTokenPrefix) - 1); - return SUCCESS; - } - } - - // If we are able to parse a meaningful known error, let's do so. Some errors - // will have HTTP_BAD_REQUEST, some will have HTTP_OK response code. + // If we are able to parse a meaningful known error, let's do so. Note that + // some errors will have HTTP_OK response code! size_t error_pos = response.find(kErrorPrefix); if (error_pos != std::string::npos) { std::string error = response.substr( error_pos + arraysize(kErrorPrefix) - 1); + DVLOG(1) << "Registration response error message: " << error; return GetStatusFromError(error); } // If we cannot tell what the error is, but at least we know response code was // not OK. if (source->GetResponseCode() != net::HTTP_OK) { - DLOG(ERROR) << "URL fetching HTTP response code is not OK. It is " - << source->GetResponseCode(); + DVLOG(1) << "Registration HTTP response code not OK: " + << source->GetResponseCode(); return HTTP_NOT_OK; } - return UNKNOWN_ERROR; + size_t token_pos = response.find(kTokenPrefix); + if (token_pos != std::string::npos) { + *token = response.substr(token_pos + arraysize(kTokenPrefix) - 1); + return SUCCESS; + } + + return RESPONSE_PARSING_FAILED; } void RegistrationRequest::OnURLFetchComplete(const net::URLFetcher* source) { std::string token; Status status = ParseResponse(source, &token); - recorder_->RecordRegistrationResponse( - request_info_.app_id, - source_to_record_, - status); + recorder_->RecordRegistrationResponse(request_info_.app_id(), + source_to_record_, status); DCHECK(custom_request_handler_.get()); custom_request_handler_->ReportUMAs( @@ -241,10 +267,8 @@ void RegistrationRequest::OnURLFetchComplete(const net::URLFetcher* source) { } status = REACHED_MAX_RETRIES; - recorder_->RecordRegistrationResponse( - request_info_.app_id, - source_to_record_, - status); + recorder_->RecordRegistrationResponse(request_info_.app_id(), + source_to_record_, status); // Only REACHED_MAX_RETRIES is reported because the function will skip // reporting count and time when status is not SUCCESS. diff --git a/chromium/google_apis/gcm/engine/registration_request.h b/chromium/google_apis/gcm/engine/registration_request.h index cb45c974a66..1b5708ce79d 100644 --- a/chromium/google_apis/gcm/engine/registration_request.h +++ b/chromium/google_apis/gcm/engine/registration_request.h @@ -49,8 +49,12 @@ class GCM_EXPORT RegistrationRequest : public net::URLFetcherDelegate { UNKNOWN_ERROR, // Unknown error. URL_FETCHING_FAILED, // URL fetching failed. HTTP_NOT_OK, // HTTP status was not OK. - RESPONSE_PARSING_FAILED, // Registration response parsing failed. + NO_RESPONSE_BODY, // No response body. REACHED_MAX_RETRIES, // Reached maximum number of retries. + RESPONSE_PARSING_FAILED, // Registration response parsing failed. + INTERNAL_SERVER_ERROR, // Internal server error during request. + QUOTA_EXCEEDED, // Registration quota exceeded. + TOO_MANY_REGISTRATIONS, // Max registrations per device exceeded. // NOTE: always keep this entry at the end. Add new status types only // immediately above this line. Make sure to update the corresponding // histogram enum accordingly. @@ -67,15 +71,24 @@ class GCM_EXPORT RegistrationRequest : public net::URLFetcherDelegate { struct GCM_EXPORT RequestInfo { RequestInfo(uint64_t android_id, uint64_t security_token, - const std::string& app_id); + const std::string& category, + const std::string& subtype); ~RequestInfo(); // Android ID of the device. uint64_t android_id; // Security token of the device. uint64_t security_token; - // Application ID. - std::string app_id; + + // Application ID used in Chrome to refer to registration/token's owner. + const std::string& app_id() const { + return subtype.empty() ? category : subtype; + } + + // GCM category field. + std::string category; + // GCM subtype field. + std::string subtype; }; // Encapsulates the custom logic that is needed to build and process the diff --git a/chromium/google_apis/gcm/engine/registration_request_unittest.cc b/chromium/google_apis/gcm/engine/registration_request_unittest.cc index 25b7ed8e1ce..42378d83c68 100644 --- a/chromium/google_apis/gcm/engine/registration_request_unittest.cc +++ b/chromium/google_apis/gcm/engine/registration_request_unittest.cc @@ -24,6 +24,7 @@ namespace gcm { namespace { const uint64_t kAndroidId = 42UL; const char kAppId[] = "TestAppId"; +const char kProductCategoryForSubtypes[] = "com.chrome.macosx"; const char kDeveloperId[] = "Project1"; const char kLoginHeader[] = "AidLogin"; const char kRegistrationURL[] = "http://foo.bar/register"; @@ -96,8 +97,9 @@ GCMRegistrationRequestTest::~GCMRegistrationRequestTest() { } void GCMRegistrationRequestTest::CreateRequest(const std::string& sender_ids) { - RegistrationRequest::RequestInfo request_info( - kAndroidId, kSecurityToken, kAppId); + RegistrationRequest::RequestInfo request_info(kAndroidId, kSecurityToken, + kAppId /* category */, + std::string() /* subtype */); std::unique_ptr<GCMRegistrationRequestHandler> request_handler( new GCMRegistrationRequestHandler(sender_ids)); request_.reset(new RegistrationRequest( @@ -149,20 +151,7 @@ TEST_F(GCMRegistrationRequestTest, RequestDataAndURL) { expected_pairs["sender"] = kDeveloperId; expected_pairs["device"] = base::Uint64ToString(kAndroidId); - // Verify data was formatted properly. - std::string upload_data = fetcher->upload_data(); - base::StringTokenizer data_tokenizer(upload_data, "&="); - while (data_tokenizer.GetNext()) { - std::map<std::string, std::string>::iterator iter = - expected_pairs.find(data_tokenizer.token()); - ASSERT_TRUE(iter != expected_pairs.end()); - ASSERT_TRUE(data_tokenizer.GetNext()); - EXPECT_EQ(iter->second, data_tokenizer.token()); - // Ensure that none of the keys appears twice. - expected_pairs.erase(iter); - } - - EXPECT_EQ(0UL, expected_pairs.size()); + ASSERT_NO_FATAL_FAILURE(VerifyFetcherUploadData(&expected_pairs)); } TEST_F(GCMRegistrationRequestTest, RequestRegistrationWithMultipleSenderIds) { @@ -204,6 +193,24 @@ TEST_F(GCMRegistrationRequestTest, ResponseParsing) { EXPECT_EQ("2501", registration_id_); } +TEST_F(GCMRegistrationRequestTest, ResponseParsingFailed) { + CreateRequest("sender1,sender2"); + request_->Start(); + + SetResponse(net::HTTP_OK, "tok"); // Simulate truncated message. + CompleteFetch(); + + EXPECT_FALSE(callback_called_); + + // Ensuring a retry happened and succeeds. + SetResponse(net::HTTP_OK, "token=2501"); + CompleteFetch(); + + EXPECT_TRUE(callback_called_); + EXPECT_EQ(RegistrationRequest::SUCCESS, status_); + EXPECT_EQ("2501", registration_id_); +} + TEST_F(GCMRegistrationRequestTest, ResponseHttpStatusNotOK) { CreateRequest("sender1,sender2"); request_->Start(); @@ -281,6 +288,24 @@ TEST_F(GCMRegistrationRequestTest, ResponseAuthenticationError) { EXPECT_EQ("2501", registration_id_); } +TEST_F(GCMRegistrationRequestTest, ResponseInternalServerError) { + CreateRequest("sender1,sender2"); + request_->Start(); + + SetResponse(net::HTTP_INTERNAL_SERVER_ERROR, "Error=InternalServerError"); + CompleteFetch(); + + EXPECT_FALSE(callback_called_); + + // Ensuring a retry happened and succeeds. + SetResponse(net::HTTP_OK, "token=2501"); + CompleteFetch(); + + EXPECT_TRUE(callback_called_); + EXPECT_EQ(RegistrationRequest::SUCCESS, status_); + EXPECT_EQ("2501", registration_id_); +} + TEST_F(GCMRegistrationRequestTest, ResponseInvalidParameters) { CreateRequest("sender1,sender2"); request_->Start(); @@ -317,6 +342,30 @@ TEST_F(GCMRegistrationRequestTest, ResponseInvalidSenderBadRequest) { EXPECT_EQ(std::string(), registration_id_); } +TEST_F(GCMRegistrationRequestTest, ResponseQuotaExceeded) { + CreateRequest("sender1"); + request_->Start(); + + SetResponse(net::HTTP_SERVICE_UNAVAILABLE, "Error=QUOTA_EXCEEDED"); + CompleteFetch(); + + EXPECT_TRUE(callback_called_); + EXPECT_EQ(RegistrationRequest::QUOTA_EXCEEDED, status_); + EXPECT_EQ(std::string(), registration_id_); +} + +TEST_F(GCMRegistrationRequestTest, ResponseTooManyRegistrations) { + CreateRequest("sender1"); + request_->Start(); + + SetResponse(net::HTTP_OK, "Error=TOO_MANY_REGISTRATIONS"); + CompleteFetch(); + + EXPECT_TRUE(callback_called_); + EXPECT_EQ(RegistrationRequest::TOO_MANY_REGISTRATIONS, status_); + EXPECT_EQ(std::string(), registration_id_); +} + TEST_F(GCMRegistrationRequestTest, RequestNotSuccessful) { CreateRequest("sender1,sender2"); request_->Start(); @@ -398,7 +447,8 @@ class InstanceIDGetTokenRequestTest : public RegistrationRequestTest { InstanceIDGetTokenRequestTest(); ~InstanceIDGetTokenRequestTest() override; - void CreateRequest(const std::string& instance_id, + void CreateRequest(bool use_subtype, + const std::string& instance_id, const std::string& authorized_entity, const std::string& scope, const std::map<std::string, std::string>& options); @@ -411,12 +461,15 @@ InstanceIDGetTokenRequestTest::~InstanceIDGetTokenRequestTest() { } void InstanceIDGetTokenRequestTest::CreateRequest( + bool use_subtype, const std::string& instance_id, const std::string& authorized_entity, const std::string& scope, const std::map<std::string, std::string>& options) { - RegistrationRequest::RequestInfo request_info( - kAndroidId, kSecurityToken, kAppId); + std::string category = use_subtype ? kProductCategoryForSubtypes : kAppId; + std::string subtype = use_subtype ? kAppId : std::string(); + RegistrationRequest::RequestInfo request_info(kAndroidId, kSecurityToken, + category, subtype); std::unique_ptr<InstanceIDGetTokenRequestHandler> request_handler( new InstanceIDGetTokenRequestHandler(instance_id, authorized_entity, scope, kGCMVersion, options)); @@ -434,7 +487,8 @@ TEST_F(InstanceIDGetTokenRequestTest, RequestSuccessful) { options["Foo"] = "Bar"; set_max_retry_count(0); - CreateRequest(kInstanceId, kDeveloperId, kScope, options); + CreateRequest(false /* use_subtype */, kInstanceId, kDeveloperId, kScope, + options); request_->Start(); SetResponse(net::HTTP_OK, "token=2501"); @@ -448,7 +502,8 @@ TEST_F(InstanceIDGetTokenRequestTest, RequestSuccessful) { TEST_F(InstanceIDGetTokenRequestTest, RequestDataAndURL) { std::map<std::string, std::string> options; options["Foo"] = "Bar"; - CreateRequest(kInstanceId, kDeveloperId, kScope, options); + CreateRequest(false /* use_subtype */, kInstanceId, kDeveloperId, kScope, + options); request_->Start(); // Get data sent by request. @@ -479,7 +534,32 @@ TEST_F(InstanceIDGetTokenRequestTest, RequestDataAndURL) { expected_pairs["gmsv"] = base::IntToString(kGCMVersion); expected_pairs["app"] = kAppId; expected_pairs["sender"] = kDeveloperId; - expected_pairs["X-subtype"] = kDeveloperId; + expected_pairs["device"] = base::Uint64ToString(kAndroidId); + expected_pairs["appid"] = kInstanceId; + expected_pairs["scope"] = kScope; + expected_pairs["X-scope"] = kScope; + expected_pairs["X-Foo"] = "Bar"; + + ASSERT_NO_FATAL_FAILURE(VerifyFetcherUploadData(&expected_pairs)); +} + +TEST_F(InstanceIDGetTokenRequestTest, RequestDataWithSubtype) { + std::map<std::string, std::string> options; + options["Foo"] = "Bar"; + CreateRequest(true /* use_subtype */, kInstanceId, kDeveloperId, kScope, + options); + request_->Start(); + + // Get data sent by request. + net::TestURLFetcher* fetcher = GetFetcher(); + ASSERT_TRUE(fetcher); + + // Same as RequestDataAndURL except "app" and "X-subtype". + std::map<std::string, std::string> expected_pairs; + expected_pairs["gmsv"] = base::IntToString(kGCMVersion); + expected_pairs["app"] = kProductCategoryForSubtypes; + expected_pairs["X-subtype"] = kAppId; + expected_pairs["sender"] = kDeveloperId; expected_pairs["device"] = base::Uint64ToString(kAndroidId); expected_pairs["appid"] = kInstanceId; expected_pairs["scope"] = kScope; @@ -490,8 +570,7 @@ TEST_F(InstanceIDGetTokenRequestTest, RequestDataAndURL) { std::string upload_data = fetcher->upload_data(); base::StringTokenizer data_tokenizer(upload_data, "&="); while (data_tokenizer.GetNext()) { - std::map<std::string, std::string>::iterator iter = - expected_pairs.find(data_tokenizer.token()); + auto iter = expected_pairs.find(data_tokenizer.token()); ASSERT_TRUE(iter != expected_pairs.end()); ASSERT_TRUE(data_tokenizer.GetNext()); EXPECT_EQ(iter->second, data_tokenizer.token()); @@ -504,7 +583,8 @@ TEST_F(InstanceIDGetTokenRequestTest, RequestDataAndURL) { TEST_F(InstanceIDGetTokenRequestTest, ResponseHttpStatusNotOK) { std::map<std::string, std::string> options; - CreateRequest(kInstanceId, kDeveloperId, kScope, options); + CreateRequest(false /* use_subtype */, kInstanceId, kDeveloperId, kScope, + options); request_->Start(); SetResponse(net::HTTP_UNAUTHORIZED, "token=2501"); diff --git a/chromium/google_apis/gcm/engine/unregistration_request.cc b/chromium/google_apis/gcm/engine/unregistration_request.cc index bd225892a79..b7a07e4e6a6 100644 --- a/chromium/google_apis/gcm/engine/unregistration_request.cc +++ b/chromium/google_apis/gcm/engine/unregistration_request.cc @@ -29,20 +29,68 @@ namespace { const char kRequestContentType[] = "application/x-www-form-urlencoded"; // Request constants. -const char kAppIdKey[] = "app"; +const char kCategoryKey[] = "app"; +const char kSubtypeKey[] = "X-subtype"; const char kDeleteKey[] = "delete"; const char kDeleteValue[] = "true"; const char kDeviceIdKey[] = "device"; const char kLoginHeader[] = "AidLogin"; +// Response constants. +const char kErrorPrefix[] = "Error="; +const char kInvalidParameters[] = "INVALID_PARAMETERS"; +const char kInternalServerError[] = "InternalServerError"; +const char kDeviceRegistrationError[] = "PHONE_REGISTRATION_ERROR"; + +// Gets correct status from the error message. +UnregistrationRequest::Status GetStatusFromError(const std::string& error) { + if (error.find(kInvalidParameters) != std::string::npos) + return UnregistrationRequest::INVALID_PARAMETERS; + if (error.find(kInternalServerError) != std::string::npos) + return UnregistrationRequest::INTERNAL_SERVER_ERROR; + if (error.find(kDeviceRegistrationError) != std::string::npos) + return UnregistrationRequest::DEVICE_REGISTRATION_ERROR; + // Should not be reached, unless the server adds new error types. + return UnregistrationRequest::UNKNOWN_ERROR; +} + +// Determines whether to retry based on the status of the last request. +bool ShouldRetryWithStatus(UnregistrationRequest::Status status) { + switch (status) { + case UnregistrationRequest::URL_FETCHING_FAILED: + case UnregistrationRequest::NO_RESPONSE_BODY: + case UnregistrationRequest::RESPONSE_PARSING_FAILED: + case UnregistrationRequest::INCORRECT_APP_ID: + case UnregistrationRequest::SERVICE_UNAVAILABLE: + case UnregistrationRequest::INTERNAL_SERVER_ERROR: + case UnregistrationRequest::HTTP_NOT_OK: + return true; + case UnregistrationRequest::SUCCESS: + case UnregistrationRequest::INVALID_PARAMETERS: + case UnregistrationRequest::DEVICE_REGISTRATION_ERROR: + case UnregistrationRequest::UNKNOWN_ERROR: + case UnregistrationRequest::REACHED_MAX_RETRIES: + return false; + case UnregistrationRequest::UNREGISTRATION_STATUS_COUNT: + NOTREACHED(); + break; + } + return false; +} + } // namespace UnregistrationRequest::RequestInfo::RequestInfo(uint64_t android_id, uint64_t security_token, - const std::string& app_id) - : android_id(android_id), security_token(security_token), app_id(app_id) { + const std::string& category, + const std::string& subtype) + : android_id(android_id), + security_token(security_token), + category(category), + subtype(subtype) { DCHECK(android_id != 0UL); DCHECK(security_token != 0UL); + DCHECK(!category.empty()); } UnregistrationRequest::RequestInfo::~RequestInfo() {} @@ -96,8 +144,9 @@ void UnregistrationRequest::Start() { DVLOG(1) << "Unregistration request: " << body; url_fetcher_->SetUploadData(kRequestContentType, body); - DVLOG(1) << "Performing unregistration for: " << request_info_.app_id; - recorder_->RecordUnregistrationSent(request_info_.app_id, source_to_record_); + DVLOG(1) << "Performing unregistration for: " << request_info_.app_id(); + recorder_->RecordUnregistrationSent(request_info_.app_id(), + source_to_record_); request_start_time_ = base::TimeTicks::Now(); url_fetcher_->Start(); } @@ -109,12 +158,14 @@ void UnregistrationRequest::BuildRequestHeaders(std::string* extra_headers) { std::string(kLoginHeader) + " " + base::Uint64ToString(request_info_.android_id) + ":" + base::Uint64ToString(request_info_.security_token)); - headers.SetHeader(kAppIdKey, request_info_.app_id); *extra_headers = headers.ToString(); } void UnregistrationRequest::BuildRequestBody(std::string* body) { - BuildFormEncoding(kAppIdKey, request_info_.app_id, body); + BuildFormEncoding(kCategoryKey, request_info_.category, body); + if (!request_info_.subtype.empty()) + BuildFormEncoding(kSubtypeKey, request_info_.subtype, body); + BuildFormEncoding(kDeviceIdKey, base::Uint64ToString(request_info_.android_id), body); @@ -127,14 +178,29 @@ void UnregistrationRequest::BuildRequestBody(std::string* body) { UnregistrationRequest::Status UnregistrationRequest::ParseResponse( const net::URLFetcher* source) { if (!source->GetStatus().is_success()) { - DVLOG(1) << "Fetcher failed"; + DVLOG(1) << "Unregistration URL fetching failed."; return URL_FETCHING_FAILED; } + std::string response; + if (!source->GetResponseAsString(&response)) { + DVLOG(1) << "Failed to get unregistration response body."; + return NO_RESPONSE_BODY; + } + + // If we are able to parse a meaningful known error, let's do so. Note that + // some errors will have HTTP_OK response code! + if (response.find(kErrorPrefix) != std::string::npos) { + std::string error = response.substr(response.find(kErrorPrefix) + + arraysize(kErrorPrefix) - 1); + DVLOG(1) << "Unregistration response error message: " << error; + return GetStatusFromError(error); + } + net::HttpStatusCode response_status = static_cast<net::HttpStatusCode>( source->GetResponseCode()); if (response_status != net::HTTP_OK) { - DVLOG(1) << "HTTP Status code is not OK, but: " << response_status; + DVLOG(1) << "Unregistration HTTP response code not OK: " << response_status; if (response_status == net::HTTP_SERVICE_UNAVAILABLE) return SERVICE_UNAVAILABLE; if (response_status == net::HTTP_INTERNAL_SERVER_ERROR) @@ -143,7 +209,7 @@ UnregistrationRequest::Status UnregistrationRequest::ParseResponse( } DCHECK(custom_request_handler_.get()); - return custom_request_handler_->ParseResponse(source); + return custom_request_handler_->ParseResponse(response); } void UnregistrationRequest::RetryWithBackoff() { @@ -152,15 +218,12 @@ void UnregistrationRequest::RetryWithBackoff() { url_fetcher_.reset(); backoff_entry_.InformOfRequest(false); - DVLOG(1) << "Delaying GCM unregistration of app: " - << request_info_.app_id << ", for " - << backoff_entry_.GetTimeUntilRelease().InMilliseconds() + 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); + 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, @@ -171,7 +234,7 @@ void UnregistrationRequest::RetryWithBackoff() { void UnregistrationRequest::OnURLFetchComplete(const net::URLFetcher* source) { UnregistrationRequest::Status status = ParseResponse(source); - DVLOG(1) << "UnregistrationRequestStauts: " << status; + DVLOG(1) << "UnregistrationRequestStatus: " << status; DCHECK(custom_request_handler_.get()); custom_request_handler_->ReportUMAs( @@ -179,24 +242,18 @@ void UnregistrationRequest::OnURLFetchComplete(const net::URLFetcher* source) { backoff_entry_.failure_count(), base::TimeTicks::Now() - request_start_time_); - recorder_->RecordUnregistrationResponse( - request_info_.app_id, source_to_record_, status); + recorder_->RecordUnregistrationResponse(request_info_.app_id(), + source_to_record_, status); - if (status == URL_FETCHING_FAILED || - status == HTTP_NOT_OK || - status == NO_RESPONSE_BODY || - status == SERVICE_UNAVAILABLE || - status == INTERNAL_SERVER_ERROR || - status == INCORRECT_APP_ID || - status == RESPONSE_PARSING_FAILED) { + if (ShouldRetryWithStatus(status)) { if (retries_left_ > 0) { RetryWithBackoff(); return; } status = REACHED_MAX_RETRIES; - recorder_->RecordUnregistrationResponse( - request_info_.app_id, source_to_record_, status); + recorder_->RecordUnregistrationResponse(request_info_.app_id(), + source_to_record_, status); // Only REACHED_MAX_RETRIES is reported because the function will skip // reporting count and time when status is not SUCCESS. @@ -204,9 +261,6 @@ void UnregistrationRequest::OnURLFetchComplete(const net::URLFetcher* source) { custom_request_handler_->ReportUMAs(status, 0, base::TimeDelta()); } - // status == SUCCESS || INVALID_PARAMETERS || UNKNOWN_ERROR || - // REACHED_MAX_RETRIES - callback_.Run(status); } diff --git a/chromium/google_apis/gcm/engine/unregistration_request.h b/chromium/google_apis/gcm/engine/unregistration_request.h index 06e9d506bdb..1f1d47a2af5 100644 --- a/chromium/google_apis/gcm/engine/unregistration_request.h +++ b/chromium/google_apis/gcm/engine/unregistration_request.h @@ -40,8 +40,7 @@ class GCM_EXPORT UnregistrationRequest : public net::URLFetcherDelegate { URL_FETCHING_FAILED, // URL fetching failed. NO_RESPONSE_BODY, // No response body. RESPONSE_PARSING_FAILED, // Failed to parse a meaningful output from - // response - // body. + // response body. INCORRECT_APP_ID, // App ID returned by the fetcher does not match // request. INVALID_PARAMETERS, // Request parameters were invalid. @@ -50,6 +49,7 @@ class GCM_EXPORT UnregistrationRequest : public net::URLFetcherDelegate { HTTP_NOT_OK, // HTTP response code was not OK. UNKNOWN_ERROR, // Unknown error. REACHED_MAX_RETRIES, // Reached maximum number of retries. + DEVICE_REGISTRATION_ERROR,// Chrome is not properly registered. // NOTE: Always keep this entry at the end. Add new status types only // immediately above this line. Make sure to update the corresponding // histogram enum accordingly. @@ -64,15 +64,22 @@ class GCM_EXPORT UnregistrationRequest : public net::URLFetcherDelegate { struct GCM_EXPORT RequestInfo { RequestInfo(uint64_t android_id, uint64_t security_token, - const std::string& app_id); + const std::string& category, + const std::string& subtype); ~RequestInfo(); // Android ID of the device. uint64_t android_id; // Security token of the device. uint64_t security_token; - // Application ID. - std::string app_id; + + // Application ID used in Chrome to refer to registration/token's owner. + const std::string& app_id() { return subtype.empty() ? category : subtype; } + + // GCM category field derived from the |app_id|. + std::string category; + // GCM subtype field derived from the |app_id|. + std::string subtype; }; // Encapsulates the custom logic that is needed to build and process the @@ -89,7 +96,7 @@ class GCM_EXPORT UnregistrationRequest : public net::URLFetcherDelegate { // Parses the HTTP response. It is called after // UnregistrationRequest::ParseResponse to proceed the parsing. - virtual Status ParseResponse(const net::URLFetcher* source) = 0; + virtual Status ParseResponse(const std::string& response) = 0; // Reports various UMAs, including status, retry count and completion time. virtual void ReportUMAs(Status status, diff --git a/chromium/google_apis/gcm/engine/unregistration_request_unittest.cc b/chromium/google_apis/gcm/engine/unregistration_request_unittest.cc index af5b1c95378..0f1a3485140 100644 --- a/chromium/google_apis/gcm/engine/unregistration_request_unittest.cc +++ b/chromium/google_apis/gcm/engine/unregistration_request_unittest.cc @@ -25,6 +25,7 @@ const char kLoginHeader[] = "AidLogin"; const char kAppId[] = "TestAppId"; const char kDeletedAppId[] = "deleted=TestAppId"; const char kDeletedToken[] = "token=SomeToken"; +const char kProductCategoryForSubtypes[] = "com.chrome.macosx"; const char kRegistrationURL[] = "http://foo.bar/register"; const uint64_t kSecurityToken = 77UL; const int kGCMVersion = 40; @@ -91,8 +92,9 @@ GCMUnregistrationRequestTest::~GCMUnregistrationRequestTest() { } void GCMUnregistrationRequestTest::CreateRequest() { - UnregistrationRequest::RequestInfo request_info( - kAndroidId, kSecurityToken, kAppId); + UnregistrationRequest::RequestInfo request_info(kAndroidId, kSecurityToken, + kAppId /* category */, + std::string() /* subtype */); std::unique_ptr<GCMUnregistrationRequestHandler> request_handler( new GCMUnregistrationRequestHandler(kAppId)); request_.reset(new UnregistrationRequest( @@ -130,9 +132,6 @@ TEST_F(GCMUnregistrationRequestTest, RequestDataPassedToFetcher) { EXPECT_EQ(base::Uint64ToString(kAndroidId), auth_tokenizer.token()); ASSERT_TRUE(auth_tokenizer.GetNext()); EXPECT_EQ(base::Uint64ToString(kSecurityToken), auth_tokenizer.token()); - std::string app_id_header; - headers.GetHeader("app", &app_id_header); - EXPECT_EQ(kAppId, app_id_header); std::map<std::string, std::string> expected_pairs; expected_pairs["app"] = kAppId; @@ -140,20 +139,7 @@ TEST_F(GCMUnregistrationRequestTest, RequestDataPassedToFetcher) { expected_pairs["delete"] = "true"; expected_pairs["gcm_unreg_caller"] = "false"; - // Verify data was formatted properly. - std::string upload_data = fetcher->upload_data(); - base::StringTokenizer data_tokenizer(upload_data, "&="); - while (data_tokenizer.GetNext()) { - std::map<std::string, std::string>::iterator iter = - expected_pairs.find(data_tokenizer.token()); - ASSERT_TRUE(iter != expected_pairs.end()) << data_tokenizer.token(); - ASSERT_TRUE(data_tokenizer.GetNext()) << data_tokenizer.token(); - EXPECT_EQ(iter->second, data_tokenizer.token()); - // Ensure that none of the keys appears twice. - expected_pairs.erase(iter); - } - - EXPECT_EQ(0UL, expected_pairs.size()); + ASSERT_NO_FATAL_FAILURE(VerifyFetcherUploadData(&expected_pairs)); } TEST_F(GCMUnregistrationRequestTest, SuccessfulUnregistration) { @@ -211,6 +197,17 @@ TEST_F(GCMUnregistrationRequestTest, InvalidParametersError) { EXPECT_EQ(UnregistrationRequest::INVALID_PARAMETERS, status_); } +TEST_F(GCMUnregistrationRequestTest, DeviceRegistrationError) { + CreateRequest(); + request_->Start(); + + SetResponse(net::HTTP_OK, "Error=PHONE_REGISTRATION_ERROR"); + CompleteFetch(); + + EXPECT_TRUE(callback_called_); + EXPECT_EQ(UnregistrationRequest::DEVICE_REGISTRATION_ERROR, status_); +} + TEST_F(GCMUnregistrationRequestTest, UnkwnownError) { CreateRequest(); request_->Start(); @@ -324,7 +321,8 @@ class InstaceIDDeleteTokenRequestTest : public UnregistrationRequestTest { InstaceIDDeleteTokenRequestTest(); ~InstaceIDDeleteTokenRequestTest() override; - void CreateRequest(const std::string& instance_id, + void CreateRequest(bool use_subtype, + const std::string& instance_id, const std::string& authorized_entity, const std::string& scope); }; @@ -336,11 +334,14 @@ InstaceIDDeleteTokenRequestTest::~InstaceIDDeleteTokenRequestTest() { } void InstaceIDDeleteTokenRequestTest::CreateRequest( + bool use_subtype, const std::string& instance_id, const std::string& authorized_entity, const std::string& scope) { - UnregistrationRequest::RequestInfo request_info( - kAndroidId, kSecurityToken, kAppId); + std::string category = use_subtype ? kProductCategoryForSubtypes : kAppId; + std::string subtype = use_subtype ? kAppId : std::string(); + UnregistrationRequest::RequestInfo request_info(kAndroidId, kSecurityToken, + category, subtype); std::unique_ptr<InstanceIDDeleteTokenRequestHandler> request_handler( new InstanceIDDeleteTokenRequestHandler(instance_id, authorized_entity, scope, kGCMVersion)); @@ -354,7 +355,7 @@ void InstaceIDDeleteTokenRequestTest::CreateRequest( } TEST_F(InstaceIDDeleteTokenRequestTest, RequestDataPassedToFetcher) { - CreateRequest(kInstanceId, kDeveloperId, kScope); + CreateRequest(false /* use_subtype */, kInstanceId, kDeveloperId, kScope); request_->Start(); // Get data sent by request. @@ -375,9 +376,6 @@ TEST_F(InstaceIDDeleteTokenRequestTest, RequestDataPassedToFetcher) { EXPECT_EQ(base::Uint64ToString(kAndroidId), auth_tokenizer.token()); ASSERT_TRUE(auth_tokenizer.GetNext()); EXPECT_EQ(base::Uint64ToString(kSecurityToken), auth_tokenizer.token()); - std::string app_id_header; - headers.GetHeader("app", &app_id_header); - EXPECT_EQ(kAppId, app_id_header); std::map<std::string, std::string> expected_pairs; expected_pairs["gmsv"] = base::IntToString(kGCMVersion); @@ -386,28 +384,37 @@ TEST_F(InstaceIDDeleteTokenRequestTest, RequestDataPassedToFetcher) { expected_pairs["delete"] = "true"; expected_pairs["appid"] = kInstanceId; expected_pairs["sender"] = kDeveloperId; - expected_pairs["X-subtype"] = kDeveloperId; expected_pairs["scope"] = kScope; expected_pairs["X-scope"] = kScope; - // Verify data was formatted properly. - std::string upload_data = fetcher->upload_data(); - base::StringTokenizer data_tokenizer(upload_data, "&="); - while (data_tokenizer.GetNext()) { - std::map<std::string, std::string>::iterator iter = - expected_pairs.find(data_tokenizer.token()); - ASSERT_TRUE(iter != expected_pairs.end()) << data_tokenizer.token(); - ASSERT_TRUE(data_tokenizer.GetNext()) << data_tokenizer.token(); - EXPECT_EQ(iter->second, data_tokenizer.token()); - // Ensure that none of the keys appears twice. - expected_pairs.erase(iter); - } + ASSERT_NO_FATAL_FAILURE(VerifyFetcherUploadData(&expected_pairs)); +} + +TEST_F(InstaceIDDeleteTokenRequestTest, RequestDataWithSubtype) { + CreateRequest(true /* use_subtype */, kInstanceId, kDeveloperId, kScope); + request_->Start(); + + // Get data sent by request. + net::TestURLFetcher* fetcher = GetFetcher(); + ASSERT_TRUE(fetcher); + + // Same as RequestDataPassedToFetcher except "app" and "X-subtype". + std::map<std::string, std::string> expected_pairs; + expected_pairs["gmsv"] = base::IntToString(kGCMVersion); + expected_pairs["app"] = kProductCategoryForSubtypes; + expected_pairs["X-subtype"] = kAppId; + expected_pairs["device"] = base::Uint64ToString(kAndroidId); + expected_pairs["delete"] = "true"; + expected_pairs["appid"] = kInstanceId; + expected_pairs["sender"] = kDeveloperId; + expected_pairs["scope"] = kScope; + expected_pairs["X-scope"] = kScope; - EXPECT_EQ(0UL, expected_pairs.size()); + ASSERT_NO_FATAL_FAILURE(VerifyFetcherUploadData(&expected_pairs)); } TEST_F(InstaceIDDeleteTokenRequestTest, SuccessfulUnregistration) { - CreateRequest(kInstanceId, kDeveloperId, kScope); + CreateRequest(false /* use_subtype */, kInstanceId, kDeveloperId, kScope); request_->Start(); SetResponse(net::HTTP_OK, kDeletedToken); @@ -418,7 +425,7 @@ TEST_F(InstaceIDDeleteTokenRequestTest, SuccessfulUnregistration) { } TEST_F(InstaceIDDeleteTokenRequestTest, ResponseHttpStatusNotOK) { - CreateRequest(kInstanceId, kDeveloperId, kScope); + CreateRequest(false /* use_subtype */, kInstanceId, kDeveloperId, kScope); request_->Start(); SetResponse(net::HTTP_UNAUTHORIZED, ""); @@ -434,7 +441,7 @@ TEST_F(InstaceIDDeleteTokenRequestTest, ResponseHttpStatusNotOK) { } TEST_F(InstaceIDDeleteTokenRequestTest, InvalidParametersError) { - CreateRequest(kInstanceId, kDeveloperId, kScope); + CreateRequest(false /* use_subtype */, kInstanceId, kDeveloperId, kScope); request_->Start(); SetResponse(net::HTTP_OK, "Error=INVALID_PARAMETERS"); @@ -445,7 +452,7 @@ TEST_F(InstaceIDDeleteTokenRequestTest, InvalidParametersError) { } TEST_F(InstaceIDDeleteTokenRequestTest, UnkwnownError) { - CreateRequest(kInstanceId, kDeveloperId, kScope); + CreateRequest(false /* use_subtype */, kInstanceId, kDeveloperId, kScope); request_->Start(); SetResponse(net::HTTP_OK, "Error=XXX"); diff --git a/chromium/google_apis/gcm/gcm.gyp b/chromium/google_apis/gcm/gcm.gyp deleted file mode 100644 index b3b9df029cd..00000000000 --- a/chromium/google_apis/gcm/gcm.gyp +++ /dev/null @@ -1,208 +0,0 @@ -# Copyright 2013 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. - -{ - 'variables': { - 'chromium_code': 1, - }, - - 'targets': [ - # The public GCM target. - { - # GN version: //google_apis/gcm - 'target_name': 'gcm', - 'type': '<(component)', - 'variables': { - 'enable_wexit_time_destructors': 1, - 'proto_in_dir': './protocol', - 'proto_out_dir': 'google_apis/gcm/protocol', - 'cc_generator_options': 'dllexport_decl=GCM_EXPORT:', - 'cc_include': 'google_apis/gcm/base/gcm_export.h', - }, - 'include_dirs': [ - '../..', - ], - 'defines': [ - 'GCM_IMPLEMENTATION', - ], - 'export_dependent_settings': [ - '../../third_party/protobuf/protobuf.gyp:protobuf_lite' - ], - 'dependencies': [ - '../../base/base.gyp:base', - '../../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations', - '../../net/net.gyp:net', - '../../third_party/leveldatabase/leveldatabase.gyp:leveldatabase', - '../../third_party/protobuf/protobuf.gyp:protobuf_lite', - '../../url/url.gyp:url_lib', - ], - 'sources': [ - # Note: sources list duplicated in GN build. - 'base/gcm_util.cc', - 'base/gcm_util.h', - 'base/mcs_message.cc', - 'base/mcs_message.h', - 'base/mcs_util.cc', - 'base/mcs_util.h', - 'base/socket_stream.cc', - 'base/socket_stream.h', - 'engine/account_mapping.cc', - 'engine/account_mapping.h', - 'engine/checkin_request.cc', - 'engine/checkin_request.h', - 'engine/connection_factory.cc', - 'engine/connection_factory.h', - 'engine/connection_factory_impl.cc', - 'engine/connection_factory_impl.h', - 'engine/connection_handler.cc', - 'engine/connection_handler.h', - 'engine/connection_handler_impl.cc', - 'engine/connection_handler_impl.h', - 'engine/gcm_registration_request_handler.cc', - 'engine/gcm_registration_request_handler.h', - 'engine/gcm_store.cc', - 'engine/gcm_store.h', - 'engine/gcm_store_impl.cc', - 'engine/gcm_store_impl.h', - 'engine/gcm_unregistration_request_handler.cc', - 'engine/gcm_unregistration_request_handler.h', - 'engine/gservices_settings.cc', - 'engine/gservices_settings.h', - 'engine/gservices_switches.cc', - 'engine/gservices_switches.h', - 'engine/heartbeat_manager.cc', - 'engine/heartbeat_manager.h', - 'engine/instance_id_delete_token_request_handler.cc', - 'engine/instance_id_delete_token_request_handler.h', - 'engine/instance_id_get_token_request_handler.cc', - 'engine/instance_id_get_token_request_handler.h', - 'engine/mcs_client.cc', - 'engine/mcs_client.h', - 'engine/registration_request.cc', - 'engine/registration_request.h', - 'engine/unregistration_request.cc', - 'engine/unregistration_request.h', - 'monitoring/gcm_stats_recorder.h', - 'protocol/android_checkin.proto', - 'protocol/checkin.proto', - 'protocol/mcs.proto', - ], - 'includes': [ - '../../build/protoc.gypi' - ], - }, - - # The test support library that is needed to test gcm. - { - # GN version: //google_apis/gcm:test_support - 'target_name': 'gcm_test_support', - 'type': 'static_library', - 'include_dirs': [ - '..', - ], - 'export_dependent_settings': [ - '../../third_party/protobuf/protobuf.gyp:protobuf_lite' - ], - 'dependencies': [ - '../../base/base.gyp:base', - '../../testing/gtest.gyp:gtest', - '../../third_party/protobuf/protobuf.gyp:protobuf_lite', - 'gcm', - ], - 'sources': [ - # Note: sources list duplicated in GN build. - 'base/fake_encryptor.cc', - 'base/fake_encryptor.h', - 'engine/fake_connection_factory.cc', - 'engine/fake_connection_factory.h', - 'engine/fake_connection_handler.cc', - 'engine/fake_connection_handler.h', - 'monitoring/fake_gcm_stats_recorder.cc', - 'monitoring/fake_gcm_stats_recorder.h', - ], - }, - - # A standalone MCS (mobile connection server) client. - { - # GN version: //google_apis/gcm:mcs_probe - 'target_name': 'mcs_probe', - 'type': 'executable', - 'variables': { 'enable_wexit_time_destructors': 1, }, - 'include_dirs': [ - '../..', - ], - 'dependencies': [ - '../../base/base.gyp:base', - '../../net/net.gyp:net', - '../../net/net.gyp:net_test_support', - '../../third_party/protobuf/protobuf.gyp:protobuf_lite', - 'gcm', - 'gcm_test_support' - ], - 'sources': [ - # Note: file list duplicated in GN build. - 'tools/mcs_probe.cc', - ], - }, - - # The main GCM unit tests. - { - 'target_name': 'gcm_unit_tests', - 'type': '<(gtest_target_type)', - 'variables': { 'enable_wexit_time_destructors': 1, }, - 'include_dirs': [ - '../..', - ], - 'export_dependent_settings': [ - '../../third_party/protobuf/protobuf.gyp:protobuf_lite' - ], - 'dependencies': [ - '../../base/base.gyp:run_all_unittests', - '../../base/base.gyp:base', - '../../net/net.gyp:net', - '../../net/net.gyp:net_test_support', - '../../testing/gtest.gyp:gtest', - '../../third_party/protobuf/protobuf.gyp:protobuf_lite', - 'gcm', - 'gcm_test_support' - ], - 'sources': [ - 'base/mcs_message_unittest.cc', - 'base/mcs_util_unittest.cc', - 'base/socket_stream_unittest.cc', - 'engine/account_mapping_unittest.cc', - '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', - 'engine/mcs_client_unittest.cc', - 'engine/registration_request_unittest.cc', - 'engine/unregistration_request_unittest.cc', - ] - }, - ], - 'conditions': [ - ['test_isolation_mode != "noop"', { - 'targets': [ - { - 'target_name': 'gcm_unit_tests_run', - 'type': 'none', - 'dependencies': [ - 'gcm_unit_tests', - ], - 'includes': [ - '../../build/isolate.gypi', - ], - 'sources': [ - 'gcm_unit_tests.isolate', - ], - }, - ], - }], - ], -} diff --git a/chromium/google_apis/gcm/gcm_unit_tests.isolate b/chromium/google_apis/gcm/gcm_unit_tests.isolate deleted file mode 100644 index ecd9dd2ff93..00000000000 --- a/chromium/google_apis/gcm/gcm_unit_tests.isolate +++ /dev/null @@ -1,26 +0,0 @@ -# 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. -{ - 'conditions': [ - ['OS=="linux" or OS=="mac" or OS=="win"', { - 'variables': { - 'command': [ - '../../testing/test_env.py', - '<(PRODUCT_DIR)/gcm_unit_tests<(EXECUTABLE_SUFFIX)', - '--brave-new-test-launcher', - '--test-launcher-bot-mode', - '--asan=<(asan)', - '--msan=<(msan)', - '--tsan=<(tsan)', - ], - 'files': [ - '../../testing/test_env.py', - ], - }, - }], - ], - 'includes': [ - '../../base/base.isolate', - ], -} diff --git a/chromium/google_apis/gcm/protocol/android_checkin.proto b/chromium/google_apis/gcm/protocol/android_checkin.proto index a4520513888..07bacd778f7 100644 --- a/chromium/google_apis/gcm/protocol/android_checkin.proto +++ b/chromium/google_apis/gcm/protocol/android_checkin.proto @@ -8,7 +8,6 @@ syntax = "proto2"; option optimize_for = LITE_RUNTIME; -option retain_unknown_fields = true; package checkin_proto; // Build characteristics unique to the Chrome browser, and Chrome OS diff --git a/chromium/google_apis/gcm/protocol/checkin.proto b/chromium/google_apis/gcm/protocol/checkin.proto index 10ad628f733..b741653d5c4 100644 --- a/chromium/google_apis/gcm/protocol/checkin.proto +++ b/chromium/google_apis/gcm/protocol/checkin.proto @@ -7,7 +7,6 @@ syntax = "proto2"; option optimize_for = LITE_RUNTIME; -option retain_unknown_fields = true; package checkin_proto; diff --git a/chromium/google_apis/gcm/protocol/mcs.proto b/chromium/google_apis/gcm/protocol/mcs.proto index cba584588d7..b8fb2d02765 100644 --- a/chromium/google_apis/gcm/protocol/mcs.proto +++ b/chromium/google_apis/gcm/protocol/mcs.proto @@ -8,7 +8,6 @@ syntax = "proto2"; option optimize_for = LITE_RUNTIME; -option retain_unknown_fields = true; package mcs_proto; diff --git a/chromium/google_apis/gcm/tools/mcs_probe.cc b/chromium/google_apis/gcm/tools/mcs_probe.cc index afc16b59876..0e76f05512f 100644 --- a/chromium/google_apis/gcm/tools/mcs_probe.cc +++ b/chromium/google_apis/gcm/tools/mcs_probe.cc @@ -146,7 +146,7 @@ class MyTestURLRequestContext : public net::TestURLRequestContext { context_storage_.set_host_resolver( net::HostResolver::CreateDefaultResolver(NULL)); context_storage_.set_transport_security_state( - base::WrapUnique(new net::TransportSecurityState())); + base::MakeUnique<net::TransportSecurityState>()); Init(); } @@ -184,7 +184,7 @@ class MyTestCertVerifier : public net::CertVerifier { net::CertVerifyResult* verify_result, const net::CompletionCallback& callback, std::unique_ptr<Request>* out_req, - const net::BoundNetLog& net_log) override { + const net::NetLogWithSource& net_log) override { return net::OK; } }; diff --git a/chromium/google_apis/google_api_keys.cc b/chromium/google_apis/google_api_keys.cc index 66de07a4ae6..f107fd8f0e5 100644 --- a/chromium/google_apis/google_api_keys.cc +++ b/chromium/google_apis/google_api_keys.cc @@ -18,6 +18,10 @@ #include "base/strings/stringize_macros.h" #include "google_apis/gaia/gaia_switches.h" +#if defined(OS_MACOSX) +#include "google_apis/google_api_keys_mac.h" +#endif + #if defined(GOOGLE_CHROME_BUILD) || defined(USE_OFFICIAL_GOOGLE_API_KEYS) #include "google_apis/internal/google_chrome_api_keys.h" #endif @@ -236,6 +240,16 @@ class APIKeyCache { base::CommandLine* command_line) { std::string key_value = baked_in_value; std::string temp; +#if defined(OS_MACOSX) + // macOS and iOS can also override the API key with a value from the + // Info.plist. + temp = ::google_apis::GetAPIKeyFromInfoPlist(environment_variable_name); + if (!temp.empty()) { + key_value = temp; + VLOG(1) << "Overriding API key " << environment_variable_name + << " with value " << key_value << " from Info.plist."; + } +#endif if (environment->GetVar(environment_variable_name, &temp)) { key_value = temp; VLOG(1) << "Overriding API key " << environment_variable_name diff --git a/chromium/google_apis/google_api_keys_mac.h b/chromium/google_apis/google_api_keys_mac.h new file mode 100644 index 00000000000..6ee268eaae3 --- /dev/null +++ b/chromium/google_apis/google_api_keys_mac.h @@ -0,0 +1,16 @@ +// Copyright 2016 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. + +#ifndef GOOGLE_API_KEYS_MAC_H_ +#define GOOGLE_API_KEYS_MAC_H_ + +#include <string> + +namespace google_apis { + +std::string GetAPIKeyFromInfoPlist(const std::string& key_name); + +} // namespace google_apis + +#endif // GOOGLE_API_KEYS_MAC_H_ diff --git a/chromium/google_apis/google_api_keys_mac.mm b/chromium/google_apis/google_api_keys_mac.mm new file mode 100644 index 00000000000..e7c8c1e5d82 --- /dev/null +++ b/chromium/google_apis/google_api_keys_mac.mm @@ -0,0 +1,22 @@ +// Copyright 2016 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. + +#import "google_api_keys_mac.h" + +#import <Foundation/Foundation.h> + +#include "base/mac/bundle_locations.h" +#include "base/mac/foundation_util.h" +#include "base/strings/sys_string_conversions.h" + +namespace google_apis { + +std::string GetAPIKeyFromInfoPlist(const std::string& key_name) { + NSString* keyName = base::SysUTF8ToNSString(key_name); + NSString* keyValue = base::mac::ObjCCast<NSString>( + [base::mac::FrameworkBundle() objectForInfoDictionaryKey:keyName]); + return base::SysNSStringToUTF8(keyValue); +} + +} // namespace google_apis diff --git a/chromium/google_apis/google_api_keys_mac_unittest.mm b/chromium/google_apis/google_api_keys_mac_unittest.mm new file mode 100644 index 00000000000..90115c8d748 --- /dev/null +++ b/chromium/google_apis/google_api_keys_mac_unittest.mm @@ -0,0 +1,124 @@ +// Copyright 2016 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. + +// Unit tests for implementation of google_api_keys namespace. +// +// Because the file deals with a lot of preprocessor defines and +// optionally includes an internal header, the way we test is by +// including the .cc file multiple times with different defines set. +// This is a little unorthodox, but it lets us test the behavior as +// close to unmodified as possible. + +#include "google_apis/google_api_keys_unittest.h" + +#include "base/mac/bundle_locations.h" +#include "base/macros.h" +#include "build/build_config.h" +#include "google_apis/gaia/gaia_switches.h" +#include "testing/gtest/include/gtest/gtest.h" +#import "third_party/ocmock/OCMock/OCMock.h" + +// We need to include everything included by google_api_keys.cc once +// at global scope so that things like STL and classes from base don't +// get defined when we re-include the google_api_keys.cc file +// below. We used to include that file in its entirety here, but that +// can cause problems if the linker decides the version of symbols +// from that file included here is the "right" version. + +#include <stddef.h> + +#include <string> +#include "base/command_line.h" +#include "base/lazy_instance.h" +#include "base/logging.h" +#include "base/strings/stringize_macros.h" +#include "google_apis/google_api_keys_mac.h" + +// After this test, for the remainder of this compilation unit, we +// need official keys to not be used. +#undef GOOGLE_CHROME_BUILD +#undef USE_OFFICIAL_GOOGLE_API_KEYS + +// Override some keys using both preprocessor defines and Info.plist entries. +// The Info.plist entries should win. +namespace override_some_keys_info_plist { + +// We start every test by creating a clean environment for the +// preprocessor defines used in google_api_keys.cc +#undef DUMMY_API_TOKEN +#undef GOOGLE_API_KEY +#undef GOOGLE_CLIENT_ID_MAIN +#undef GOOGLE_CLIENT_SECRET_MAIN +#undef GOOGLE_CLIENT_ID_CLOUD_PRINT +#undef GOOGLE_CLIENT_SECRET_CLOUD_PRINT +#undef GOOGLE_CLIENT_ID_REMOTING +#undef GOOGLE_CLIENT_SECRET_REMOTING +#undef GOOGLE_CLIENT_ID_REMOTING_HOST +#undef GOOGLE_CLIENT_SECRET_REMOTING_HOST +#undef GOOGLE_DEFAULT_CLIENT_ID +#undef GOOGLE_DEFAULT_CLIENT_SECRET + +#define GOOGLE_API_KEY "API_KEY" +#define GOOGLE_CLIENT_ID_MAIN "ID_MAIN" +#define GOOGLE_CLIENT_SECRET_MAIN "SECRET_MAIN" +#define GOOGLE_CLIENT_ID_CLOUD_PRINT "ID_CLOUD_PRINT" +#define GOOGLE_CLIENT_SECRET_CLOUD_PRINT "SECRET_CLOUD_PRINT" +#define GOOGLE_CLIENT_ID_REMOTING "ID_REMOTING" +#define GOOGLE_CLIENT_SECRET_REMOTING "SECRET_REMOTING" +#define GOOGLE_CLIENT_ID_REMOTING_HOST "ID_REMOTING_HOST" +#define GOOGLE_CLIENT_SECRET_REMOTING_HOST "SECRET_REMOTING_HOST" + +// Undef include guard so things get defined again, within this namespace. +#undef GOOGLE_APIS_GOOGLE_API_KEYS_H_ +#undef GOOGLE_APIS_INTERNAL_GOOGLE_CHROME_API_KEYS_ +#include "google_apis/google_api_keys.cc" + +} // namespace override_all_keys_env + +TEST_F(GoogleAPIKeysTest, OverrideSomeKeysUsingInfoPlist) { + namespace testcase = override_some_keys_info_plist::google_apis; + + id mock_bundle = [OCMockObject mockForClass:[NSBundle class]]; + [[[mock_bundle stub] andReturn:@"plist-API_KEY"] + objectForInfoDictionaryKey:@"GOOGLE_API_KEY"]; + [[[mock_bundle stub] andReturn:@"plist-ID_MAIN"] + objectForInfoDictionaryKey:@"GOOGLE_CLIENT_ID_MAIN"]; + [[[mock_bundle stub] andReturn:nil] objectForInfoDictionaryKey:[OCMArg any]]; + base::mac::SetOverrideFrameworkBundle(mock_bundle); + + EXPECT_TRUE(testcase::HasKeysConfigured()); + + // Once the keys have been configured, the bundle isn't used anymore. + base::mac::SetOverrideFrameworkBundle(nil); + + std::string api_key = testcase::g_api_key_cache.Get().api_key(); + std::string id_main = + testcase::g_api_key_cache.Get().GetClientID(testcase::CLIENT_MAIN); + std::string secret_main = + testcase::g_api_key_cache.Get().GetClientSecret(testcase::CLIENT_MAIN); + std::string id_cloud_print = + testcase::g_api_key_cache.Get().GetClientID(testcase::CLIENT_CLOUD_PRINT); + std::string secret_cloud_print = + testcase::g_api_key_cache.Get().GetClientSecret( + testcase::CLIENT_CLOUD_PRINT); + std::string id_remoting = + testcase::g_api_key_cache.Get().GetClientID(testcase::CLIENT_REMOTING); + std::string secret_remoting = testcase::g_api_key_cache.Get().GetClientSecret( + testcase::CLIENT_REMOTING); + std::string id_remoting_host = testcase::g_api_key_cache.Get().GetClientID( + testcase::CLIENT_REMOTING_HOST); + std::string secret_remoting_host = + testcase::g_api_key_cache.Get().GetClientSecret( + testcase::CLIENT_REMOTING_HOST); + + EXPECT_EQ("plist-API_KEY", api_key); + EXPECT_EQ("plist-ID_MAIN", id_main); + EXPECT_EQ("SECRET_MAIN", secret_main); + EXPECT_EQ("ID_CLOUD_PRINT", id_cloud_print); + EXPECT_EQ("SECRET_CLOUD_PRINT", secret_cloud_print); + EXPECT_EQ("ID_REMOTING", id_remoting); + EXPECT_EQ("SECRET_REMOTING", secret_remoting); + EXPECT_EQ("ID_REMOTING_HOST", id_remoting_host); + EXPECT_EQ("SECRET_REMOTING_HOST", secret_remoting_host); +} diff --git a/chromium/google_apis/google_api_keys_unittest.cc b/chromium/google_apis/google_api_keys_unittest.cc index a04b32263da..5ce1fb31622 100644 --- a/chromium/google_apis/google_api_keys_unittest.cc +++ b/chromium/google_apis/google_api_keys_unittest.cc @@ -10,14 +10,11 @@ // This is a little unorthodox, but it lets us test the behavior as // close to unmodified as possible. -#include "google_apis/google_api_keys.h" - -#include <memory> +#include "google_apis/google_api_keys_unittest.h" #include "base/macros.h" #include "build/build_config.h" #include "google_apis/gaia/gaia_switches.h" -#include "testing/gtest/include/gtest/gtest.h" // The Win builders fail (with a linker crash) when trying to link // unit_tests, and the Android builders complain about multiply @@ -38,74 +35,58 @@ #include <string> #include "base/command_line.h" -#include "base/environment.h" #include "base/lazy_instance.h" #include "base/logging.h" #include "base/strings/stringize_macros.h" -// This is the default baked-in value for OAuth IDs and secrets. -static const char kDummyToken[] = "dummytoken"; - -struct EnvironmentCache { - public: - EnvironmentCache() : variable_name(NULL), was_set(false) {} - - const char* variable_name; - bool was_set; - std::string value; -}; - -class GoogleAPIKeysTest : public testing::Test { - public: - GoogleAPIKeysTest() : env_(base::Environment::Create()) { - env_cache_[0].variable_name = "GOOGLE_API_KEY"; - env_cache_[1].variable_name = "GOOGLE_CLIENT_ID_MAIN"; - env_cache_[2].variable_name = "GOOGLE_CLIENT_SECRET_MAIN"; - env_cache_[3].variable_name = "GOOGLE_CLIENT_ID_CLOUD_PRINT"; - env_cache_[4].variable_name = "GOOGLE_CLIENT_SECRET_CLOUD_PRINT"; - env_cache_[5].variable_name = "GOOGLE_CLIENT_ID_REMOTING"; - env_cache_[6].variable_name = "GOOGLE_CLIENT_SECRET_REMOTING"; - env_cache_[7].variable_name = "GOOGLE_CLIENT_ID_REMOTING_HOST"; - env_cache_[8].variable_name = "GOOGLE_CLIENT_SECRET_REMOTING_HOST"; - env_cache_[9].variable_name = "GOOGLE_DEFAULT_CLIENT_ID"; - env_cache_[10].variable_name = "GOOGLE_DEFAULT_CLIENT_SECRET"; - } +#if defined(OS_MACOSX) +#include "google_apis/google_api_keys_mac.h" +#endif + +GoogleAPIKeysTest::GoogleAPIKeysTest() : env_(base::Environment::Create()) { + static_assert(11 == 3 + 2 * google_apis::CLIENT_NUM_ITEMS, + "Unexpected number of key entries."); + env_cache_[0].variable_name = "GOOGLE_API_KEY"; + env_cache_[1].variable_name = "GOOGLE_CLIENT_ID_MAIN"; + env_cache_[2].variable_name = "GOOGLE_CLIENT_SECRET_MAIN"; + env_cache_[3].variable_name = "GOOGLE_CLIENT_ID_CLOUD_PRINT"; + env_cache_[4].variable_name = "GOOGLE_CLIENT_SECRET_CLOUD_PRINT"; + env_cache_[5].variable_name = "GOOGLE_CLIENT_ID_REMOTING"; + env_cache_[6].variable_name = "GOOGLE_CLIENT_SECRET_REMOTING"; + env_cache_[7].variable_name = "GOOGLE_CLIENT_ID_REMOTING_HOST"; + env_cache_[8].variable_name = "GOOGLE_CLIENT_SECRET_REMOTING_HOST"; + env_cache_[9].variable_name = "GOOGLE_DEFAULT_CLIENT_ID"; + env_cache_[10].variable_name = "GOOGLE_DEFAULT_CLIENT_SECRET"; +} - void SetUp() override { - // Unset all environment variables that can affect these tests, - // for the duration of the tests. - for (size_t i = 0; i < arraysize(env_cache_); ++i) { - EnvironmentCache& cache = env_cache_[i]; - cache.was_set = env_->HasVar(cache.variable_name); - cache.value.clear(); - if (cache.was_set) { - env_->GetVar(cache.variable_name, &cache.value); - env_->UnSetVar(cache.variable_name); - } +GoogleAPIKeysTest::~GoogleAPIKeysTest() {} + +void GoogleAPIKeysTest::SetUp() { + // Unset all environment variables that can affect these tests, + // for the duration of the tests. + for (size_t i = 0; i < arraysize(env_cache_); ++i) { + EnvironmentCache& cache = env_cache_[i]; + cache.was_set = env_->HasVar(cache.variable_name); + cache.value.clear(); + if (cache.was_set) { + env_->GetVar(cache.variable_name, &cache.value); + env_->UnSetVar(cache.variable_name); } } +} - void TearDown() override { - // Restore environment. - for (size_t i = 0; i < arraysize(env_cache_); ++i) { - EnvironmentCache& cache = env_cache_[i]; - if (cache.was_set) { - env_->SetVar(cache.variable_name, cache.value); - } +void GoogleAPIKeysTest::TearDown() { + // Restore environment. + for (size_t i = 0; i < arraysize(env_cache_); ++i) { + EnvironmentCache& cache = env_cache_[i]; + if (cache.was_set) { + env_->SetVar(cache.variable_name, cache.value); } } +} - private: - std::unique_ptr<base::Environment> env_; - - // Why 3? It is for GOOGLE_API_KEY, GOOGLE_DEFAULT_CLIENT_ID and - // GOOGLE_DEFAULT_CLIENT_SECRET. - // - // Why 2 times CLIENT_NUM_ITEMS? This is the number of different - // clients in the OAuth2Client enumeration, and for each of these we - // have both an ID and a secret. - EnvironmentCache env_cache_[3 + 2 * google_apis::CLIENT_NUM_ITEMS]; -}; +// This is the default baked-in value for OAuth IDs and secrets. +static const char kDummyToken[] = "dummytoken"; #if defined(GOOGLE_CHROME_BUILD) || defined(USE_OFFICIAL_GOOGLE_API_KEYS) // Test official build behavior, since we are in a checkout where this diff --git a/chromium/google_apis/google_api_keys_unittest.h b/chromium/google_apis/google_api_keys_unittest.h new file mode 100644 index 00000000000..05bf1bf1fc4 --- /dev/null +++ b/chromium/google_apis/google_api_keys_unittest.h @@ -0,0 +1,42 @@ +// Copyright 2016 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. + +#ifndef GOOGLE_APIS_GOOGLE_API_KEYS_UNITTEST_H_ +#define GOOGLE_APIS_GOOGLE_API_KEYS_UNITTEST_H_ + +#include <memory> +#include <string> + +#include "base/environment.h" +#include "google_apis/google_api_keys.h" +#include "testing/gtest/include/gtest/gtest.h" + +struct EnvironmentCache { + EnvironmentCache() : variable_name(nullptr), was_set(false) {} + + const char* variable_name; + bool was_set; + std::string value; +}; + +class GoogleAPIKeysTest : public testing::Test { + public: + GoogleAPIKeysTest(); + ~GoogleAPIKeysTest() override; + void SetUp() override; + void TearDown() override; + + private: + std::unique_ptr<base::Environment> env_; + + // Why 3? It is for GOOGLE_API_KEY, GOOGLE_DEFAULT_CLIENT_ID and + // GOOGLE_DEFAULT_CLIENT_SECRET. + // + // Why 2 times CLIENT_NUM_ITEMS? This is the number of different + // clients in the OAuth2Client enumeration, and for each of these we + // have both an ID and a secret. + EnvironmentCache env_cache_[3 + 2 * google_apis::CLIENT_NUM_ITEMS]; +}; + +#endif // GOOGLE_APIS_GOOGLE_API_KEYS_UNITTEST_H_ diff --git a/chromium/google_apis/google_apis.gyp b/chromium/google_apis/google_apis.gyp deleted file mode 100644 index 2ef9a86af5c..00000000000 --- a/chromium/google_apis/google_apis.gyp +++ /dev/null @@ -1,228 +0,0 @@ -# Copyright (c) 2012 The Chromium Authors. All rights reserved. -# Use of this source code is governed by a BSD-style license that can be -# found in the LICENSE file. - -{ - 'variables': { - 'chromium_code': 1, # Use higher warning level. - }, - 'includes': [ - '../build/win_precompile.gypi', - ], - 'targets': [ - { - # GN version: //google_apis - 'target_name': 'google_apis', - 'type': 'static_library', - 'includes': [ - 'determine_use_official_keys.gypi', - ], - 'dependencies': [ - '../base/base.gyp:base', - '../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations', - '../crypto/crypto.gyp:crypto', - '../net/net.gyp:net', - '../url/url.gyp:url_lib', - ], - 'conditions': [ - ['google_api_key!=""', { - 'defines': ['GOOGLE_API_KEY="<(google_api_key)"'], - }], - ['google_default_client_id!=""', { - 'defines': [ - 'GOOGLE_DEFAULT_CLIENT_ID="<(google_default_client_id)"', - ] - }], - ['google_default_client_secret!=""', { - 'defines': [ - 'GOOGLE_DEFAULT_CLIENT_SECRET="<(google_default_client_secret)"', - ] - }], - ['enable_extensions==1', { - 'sources': [ - # Note: sources list duplicated in GN build. - 'drive/auth_service.cc', - 'drive/auth_service.h', - 'drive/auth_service_interface.h', - 'drive/auth_service_observer.h', - 'drive/base_requests.cc', - 'drive/base_requests.h', - 'drive/drive_api_error_codes.cc', - 'drive/drive_api_error_codes.h', - 'drive/drive_api_parser.cc', - 'drive/drive_api_parser.h', - 'drive/drive_api_requests.cc', - 'drive/drive_api_requests.h', - 'drive/drive_api_url_generator.cc', - 'drive/drive_api_url_generator.h', - 'drive/drive_common_callbacks.h', - 'drive/files_list_request_runner.cc', - 'drive/files_list_request_runner.h', - 'drive/request_sender.cc', - 'drive/request_sender.h', - 'drive/request_util.cc', - 'drive/request_util.h', - 'drive/task_util.cc', - 'drive/task_util.h', - 'drive/time_util.cc', - 'drive/time_util.h', - ], - }], - ], - 'sources': [ - # Note: sources list duplicated in GN build. - 'gaia/account_tracker.cc', - 'gaia/account_tracker.h', - 'gaia/gaia_auth_consumer.cc', - 'gaia/gaia_auth_consumer.h', - 'gaia/gaia_auth_fetcher.cc', - 'gaia/gaia_auth_fetcher.h', - 'gaia/gaia_auth_util.cc', - 'gaia/gaia_auth_util.h', - 'gaia/gaia_constants.cc', - 'gaia/gaia_constants.h', - 'gaia/gaia_oauth_client.cc', - 'gaia/gaia_oauth_client.h', - 'gaia/gaia_switches.cc', - 'gaia/gaia_switches.h', - 'gaia/gaia_urls.cc', - 'gaia/gaia_urls.h', - 'gaia/google_service_auth_error.cc', - 'gaia/google_service_auth_error.h', - 'gaia/identity_provider.cc', - 'gaia/identity_provider.h', - 'gaia/oauth2_access_token_consumer.h', - 'gaia/oauth2_access_token_fetcher.cc', - 'gaia/oauth2_access_token_fetcher.h', - 'gaia/oauth2_access_token_fetcher_impl.cc', - 'gaia/oauth2_access_token_fetcher_impl.h', - 'gaia/oauth2_access_token_fetcher_immediate_error.cc', - 'gaia/oauth2_access_token_fetcher_immediate_error.h', - 'gaia/oauth2_api_call_flow.cc', - 'gaia/oauth2_api_call_flow.h', - 'gaia/oauth2_mint_token_flow.cc', - 'gaia/oauth2_mint_token_flow.h', - 'gaia/oauth2_token_service.cc', - 'gaia/oauth2_token_service.h', - 'gaia/oauth2_token_service_delegate.cc', - 'gaia/oauth2_token_service_delegate.h', - 'gaia/oauth2_token_service_request.cc', - 'gaia/oauth2_token_service_request.h', - 'gaia/oauth_request_signer.cc', - 'gaia/oauth_request_signer.h', - 'gaia/ubertoken_fetcher.cc', - 'gaia/ubertoken_fetcher.h', - 'google_api_keys.cc', - 'google_api_keys.h', - ], - # TODO(jschuh): crbug.com/167187 fix size_t to int truncations. - 'msvs_disabled_warnings': [4267, ], - }, - { - 'target_name': 'google_apis_unittests', - 'type': 'executable', - 'dependencies': [ - '../base/base.gyp:run_all_unittests', - '../testing/gmock.gyp:gmock', - '../testing/gtest.gyp:gtest', - 'google_apis', - 'google_apis_test_support', - ], - 'includes': [ - 'determine_use_official_keys.gypi', - ], - 'include_dirs': [ - '..', - ], - 'sources': [ - 'gaia/account_tracker_unittest.cc', - 'gaia/gaia_auth_fetcher_unittest.cc', - 'gaia/gaia_auth_util_unittest.cc', - 'gaia/gaia_oauth_client_unittest.cc', - 'gaia/google_service_auth_error_unittest.cc', - 'gaia/oauth2_access_token_fetcher_impl_unittest.cc', - 'gaia/oauth2_api_call_flow_unittest.cc', - 'gaia/oauth2_mint_token_flow_unittest.cc', - 'gaia/oauth2_token_service_request_unittest.cc', - 'gaia/oauth2_token_service_unittest.cc', - 'gaia/oauth_request_signer_unittest.cc', - 'gaia/ubertoken_fetcher_unittest.cc', - 'google_api_keys_unittest.cc', - ], - 'conditions': [ - ['enable_extensions==1', { - 'sources': [ - 'drive/base_requests_server_unittest.cc', - 'drive/base_requests_unittest.cc', - 'drive/drive_api_parser_unittest.cc', - 'drive/drive_api_requests_unittest.cc', - 'drive/drive_api_url_generator_unittest.cc', - 'drive/files_list_request_runner_unittest.cc', - 'drive/request_sender_unittest.cc', - 'drive/request_util_unittest.cc', - 'drive/time_util_unittest.cc', - ], - }], - ], - }, - { - # GN version: //google_apis:test_support - 'target_name': 'google_apis_test_support', - 'type': 'static_library', - 'dependencies': [ - '../base/base.gyp:base', - '../base/base.gyp:test_support_base', - '../net/net.gyp:net', - '../net/net.gyp:net_test_support', - ], - 'export_dependent_settings': [ - '../base/base.gyp:base', - '../base/base.gyp:test_support_base', - '../net/net.gyp:net', - '../net/net.gyp:net_test_support', - ], - 'sources': [ - 'gaia/fake_gaia.cc', - '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/mock_url_fetcher_factory.h', - 'gaia/oauth2_token_service_test_util.cc', - 'gaia/oauth2_token_service_test_util.h', - ], - 'conditions': [ - ['enable_extensions==1', { - 'sources': [ - 'drive/dummy_auth_service.cc', - 'drive/dummy_auth_service.h', - 'drive/test_util.cc', - 'drive/test_util.h', - ], - }], - ], - }, - ], - '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 deleted file mode 100644 index 5d9e22e8ba9..00000000000 --- a/chromium/google_apis/google_apis_unittests.isolate +++ /dev/null @@ -1,24 +0,0 @@ -# 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/', - ], - }, - }], - ], -} |