diff options
Diffstat (limited to 'chromium/components/update_client')
33 files changed, 1419 insertions, 839 deletions
diff --git a/chromium/components/update_client/BUILD.gn b/chromium/components/update_client/BUILD.gn index a105b0414ae..f5d203de8f6 100644 --- a/chromium/components/update_client/BUILD.gn +++ b/chromium/components/update_client/BUILD.gn @@ -76,6 +76,7 @@ static_library("update_client") { "//courgette:courgette_lib", "//crypto", "//net", + "//services/network/public/cpp:cpp", "//third_party/libxml", "//url", ] @@ -92,8 +93,8 @@ static_library("test_support") { "test_configurator.h", "test_installer.cc", "test_installer.h", - "url_request_post_interceptor.cc", - "url_request_post_interceptor.h", + "url_loader_post_interceptor.cc", + "url_loader_post_interceptor.h", ] public_deps = [ @@ -107,6 +108,7 @@ static_library("test_support") { "//components/services/unzip:lib", "//mojo/public/cpp/bindings", "//net:test_support", + "//services/network:test_support", "//services/service_manager/public/cpp", "//services/service_manager/public/cpp/test:test_support", "//services/service_manager/public/mojom", @@ -132,6 +134,8 @@ bundle_data("unit_tests_bundle_data") { "//components/test/data/update_client/updatecheck_reply_1.xml", "//components/test/data/update_client/updatecheck_reply_4.xml", "//components/test/data/update_client/updatecheck_reply_noupdate.xml", + "//components/test/data/update_client/updatecheck_reply_parse_error.xml", + "//components/test/data/update_client/updatecheck_reply_unknownapp.xml", ] outputs = [ "{{bundle_resources_dir}}/" + @@ -173,6 +177,7 @@ source_set("unit_tests") { "//components/version_info:version_info", "//courgette:courgette_lib", "//net:test_support", + "//services/network/public/cpp:cpp_base", "//services/service_manager/public/cpp", "//services/service_manager/public/cpp/test:test_support", "//testing/gmock", diff --git a/chromium/components/update_client/DEPS b/chromium/components/update_client/DEPS index bcf55884694..11fec055972 100644 --- a/chromium/components/update_client/DEPS +++ b/chromium/components/update_client/DEPS @@ -11,6 +11,8 @@ include_rules = [ "+libxml", "+mojo", "+net", + "+services/network/public/cpp", + "+services/network/test", "+services/service_manager/public", "+third_party/libxml", "+third_party/zlib", diff --git a/chromium/components/update_client/action_runner_win.cc b/chromium/components/update_client/action_runner_win.cc index 5b11f3ff807..b92a5089543 100644 --- a/chromium/components/update_client/action_runner_win.cc +++ b/chromium/components/update_client/action_runner_win.cc @@ -56,6 +56,10 @@ base::CommandLine ActionRunner::MakeCommandLine( command_line.AppendSwitchASCII( "browser-version", component_.config()->GetBrowserVersion().GetString()); command_line.AppendSwitchASCII("sessionid", component_.session_id()); + const auto app_guid = component_.config()->GetAppGuid(); + if (!app_guid.empty()) + command_line.AppendSwitchASCII("appguid", app_guid); + VLOG(1) << "run action: " << command_line.GetCommandLineString(); return command_line; } diff --git a/chromium/components/update_client/background_downloader_win.cc b/chromium/components/update_client/background_downloader_win.cc index fb8c9ed1591..29d3db7ce59 100644 --- a/chromium/components/update_client/background_downloader_win.cc +++ b/chromium/components/update_client/background_downloader_win.cc @@ -141,7 +141,7 @@ const int kMaxQueuedJobs = 10; // Retrieves the singleton instance of GIT for this process. HRESULT GetGit(ComPtr<IGlobalInterfaceTable>* git) { - return ::CoCreateInstance(CLSID_StdGlobalInterfaceTable, NULL, + return ::CoCreateInstance(CLSID_StdGlobalInterfaceTable, nullptr, CLSCTX_INPROC_SERVER, IID_PPV_ARGS(git->GetAddressOf())); } @@ -190,7 +190,7 @@ HRESULT GetFilesInJob(const ComPtr<IBackgroundCopyJob>& job, for (ULONG i = 0; i != num_files; ++i) { ComPtr<IBackgroundCopyFile> file; - if (enum_files->Next(1, file.GetAddressOf(), NULL) == S_OK && file.Get()) + if (enum_files->Next(1, file.GetAddressOf(), nullptr) == S_OK && file.Get()) files->push_back(file); } @@ -316,7 +316,7 @@ HRESULT FindBitsJobIf(Predicate pred, // the job description matches the component updater jobs. for (ULONG i = 0; i != job_count; ++i) { ComPtr<IBackgroundCopyJob> current_job; - if (enum_jobs->Next(1, current_job.GetAddressOf(), NULL) == S_OK && + if (enum_jobs->Next(1, current_job.GetAddressOf(), nullptr) == S_OK && pred(current_job)) { base::string16 job_name; hr = GetJobDisplayName(current_job, &job_name); @@ -384,7 +384,7 @@ void CleanupJob(const ComPtr<IBackgroundCopyJob>& job) { std::vector<base::FilePath> paths; for (const auto& file : files) { base::string16 local_name; - HRESULT hr = GetJobFileProperties(file, &local_name, NULL, NULL); + HRESULT hr = GetJobFileProperties(file, &local_name, nullptr, nullptr); if (SUCCEEDED(hr)) paths.push_back(base::FilePath(local_name)); } @@ -804,7 +804,7 @@ HRESULT BackgroundDownloader::CompleteJob() { base::string16 local_name; BG_FILE_PROGRESS progress = {0}; - hr = GetJobFileProperties(files.front(), &local_name, NULL, &progress); + hr = GetJobFileProperties(files.front(), &local_name, nullptr, &progress); if (FAILED(hr)) return hr; diff --git a/chromium/components/update_client/command_line_config_policy.cc b/chromium/components/update_client/command_line_config_policy.cc index a6456019b22..899d8ace277 100644 --- a/chromium/components/update_client/command_line_config_policy.cc +++ b/chromium/components/update_client/command_line_config_policy.cc @@ -37,4 +37,8 @@ GURL CommandLineConfigPolicy::UrlSourceOverride() const { return GURL(); } +int CommandLineConfigPolicy::InitialDelay() const { + return 0; +} + } // namespace update_client diff --git a/chromium/components/update_client/command_line_config_policy.h b/chromium/components/update_client/command_line_config_policy.h index d515c994180..2d201a526ba 100644 --- a/chromium/components/update_client/command_line_config_policy.h +++ b/chromium/components/update_client/command_line_config_policy.h @@ -32,6 +32,10 @@ class CommandLineConfigPolicy { // The override URL for updates. Can be empty. virtual GURL UrlSourceOverride() const; + // If non-zero, time interval in seconds until the first component + // update check. + virtual int InitialDelay() const; + virtual ~CommandLineConfigPolicy() {} }; diff --git a/chromium/components/update_client/component.cc b/chromium/components/update_client/component.cc index 6c8ce2c8bcb..4cdaaf730c7 100644 --- a/chromium/components/update_client/component.cc +++ b/chromium/components/update_client/component.cc @@ -266,11 +266,18 @@ void Component::Uninstall(const base::Version& version, int reason) { state_ = std::make_unique<StateUninstalled>(this); } -void Component::UpdateCheckComplete() { +void Component::SetUpdateCheckResult( + const base::Optional<ProtocolParser::Result>& result, + ErrorCategory error_category, + int error) { DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK_EQ(ComponentState::kChecking, state()); + error_category_ = error_category; + error_code_ = error; + if (result) + SetParseResult(result.value()); + base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, std::move(update_check_complete_)); } @@ -412,6 +419,9 @@ void Component::StateUpdateError::DoHandle() { auto& component = State::component(); + DCHECK_NE(ErrorCategory::kNone, component.error_category_); + DCHECK_NE(0, component.error_code_); + // Create an event only when the server response included an update. if (component.IsUpdateAvailable()) component.AppendEvent(BuildUpdateCompleteEventElement(component)); diff --git a/chromium/components/update_client/component.h b/chromium/components/update_client/component.h index 749e3009bba..e9adacd7f38 100644 --- a/chromium/components/update_client/component.h +++ b/chromium/components/update_client/component.h @@ -16,6 +16,7 @@ #include "base/gtest_prod_util.h" #include "base/macros.h" #include "base/memory/ref_counted.h" +#include "base/optional.h" #include "base/threading/thread_checker.h" #include "base/time/time.h" #include "base/version.h" @@ -49,14 +50,14 @@ class Component { CrxUpdateItem GetCrxUpdateItem() const; - // Called by the UpdateChecker to set the update response for this component. - void SetParseResult(const ProtocolParser::Result& result); - // Sets the uninstall state for this component. void Uninstall(const base::Version& cur_version, int reason); // Called by the UpdateEngine when an update check for this component is done. - void UpdateCheckComplete(); + void SetUpdateCheckResult( + const base::Optional<ProtocolParser::Result>& result, + ErrorCategory error_category, + int error); // Returns true if the component has reached a final state and no further // handling and state transitions are possible. @@ -92,12 +93,6 @@ class Component { std::string next_fp() const { return next_fp_; } void set_next_fp(const std::string& next_fp) { next_fp_ = next_fp; } - void set_update_check_error(int update_check_error) { - error_category_ = ErrorCategory::kUpdateCheck; - error_code_ = update_check_error; - extra_code1_ = 0; - } - bool is_foreground() const; const std::vector<std::string>& events() const { return events_; } @@ -371,6 +366,8 @@ class Component { // Notifies registered observers about changes in the state of the component. void NotifyObservers(Events event) const; + void SetParseResult(const ProtocolParser::Result& result); + base::ThreadChecker thread_checker_; const std::string id_; diff --git a/chromium/components/update_client/component_patcher.cc b/chromium/components/update_client/component_patcher.cc index e0bf014d389..33d1df803d5 100644 --- a/chromium/components/update_client/component_patcher.cc +++ b/chromium/components/update_client/component_patcher.cc @@ -66,7 +66,7 @@ void ComponentPatcher::Start(Callback callback) { void ComponentPatcher::StartPatching() { commands_.reset(ReadCommands(input_dir_)); - if (!commands_.get()) { + if (!commands_) { DonePatching(UnpackerError::kDeltaBadCommands, 0); } else { next_command_ = commands_->begin(); @@ -90,7 +90,7 @@ void ComponentPatcher::PatchNextFile() { current_operation_ = CreateDeltaUpdateOp(operation, connector_.get()); } - if (!current_operation_.get()) { + if (!current_operation_) { DonePatching(UnpackerError::kDeltaUnsupportedCommand, 0); return; } diff --git a/chromium/components/update_client/configurator.h b/chromium/components/update_client/configurator.h index 2d08795d590..370327315c8 100644 --- a/chromium/components/update_client/configurator.h +++ b/chromium/components/update_client/configurator.h @@ -11,6 +11,7 @@ #include "base/memory/ref_counted.h" #include "net/url_request/url_request_context_getter.h" +#include "services/network/public/cpp/shared_url_loader_factory.h" class GURL; class PrefService; @@ -96,6 +97,9 @@ class Configurator : public base::RefCountedThreadSafe<Configurator> { virtual scoped_refptr<net::URLRequestContextGetter> RequestContext() const = 0; + virtual scoped_refptr<network::SharedURLLoaderFactory> URLLoaderFactory() + const = 0; + // Returns a new connector to the service manager. That connector is not bound // to any thread yet. virtual std::unique_ptr<service_manager::Connector> @@ -146,6 +150,10 @@ class Configurator : public base::RefCountedThreadSafe<Configurator> { // feature to support testing. virtual std::vector<uint8_t> GetRunActionKeyHash() const = 0; + // Returns the app GUID with which Chrome is registered with Google Update, or + // an empty string if this brand does not integrate with Google Update. + virtual std::string GetAppGuid() const = 0; + protected: friend class base::RefCountedThreadSafe<Configurator>; diff --git a/chromium/components/update_client/ping_manager_unittest.cc b/chromium/components/update_client/ping_manager_unittest.cc index 32cd92ba9dd..29e5481cfcf 100644 --- a/chromium/components/update_client/ping_manager_unittest.cc +++ b/chromium/components/update_client/ping_manager_unittest.cc @@ -19,7 +19,7 @@ #include "components/update_client/protocol_builder.h" #include "components/update_client/test_configurator.h" #include "components/update_client/update_engine.h" -#include "components/update_client/url_request_post_interceptor.h" +#include "components/update_client/url_loader_post_interceptor.h" #include "net/url_request/url_request_test_util.h" #include "testing/gtest/include/gtest/gtest.h" @@ -103,9 +103,8 @@ scoped_refptr<UpdateContext> PingManagerTest::MakeMockUpdateContext() const { } TEST_F(PingManagerTest, SendPing) { - auto interceptor_factory = - std::make_unique<InterceptorFactory>(base::ThreadTaskRunnerHandle::Get()); - auto interceptor = interceptor_factory->CreateInterceptor(); + auto interceptor = std::make_unique<URLLoaderPostInterceptor>( + config_->test_url_loader_factory()); EXPECT_TRUE(interceptor); // Test eventresult="1" is sent for successful updates. @@ -133,12 +132,12 @@ TEST_F(PingManagerTest, SendPing) { EXPECT_NE(string::npos, interceptor->GetRequestBody(0).find(" sessionid=")); // Check the ping request does not carry the specific extra request headers. - EXPECT_FALSE(interceptor->GetRequests()[0].second.HasHeader( - "X-Goog-Update-Interactivity")); - EXPECT_FALSE(interceptor->GetRequests()[0].second.HasHeader( - "X-Goog-Update-Updater")); - EXPECT_FALSE( - interceptor->GetRequests()[0].second.HasHeader("X-Goog-Update-AppId")); + EXPECT_FALSE(std::get<1>(interceptor->GetRequests()[0]) + .HasHeader("X-Goog-Update-Interactivity")); + EXPECT_FALSE(std::get<1>(interceptor->GetRequests()[0]) + .HasHeader("X-Goog-Update-Updater")); + EXPECT_FALSE(std::get<1>(interceptor->GetRequests()[0]) + .HasHeader("X-Goog-Update-AppId")); interceptor->Reset(); } diff --git a/chromium/components/update_client/protocol_builder_unittest.cc b/chromium/components/update_client/protocol_builder_unittest.cc index f748ced1951..0570e70e4f2 100644 --- a/chromium/components/update_client/protocol_builder_unittest.cc +++ b/chromium/components/update_client/protocol_builder_unittest.cc @@ -48,8 +48,7 @@ TEST(BuildProtocolRequest, UpdaterStateAttributes) { // When no updater state is provided, then check that the elements and // attributes related to the updater state are not serialized. std::string request = - BuildProtocolRequest("1", "", "", "", "", "", "", "", "", nullptr) - .c_str(); + BuildProtocolRequest("1", "", "", "", "", "", "", "", "", nullptr); EXPECT_EQ(std::string::npos, request.find(" domainjoined")); EXPECT_EQ(std::string::npos, request.find("<updater")); diff --git a/chromium/components/update_client/protocol_parser.cc b/chromium/components/update_client/protocol_parser.cc index 95902f695be..efd88dde6fc 100644 --- a/chromium/components/update_client/protocol_parser.cc +++ b/chromium/components/update_client/protocol_parser.cc @@ -321,7 +321,25 @@ bool ParseAppTag(xmlNode* app, return false; } - // Get the <updatecheck> tag. + // Read the |status| attribute for the app. + // If the status is one of the defined app status error literals, then return + // it in the result as if it were an updatecheck status, then stop parsing, + // and return success. + result->status = GetAttribute(app, "status"); + if (result->status == "restricted" || + result->status == "error-unknownApplication" || + result->status == "error-invalidAppId") + return true; + + // If the status was not handled above and the status is not "ok", then + // this must be a status literal that that the parser does not know about. + if (!result->status.empty() && result->status != "ok") { + *error = "Unknown app status"; + return false; + } + + // Get the <updatecheck> tag if the status is missing or the status is "ok". + DCHECK(result->status.empty() || result->status == "ok"); std::vector<xmlNode*> updates = GetChildren(app, "updatecheck"); if (updates.empty()) { *error = "Missing updatecheck on app."; diff --git a/chromium/components/update_client/protocol_parser_unittest.cc b/chromium/components/update_client/protocol_parser_unittest.cc index e34f1869dcf..0a09a6433bf 100644 --- a/chromium/components/update_client/protocol_parser_unittest.cc +++ b/chromium/components/update_client/protocol_parser_unittest.cc @@ -289,6 +289,24 @@ const char* kUpdateCheckStatusErrorWithRunAction = " </app>" "</response>"; +// Includes four <app> tags with status different than "ok". +const char* kAppsStatusError = + "<?xml version='1.0' encoding='UTF-8'?>" + "<response protocol='3.1'>" + " <app appid='aaaaaaaa' status='error-unknownApplication'>" + " <updatecheck status='error-internal'/>" + " </app>" + " <app appid='bbbbbbbb' status='restricted'>" + " <updatecheck status='error-internal'/>" + " </app>" + " <app appid='cccccccc' status='error-invalidAppId'>" + " <updatecheck status='error-internal'/>" + " </app>" + " <app appid='dddddddd' status='foobar'>" + " <updatecheck status='error-internal'/>" + " </app>" + "</response>"; + TEST(ComponentUpdaterProtocolParserTest, Parse) { ProtocolParser parser; @@ -324,15 +342,15 @@ TEST(ComponentUpdaterProtocolParserTest, Parse) { EXPECT_TRUE(parser.Parse(kValidXml)); EXPECT_TRUE(parser.errors().empty()); EXPECT_EQ(1u, parser.results().list.size()); - const ProtocolParser::Result* firstResult = &parser.results().list[0]; - EXPECT_STREQ("ok", firstResult->status.c_str()); - EXPECT_EQ(1u, firstResult->crx_urls.size()); - EXPECT_EQ(GURL("http://example.com/"), firstResult->crx_urls[0]); - EXPECT_EQ(GURL("http://diff.example.com/"), firstResult->crx_diffurls[0]); - EXPECT_EQ("1.2.3.4", firstResult->manifest.version); - EXPECT_EQ("2.0.143.0", firstResult->manifest.browser_min_version); - EXPECT_EQ(1u, firstResult->manifest.packages.size()); - EXPECT_EQ("extension_1_2_3_4.crx", firstResult->manifest.packages[0].name); + const ProtocolParser::Result* first_result = &parser.results().list[0]; + EXPECT_STREQ("ok", first_result->status.c_str()); + EXPECT_EQ(1u, first_result->crx_urls.size()); + EXPECT_EQ(GURL("http://example.com/"), first_result->crx_urls[0]); + EXPECT_EQ(GURL("http://diff.example.com/"), first_result->crx_diffurls[0]); + EXPECT_EQ("1.2.3.4", first_result->manifest.version); + EXPECT_EQ("2.0.143.0", first_result->manifest.browser_min_version); + EXPECT_EQ(1u, first_result->manifest.packages.size()); + EXPECT_EQ("extension_1_2_3_4.crx", first_result->manifest.packages[0].name); // Parse some xml that uses namespace prefixes. EXPECT_TRUE(parser.Parse(kUsesNamespacePrefix)); @@ -344,23 +362,23 @@ TEST(ComponentUpdaterProtocolParserTest, Parse) { EXPECT_TRUE(parser.Parse(valid_xml_with_hash)); EXPECT_TRUE(parser.errors().empty()); EXPECT_FALSE(parser.results().list.empty()); - firstResult = &parser.results().list[0]; - EXPECT_FALSE(firstResult->manifest.packages.empty()); - EXPECT_EQ("1234", firstResult->manifest.packages[0].hash_sha256); - EXPECT_EQ("5678", firstResult->manifest.packages[0].hashdiff_sha256); + first_result = &parser.results().list[0]; + EXPECT_FALSE(first_result->manifest.packages.empty()); + EXPECT_EQ("1234", first_result->manifest.packages[0].hash_sha256); + EXPECT_EQ("5678", first_result->manifest.packages[0].hashdiff_sha256); // Parse xml with package size value EXPECT_TRUE(parser.Parse(valid_xml_with_invalid_sizes)); EXPECT_TRUE(parser.errors().empty()); EXPECT_FALSE(parser.results().list.empty()); - firstResult = &parser.results().list[0]; - EXPECT_FALSE(firstResult->manifest.packages.empty()); - EXPECT_EQ(1234, firstResult->manifest.packages[0].size); - EXPECT_EQ(-1234, firstResult->manifest.packages[1].size); - EXPECT_EQ(0, firstResult->manifest.packages[2].size); - EXPECT_EQ(0, firstResult->manifest.packages[3].size); - EXPECT_EQ(0, firstResult->manifest.packages[4].size); - EXPECT_EQ(0, firstResult->manifest.packages[5].size); + first_result = &parser.results().list[0]; + EXPECT_FALSE(first_result->manifest.packages.empty()); + EXPECT_EQ(1234, first_result->manifest.packages[0].size); + EXPECT_EQ(-1234, first_result->manifest.packages[1].size); + EXPECT_EQ(0, first_result->manifest.packages[2].size); + EXPECT_EQ(0, first_result->manifest.packages[3].size); + EXPECT_EQ(0, first_result->manifest.packages[4].size); + EXPECT_EQ(0, first_result->manifest.packages[5].size); // Parse xml with a <daystart> element. EXPECT_TRUE(parser.Parse(kWithDaystart)); @@ -372,63 +390,82 @@ TEST(ComponentUpdaterProtocolParserTest, Parse) { EXPECT_TRUE(parser.Parse(kNoUpdate)); EXPECT_TRUE(parser.errors().empty()); EXPECT_FALSE(parser.results().list.empty()); - firstResult = &parser.results().list[0]; - EXPECT_STREQ("noupdate", firstResult->status.c_str()); - EXPECT_EQ(firstResult->extension_id, "12345"); - EXPECT_EQ(firstResult->manifest.version, ""); + first_result = &parser.results().list[0]; + EXPECT_STREQ("noupdate", first_result->status.c_str()); + EXPECT_EQ(first_result->extension_id, "12345"); + EXPECT_EQ(first_result->manifest.version, ""); // Parse xml with one error and one success <app> tag. EXPECT_TRUE(parser.Parse(kTwoAppsOneError)); - EXPECT_FALSE(parser.errors().empty()); - EXPECT_EQ(1u, parser.results().list.size()); - firstResult = &parser.results().list[0]; - EXPECT_EQ(firstResult->extension_id, "bbbbbbbb"); - EXPECT_STREQ("ok", firstResult->status.c_str()); - EXPECT_EQ("1.2.3.4", firstResult->manifest.version); + EXPECT_TRUE(parser.errors().empty()); + EXPECT_EQ(2u, parser.results().list.size()); + first_result = &parser.results().list[0]; + EXPECT_EQ(first_result->extension_id, "aaaaaaaa"); + EXPECT_STREQ("error-unknownApplication", first_result->status.c_str()); + EXPECT_TRUE(first_result->manifest.version.empty()); + const ProtocolParser::Result* second_result = &parser.results().list[1]; + EXPECT_EQ(second_result->extension_id, "bbbbbbbb"); + EXPECT_STREQ("ok", second_result->status.c_str()); + EXPECT_EQ("1.2.3.4", second_result->manifest.version); // Parse xml with two apps setting the cohort info. EXPECT_TRUE(parser.Parse(kTwoAppsSetCohort)); EXPECT_TRUE(parser.errors().empty()); EXPECT_EQ(2u, parser.results().list.size()); - firstResult = &parser.results().list[0]; - EXPECT_EQ(firstResult->extension_id, "aaaaaaaa"); - EXPECT_NE(firstResult->cohort_attrs.find("cohort"), - firstResult->cohort_attrs.end()); - EXPECT_EQ(firstResult->cohort_attrs.find("cohort")->second, "1:2q3/"); - EXPECT_EQ(firstResult->cohort_attrs.find("cohortname"), - firstResult->cohort_attrs.end()); - EXPECT_EQ(firstResult->cohort_attrs.find("cohorthint"), - firstResult->cohort_attrs.end()); - const ProtocolParser::Result* secondResult = &parser.results().list[1]; - EXPECT_EQ(secondResult->extension_id, "bbbbbbbb"); - EXPECT_NE(secondResult->cohort_attrs.find("cohort"), - secondResult->cohort_attrs.end()); - EXPECT_EQ(secondResult->cohort_attrs.find("cohort")->second, "1:33z@0.33"); - EXPECT_NE(secondResult->cohort_attrs.find("cohortname"), - secondResult->cohort_attrs.end()); - EXPECT_EQ(secondResult->cohort_attrs.find("cohortname")->second, "cname"); - EXPECT_EQ(secondResult->cohort_attrs.find("cohorthint"), - secondResult->cohort_attrs.end()); + first_result = &parser.results().list[0]; + EXPECT_EQ(first_result->extension_id, "aaaaaaaa"); + EXPECT_NE(first_result->cohort_attrs.find("cohort"), + first_result->cohort_attrs.end()); + EXPECT_EQ(first_result->cohort_attrs.find("cohort")->second, "1:2q3/"); + EXPECT_EQ(first_result->cohort_attrs.find("cohortname"), + first_result->cohort_attrs.end()); + EXPECT_EQ(first_result->cohort_attrs.find("cohorthint"), + first_result->cohort_attrs.end()); + EXPECT_EQ(second_result->extension_id, "bbbbbbbb"); + EXPECT_NE(second_result->cohort_attrs.find("cohort"), + second_result->cohort_attrs.end()); + EXPECT_EQ(second_result->cohort_attrs.find("cohort")->second, "1:33z@0.33"); + EXPECT_NE(second_result->cohort_attrs.find("cohortname"), + second_result->cohort_attrs.end()); + EXPECT_EQ(second_result->cohort_attrs.find("cohortname")->second, "cname"); + EXPECT_EQ(second_result->cohort_attrs.find("cohorthint"), + second_result->cohort_attrs.end()); EXPECT_TRUE(parser.Parse(kUpdateCheckStatusOkWithRunAction)); EXPECT_TRUE(parser.errors().empty()); EXPECT_FALSE(parser.results().list.empty()); - firstResult = &parser.results().list[0]; - EXPECT_STREQ("ok", firstResult->status.c_str()); - EXPECT_EQ(firstResult->extension_id, "12345"); - EXPECT_STREQ("this", firstResult->action_run.c_str()); + first_result = &parser.results().list[0]; + EXPECT_STREQ("ok", first_result->status.c_str()); + EXPECT_EQ(first_result->extension_id, "12345"); + EXPECT_STREQ("this", first_result->action_run.c_str()); EXPECT_TRUE(parser.Parse(kUpdateCheckStatusNoUpdateWithRunAction)); EXPECT_TRUE(parser.errors().empty()); EXPECT_FALSE(parser.results().list.empty()); - firstResult = &parser.results().list[0]; - EXPECT_STREQ("noupdate", firstResult->status.c_str()); - EXPECT_EQ(firstResult->extension_id, "12345"); - EXPECT_STREQ("this", firstResult->action_run.c_str()); + first_result = &parser.results().list[0]; + EXPECT_STREQ("noupdate", first_result->status.c_str()); + EXPECT_EQ(first_result->extension_id, "12345"); + EXPECT_STREQ("this", first_result->action_run.c_str()); EXPECT_TRUE(parser.Parse(kUpdateCheckStatusErrorWithRunAction)); EXPECT_FALSE(parser.errors().empty()); EXPECT_TRUE(parser.results().list.empty()); + + EXPECT_TRUE(parser.Parse(kAppsStatusError)); + EXPECT_STREQ("Unknown app status", parser.errors().c_str()); + EXPECT_EQ(3u, parser.results().list.size()); + first_result = &parser.results().list[0]; + EXPECT_EQ(first_result->extension_id, "aaaaaaaa"); + EXPECT_STREQ("error-unknownApplication", first_result->status.c_str()); + EXPECT_TRUE(first_result->manifest.version.empty()); + second_result = &parser.results().list[1]; + EXPECT_EQ(second_result->extension_id, "bbbbbbbb"); + EXPECT_STREQ("restricted", second_result->status.c_str()); + EXPECT_TRUE(second_result->manifest.version.empty()); + const ProtocolParser::Result* third_result = &parser.results().list[2]; + EXPECT_EQ(third_result->extension_id, "cccccccc"); + EXPECT_STREQ("error-invalidAppId", third_result->status.c_str()); + EXPECT_TRUE(third_result->manifest.version.empty()); } } // namespace update_client diff --git a/chromium/components/update_client/request_sender.cc b/chromium/components/update_client/request_sender.cc index 190cf58ec48..be6e856db2b 100644 --- a/chromium/components/update_client/request_sender.cc +++ b/chromium/components/update_client/request_sender.cc @@ -15,10 +15,10 @@ #include "base/threading/thread_task_runner_handle.h" #include "components/client_update_protocol/ecdsa.h" #include "components/update_client/configurator.h" +#include "components/update_client/update_client_errors.h" #include "components/update_client/utils.h" #include "net/http/http_response_headers.h" -#include "net/url_request/url_fetcher.h" -#include "net/url_request/url_request_status.h" +#include "services/network/public/cpp/simple_url_loader.h" namespace update_client { @@ -73,7 +73,7 @@ void RequestSender::Send( request_sender_callback_ = std::move(request_sender_callback); if (urls_.empty()) { - return HandleSendError(-1, 0); + return HandleSendError(static_cast<int>(ProtocolError::MISSING_URLS), 0); } cur_url_ = urls_.begin(); @@ -81,7 +81,8 @@ void RequestSender::Send( if (use_signing_) { public_key_ = GetKey(kKeyPubBytesBase64); if (public_key_.empty()) - return HandleSendError(-1, 0); + return HandleSendError( + static_cast<int>(ProtocolError::MISSING_PUBLIC_KEY), 0); } SendInternal(); @@ -103,13 +104,19 @@ void RequestSender::SendInternal() { url = BuildUpdateUrl(url, request_query_string); } - url_fetcher_ = SendProtocolRequest(url, request_extra_headers_, request_body_, - this, config_->RequestContext()); - if (!url_fetcher_.get()) + update_client::LoadCompleteCallback callback = base::BindOnce( + &RequestSender::OnSimpleURLLoaderComplete, base::Unretained(this), url); + + url_loader_ = + SendProtocolRequest(url, request_extra_headers_, request_body_, + std::move(callback), config_->URLLoaderFactory()); + if (!url_loader_) base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(&RequestSender::SendInternalComplete, - base::Unretained(this), -1, std::string(), - std::string(), 0)); + FROM_HERE, + base::BindOnce(&RequestSender::SendInternalComplete, + base::Unretained(this), + static_cast<int>(ProtocolError::URL_FETCHER_FAILED), + std::string(), std::string(), 0)); } void RequestSender::SendInternalComplete(int error, @@ -125,7 +132,7 @@ void RequestSender::SendInternalComplete(int error, } DCHECK(use_signing_); - DCHECK(signer_.get()); + DCHECK(signer_); if (signer_->ValidateResponse(response_body, response_etag)) { base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::BindOnce(std::move(request_sender_callback_), 0, @@ -133,7 +140,7 @@ void RequestSender::SendInternalComplete(int error, return; } - error = kErrorResponseNotTrusted; + error = static_cast<int>(ProtocolError::RESPONSE_NOT_TRUSTED); } DCHECK(error); @@ -150,30 +157,40 @@ void RequestSender::SendInternalComplete(int error, HandleSendError(error, retry_after_sec); } -void RequestSender::OnURLFetchComplete(const net::URLFetcher* source) { +void RequestSender::OnSimpleURLLoaderComplete( + const GURL& original_url, + std::unique_ptr<std::string> response_body) { DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(source); - const GURL original_url(source->GetOriginalURL()); VLOG(1) << "request completed from url: " << original_url.spec(); - const int fetch_error(GetFetchError(*source)); - std::string response_body; - CHECK(source->GetResponseAsString(&response_body)); + int response_code = -1; + if (url_loader_->ResponseInfo() && url_loader_->ResponseInfo()->headers) { + response_code = url_loader_->ResponseInfo()->headers->response_code(); + } + + int fetch_error = -1; + if (response_body && response_code == 200) { + fetch_error = 0; + } else if (response_code != -1) { + fetch_error = response_code; + } else { + fetch_error = url_loader_->NetError(); + } int64_t retry_after_sec(-1); - const auto status(source->GetStatus().status()); - if (original_url.SchemeIsCryptographic() && - status == net::URLRequestStatus::SUCCESS) { - retry_after_sec = GetInt64HeaderValue(source, kHeaderXRetryAfter); + if (original_url.SchemeIsCryptographic() && fetch_error > 0) { + retry_after_sec = + GetInt64HeaderValue(url_loader_.get(), kHeaderXRetryAfter); retry_after_sec = std::min(retry_after_sec, kMaxRetryAfterSec); } base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::BindOnce(&RequestSender::SendInternalComplete, - base::Unretained(this), fetch_error, response_body, - GetStringHeaderValue(source, kHeaderEtag), + base::Unretained(this), fetch_error, + response_body ? *response_body : std::string(), + GetStringHeaderValue(url_loader_.get(), kHeaderEtag), static_cast<int>(retry_after_sec))); } @@ -202,23 +219,30 @@ GURL RequestSender::BuildUpdateUrl(const GURL& url, return url.ReplaceComponents(replacements); } -std::string RequestSender::GetStringHeaderValue(const net::URLFetcher* source, - const char* header_name) { - auto* response_headers(source->GetResponseHeaders()); - if (!response_headers) - return std::string(); +std::string RequestSender::GetStringHeaderValue( + const network::SimpleURLLoader* url_loader, + const char* header_name) { + DCHECK(url_loader); + if (url_loader->ResponseInfo() && url_loader->ResponseInfo()->headers) { + std::string etag; + return url_loader->ResponseInfo()->headers->EnumerateHeader( + nullptr, header_name, &etag) + ? etag + : std::string(); + } - std::string etag; - return response_headers->EnumerateHeader(nullptr, header_name, &etag) - ? etag - : std::string(); + return std::string(); } -int64_t RequestSender::GetInt64HeaderValue(const net::URLFetcher* source, - const char* header_name) { - auto* response_headers(source->GetResponseHeaders()); - return response_headers ? response_headers->GetInt64HeaderValue(header_name) - : -1; +int64_t RequestSender::GetInt64HeaderValue( + const network::SimpleURLLoader* url_loader, + const char* header_name) { + DCHECK(url_loader); + if (url_loader->ResponseInfo() && url_loader->ResponseInfo()->headers) { + return url_loader->ResponseInfo()->headers->GetInt64HeaderValue( + header_name); + } + return -1; } } // namespace update_client diff --git a/chromium/components/update_client/request_sender.h b/chromium/components/update_client/request_sender.h index 91a83c9e536..10722312152 100644 --- a/chromium/components/update_client/request_sender.h +++ b/chromium/components/update_client/request_sender.h @@ -16,15 +16,14 @@ #include "base/macros.h" #include "base/memory/ref_counted.h" #include "base/threading/thread_checker.h" -#include "net/url_request/url_fetcher_delegate.h" #include "url/gurl.h" namespace client_update_protocol { class Ecdsa; } -namespace net { -class URLFetcher; +namespace network { +class SimpleURLLoader; } namespace update_client { @@ -35,7 +34,7 @@ class Configurator; // of responsibility design pattern, where the urls are tried in the order they // are specified, until the request to one of them succeeds or all have failed. // CUP signing is optional. -class RequestSender : public net::URLFetcherDelegate { +class RequestSender { public: // If |error| is 0, then the response is provided in the |response| parameter. // |retry_after_sec| contains the value of the X-Retry-After response header, @@ -46,14 +45,8 @@ class RequestSender : public net::URLFetcherDelegate { using RequestSenderCallback = base::OnceCallback< void(int error, const std::string& response, int retry_after_sec)>; - // This value is chosen not to conflict with network errors defined by - // net/base/net_error_list.h. The callers don't have to handle this error in - // any meaningful way, but this value may be reported in UMA stats, therefore - // avoiding collisions with known network errors is desirable. - enum : int { kErrorResponseNotTrusted = -10000 }; - explicit RequestSender(scoped_refptr<Configurator> config); - ~RequestSender() override; + ~RequestSender(); // |use_signing| enables CUP signing of protocol messages exchanged using // this class. |is_foreground| controls the presence and the value for the @@ -76,16 +69,17 @@ class RequestSender : public net::URLFetcherDelegate { // Returns the string value of a header of the server response or an empty // string if the header is not available. - static std::string GetStringHeaderValue(const net::URLFetcher* source, - const char* header_name); + static std::string GetStringHeaderValue( + const network::SimpleURLLoader* url_loader, + const char* header_name); // Returns the integral value of a header of the server response or -1 if // if the header is not available or a conversion error has occured. - static int64_t GetInt64HeaderValue(const net::URLFetcher* source, + static int64_t GetInt64HeaderValue(const network::SimpleURLLoader* loader, const char* header_name); - // Overrides for URLFetcherDelegate. - void OnURLFetchComplete(const net::URLFetcher* source) override; + void OnSimpleURLLoaderComplete(const GURL& original_url, + std::unique_ptr<std::string> response_body); // Implements the error handling and url fallback mechanism. void SendInternal(); @@ -112,7 +106,7 @@ class RequestSender : public net::URLFetcherDelegate { std::string public_key_; std::vector<GURL>::const_iterator cur_url_; - std::unique_ptr<net::URLFetcher> url_fetcher_; + std::unique_ptr<network::SimpleURLLoader> url_loader_; std::unique_ptr<client_update_protocol::Ecdsa> signer_; DISALLOW_COPY_AND_ASSIGN(RequestSender); diff --git a/chromium/components/update_client/request_sender_unittest.cc b/chromium/components/update_client/request_sender_unittest.cc index f480cac3a66..e2eb26316d8 100644 --- a/chromium/components/update_client/request_sender_unittest.cc +++ b/chromium/components/update_client/request_sender_unittest.cc @@ -15,8 +15,7 @@ #include "base/test/scoped_task_environment.h" #include "base/threading/thread_task_runner_handle.h" #include "components/update_client/test_configurator.h" -#include "components/update_client/url_request_post_interceptor.h" -#include "net/url_request/url_fetcher.h" +#include "components/update_client/url_loader_post_interceptor.h" #include "testing/gtest/include/gtest/gtest.h" namespace update_client { @@ -25,8 +24,6 @@ namespace { const char kUrl1[] = "https://localhost2/path1"; const char kUrl2[] = "https://localhost2/path2"; -const char kUrlPath1[] = "path1"; -const char kUrlPath2[] = "path2"; // TODO(sorin): refactor as a utility function for unit tests. base::FilePath test_file(const char* file) { @@ -63,10 +60,8 @@ class RequestSenderTest : public testing::Test, scoped_refptr<TestConfigurator> config_; std::unique_ptr<RequestSender> request_sender_; - std::unique_ptr<InterceptorFactory> interceptor_factory_; - scoped_refptr<URLRequestPostInterceptor> post_interceptor_1_; - scoped_refptr<URLRequestPostInterceptor> post_interceptor_2_; + std::unique_ptr<URLLoaderPostInterceptor> post_interceptor_; int error_ = 0; std::string response_; @@ -89,23 +84,19 @@ void RequestSenderTest::SetUp() { config_ = base::MakeRefCounted<TestConfigurator>(); request_sender_ = std::make_unique<RequestSender>(config_); - interceptor_factory_ = - std::make_unique<InterceptorFactory>(base::ThreadTaskRunnerHandle::Get()); - post_interceptor_1_ = - interceptor_factory_->CreateInterceptorForPath(kUrlPath1); - post_interceptor_2_ = - interceptor_factory_->CreateInterceptorForPath(kUrlPath2); - EXPECT_TRUE(post_interceptor_1_); - EXPECT_TRUE(post_interceptor_2_); + std::vector<GURL> urls; + urls.push_back(GURL(kUrl1)); + urls.push_back(GURL(kUrl2)); + + post_interceptor_ = std::make_unique<URLLoaderPostInterceptor>( + urls, config_->test_url_loader_factory()); + EXPECT_TRUE(post_interceptor_); } void RequestSenderTest::TearDown() { request_sender_ = nullptr; - post_interceptor_1_ = nullptr; - post_interceptor_2_ = nullptr; - - interceptor_factory_ = nullptr; + post_interceptor_.reset(); // Run the threads until they are idle to allow the clean up // of the network interceptors on the IO thread. @@ -137,8 +128,8 @@ void RequestSenderTest::RequestSenderComplete(int error, // not tried. TEST_P(RequestSenderTest, RequestSendSuccess) { EXPECT_TRUE( - post_interceptor_1_->ExpectRequest(std::make_unique<PartialMatch>("test"), - test_file("updatecheck_reply_1.xml"))); + post_interceptor_->ExpectRequest(std::make_unique<PartialMatch>("test"), + test_file("updatecheck_reply_1.xml"))); const bool is_foreground = GetParam(); request_sender_->Send( @@ -149,18 +140,16 @@ TEST_P(RequestSenderTest, RequestSendSuccess) { base::Unretained(this))); RunThreads(); - EXPECT_EQ(1, post_interceptor_1_->GetHitCount()) - << post_interceptor_1_->GetRequestsAsString(); - EXPECT_EQ(1, post_interceptor_1_->GetCount()) - << post_interceptor_1_->GetRequestsAsString(); + EXPECT_EQ(1, post_interceptor_->GetHitCount()) + << post_interceptor_->GetRequestsAsString(); + EXPECT_EQ(1, post_interceptor_->GetCount()) + << post_interceptor_->GetRequestsAsString(); - EXPECT_EQ(0, post_interceptor_2_->GetHitCount()) - << post_interceptor_2_->GetRequestsAsString(); - EXPECT_EQ(0, post_interceptor_2_->GetCount()) - << post_interceptor_2_->GetRequestsAsString(); + EXPECT_EQ(0, post_interceptor_->GetHitCountForURL(GURL(kUrl2))) + << post_interceptor_->GetRequestsAsString(); // Sanity check the request. - EXPECT_STREQ("test", post_interceptor_1_->GetRequestBody(0).c_str()); + EXPECT_STREQ("test", post_interceptor_->GetRequestBody(0).c_str()); // Check the response post conditions. EXPECT_EQ(0, error_); @@ -171,7 +160,7 @@ TEST_P(RequestSenderTest, RequestSendSuccess) { // Check the interactivity header value. const auto extra_request_headers = - post_interceptor_1_->GetRequests()[0].second; + std::get<1>(post_interceptor_->GetRequests()[0]); EXPECT_TRUE(extra_request_headers.HasHeader("X-Goog-Update-Interactivity")); std::string header; extra_request_headers.GetHeader("X-Goog-Update-Interactivity", &header); @@ -181,10 +170,10 @@ TEST_P(RequestSenderTest, RequestSendSuccess) { // Tests that the request succeeds using the second url after the first url // has failed. TEST_F(RequestSenderTest, RequestSendSuccessWithFallback) { - EXPECT_TRUE(post_interceptor_1_->ExpectRequest( - std::make_unique<PartialMatch>("test"), 403)); - EXPECT_TRUE(post_interceptor_2_->ExpectRequest( - std::make_unique<PartialMatch>("test"))); + EXPECT_TRUE(post_interceptor_->ExpectRequest( + std::make_unique<PartialMatch>("test"), net::HTTP_FORBIDDEN)); + EXPECT_TRUE( + post_interceptor_->ExpectRequest(std::make_unique<PartialMatch>("test"))); request_sender_->Send( {GURL(kUrl1), GURL(kUrl2)}, {}, "test", false, @@ -192,26 +181,26 @@ TEST_F(RequestSenderTest, RequestSendSuccessWithFallback) { base::Unretained(this))); RunThreads(); - EXPECT_EQ(1, post_interceptor_1_->GetHitCount()) - << post_interceptor_1_->GetRequestsAsString(); - EXPECT_EQ(1, post_interceptor_1_->GetCount()) - << post_interceptor_1_->GetRequestsAsString(); - EXPECT_EQ(1, post_interceptor_2_->GetHitCount()) - << post_interceptor_2_->GetRequestsAsString(); - EXPECT_EQ(1, post_interceptor_2_->GetCount()) - << post_interceptor_2_->GetRequestsAsString(); - - EXPECT_STREQ("test", post_interceptor_1_->GetRequestBody(0).c_str()); - EXPECT_STREQ("test", post_interceptor_2_->GetRequestBody(0).c_str()); + EXPECT_EQ(2, post_interceptor_->GetHitCount()) + << post_interceptor_->GetRequestsAsString(); + EXPECT_EQ(2, post_interceptor_->GetCount()) + << post_interceptor_->GetRequestsAsString(); + EXPECT_EQ(1, post_interceptor_->GetHitCountForURL(GURL(kUrl1))) + << post_interceptor_->GetRequestsAsString(); + EXPECT_EQ(1, post_interceptor_->GetHitCountForURL(GURL(kUrl2))) + << post_interceptor_->GetRequestsAsString(); + + EXPECT_STREQ("test", post_interceptor_->GetRequestBody(0).c_str()); + EXPECT_STREQ("test", post_interceptor_->GetRequestBody(1).c_str()); EXPECT_EQ(0, error_); } // Tests that the request fails when both urls have failed. TEST_F(RequestSenderTest, RequestSendFailed) { - EXPECT_TRUE(post_interceptor_1_->ExpectRequest( - std::make_unique<PartialMatch>("test"), 403)); - EXPECT_TRUE(post_interceptor_2_->ExpectRequest( - std::make_unique<PartialMatch>("test"), 403)); + EXPECT_TRUE(post_interceptor_->ExpectRequest( + std::make_unique<PartialMatch>("test"), net::HTTP_FORBIDDEN)); + EXPECT_TRUE(post_interceptor_->ExpectRequest( + std::make_unique<PartialMatch>("test"), net::HTTP_FORBIDDEN)); const std::vector<GURL> urls = {GURL(kUrl1), GURL(kUrl2)}; request_sender_ = std::make_unique<RequestSender>(config_); @@ -221,17 +210,17 @@ TEST_F(RequestSenderTest, RequestSendFailed) { base::Unretained(this))); RunThreads(); - EXPECT_EQ(1, post_interceptor_1_->GetHitCount()) - << post_interceptor_1_->GetRequestsAsString(); - EXPECT_EQ(1, post_interceptor_1_->GetCount()) - << post_interceptor_1_->GetRequestsAsString(); - EXPECT_EQ(1, post_interceptor_2_->GetHitCount()) - << post_interceptor_2_->GetRequestsAsString(); - EXPECT_EQ(1, post_interceptor_2_->GetCount()) - << post_interceptor_2_->GetRequestsAsString(); - - EXPECT_STREQ("test", post_interceptor_1_->GetRequestBody(0).c_str()); - EXPECT_STREQ("test", post_interceptor_2_->GetRequestBody(0).c_str()); + EXPECT_EQ(2, post_interceptor_->GetHitCount()) + << post_interceptor_->GetRequestsAsString(); + EXPECT_EQ(2, post_interceptor_->GetCount()) + << post_interceptor_->GetRequestsAsString(); + EXPECT_EQ(1, post_interceptor_->GetHitCountForURL(GURL(kUrl1))) + << post_interceptor_->GetRequestsAsString(); + EXPECT_EQ(1, post_interceptor_->GetHitCountForURL(GURL(kUrl2))) + << post_interceptor_->GetRequestsAsString(); + + EXPECT_STREQ("test", post_interceptor_->GetRequestBody(0).c_str()); + EXPECT_STREQ("test", post_interceptor_->GetRequestBody(1).c_str()); EXPECT_EQ(403, error_); } @@ -245,14 +234,14 @@ TEST_F(RequestSenderTest, RequestSendFailedNoUrls) { base::Unretained(this))); RunThreads(); - EXPECT_EQ(-1, error_); + EXPECT_EQ(-10002, error_); } // Tests that a CUP request fails if the response is not signed. TEST_F(RequestSenderTest, RequestSendCupError) { EXPECT_TRUE( - post_interceptor_1_->ExpectRequest(std::make_unique<PartialMatch>("test"), - test_file("updatecheck_reply_1.xml"))); + post_interceptor_->ExpectRequest(std::make_unique<PartialMatch>("test"), + test_file("updatecheck_reply_1.xml"))); const std::vector<GURL> urls = {GURL(kUrl1)}; request_sender_ = std::make_unique<RequestSender>(config_); @@ -262,13 +251,13 @@ TEST_F(RequestSenderTest, RequestSendCupError) { base::Unretained(this))); RunThreads(); - EXPECT_EQ(1, post_interceptor_1_->GetHitCount()) - << post_interceptor_1_->GetRequestsAsString(); - EXPECT_EQ(1, post_interceptor_1_->GetCount()) - << post_interceptor_1_->GetRequestsAsString(); + EXPECT_EQ(1, post_interceptor_->GetHitCount()) + << post_interceptor_->GetRequestsAsString(); + EXPECT_EQ(1, post_interceptor_->GetCount()) + << post_interceptor_->GetRequestsAsString(); - EXPECT_STREQ("test", post_interceptor_1_->GetRequestBody(0).c_str()); - EXPECT_EQ(RequestSender::kErrorResponseNotTrusted, error_); + EXPECT_STREQ("test", post_interceptor_->GetRequestBody(0).c_str()); + EXPECT_EQ(-10000, error_); EXPECT_TRUE(response_.empty()); } diff --git a/chromium/components/update_client/test_configurator.cc b/chromium/components/update_client/test_configurator.cc index cbc177ee6a9..882e5db3cb0 100644 --- a/chromium/components/update_client/test_configurator.cc +++ b/chromium/components/update_client/test_configurator.cc @@ -13,6 +13,7 @@ #include "components/services/unzip/unzip_service.h" #include "components/update_client/activity_data_service.h" #include "net/url_request/url_request_test_util.h" +#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h" #include "services/service_manager/public/cpp/connector.h" #include "services/service_manager/public/cpp/service.h" #include "services/service_manager/public/cpp/test/test_connector_factory.h" @@ -38,7 +39,10 @@ TestConfigurator::TestConfigurator() enabled_cup_signing_(false), enabled_component_updates_(true), context_(base::MakeRefCounted<net::TestURLRequestContextGetter>( - base::ThreadTaskRunnerHandle::Get())) { + base::ThreadTaskRunnerHandle::Get())), + test_shared_loader_factory_( + base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>( + &test_url_loader_factory_)) { service_manager::TestConnectorFactory::NameToServiceMap services; services.insert( std::make_pair("patch_service", std::make_unique<patch::PatchService>())); @@ -120,6 +124,11 @@ scoped_refptr<net::URLRequestContextGetter> TestConfigurator::RequestContext() return context_; } +scoped_refptr<network::SharedURLLoaderFactory> +TestConfigurator::URLLoaderFactory() const { + return test_shared_loader_factory_; +} + std::unique_ptr<service_manager::Connector> TestConfigurator::CreateServiceManagerConnector() const { return connector_->Clone(); @@ -175,6 +184,10 @@ void TestConfigurator::SetPingUrl(const GURL& url) { ping_url_ = url; } +void TestConfigurator::SetAppGuid(const std::string& app_guid) { + app_guid_ = app_guid; +} + PrefService* TestConfigurator::GetPrefService() const { return nullptr; } @@ -191,4 +204,8 @@ std::vector<uint8_t> TestConfigurator::GetRunActionKeyHash() const { return std::vector<uint8_t>(std::begin(gjpm_hash), std::end(gjpm_hash)); } +std::string TestConfigurator::GetAppGuid() const { + return app_guid_; +} + } // namespace update_client diff --git a/chromium/components/update_client/test_configurator.h b/chromium/components/update_client/test_configurator.h index 60ba6579c06..c7d4bbf715d 100644 --- a/chromium/components/update_client/test_configurator.h +++ b/chromium/components/update_client/test_configurator.h @@ -15,6 +15,7 @@ #include "base/macros.h" #include "base/memory/ref_counted.h" #include "components/update_client/configurator.h" +#include "services/network/test/test_url_loader_factory.h" #include "url/gurl.h" class PrefService; @@ -24,6 +25,10 @@ class TestURLRequestContextGetter; class URLRequestContextGetter; } // namespace net +namespace network { +class SharedURLLoaderFactory; +} // namespace network + namespace service_manager { class Connector; class TestConnectorFactory; @@ -87,6 +92,8 @@ class TestConfigurator : public Configurator { std::string ExtraRequestParams() const override; std::string GetDownloadPreference() const override; scoped_refptr<net::URLRequestContextGetter> RequestContext() const override; + scoped_refptr<network::SharedURLLoaderFactory> URLLoaderFactory() + const override; std::unique_ptr<service_manager::Connector> CreateServiceManagerConnector() const override; bool EnabledDeltas() const override; @@ -97,6 +104,7 @@ class TestConfigurator : public Configurator { ActivityDataService* GetActivityDataService() const override; bool IsPerUserInstall() const override; std::vector<uint8_t> GetRunActionKeyHash() const override; + std::string GetAppGuid() const override; void SetBrand(const std::string& brand); void SetOnDemandTime(int seconds); @@ -106,6 +114,10 @@ class TestConfigurator : public Configurator { void SetEnabledComponentUpdates(bool enabled_component_updates); void SetUpdateCheckUrl(const GURL& url); void SetPingUrl(const GURL& url); + void SetAppGuid(const std::string& app_guid); + network::TestURLLoaderFactory* test_url_loader_factory() { + return &test_url_loader_factory_; + } private: friend class base::RefCountedThreadSafe<TestConfigurator>; @@ -121,11 +133,15 @@ class TestConfigurator : public Configurator { bool enabled_component_updates_; GURL update_check_url_; GURL ping_url_; + std::string app_guid_; std::unique_ptr<service_manager::TestConnectorFactory> connector_factory_; std::unique_ptr<service_manager::Connector> connector_; scoped_refptr<net::TestURLRequestContextGetter> context_; + scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_; + network::TestURLLoaderFactory test_url_loader_factory_; + DISALLOW_COPY_AND_ASSIGN(TestConfigurator); }; diff --git a/chromium/components/update_client/test_installer.cc b/chromium/components/update_client/test_installer.cc index 22eef68c64e..3ed161acb03 100644 --- a/chromium/components/update_client/test_installer.cc +++ b/chromium/components/update_client/test_installer.cc @@ -83,7 +83,7 @@ void VersionedTestInstaller::Install(const base::FilePath& unpack_path, const auto manifest = update_client::ReadManifest(unpack_path); std::string version_string; manifest->GetStringASCII("version", &version_string); - const base::Version version(version_string.c_str()); + const base::Version version(version_string); const base::FilePath path = install_directory_.AppendASCII(version.GetString()); diff --git a/chromium/components/update_client/update_checker.cc b/chromium/components/update_client/update_checker.cc index 1fb0479865e..d4ecc9e73ee 100644 --- a/chromium/components/update_client/update_checker.cc +++ b/chromium/components/update_client/update_checker.cc @@ -25,7 +25,6 @@ #include "components/update_client/configurator.h" #include "components/update_client/persisted_data.h" #include "components/update_client/protocol_builder.h" -#include "components/update_client/protocol_parser.h" #include "components/update_client/request_sender.h" #include "components/update_client/task_traits.h" #include "components/update_client/update_client.h" @@ -69,14 +68,12 @@ class UpdateCheckerImpl : public UpdateChecker { const IdToComponentPtrMap& components, const std::string& additional_attributes, bool enabled_component_updates); - void OnRequestSenderComplete(const IdToComponentPtrMap& components, - int error, + void OnRequestSenderComplete(int error, const std::string& response, int retry_after_sec); - void UpdateCheckSucceeded(const IdToComponentPtrMap& components, - const ProtocolParser::Results& results, + void UpdateCheckSucceeded(const ProtocolParser::Results& results, int retry_after_sec); - void UpdateCheckFailed(const IdToComponentPtrMap& components, + void UpdateCheckFailed(ErrorCategory error_category, int error, int retry_after_sec); @@ -168,11 +165,10 @@ void UpdateCheckerImpl::CheckForUpdatesHelper( updater_state_attributes_), config_->EnabledCupSigning(), base::BindOnce(&UpdateCheckerImpl::OnRequestSenderComplete, - base::Unretained(this), base::ConstRef(components))); + base::Unretained(this))); } void UpdateCheckerImpl::OnRequestSenderComplete( - const IdToComponentPtrMap& components, int error, const std::string& response, int retry_after_sec) { @@ -180,23 +176,24 @@ void UpdateCheckerImpl::OnRequestSenderComplete( if (error) { VLOG(1) << "RequestSender failed " << error; - UpdateCheckFailed(components, error, retry_after_sec); + UpdateCheckFailed(ErrorCategory::kUpdateCheck, error, retry_after_sec); return; } ProtocolParser update_response; if (!update_response.Parse(response)) { VLOG(1) << "Parse failed " << update_response.errors(); - UpdateCheckFailed(components, -1, retry_after_sec); + UpdateCheckFailed(ErrorCategory::kUpdateCheck, + static_cast<int>(ProtocolError::PARSE_FAILED), + retry_after_sec); return; } DCHECK_EQ(0, error); - UpdateCheckSucceeded(components, update_response.results(), retry_after_sec); + UpdateCheckSucceeded(update_response.results(), retry_after_sec); } void UpdateCheckerImpl::UpdateCheckSucceeded( - const IdToComponentPtrMap& components, const ProtocolParser::Results& results, int retry_after_sec) { DCHECK(thread_checker_.CalledOnValidThread()); @@ -218,32 +215,23 @@ void UpdateCheckerImpl::UpdateCheckSucceeded( metadata_->SetCohortHint(result.extension_id, entry->second); } - for (const auto& result : results.list) { - const auto& id = result.extension_id; - const auto it = components.find(id); - if (it != components.end()) - it->second->SetParseResult(result); - } - base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, - base::BindOnce(std::move(update_check_callback_), 0, retry_after_sec)); + base::BindOnce(std::move(update_check_callback_), + base::make_optional<ProtocolParser::Results>(results), + ErrorCategory::kNone, 0, retry_after_sec)); } -void UpdateCheckerImpl::UpdateCheckFailed(const IdToComponentPtrMap& components, +void UpdateCheckerImpl::UpdateCheckFailed(ErrorCategory error_category, int error, int retry_after_sec) { DCHECK(thread_checker_.CalledOnValidThread()); DCHECK_NE(0, error); - for (const auto& item : components) { - DCHECK(item.second); - Component& component = *item.second; - component.set_update_check_error(error); - } base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(std::move(update_check_callback_), error, - retry_after_sec)); + FROM_HERE, + base::BindOnce(std::move(update_check_callback_), base::nullopt, + error_category, error, retry_after_sec)); } } // namespace diff --git a/chromium/components/update_client/update_checker.h b/chromium/components/update_client/update_checker.h index 408b3bbb65b..241e3a0a73c 100644 --- a/chromium/components/update_client/update_checker.h +++ b/chromium/components/update_client/update_checker.h @@ -12,7 +12,9 @@ #include "base/callback.h" #include "base/macros.h" #include "base/memory/ref_counted.h" +#include "base/optional.h" #include "components/update_client/component.h" +#include "components/update_client/protocol_parser.h" #include "url/gurl.h" namespace update_client { @@ -22,8 +24,11 @@ class PersistedData; class UpdateChecker { public: - using UpdateCheckCallback = - base::OnceCallback<void(int error, int retry_after_sec)>; + using UpdateCheckCallback = base::OnceCallback<void( + const base::Optional<ProtocolParser::Results>& results, + ErrorCategory error_category, + int error, + int retry_after_sec)>; using Factory = std::unique_ptr<UpdateChecker> (*)(scoped_refptr<Configurator> config, diff --git a/chromium/components/update_client/update_checker_unittest.cc b/chromium/components/update_client/update_checker_unittest.cc index a3249b985c3..bb8cf9ebcdf 100644 --- a/chromium/components/update_client/update_checker_unittest.cc +++ b/chromium/components/update_client/update_checker_unittest.cc @@ -13,9 +13,13 @@ #include "base/files/file_util.h" #include "base/macros.h" #include "base/memory/ref_counted.h" +#include "base/optional.h" #include "base/path_service.h" #include "base/run_loop.h" +#include "base/stl_util.h" +#include "base/strings/stringprintf.h" #include "base/task_scheduler/post_task.h" +#include "base/test/bind_test_util.h" #include "base/test/scoped_task_environment.h" #include "base/threading/thread_task_runner_handle.h" #include "base/version.h" @@ -26,8 +30,9 @@ #include "components/update_client/persisted_data.h" #include "components/update_client/test_configurator.h" #include "components/update_client/update_engine.h" -#include "components/update_client/url_request_post_interceptor.h" +#include "components/update_client/url_loader_post_interceptor.h" #include "net/url_request/url_request_test_util.h" +#include "services/network/public/cpp/resource_request.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "url/gurl.h" @@ -114,7 +119,11 @@ class UpdateCheckerTest : public testing::Test, void SetUp() override; void TearDown() override; - void UpdateCheckComplete(int error, int retry_after_sec); + void UpdateCheckComplete( + const base::Optional<ProtocolParser::Results>& results, + ErrorCategory error_category, + int error, + int retry_after_sec); protected: void Quit(); @@ -129,9 +138,10 @@ class UpdateCheckerTest : public testing::Test, std::unique_ptr<UpdateChecker> update_checker_; - std::unique_ptr<InterceptorFactory> interceptor_factory_; - scoped_refptr<URLRequestPostInterceptor> post_interceptor_; + std::unique_ptr<URLLoaderPostInterceptor> post_interceptor_; + base::Optional<ProtocolParser::Results> results_; + ErrorCategory error_category_ = ErrorCategory::kNone; int error_ = 0; int retry_after_sec_ = 0; @@ -162,9 +172,9 @@ void UpdateCheckerTest::SetUp() { PersistedData::RegisterPrefs(pref_->registry()); metadata_ = std::make_unique<PersistedData>(pref_.get(), activity_data_service_.get()); - interceptor_factory_ = - std::make_unique<InterceptorFactory>(base::ThreadTaskRunnerHandle::Get()); - post_interceptor_ = interceptor_factory_->CreateInterceptor(); + + post_interceptor_ = std::make_unique<URLLoaderPostInterceptor>( + config_->test_url_loader_factory()); EXPECT_TRUE(post_interceptor_); update_checker_ = nullptr; @@ -177,8 +187,7 @@ void UpdateCheckerTest::SetUp() { void UpdateCheckerTest::TearDown() { update_checker_ = nullptr; - post_interceptor_ = nullptr; - interceptor_factory_ = nullptr; + post_interceptor_.reset(); config_ = nullptr; @@ -198,7 +207,13 @@ void UpdateCheckerTest::Quit() { std::move(quit_closure_).Run(); } -void UpdateCheckerTest::UpdateCheckComplete(int error, int retry_after_sec) { +void UpdateCheckerTest::UpdateCheckComplete( + const base::Optional<ProtocolParser::Results>& results, + ErrorCategory error_category, + int error, + int retry_after_sec) { + results_ = results; + error_category_ = error_category; error_ = error; retry_after_sec_ = retry_after_sec; Quit(); @@ -215,7 +230,7 @@ std::unique_ptr<Component> UpdateCheckerTest::MakeComponent() const { std::unique_ptr<CrxComponent> crx_component = std::make_unique<CrxComponent>(); crx_component->name = "test_jebg"; - crx_component->pk_hash.assign(jebg_hash, jebg_hash + arraysize(jebg_hash)); + crx_component->pk_hash.assign(jebg_hash, jebg_hash + base::size(jebg_hash)); crx_component->installer = nullptr; crx_component->version = base::Version("0.9"); crx_component->fingerprint = "fp1"; @@ -280,15 +295,20 @@ TEST_P(UpdateCheckerTest, UpdateCheckSuccess) { EXPECT_THAT(request, testing::HasSubstr(" sessionid=")); // Sanity check the arguments of the callback after parsing. + EXPECT_EQ(ErrorCategory::kNone, error_category_); EXPECT_EQ(0, error_); - - EXPECT_EQ(base::Version("1.0"), component->next_version_); - EXPECT_EQ(1u, component->crx_urls_.size()); - EXPECT_EQ( - GURL("http://localhost/download/jebgalgnebhfojomionfpkfelancnnkf.crx"), - component->crx_urls_.front()); - - EXPECT_STREQ("this", component->action_run_.c_str()); + EXPECT_TRUE(results_); + EXPECT_EQ(1u, results_->list.size()); + const auto& result = results_->list.front(); + EXPECT_STREQ("jebgalgnebhfojomionfpkfelancnnkf", result.extension_id.c_str()); + EXPECT_EQ("1.0", result.manifest.version); + EXPECT_EQ("11.0.1.0", result.manifest.browser_min_version); + EXPECT_EQ(1u, result.manifest.packages.size()); + EXPECT_STREQ("jebgalgnebhfojomionfpkfelancnnkf.crx", + result.manifest.packages.front().name.c_str()); + EXPECT_EQ(1u, result.crx_urls.size()); + EXPECT_EQ(GURL("http://localhost/download/"), result.crx_urls.front()); + EXPECT_STREQ("this", result.action_run.c_str()); #if (OS_WIN) EXPECT_THAT(request, testing::HasSubstr(" domainjoined=")); @@ -300,7 +320,8 @@ TEST_P(UpdateCheckerTest, UpdateCheckSuccess) { #endif // OS_WINDOWS // Check the DDOS protection header values. - const auto extra_request_headers = post_interceptor_->GetRequests()[0].second; + const auto extra_request_headers = + std::get<1>(post_interceptor_->GetRequests()[0]); EXPECT_TRUE(extra_request_headers.HasHeader("X-Goog-Update-Interactivity")); std::string header; extra_request_headers.GetHeader("X-Goog-Update-Interactivity", &header); @@ -375,15 +396,13 @@ TEST_F(UpdateCheckerTest, UpdateCheckSuccessNoBrand) { // Simulates a 403 server response error. TEST_F(UpdateCheckerTest, UpdateCheckError) { EXPECT_TRUE(post_interceptor_->ExpectRequest( - std::make_unique<PartialMatch>("updatecheck"), 403)); + std::make_unique<PartialMatch>("updatecheck"), net::HTTP_FORBIDDEN)); update_checker_ = UpdateChecker::Create(config_, metadata_.get()); IdToComponentPtrMap components; components[kUpdateItemId] = MakeComponent(); - auto& component = components[kUpdateItemId]; - update_checker_->CheckForUpdates( update_context_->session_id, {kUpdateItemId}, components, "", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, @@ -395,8 +414,9 @@ TEST_F(UpdateCheckerTest, UpdateCheckError) { EXPECT_EQ(1, post_interceptor_->GetCount()) << post_interceptor_->GetRequestsAsString(); + EXPECT_EQ(ErrorCategory::kUpdateCheck, error_category_); EXPECT_EQ(403, error_); - EXPECT_FALSE(component->next_version_.IsValid()); + EXPECT_FALSE(results_); } TEST_F(UpdateCheckerTest, UpdateCheckDownloadPreference) { @@ -416,7 +436,6 @@ TEST_F(UpdateCheckerTest, UpdateCheckDownloadPreference) { "extra=\"params\"", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, base::Unretained(this))); - RunThreads(); // The request must contain dlpref="cacheable". @@ -438,8 +457,6 @@ TEST_F(UpdateCheckerTest, UpdateCheckCupError) { IdToComponentPtrMap components; components[kUpdateItemId] = MakeComponent(); - const auto& component = components[kUpdateItemId]; - update_checker_->CheckForUpdates( update_context_->session_id, {kUpdateItemId}, components, "", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, @@ -463,8 +480,9 @@ TEST_F(UpdateCheckerTest, UpdateCheckCupError) { testing::HasSubstr(R"(<packages><package fp="fp1"/></packages></app>)")); // Expect an error since the response is not trusted. + EXPECT_EQ(ErrorCategory::kUpdateCheck, error_category_); EXPECT_EQ(-10000, error_); - EXPECT_FALSE(component->next_version_.IsValid()); + EXPECT_FALSE(results_); } // Tests that the UpdateCheckers will not make an update check for a @@ -486,7 +504,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckRequiresEncryptionError) { base::Unretained(this))); RunThreads(); - EXPECT_EQ(-1, error_); + EXPECT_EQ(ErrorCategory::kUpdateCheck, error_category_); + EXPECT_EQ(-10002, error_); EXPECT_FALSE(component->next_version_.IsValid()); } @@ -867,8 +886,6 @@ TEST_F(UpdateCheckerTest, NoUpdateActionRun) { IdToComponentPtrMap components; components[kUpdateItemId] = MakeComponent(); - auto& component = components[kUpdateItemId]; - update_checker_->CheckForUpdates( update_context_->session_id, {kUpdateItemId}, components, "", true, base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, @@ -880,8 +897,14 @@ TEST_F(UpdateCheckerTest, NoUpdateActionRun) { ASSERT_EQ(1, post_interceptor_->GetCount()) << post_interceptor_->GetRequestsAsString(); + // Sanity check the arguments of the callback after parsing. + EXPECT_EQ(ErrorCategory::kNone, error_category_); EXPECT_EQ(0, error_); - EXPECT_STREQ("this", component->action_run_.c_str()); + EXPECT_TRUE(results_); + EXPECT_EQ(1u, results_->list.size()); + const auto& result = results_->list.front(); + EXPECT_STREQ("jebgalgnebhfojomionfpkfelancnnkf", result.extension_id.c_str()); + EXPECT_STREQ("this", result.action_run.c_str()); } TEST_F(UpdateCheckerTest, UpdatePauseResume) { @@ -889,10 +912,10 @@ TEST_F(UpdateCheckerTest, UpdatePauseResume) { std::make_unique<PartialMatch>("updatecheck"), test_file("updatecheck_reply_1.xml"))); post_interceptor_->url_job_request_ready_callback(base::BindOnce( - [](scoped_refptr<URLRequestPostInterceptor> post_interceptor) { + [](URLLoaderPostInterceptor* post_interceptor) { post_interceptor->Resume(); }, - post_interceptor_)); + post_interceptor_.get())); post_interceptor_->Pause(); update_checker_ = UpdateChecker::Create(config_, metadata_.get()); @@ -941,4 +964,64 @@ TEST_F(UpdateCheckerTest, UpdateResetUpdateChecker) { runloop.Run(); } +// The update response contains a protocol version which does not match the +// expected protocol version. +TEST_F(UpdateCheckerTest, ParseErrorProtocolVersionMismatch) { + EXPECT_TRUE(post_interceptor_->ExpectRequest( + std::make_unique<PartialMatch>("updatecheck"), + test_file("updatecheck_reply_parse_error.xml"))); + + update_checker_ = UpdateChecker::Create(config_, metadata_.get()); + + IdToComponentPtrMap components; + components[kUpdateItemId] = MakeComponent(); + + update_checker_->CheckForUpdates( + update_context_->session_id, {kUpdateItemId}, components, "", true, + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); + RunThreads(); + + EXPECT_EQ(1, post_interceptor_->GetHitCount()) + << post_interceptor_->GetRequestsAsString(); + ASSERT_EQ(1, post_interceptor_->GetCount()) + << post_interceptor_->GetRequestsAsString(); + + EXPECT_EQ(ErrorCategory::kUpdateCheck, error_category_); + EXPECT_EQ(-10003, error_); + EXPECT_FALSE(results_); +} + +// The update response contains a status |error-unknownApplication| for the +// app. The response is succesfully parsed and a result is extracted to +// indicate this status. +TEST_F(UpdateCheckerTest, ParseErrorAppStatusErrorUnknownApplication) { + EXPECT_TRUE(post_interceptor_->ExpectRequest( + std::make_unique<PartialMatch>("updatecheck"), + test_file("updatecheck_reply_unknownapp.xml"))); + + update_checker_ = UpdateChecker::Create(config_, metadata_.get()); + + IdToComponentPtrMap components; + components[kUpdateItemId] = MakeComponent(); + + update_checker_->CheckForUpdates( + update_context_->session_id, {kUpdateItemId}, components, "", true, + base::BindOnce(&UpdateCheckerTest::UpdateCheckComplete, + base::Unretained(this))); + RunThreads(); + + EXPECT_EQ(1, post_interceptor_->GetHitCount()) + << post_interceptor_->GetRequestsAsString(); + ASSERT_EQ(1, post_interceptor_->GetCount()) + << post_interceptor_->GetRequestsAsString(); + + EXPECT_EQ(ErrorCategory::kNone, error_category_); + EXPECT_EQ(0, error_); + EXPECT_TRUE(results_); + EXPECT_EQ(1u, results_->list.size()); + const auto& result = results_->list.front(); + EXPECT_STREQ("error-unknownApplication", result.status.c_str()); +} + } // namespace update_client diff --git a/chromium/components/update_client/update_client.cc b/chromium/components/update_client/update_client.cc index 4d3c9b4c470..561bbcf9902 100644 --- a/chromium/components/update_client/update_client.cc +++ b/chromium/components/update_client/update_client.cc @@ -16,6 +16,7 @@ #include "base/logging.h" #include "base/macros.h" #include "base/observer_list.h" +#include "base/stl_util.h" #include "base/threading/thread_checker.h" #include "base/threading/thread_task_runner_handle.h" #include "components/prefs/pref_registry_simple.h" @@ -183,14 +184,14 @@ bool UpdateClientImpl::IsUpdating(const std::string& id) const { for (const auto task : tasks_) { const auto ids = task->GetIds(); - if (std::find(ids.begin(), ids.end(), id) != ids.end()) { + if (base::ContainsValue(ids, id)) { return true; } } for (const auto task : task_queue_) { const auto ids = task->GetIds(); - if (std::find(ids.begin(), ids.end(), id) != ids.end()) { + if (base::ContainsValue(ids, id)) { return true; } } diff --git a/chromium/components/update_client/update_client_errors.h b/chromium/components/update_client/update_client_errors.h index 129d7aa9763..3ee05098ecb 100644 --- a/chromium/components/update_client/update_client_errors.h +++ b/chromium/components/update_client/update_client_errors.h @@ -10,7 +10,6 @@ namespace update_client { // Errors generated as a result of calling UpdateClient member functions. // These errors are not sent in pings. enum class Error { - INVALID_ARGUMENT = -1, NONE = 0, UPDATE_IN_PROGRESS = 1, UPDATE_CANCELED = 2, @@ -18,6 +17,8 @@ enum class Error { SERVICE_ERROR = 4, UPDATE_CHECK_ERROR = 5, CRX_NOT_FOUND = 6, + INVALID_ARGUMENT = 7, + MAX_VALUE, }; // These errors are sent in pings. Add new values only to the bottom of @@ -85,7 +86,7 @@ enum class InstallError { CUSTOM_ERROR_BASE = 100, // Specific installer errors go above this value. }; -// These errors are returned with the |kInstall| error category and +// These errors are returned with the |kService| error category and // indicate critical or configuration errors in the update service. enum class ServiceError { NONE = 0, @@ -93,6 +94,25 @@ enum class ServiceError { UPDATE_DISABLED = 2, }; +// These errors are related to serialization, deserialization, and parsing of +// protocol requests. +// The begin value for this enum is chosen not to conflict with network errors +// defined by net/base/net_error_list.h. The callers don't have to handle this +// error in any meaningful way, but this value may be reported in UMA stats, +// therefore avoiding collisions with known network errors is desirable. +enum class ProtocolError : int { + NONE = 0, + RESPONSE_NOT_TRUSTED = -10000, + MISSING_PUBLIC_KEY = -10001, + MISSING_URLS = -10002, + PARSE_FAILED = -10003, + UPDATE_RESPONSE_NOT_FOUND = -10004, + URL_FETCHER_FAILED = -10005, + UNKNOWN_APPLICATION = -10006, + RESTRICTED_APPLICATION = -10007, + INVALID_APPID = -10008, +}; + } // namespace update_client #endif // COMPONENTS_UPDATE_CLIENT_UPDATE_CLIENT_ERRORS_H_ diff --git a/chromium/components/update_client/update_client_unittest.cc b/chromium/components/update_client/update_client_unittest.cc index 581cbf9e9b9..4e9900571dd 100644 --- a/chromium/components/update_client/update_client_unittest.cc +++ b/chromium/components/update_client/update_client_unittest.cc @@ -11,6 +11,7 @@ #include "base/location.h" #include "base/macros.h" #include "base/memory/ref_counted.h" +#include "base/optional.h" #include "base/path_service.h" #include "base/run_loop.h" #include "base/stl_util.h" @@ -263,10 +264,13 @@ TEST_F(UpdateClientTest, OneCrxNoUpdate) { ProtocolParser::Result result; result.extension_id = id; result.status = "noupdate"; - component->SetParseResult(result); + + ProtocolParser::Results results; + results.list.push_back(result); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), results, + ErrorCategory::kNone, 0, 0)); } }; @@ -384,12 +388,16 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoUpdate) { </manifest> </updatecheck> </app> + <app appid='abagagagagagagagagagagagagagagag'> + <updatecheck status='noupdate'/> + </app> </response> */ EXPECT_FALSE(session_id.empty()); EXPECT_TRUE(enabled_component_updates); EXPECT_EQ(2u, ids_to_check.size()); + ProtocolParser::Results results; { const std::string id = "jebgalgnebhfojomionfpkfelancnnkf"; EXPECT_EQ(id, ids_to_check[0]); @@ -407,11 +415,9 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoUpdate) { result.manifest.version = "1.0"; result.manifest.browser_min_version = "11.0.1.0"; result.manifest.packages.push_back(package); + results.list.push_back(result); - auto& component = components.at(id); - component->SetParseResult(result); - - EXPECT_FALSE(component->is_foreground()); + EXPECT_FALSE(components.at(id)->is_foreground()); } { @@ -422,15 +428,14 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoUpdate) { ProtocolParser::Result result; result.extension_id = id; result.status = "noupdate"; + results.list.push_back(result); - auto& component = components.at(id); - component->SetParseResult(result); - - EXPECT_FALSE(component->is_foreground()); + EXPECT_FALSE(components.at(id)->is_foreground()); } base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), results, + ErrorCategory::kNone, 0, 0)); } }; @@ -533,6 +538,223 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoUpdate) { update_client->RemoveObserver(&observer); } +// Tests the scenario where two CRXs are checked for updates. One CRX has +// an update but the server ignores the second CRX and returns no response for +// it. The second component gets an |UPDATE_RESPONSE_NOT_FOUND| error and +// transitions to the error state. +TEST_F(UpdateClientTest, TwoCrxUpdateFirstServerIgnoresSecond) { + class DataCallbackMock { + public: + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { + std::unique_ptr<CrxComponent> crx1 = std::make_unique<CrxComponent>(); + crx1->name = "test_jebg"; + crx1->pk_hash.assign(jebg_hash, jebg_hash + base::size(jebg_hash)); + crx1->version = base::Version("0.9"); + crx1->installer = base::MakeRefCounted<TestInstaller>(); + + std::unique_ptr<CrxComponent> crx2 = std::make_unique<CrxComponent>(); + crx2->name = "test_abag"; + crx2->pk_hash.assign(abag_hash, abag_hash + base::size(abag_hash)); + crx2->version = base::Version("2.2"); + crx2->installer = base::MakeRefCounted<TestInstaller>(); + + std::vector<std::unique_ptr<CrxComponent>> component; + component.push_back(std::move(crx1)); + component.push_back(std::move(crx2)); + return component; + } + }; + + class CompletionCallbackMock { + public: + static void Callback(base::OnceClosure quit_closure, Error error) { + EXPECT_EQ(Error::NONE, error); + std::move(quit_closure).Run(); + } + }; + + class MockUpdateChecker : public UpdateChecker { + public: + static std::unique_ptr<UpdateChecker> Create( + scoped_refptr<Configurator> config, + PersistedData* metadata) { + return std::make_unique<MockUpdateChecker>(); + } + + void CheckForUpdates(const std::string& session_id, + const std::vector<std::string>& ids_to_check, + const IdToComponentPtrMap& components, + const std::string& additional_attributes, + bool enabled_component_updates, + UpdateCheckCallback update_check_callback) override { + /* + Mock the following response: + + <?xml version='1.0' encoding='UTF-8'?> + <response protocol='3.1'> + <app appid='jebgalgnebhfojomionfpkfelancnnkf'> + <updatecheck status='ok'> + <urls> + <url codebase='http://localhost/download/'/> + </urls> + <manifest version='1.0' prodversionmin='11.0.1.0'> + <packages> + <package name='jebgalgnebhfojomionfpkfelancnnkf.crx' + hash_sha256='6fc4b93fd11134de1300c2c0bb88c12b644a4ec0fd + 7c9b12cb7cc067667bde87'/> + </packages> + </manifest> + </updatecheck> + </app> + </response> + */ + EXPECT_FALSE(session_id.empty()); + EXPECT_TRUE(enabled_component_updates); + EXPECT_EQ(2u, ids_to_check.size()); + + ProtocolParser::Results results; + { + const std::string id = "jebgalgnebhfojomionfpkfelancnnkf"; + EXPECT_EQ(id, ids_to_check[0]); + EXPECT_EQ(1u, components.count(id)); + + ProtocolParser::Result::Manifest::Package package; + package.name = "jebgalgnebhfojomionfpkfelancnnkf.crx"; + package.hash_sha256 = + "6fc4b93fd11134de1300c2c0bb88c12b644a4ec0fd7c9b12cb7cc067667bde87"; + + ProtocolParser::Result result; + result.extension_id = "jebgalgnebhfojomionfpkfelancnnkf"; + result.status = "ok"; + result.crx_urls.push_back(GURL("http://localhost/download/")); + result.manifest.version = "1.0"; + result.manifest.browser_min_version = "11.0.1.0"; + result.manifest.packages.push_back(package); + results.list.push_back(result); + + EXPECT_FALSE(components.at(id)->is_foreground()); + } + + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::BindOnce(std::move(update_check_callback), results, + ErrorCategory::kNone, 0, 0)); + } + }; + + class MockCrxDownloader : public CrxDownloader { + public: + static std::unique_ptr<CrxDownloader> Create( + bool is_background_download, + scoped_refptr<net::URLRequestContextGetter> context_getter) { + return std::make_unique<MockCrxDownloader>(); + } + + MockCrxDownloader() : CrxDownloader(nullptr) {} + + private: + void DoStartDownload(const GURL& url) override { + DownloadMetrics download_metrics; + download_metrics.url = url; + download_metrics.downloader = DownloadMetrics::kNone; + download_metrics.error = 0; + download_metrics.downloaded_bytes = 1843; + download_metrics.total_bytes = 1843; + download_metrics.download_time_ms = 1000; + + FilePath path; + EXPECT_TRUE(MakeTestFile( + TestFilePath("jebgalgnebhfojomionfpkfelancnnkf.crx"), &path)); + + Result result; + result.error = 0; + result.response = path; + result.downloaded_bytes = 1843; + result.total_bytes = 1843; + + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::BindOnce(&MockCrxDownloader::OnDownloadProgress, + base::Unretained(this), result)); + + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::BindOnce(&MockCrxDownloader::OnDownloadComplete, + base::Unretained(this), true, result, + download_metrics)); + } + }; + + class MockPingManager : public MockPingManagerImpl { + public: + explicit MockPingManager(scoped_refptr<Configurator> config) + : MockPingManagerImpl(config) {} + + protected: + ~MockPingManager() override { + const auto ping_data = MockPingManagerImpl::ping_data(); + EXPECT_EQ(1u, ping_data.size()); + EXPECT_EQ("jebgalgnebhfojomionfpkfelancnnkf", ping_data[0].id); + EXPECT_EQ(base::Version("0.9"), ping_data[0].previous_version); + EXPECT_EQ(base::Version("1.0"), ping_data[0].next_version); + EXPECT_EQ(0, static_cast<int>(ping_data[0].error_category)); + EXPECT_EQ(0, ping_data[0].error_code); + } + }; + + scoped_refptr<UpdateClient> update_client = + base::MakeRefCounted<UpdateClientImpl>( + config(), base::MakeRefCounted<MockPingManager>(config()), + &MockUpdateChecker::Create, &MockCrxDownloader::Create); + + MockObserver observer; + { + InSequence seq; + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_CHECKING_FOR_UPDATES, + "jebgalgnebhfojomionfpkfelancnnkf")) + .Times(1); + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_FOUND, + "jebgalgnebhfojomionfpkfelancnnkf")) + .Times(1); + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_DOWNLOADING, + "jebgalgnebhfojomionfpkfelancnnkf")) + .Times(AtLeast(1)); + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_READY, + "jebgalgnebhfojomionfpkfelancnnkf")) + .Times(1); + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATED, + "jebgalgnebhfojomionfpkfelancnnkf")) + .Times(1); + } + { + InSequence seq; + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_CHECKING_FOR_UPDATES, + "abagagagagagagagagagagagagagagag")) + .Times(1); + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_ERROR, + "abagagagagagagagagagagagagagagag")) + .Times(1) + .WillOnce(Invoke([&update_client](Events event, const std::string& id) { + CrxUpdateItem item; + update_client->GetCrxUpdateState(id, &item); + EXPECT_EQ(ComponentState::kUpdateError, item.state); + EXPECT_EQ(5, static_cast<int>(item.error_category)); + EXPECT_EQ(-10004, item.error_code); + EXPECT_EQ(0, item.extra_code1); + })); + } + + update_client->AddObserver(&observer); + + const std::vector<std::string> ids = {"jebgalgnebhfojomionfpkfelancnnkf", + "abagagagagagagagagagagagagagagag"}; + update_client->Update( + ids, base::BindOnce(&DataCallbackMock::Callback), false, + base::BindOnce(&CompletionCallbackMock::Callback, quit_closure())); + + RunThreads(); + + update_client->RemoveObserver(&observer); +} + // Tests the update check for two CRXs scenario when the second CRX does not // provide a CrxComponent instance. In this case, the update is handled as // if only one component were provided as an argument to the |Update| call @@ -603,6 +825,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoCrxComponentData) { EXPECT_TRUE(enabled_component_updates); EXPECT_EQ(1u, ids_to_check.size()); + ProtocolParser::Results results; { const std::string id = "jebgalgnebhfojomionfpkfelancnnkf"; EXPECT_EQ(id, ids_to_check[0]); @@ -620,15 +843,14 @@ TEST_F(UpdateClientTest, TwoCrxUpdateNoCrxComponentData) { result.manifest.version = "1.0"; result.manifest.browser_min_version = "11.0.1.0"; result.manifest.packages.push_back(package); + results.list.push_back(result); - auto& component = components.at(id); - component->SetParseResult(result); - - EXPECT_FALSE(component->is_foreground()); + EXPECT_FALSE(components.at(id)->is_foreground()); } base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), results, + ErrorCategory::kNone, 0, 0)); } }; @@ -918,6 +1140,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateDownloadTimeout) { EXPECT_TRUE(enabled_component_updates); EXPECT_EQ(2u, ids_to_check.size()); + ProtocolParser::Results results; { const std::string id = "jebgalgnebhfojomionfpkfelancnnkf"; EXPECT_EQ(id, ids_to_check[0]); @@ -935,9 +1158,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateDownloadTimeout) { result.manifest.version = "1.0"; result.manifest.browser_min_version = "11.0.1.0"; result.manifest.packages.push_back(package); - - auto& component = components.at(id); - component->SetParseResult(result); + results.list.push_back(result); } { @@ -957,13 +1178,12 @@ TEST_F(UpdateClientTest, TwoCrxUpdateDownloadTimeout) { result.manifest.version = "1.0"; result.manifest.browser_min_version = "11.0.1.0"; result.manifest.packages.push_back(package); - - auto& component = components.at(id); - component->SetParseResult(result); + results.list.push_back(result); } base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), results, + ErrorCategory::kNone, 0, 0)); } }; @@ -1203,9 +1423,7 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdate) { result.manifest.version = "1.0"; result.manifest.browser_min_version = "11.0.1.0"; result.manifest.packages.push_back(package); - - auto& component = components.at(id); - component->SetParseResult(result); + results.list.push_back(result); } else if (num_call == 2) { /* Mock the following response: @@ -1253,15 +1471,14 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdate) { result.manifest.version = "2.0"; result.manifest.browser_min_version = "11.0.1.0"; result.manifest.packages.push_back(package); - - auto& component = components.at(id); - component->SetParseResult(result); + results.list.push_back(result); } else { NOTREACHED(); } base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), results, + ErrorCategory::kNone, 0, 0)); } }; @@ -1535,11 +1752,11 @@ TEST_F(UpdateClientTest, OneCrxInstallError) { result.manifest.browser_min_version = "11.0.1.0"; result.manifest.packages.push_back(package); - auto& component = components.at(id); - component->SetParseResult(result); - + ProtocolParser::Results results; + results.list.push_back(result); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), results, + ErrorCategory::kNone, 0, 0)); } }; @@ -1735,9 +1952,7 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdateFailsFullUpdateSucceeds) { result.manifest.version = "1.0"; result.manifest.browser_min_version = "11.0.1.0"; result.manifest.packages.push_back(package); - - auto& component = components.at(id); - component->SetParseResult(result); + results.list.push_back(result); } else if (num_call == 2) { /* Mock the following response: @@ -1785,15 +2000,14 @@ TEST_F(UpdateClientTest, OneCrxDiffUpdateFailsFullUpdateSucceeds) { result.manifest.version = "2.0"; result.manifest.browser_min_version = "11.0.1.0"; result.manifest.packages.push_back(package); - - auto& component = components.at(id); - component->SetParseResult(result); + results.list.push_back(result); } else { NOTREACHED(); } base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), results, + ErrorCategory::kNone, 0, 0)); } }; @@ -2013,10 +2227,12 @@ TEST_F(UpdateClientTest, OneCrxNoUpdateQueuedCall) { ProtocolParser::Result result; result.extension_id = id; result.status = "noupdate"; - component->SetParseResult(result); + ProtocolParser::Results results; + results.list.push_back(result); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), results, + ErrorCategory::kNone, 0, 0)); } }; @@ -2156,14 +2372,15 @@ TEST_F(UpdateClientTest, OneCrxInstall) { result.manifest.browser_min_version = "11.0.1.0"; result.manifest.packages.push_back(package); - auto& component = components.at(id); - component->SetParseResult(result); + ProtocolParser::Results results; + results.list.push_back(result); // Verify that calling Install sets ondemand. - EXPECT_TRUE(component->is_foreground()); + EXPECT_TRUE(components.at(id)->is_foreground()); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), results, + ErrorCategory::kNone, 0, 0)); } }; @@ -2408,14 +2625,15 @@ TEST_F(UpdateClientTest, ConcurrentInstallSameCRX) { result.extension_id = id; result.status = "noupdate"; - auto& component = components.at(id); - component->SetParseResult(result); + ProtocolParser::Results results; + results.list.push_back(result); // Verify that calling Install sets |is_foreground| for the component. - EXPECT_TRUE(component->is_foreground()); + EXPECT_TRUE(components.at(id)->is_foreground()); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), results, + ErrorCategory::kNone, 0, 0)); } }; @@ -2689,16 +2907,16 @@ TEST_F(UpdateClientTest, RetryAfter) { EXPECT_EQ(id, ids_to_check.front()); EXPECT_EQ(1u, components.count(id)); - auto& component = components.at(id); - ProtocolParser::Result result; result.extension_id = id; result.status = "noupdate"; - component->SetParseResult(result); + + ProtocolParser::Results results; + results.list.push_back(result); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, - base::BindOnce(std::move(update_check_callback), 0, retry_after_sec)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), results, + ErrorCategory::kNone, 0, retry_after_sec)); } }; @@ -2805,7 +3023,7 @@ TEST_F(UpdateClientTest, RetryAfter) { // the group policy to enable updates, and has its updates disabled. The second // component has an update. The server does not honor the "updatedisabled" // attribute and returns updates for both components. However, the update for -// the first component is not apply and the client responds with a +// the first component is not applied and the client responds with a // (SERVICE_ERROR, UPDATE_DISABLED) TEST_F(UpdateClientTest, TwoCrxUpdateOneUpdateDisabled) { class DataCallbackMock { @@ -2898,6 +3116,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateOneUpdateDisabled) { EXPECT_FALSE(enabled_component_updates); EXPECT_EQ(2u, ids_to_check.size()); + ProtocolParser::Results results; { const std::string id = "jebgalgnebhfojomionfpkfelancnnkf"; EXPECT_EQ(id, ids_to_check[0]); @@ -2915,9 +3134,7 @@ TEST_F(UpdateClientTest, TwoCrxUpdateOneUpdateDisabled) { result.manifest.version = "1.0"; result.manifest.browser_min_version = "11.0.1.0"; result.manifest.packages.push_back(package); - - auto& component = components.at(id); - component->SetParseResult(result); + results.list.push_back(result); } { @@ -2937,13 +3154,12 @@ TEST_F(UpdateClientTest, TwoCrxUpdateOneUpdateDisabled) { result.manifest.version = "1.0"; result.manifest.browser_min_version = "11.0.1.0"; result.manifest.packages.push_back(package); - - auto& component = components.at(id); - component->SetParseResult(result); + results.list.push_back(result); } base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), results, + ErrorCategory::kNone, 0, 0)); } }; @@ -3111,11 +3327,10 @@ TEST_F(UpdateClientTest, OneCrxUpdateCheckFails) { const std::string id = "jebgalgnebhfojomionfpkfelancnnkf"; EXPECT_EQ(id, ids_to_check.front()); EXPECT_EQ(1u, components.count(id)); - constexpr int update_check_error = -1; - components.at(id)->set_update_check_error(update_check_error); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(std::move(update_check_callback), - update_check_error, 0)); + FROM_HERE, + base::BindOnce(std::move(update_check_callback), base::nullopt, + ErrorCategory::kUpdateCheck, -1, 0)); } }; @@ -3176,6 +3391,208 @@ TEST_F(UpdateClientTest, OneCrxUpdateCheckFails) { update_client->RemoveObserver(&observer); } +// Tests the scenario where the server responds with different values for +// application status. +TEST_F(UpdateClientTest, OneCrxErrorUnknownApp) { + class DataCallbackMock { + public: + static std::vector<std::unique_ptr<CrxComponent>> Callback( + const std::vector<std::string>& ids) { + std::vector<std::unique_ptr<CrxComponent>> component; + + std::unique_ptr<CrxComponent> crx = std::make_unique<CrxComponent>(); + crx->name = "test_jebg"; + crx->pk_hash.assign(jebg_hash, jebg_hash + base::size(jebg_hash)); + crx->version = base::Version("0.9"); + crx->installer = base::MakeRefCounted<TestInstaller>(); + component.push_back(std::move(crx)); + + crx = std::make_unique<CrxComponent>(); + crx->name = "test_abag"; + crx->pk_hash.assign(abag_hash, abag_hash + base::size(abag_hash)); + crx->version = base::Version("0.1"); + crx->installer = base::MakeRefCounted<TestInstaller>(); + component.push_back(std::move(crx)); + + crx = std::make_unique<CrxComponent>(); + crx->name = "test_ihfo"; + crx->pk_hash.assign(ihfo_hash, ihfo_hash + base::size(ihfo_hash)); + crx->version = base::Version("0.2"); + crx->installer = base::MakeRefCounted<TestInstaller>(); + component.push_back(std::move(crx)); + + crx = std::make_unique<CrxComponent>(); + crx->name = "test_gjpm"; + crx->pk_hash.assign(gjpm_hash, gjpm_hash + base::size(gjpm_hash)); + crx->version = base::Version("0.3"); + crx->installer = base::MakeRefCounted<TestInstaller>(); + component.push_back(std::move(crx)); + + return component; + } + }; + + class CompletionCallbackMock { + public: + static void Callback(base::OnceClosure quit_closure, Error error) { + EXPECT_EQ(Error::NONE, error); + std::move(quit_closure).Run(); + } + }; + + class MockUpdateChecker : public UpdateChecker { + public: + static std::unique_ptr<UpdateChecker> Create( + scoped_refptr<Configurator> config, + PersistedData* metadata) { + return std::make_unique<MockUpdateChecker>(); + } + + void CheckForUpdates(const std::string& session_id, + const std::vector<std::string>& ids_to_check, + const IdToComponentPtrMap& components, + const std::string& additional_attributes, + bool enabled_component_updates, + UpdateCheckCallback update_check_callback) override { + EXPECT_FALSE(session_id.empty()); + EXPECT_TRUE(enabled_component_updates); + EXPECT_EQ(4u, ids_to_check.size()); + + const std::string update_response = + R"(<?xml version="1.0" encoding="UTF-8"?>)" + R"(<response protocol="3.1">)" + R"(<app appid="jebgalgnebhfojomionfpkfelancnnkf")" + R"( status="error-unknownApplication"/>)" + R"(<app appid="abagagagagagagagagagagagagagagag")" + R"( status="restricted"/>)" + R"(<app appid="ihfokbkgjpifnbbojhneepfflplebdkc")" + R"( status="error-invalidAppId"/>)" + R"(<app appid="gjpmebpgbhcamgdgjcmnjfhggjpgcimm")" + R"( status="error-foobarApp"/>)" + R"(</response>)"; + + ProtocolParser parser; + EXPECT_TRUE(parser.Parse(update_response)); + + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, + base::BindOnce(std::move(update_check_callback), parser.results(), + ErrorCategory::kNone, 0, 0)); + } + }; + + class MockCrxDownloader : public CrxDownloader { + public: + static std::unique_ptr<CrxDownloader> Create( + bool is_background_download, + scoped_refptr<net::URLRequestContextGetter> context_getter) { + return std::make_unique<MockCrxDownloader>(); + } + + MockCrxDownloader() : CrxDownloader(nullptr) {} + + private: + void DoStartDownload(const GURL& url) override { EXPECT_TRUE(false); } + }; + + class MockPingManager : public MockPingManagerImpl { + public: + explicit MockPingManager(scoped_refptr<Configurator> config) + : MockPingManagerImpl(config) {} + + protected: + ~MockPingManager() override { EXPECT_TRUE(ping_data().empty()); } + }; + + scoped_refptr<UpdateClient> update_client = + base::MakeRefCounted<UpdateClientImpl>( + config(), base::MakeRefCounted<MockPingManager>(config()), + &MockUpdateChecker::Create, &MockCrxDownloader::Create); + + MockObserver observer; + { + InSequence seq; + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_CHECKING_FOR_UPDATES, + "jebgalgnebhfojomionfpkfelancnnkf")) + .Times(1); + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_ERROR, + "jebgalgnebhfojomionfpkfelancnnkf")) + .Times(1) + .WillOnce(Invoke([&update_client](Events event, const std::string& id) { + CrxUpdateItem item; + update_client->GetCrxUpdateState(id, &item); + EXPECT_EQ(ComponentState::kUpdateError, item.state); + EXPECT_EQ(5, static_cast<int>(item.error_category)); + EXPECT_EQ(-10006, item.error_code); // UNKNOWN_APPPLICATION. + EXPECT_EQ(0, item.extra_code1); + })); + } + { + InSequence seq; + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_CHECKING_FOR_UPDATES, + "abagagagagagagagagagagagagagagag")) + .Times(1); + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_ERROR, + "abagagagagagagagagagagagagagagag")) + .Times(1) + .WillOnce(Invoke([&update_client](Events event, const std::string& id) { + CrxUpdateItem item; + update_client->GetCrxUpdateState(id, &item); + EXPECT_EQ(ComponentState::kUpdateError, item.state); + EXPECT_EQ(5, static_cast<int>(item.error_category)); + EXPECT_EQ(-10007, item.error_code); // RESTRICTED_APPLICATION. + EXPECT_EQ(0, item.extra_code1); + })); + } + { + InSequence seq; + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_CHECKING_FOR_UPDATES, + "ihfokbkgjpifnbbojhneepfflplebdkc")) + .Times(1); + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_ERROR, + "ihfokbkgjpifnbbojhneepfflplebdkc")) + .Times(1) + .WillOnce(Invoke([&update_client](Events event, const std::string& id) { + CrxUpdateItem item; + update_client->GetCrxUpdateState(id, &item); + EXPECT_EQ(ComponentState::kUpdateError, item.state); + EXPECT_EQ(5, static_cast<int>(item.error_category)); + EXPECT_EQ(-10008, item.error_code); // INVALID_APPID. + EXPECT_EQ(0, item.extra_code1); + })); + } + { + InSequence seq; + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_CHECKING_FOR_UPDATES, + "gjpmebpgbhcamgdgjcmnjfhggjpgcimm")) + .Times(1); + EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_ERROR, + "gjpmebpgbhcamgdgjcmnjfhggjpgcimm")) + .Times(1) + .WillOnce(Invoke([&update_client](Events event, const std::string& id) { + CrxUpdateItem item; + update_client->GetCrxUpdateState(id, &item); + EXPECT_EQ(ComponentState::kUpdateError, item.state); + EXPECT_EQ(5, static_cast<int>(item.error_category)); + EXPECT_EQ(-10004, item.error_code); // UPDATE_RESPONSE_NOT_FOUND. + EXPECT_EQ(0, item.extra_code1); + })); + } + + update_client->AddObserver(&observer); + + const std::vector<std::string> ids = { + "jebgalgnebhfojomionfpkfelancnnkf", "abagagagagagagagagagagagagagagag", + "ihfokbkgjpifnbbojhneepfflplebdkc", "gjpmebpgbhcamgdgjcmnjfhggjpgcimm"}; + update_client->Update( + ids, base::BindOnce(&DataCallbackMock::Callback), true, + base::BindOnce(&CompletionCallbackMock::Callback, quit_closure())); + + RunThreads(); + + update_client->RemoveObserver(&observer); +} + #if defined(OS_WIN) // ActionRun is only implemented on Windows. // Tests that a run action in invoked in the CRX install scenario. @@ -3239,11 +3656,12 @@ TEST_F(UpdateClientTest, ActionRun_Install) { result.manifest.packages.push_back(package); result.action_run = "ChromeRecovery.crx3"; - auto& component = components.at(id); - component->SetParseResult(result); + ProtocolParser::Results results; + results.list.push_back(result); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), results, + ErrorCategory::kNone, 0, 0)); } }; @@ -3380,17 +3798,17 @@ TEST_F(UpdateClientTest, ActionRun_NoUpdate) { EXPECT_EQ(id, ids_to_check[0]); EXPECT_EQ(1u, components.count(id)); - auto& component = components.at(id); - ProtocolParser::Result result; result.extension_id = id; result.status = "noupdate"; result.action_run = "ChromeRecovery.crx3"; - component->SetParseResult(result); + ProtocolParser::Results results; + results.list.push_back(result); base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::BindOnce(std::move(update_check_callback), 0, 0)); + FROM_HERE, base::BindOnce(std::move(update_check_callback), results, + ErrorCategory::kNone, 0, 0)); } }; diff --git a/chromium/components/update_client/update_engine.cc b/chromium/components/update_client/update_engine.cc index 351cb3893c2..a778b7d9628 100644 --- a/chromium/components/update_client/update_engine.cc +++ b/chromium/components/update_client/update_engine.cc @@ -18,6 +18,7 @@ #include "components/update_client/configurator.h" #include "components/update_client/crx_update_item.h" #include "components/update_client/persisted_data.h" +#include "components/update_client/protocol_parser.h" #include "components/update_client/update_checker.h" #include "components/update_client/update_client_errors.h" #include "components/update_client/utils.h" @@ -81,6 +82,11 @@ void UpdateEngine::Update(bool is_foreground, } if (IsThrottled(is_foreground)) { + // TODO(xiaochu): remove this log after https://crbug.com/851151 is fixed. + VLOG(1) << "Background update is throttled for following components:"; + for (const auto& id : ids) { + VLOG(1) << "id:" << id; + } base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::BindOnce(std::move(callback), Error::RETRY_LATER)); return; @@ -175,13 +181,16 @@ void UpdateEngine::DoUpdateCheck(scoped_refptr<UpdateContext> update_context) { update_context->components_to_check_for_updates, update_context->components, config_->ExtraRequestParams(), update_context->enabled_component_updates, - base::BindOnce(&UpdateEngine::UpdateCheckDone, base::Unretained(this), - update_context)); + base::BindOnce(&UpdateEngine::UpdateCheckResultsAvailable, + base::Unretained(this), update_context)); } -void UpdateEngine::UpdateCheckDone(scoped_refptr<UpdateContext> update_context, - int error, - int retry_after_sec) { +void UpdateEngine::UpdateCheckResultsAvailable( + scoped_refptr<UpdateContext> update_context, + const base::Optional<ProtocolParser::Results>& results, + ErrorCategory error_category, + int error, + int retry_after_sec) { DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(update_context); @@ -202,12 +211,53 @@ void UpdateEngine::UpdateCheckDone(scoped_refptr<UpdateContext> update_context, update_context->update_check_error = error; + if (error) { + DCHECK(!results); + for (const auto& id : update_context->components_to_check_for_updates) { + DCHECK_EQ(1u, update_context->components.count(id)); + auto& component = update_context->components.at(id); + component->SetUpdateCheckResult(base::nullopt, + ErrorCategory::kUpdateCheck, error); + } + return; + } + + DCHECK(results); + DCHECK_EQ(0, error); + + std::map<std::string, ProtocolParser::Result> id_to_result; + for (const auto& result : results->list) + id_to_result[result.extension_id] = result; + for (const auto& id : update_context->components_to_check_for_updates) { DCHECK_EQ(1u, update_context->components.count(id)); - DCHECK(update_context->components.at(id)); - - auto& component = *update_context->components.at(id); - component.UpdateCheckComplete(); + auto& component = update_context->components.at(id); + const auto& it = id_to_result.find(id); + if (it != id_to_result.end()) { + const auto result = it->second; + const auto error = [](const std::string& status) { + // First, handle app status literals which can be folded down as an + // updatecheck status + if (status == "error-unknownApplication") + return std::make_pair(ErrorCategory::kUpdateCheck, + ProtocolError::UNKNOWN_APPLICATION); + if (status == "restricted") + return std::make_pair(ErrorCategory::kUpdateCheck, + ProtocolError::RESTRICTED_APPLICATION); + if (status == "error-invalidAppId") + return std::make_pair(ErrorCategory::kUpdateCheck, + ProtocolError::INVALID_APPID); + // If the parser has return a valid result and the status is not one of + // the literals above, then this must be a success an not a parse error. + return std::make_pair(ErrorCategory::kNone, ProtocolError::NONE); + }(result.status); + component->SetUpdateCheckResult(result, error.first, + static_cast<int>(error.second)); + } else { + component->SetUpdateCheckResult( + base::nullopt, ErrorCategory::kUpdateCheck, + static_cast<int>(ProtocolError::UPDATE_RESPONSE_NOT_FOUND)); + } } } diff --git a/chromium/components/update_client/update_engine.h b/chromium/components/update_client/update_engine.h index 2cfe9da0465..96018736a9e 100644 --- a/chromium/components/update_client/update_engine.h +++ b/chromium/components/update_client/update_engine.h @@ -15,6 +15,7 @@ #include "base/containers/queue.h" #include "base/macros.h" #include "base/memory/ref_counted.h" +#include "base/optional.h" #include "base/threading/thread_checker.h" #include "base/time/time.h" #include "components/update_client/component.h" @@ -81,9 +82,12 @@ class UpdateEngine : public base::RefCounted<UpdateEngine> { void UpdateCheckComplete(scoped_refptr<UpdateContext> update_context); void DoUpdateCheck(scoped_refptr<UpdateContext> update_context); - void UpdateCheckDone(scoped_refptr<UpdateContext> update_context, - int error, - int retry_after_sec); + void UpdateCheckResultsAvailable( + scoped_refptr<UpdateContext> update_context, + const base::Optional<ProtocolParser::Results>& results, + ErrorCategory error_category, + int error, + int retry_after_sec); void HandleComponent(scoped_refptr<UpdateContext> update_context); void HandleComponentComplete(scoped_refptr<UpdateContext> update_context); diff --git a/chromium/components/update_client/url_loader_post_interceptor.cc b/chromium/components/update_client/url_loader_post_interceptor.cc new file mode 100644 index 00000000000..6f374787dce --- /dev/null +++ b/chromium/components/update_client/url_loader_post_interceptor.cc @@ -0,0 +1,263 @@ +// Copyright 2018 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 "components/update_client/url_loader_post_interceptor.h" + +#include <memory> +#include <utility> +#include <vector> + +#include "base/files/file_util.h" +#include "base/macros.h" +#include "base/run_loop.h" +#include "base/strings/stringprintf.h" +#include "base/test/bind_test_util.h" +#include "components/update_client/test_configurator.h" +#include "net/test/embedded_test_server/embedded_test_server.h" +#include "net/test/embedded_test_server/http_request.h" +#include "net/test/embedded_test_server/http_response.h" +#include "services/network/public/cpp/resource_request.h" +#include "services/network/test/test_url_loader_factory.h" +#include "services/network/test/test_utils.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace update_client { + +URLLoaderPostInterceptor::URLLoaderPostInterceptor( + network::TestURLLoaderFactory* url_loader_factory) + : url_loader_factory_(url_loader_factory) { + filtered_urls_.push_back( + GURL(base::StringPrintf("%s://%s%s", POST_INTERCEPT_SCHEME, + POST_INTERCEPT_HOSTNAME, POST_INTERCEPT_PATH))); + InitializeWithInterceptor(); +} + +URLLoaderPostInterceptor::URLLoaderPostInterceptor( + std::vector<GURL> supported_urls, + network::TestURLLoaderFactory* url_loader_factory) + : url_loader_factory_(url_loader_factory) { + DCHECK_LT(0u, supported_urls.size()); + filtered_urls_.swap(supported_urls); + InitializeWithInterceptor(); +} + +URLLoaderPostInterceptor::URLLoaderPostInterceptor( + std::vector<GURL> supported_urls, + net::test_server::EmbeddedTestServer* embedded_test_server) + : embedded_test_server_(embedded_test_server) { + DCHECK_LT(0u, supported_urls.size()); + filtered_urls_.swap(supported_urls); + InitializeWithRequestHandler(); +} + +URLLoaderPostInterceptor::~URLLoaderPostInterceptor() {} + +bool URLLoaderPostInterceptor::ExpectRequest( + std::unique_ptr<RequestMatcher> request_matcher) { + return ExpectRequest(std::move(request_matcher), net::HTTP_OK); +} + +bool URLLoaderPostInterceptor::ExpectRequest( + std::unique_ptr<RequestMatcher> request_matcher, + net::HttpStatusCode response_code) { + expectations_.push( + {std::move(request_matcher), ExpectationResponse(response_code, "")}); + return true; +} + +bool URLLoaderPostInterceptor::ExpectRequest( + std::unique_ptr<RequestMatcher> request_matcher, + const base::FilePath& filepath) { + std::string response; + if (filepath.empty() || !base::ReadFileToString(filepath, &response)) + return false; + + expectations_.push({std::move(request_matcher), + ExpectationResponse(net::HTTP_OK, response)}); + return true; +} + +// Returns how many requests have been intercepted and matched by +// an expectation. One expectation can only be matched by one request. +int URLLoaderPostInterceptor::GetHitCount() const { + return hit_count_; +} + +// Returns how many requests in total have been captured by the interceptor. +int URLLoaderPostInterceptor::GetCount() const { + return static_cast<int>(requests_.size()); +} + +// Returns all requests that have been intercepted, matched or not. +std::vector<URLLoaderPostInterceptor::InterceptedRequest> +URLLoaderPostInterceptor::GetRequests() const { + return requests_; +} + +// Return the body of the n-th request, zero-based. +std::string URLLoaderPostInterceptor::GetRequestBody(size_t n) const { + return std::get<0>(requests_[n]); +} + +// Returns the joined bodies of all requests for debugging purposes. +std::string URLLoaderPostInterceptor::GetRequestsAsString() const { + const std::vector<InterceptedRequest> requests = GetRequests(); + std::string s = "Requests are:"; + int i = 0; + for (auto it = requests.cbegin(); it != requests.cend(); ++it) + s.append(base::StringPrintf("\n [%d]: %s", ++i, std::get<0>(*it).c_str())); + return s; +} + +// Resets the state of the interceptor so that new expectations can be set. +void URLLoaderPostInterceptor::Reset() { + hit_count_ = 0; + requests_.clear(); + base::queue<Expectation>().swap(expectations_); +} + +void URLLoaderPostInterceptor::Pause() { + is_paused_ = true; +} + +void URLLoaderPostInterceptor::Resume() { + is_paused_ = false; + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::BindLambdaForTesting([&]() { + if (!pending_expectations_.size()) + return; + + PendingExpectation expectation = + std::move(pending_expectations_.front()); + pending_expectations_.pop(); + url_loader_factory_->AddResponse(expectation.first.spec(), + expectation.second.response_body, + expectation.second.response_code); + })); +} + +void URLLoaderPostInterceptor::url_job_request_ready_callback( + UrlJobRequestReadyCallback url_job_request_ready_callback) { + url_job_request_ready_callback_ = std::move(url_job_request_ready_callback); +} + +int URLLoaderPostInterceptor::GetHitCountForURL(const GURL& url) { + int hit_count = 0; + const std::vector<InterceptedRequest> requests = GetRequests(); + for (auto it = requests.cbegin(); it != requests.cend(); ++it) { + GURL url_no_query = std::get<2>(*it); + if (url_no_query.has_query()) { + GURL::Replacements replacements; + replacements.ClearQuery(); + url_no_query = url_no_query.ReplaceComponents(replacements); + } + if (url_no_query == url) + hit_count++; + } + return hit_count; +} + +void URLLoaderPostInterceptor::InitializeWithInterceptor() { + DCHECK(url_loader_factory_); + url_loader_factory_->SetInterceptor( + base::BindLambdaForTesting([&](const network::ResourceRequest& request) { + GURL url = request.url; + if (url.has_query()) { + GURL::Replacements replacements; + replacements.ClearQuery(); + url = url.ReplaceComponents(replacements); + } + auto it = std::find_if( + filtered_urls_.begin(), filtered_urls_.end(), + [url](const GURL& filtered_url) { return filtered_url == url; }); + if (it == filtered_urls_.end()) + return; + + std::string request_body = network::GetUploadData(request); + requests_.push_back({request_body, request.headers, request.url}); + if (expectations_.empty()) + return; + const auto& expectation = expectations_.front(); + if (expectation.first->Match(request_body)) { + const net::HttpStatusCode response_code( + expectation.second.response_code); + const std::string response_body(expectation.second.response_body); + + if (url_job_request_ready_callback_) { + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, std::move(url_job_request_ready_callback_)); + } + + if (!is_paused_) { + url_loader_factory_->AddResponse(request.url.spec(), response_body, + response_code); + } else { + pending_expectations_.push({request.url, expectation.second}); + } + expectations_.pop(); + ++hit_count_; + } + })); +} + +void URLLoaderPostInterceptor::InitializeWithRequestHandler() { + DCHECK(embedded_test_server_); + DCHECK(!url_loader_factory_); + embedded_test_server_->RegisterRequestHandler(base::BindRepeating( + &URLLoaderPostInterceptor::RequestHandler, base::Unretained(this))); +} + +std::unique_ptr<net::test_server::HttpResponse> +URLLoaderPostInterceptor::RequestHandler( + const net::test_server::HttpRequest& request) { + // Only intercepts POST. + if (request.method != net::test_server::METHOD_POST) + return nullptr; + + GURL url = request.GetURL(); + if (url.has_query()) { + GURL::Replacements replacements; + replacements.ClearQuery(); + url = url.ReplaceComponents(replacements); + } + auto it = std::find_if( + filtered_urls_.begin(), filtered_urls_.end(), + [url](const GURL& filtered_url) { return filtered_url == url; }); + if (it == filtered_urls_.end()) + return nullptr; + + std::string request_body = request.content; + net::HttpRequestHeaders headers; + for (auto it : request.headers) + headers.SetHeader(it.first, it.second); + requests_.push_back({request_body, headers, url}); + if (expectations_.empty()) + return nullptr; + + const auto& expectation = expectations_.front(); + if (expectation.first->Match(request_body)) { + const net::HttpStatusCode response_code(expectation.second.response_code); + const std::string response_body(expectation.second.response_body); + expectations_.pop(); + ++hit_count_; + + std::unique_ptr<net::test_server::BasicHttpResponse> http_response( + new net::test_server::BasicHttpResponse); + http_response->set_code(response_code); + http_response->set_content(response_body); + return http_response; + } + + return nullptr; +} + +bool PartialMatch::Match(const std::string& actual) const { + return actual.find(expected_) != std::string::npos; +} + +bool AnyMatch::Match(const std::string& actual) const { + return true; +} + +} // namespace update_client diff --git a/chromium/components/update_client/url_request_post_interceptor.h b/chromium/components/update_client/url_loader_post_interceptor.h index 4355f6e4e1b..c9c264bb66f 100644 --- a/chromium/components/update_client/url_request_post_interceptor.h +++ b/chromium/components/update_client/url_loader_post_interceptor.h @@ -1,49 +1,44 @@ -// Copyright 2013 The Chromium Authors. All rights reserved. +// Copyright 2018 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 COMPONENTS_UPDATE_CLIENT_URL_REQUEST_POST_INTERCEPTOR_H_ -#define COMPONENTS_UPDATE_CLIENT_URL_REQUEST_POST_INTERCEPTOR_H_ - -#include <stdint.h> -#include <map> -#include <memory> -#include <string> -#include <utility> -#include <vector> +#ifndef COMPONENTS_UPDATE_CLIENT_URL_LOADER_POST_INTERCEPTOR_H_ +#define COMPONENTS_UPDATE_CLIENT_URL_LOADER_POST_INTERCEPTOR_H_ #include "base/callback.h" #include "base/containers/queue.h" +#include "base/files/file_path.h" #include "base/macros.h" -#include "base/memory/ref_counted.h" -#include "base/synchronization/lock.h" +#include "net/http/http_request_headers.h" +#include "net/http/http_status_code.h" #include "url/gurl.h" -namespace base { -class FilePath; -class SequencedTaskRunner; +namespace network { +class TestURLLoaderFactory; } namespace net { -class HttpRequestHeaders; -} +namespace test_server { +class EmbeddedTestServer; +class HttpResponse; +struct HttpRequest; +} // namespace test_server +} // namespace net namespace update_client { -class URLRequestMockJob; - // Intercepts requests to a file path, counts them, and captures the body of // the requests. Optionally, for each request, it can return a canned response // from a given file. The class maintains a queue of expectations, and returns // one and only one response for each request that matches the expectation. // Then, the expectation is removed from the queue. -class URLRequestPostInterceptor - : public base::RefCountedThreadSafe<URLRequestPostInterceptor> { +class URLLoaderPostInterceptor { public: - using InterceptedRequest = std::pair<std::string, net::HttpRequestHeaders>; + using InterceptedRequest = + std::tuple<std::string, net::HttpRequestHeaders, GURL>; - // Called when the job associated with the url request which is intercepted - // by this object has been created. + // Called when the load associated with the url request is intercepted + // by this object:. using UrlJobRequestReadyCallback = base::OnceCallback<void()>; // Allows a generic string maching interface when setting up expectations. @@ -53,8 +48,14 @@ class URLRequestPostInterceptor virtual ~RequestMatcher() {} }; - // Returns the url that is intercepted. - GURL GetUrl() const; + explicit URLLoaderPostInterceptor( + network::TestURLLoaderFactory* url_loader_factory); + URLLoaderPostInterceptor(std::vector<GURL> supported_urls, + network::TestURLLoaderFactory* url_loader_factory); + URLLoaderPostInterceptor(std::vector<GURL> supported_urls, + net::test_server::EmbeddedTestServer*); + + ~URLLoaderPostInterceptor(); // Sets an expection for the body of the POST request and optionally, // provides a canned response identified by a |file_path| to be returned when @@ -63,8 +64,10 @@ class URLRequestPostInterceptor // response body with that response code is returned. // Returns |true| if the expectation was set. bool ExpectRequest(std::unique_ptr<RequestMatcher> request_matcher); + bool ExpectRequest(std::unique_ptr<RequestMatcher> request_matcher, - int response_code); + net::HttpStatusCode response_code); + bool ExpectRequest(std::unique_ptr<RequestMatcher> request_matcher, const base::FilePath& filepath); @@ -103,38 +106,28 @@ class URLRequestPostInterceptor void url_job_request_ready_callback( UrlJobRequestReadyCallback url_job_request_ready_callback); - private: - class Delegate; - class URLRequestMockJob; + int GetHitCountForURL(const GURL& url); - friend class URLRequestPostInterceptorFactory; - friend class base::RefCountedThreadSafe<URLRequestPostInterceptor>; + private: + void InitializeWithInterceptor(); + void InitializeWithRequestHandler(); - static const int kResponseCode200 = 200; + std::unique_ptr<net::test_server::HttpResponse> RequestHandler( + const net::test_server::HttpRequest& request); struct ExpectationResponse { - ExpectationResponse(int code, const std::string& body) + ExpectationResponse(net::HttpStatusCode code, const std::string& body) : response_code(code), response_body(body) {} - const int response_code; + const net::HttpStatusCode response_code; const std::string response_body; }; using Expectation = std::pair<std::unique_ptr<RequestMatcher>, ExpectationResponse>; - URLRequestPostInterceptor( - const GURL& url, - scoped_refptr<base::SequencedTaskRunner> io_task_runner); - ~URLRequestPostInterceptor(); - - void ClearExpectations(); - - const GURL url_; - scoped_refptr<base::SequencedTaskRunner> io_task_runner_; - - mutable base::Lock interceptor_lock_; + using PendingExpectation = std::pair<GURL, ExpectationResponse>; // Contains the count of the request matching expectations. - int hit_count_; + int hit_count_ = 0; // Contains the request body and the extra headers of the intercepted // requests. @@ -143,59 +136,21 @@ class URLRequestPostInterceptor // Contains the expectations which this interceptor tries to match. base::queue<Expectation> expectations_; - URLRequestMockJob* request_job_ = nullptr; - - bool is_paused_ = false; - - UrlJobRequestReadyCallback url_job_request_ready_callback_; - - DISALLOW_COPY_AND_ASSIGN(URLRequestPostInterceptor); -}; - -class URLRequestPostInterceptorFactory { - public: - URLRequestPostInterceptorFactory( - const std::string& scheme, - const std::string& hostname, - scoped_refptr<base::SequencedTaskRunner> io_task_runner); - ~URLRequestPostInterceptorFactory(); - - // Creates an interceptor object for the specified url path. - scoped_refptr<URLRequestPostInterceptor> CreateInterceptor( - const base::FilePath& filepath); - - private: - const std::string scheme_; - const std::string hostname_; - scoped_refptr<base::SequencedTaskRunner> io_task_runner_; - - // After creation, |delegate_| lives on the IO thread and it is owned by - // a URLRequestFilter after registration. A task to unregister it and - // implicitly destroy it is posted from ~URLRequestPostInterceptorFactory(). - URLRequestPostInterceptor::Delegate* delegate_; + base::queue<PendingExpectation> pending_expectations_; - DISALLOW_COPY_AND_ASSIGN(URLRequestPostInterceptorFactory); -}; + network::TestURLLoaderFactory* url_loader_factory_ = nullptr; + net::test_server::EmbeddedTestServer* embedded_test_server_ = nullptr; -// Intercepts HTTP POST requests sent to "localhost2". -class InterceptorFactory : public URLRequestPostInterceptorFactory { - public: - explicit InterceptorFactory( - scoped_refptr<base::SequencedTaskRunner> io_task_runner); - ~InterceptorFactory(); + bool is_paused_ = false; - // Creates an interceptor for the url path defined by POST_INTERCEPT_PATH. - scoped_refptr<URLRequestPostInterceptor> CreateInterceptor(); + std::vector<GURL> filtered_urls_; - // Creates an interceptor for the given url path. - scoped_refptr<URLRequestPostInterceptor> CreateInterceptorForPath( - const char* url_path); + UrlJobRequestReadyCallback url_job_request_ready_callback_; - private: - DISALLOW_COPY_AND_ASSIGN(InterceptorFactory); + DISALLOW_COPY_AND_ASSIGN(URLLoaderPostInterceptor); }; -class PartialMatch : public URLRequestPostInterceptor::RequestMatcher { +class PartialMatch : public URLLoaderPostInterceptor::RequestMatcher { public: explicit PartialMatch(const std::string& expected) : expected_(expected) {} bool Match(const std::string& actual) const override; @@ -206,7 +161,7 @@ class PartialMatch : public URLRequestPostInterceptor::RequestMatcher { DISALLOW_COPY_AND_ASSIGN(PartialMatch); }; -class AnyMatch : public URLRequestPostInterceptor::RequestMatcher { +class AnyMatch : public URLLoaderPostInterceptor::RequestMatcher { public: AnyMatch() = default; bool Match(const std::string& actual) const override; @@ -217,4 +172,4 @@ class AnyMatch : public URLRequestPostInterceptor::RequestMatcher { } // namespace update_client -#endif // COMPONENTS_UPDATE_CLIENT_URL_REQUEST_POST_INTERCEPTOR_H_ +#endif // COMPONENTS_UPDATE_CLIENT_URL_LOADER_POST_INTERCEPTOR_H_ diff --git a/chromium/components/update_client/url_request_post_interceptor.cc b/chromium/components/update_client/url_request_post_interceptor.cc deleted file mode 100644 index b177f533287..00000000000 --- a/chromium/components/update_client/url_request_post_interceptor.cc +++ /dev/null @@ -1,340 +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. - -#include "components/update_client/url_request_post_interceptor.h" - -#include <memory> - -#include "base/bind.h" -#include "base/files/file_util.h" -#include "base/macros.h" -#include "base/memory/ref_counted.h" -#include "base/sequenced_task_runner.h" -#include "base/strings/stringprintf.h" -#include "components/update_client/test_configurator.h" -#include "net/base/upload_bytes_element_reader.h" -#include "net/base/upload_data_stream.h" -#include "net/http/http_response_headers.h" -#include "net/http/http_util.h" -#include "net/url_request/url_request.h" -#include "net/url_request/url_request_filter.h" -#include "net/url_request/url_request_interceptor.h" -#include "net/url_request/url_request_simple_job.h" -#include "net/url_request/url_request_test_util.h" - -namespace update_client { - -// Returns a canned response. -class URLRequestPostInterceptor::URLRequestMockJob - : public net::URLRequestSimpleJob { - public: - URLRequestMockJob(scoped_refptr<URLRequestPostInterceptor> interceptor, - net::URLRequest* request, - net::NetworkDelegate* network_delegate, - int response_code, - const std::string& response_body) - : net::URLRequestSimpleJob(request, network_delegate), - interceptor_(interceptor), - response_code_(response_code), - response_body_(response_body) {} - - void Start() override { - if (interceptor_->is_paused_) - return; - net::URLRequestSimpleJob::Start(); - } - - protected: - void GetResponseInfo(net::HttpResponseInfo* info) override { - const std::string headers = - base::StringPrintf("HTTP/1.1 %i OK\r\n\r\n", response_code_); - info->headers = base::MakeRefCounted<net::HttpResponseHeaders>( - net::HttpUtil::AssembleRawHeaders(headers.c_str(), headers.length())); - } - - int GetData(std::string* mime_type, - std::string* charset, - std::string* data, - const net::CompletionCallback& callback) const override { - mime_type->assign("text/plain"); - charset->assign("US-ASCII"); - data->assign(response_body_); - return net::OK; - } - - private: - ~URLRequestMockJob() override {} - - scoped_refptr<URLRequestPostInterceptor> interceptor_; - - int response_code_; - std::string response_body_; - DISALLOW_COPY_AND_ASSIGN(URLRequestMockJob); -}; - -URLRequestPostInterceptor::URLRequestPostInterceptor( - const GURL& url, - scoped_refptr<base::SequencedTaskRunner> io_task_runner) - : url_(url), io_task_runner_(io_task_runner), hit_count_(0) {} - -URLRequestPostInterceptor::~URLRequestPostInterceptor() { - DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); -} - -GURL URLRequestPostInterceptor::GetUrl() const { - return url_; -} - -bool URLRequestPostInterceptor::ExpectRequest( - std::unique_ptr<RequestMatcher> request_matcher) { - return ExpectRequest(std::move(request_matcher), kResponseCode200); -} - -bool URLRequestPostInterceptor::ExpectRequest( - std::unique_ptr<RequestMatcher> request_matcher, - int response_code) { - expectations_.push( - {std::move(request_matcher), ExpectationResponse(response_code, "")}); - return true; -} - -bool URLRequestPostInterceptor::ExpectRequest( - std::unique_ptr<RequestMatcher> request_matcher, - const base::FilePath& filepath) { - std::string response; - if (filepath.empty() || !base::ReadFileToString(filepath, &response)) - return false; - - expectations_.push({std::move(request_matcher), - ExpectationResponse(kResponseCode200, response)}); - return true; -} - -int URLRequestPostInterceptor::GetHitCount() const { - base::AutoLock auto_lock(interceptor_lock_); - return hit_count_; -} - -int URLRequestPostInterceptor::GetCount() const { - base::AutoLock auto_lock(interceptor_lock_); - return static_cast<int>(requests_.size()); -} - -std::vector<URLRequestPostInterceptor::InterceptedRequest> -URLRequestPostInterceptor::GetRequests() const { - base::AutoLock auto_lock(interceptor_lock_); - return requests_; -} - -std::string URLRequestPostInterceptor::GetRequestBody(size_t n) const { - base::AutoLock auto_lock(interceptor_lock_); - return requests_[n].first; -} - -std::string URLRequestPostInterceptor::GetRequestsAsString() const { - const std::vector<InterceptedRequest> requests = GetRequests(); - - std::string s = "Requests are:"; - - int i = 0; - for (auto it = requests.cbegin(); it != requests.cend(); ++it) - s.append(base::StringPrintf("\n [%d]: %s", ++i, it->first.c_str())); - - return s; -} - -void URLRequestPostInterceptor::Reset() { - base::AutoLock auto_lock(interceptor_lock_); - hit_count_ = 0; - requests_.clear(); - base::queue<Expectation>().swap(expectations_); -} - -void URLRequestPostInterceptor::Pause() { - base::AutoLock auto_lock(interceptor_lock_); - is_paused_ = true; -} - -void URLRequestPostInterceptor::Resume() { - base::AutoLock auto_lock(interceptor_lock_); - is_paused_ = false; - io_task_runner_->PostTask(FROM_HERE, - base::BindOnce(&URLRequestMockJob::Start, - base::Unretained(request_job_))); -} - -void URLRequestPostInterceptor::url_job_request_ready_callback( - UrlJobRequestReadyCallback url_job_request_ready_callback) { - base::AutoLock auto_lock(interceptor_lock_); - url_job_request_ready_callback_ = std::move(url_job_request_ready_callback); -} - -class URLRequestPostInterceptor::Delegate : public net::URLRequestInterceptor { - public: - Delegate(const std::string& scheme, - const std::string& hostname, - scoped_refptr<base::SequencedTaskRunner> io_task_runner) - : scheme_(scheme), hostname_(hostname), io_task_runner_(io_task_runner) {} - - void Register() { - DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); - net::URLRequestFilter::GetInstance()->AddHostnameInterceptor( - scheme_, hostname_, std::unique_ptr<net::URLRequestInterceptor>(this)); - } - - void Unregister() { - DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); - interceptors_.clear(); - net::URLRequestFilter::GetInstance()->RemoveHostnameHandler(scheme_, - hostname_); - } - - void OnCreateInterceptor( - scoped_refptr<URLRequestPostInterceptor> interceptor) { - DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); - DCHECK_EQ(0u, interceptors_.count(interceptor->GetUrl())); - - interceptors_.insert(std::make_pair(interceptor->GetUrl(), interceptor)); - } - - private: - ~Delegate() override {} - - net::URLRequestJob* MaybeInterceptRequest( - net::URLRequest* request, - net::NetworkDelegate* network_delegate) const override { - DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); - - // Only intercepts POST. - if (!request->has_upload()) - return nullptr; - - GURL url = request->url(); - if (url.has_query()) { - GURL::Replacements replacements; - replacements.ClearQuery(); - url = url.ReplaceComponents(replacements); - } - - const auto it = interceptors_.find(url); - if (it == interceptors_.end()) - return nullptr; - - // There is an interceptor hooked up for this url. Read the request body, - // check the existing expectations, and handle the matching case by - // popping the expectation off the queue, counting the match, and - // returning a mock object to serve the canned response. - auto interceptor = it->second; - - const net::UploadDataStream* stream = request->get_upload(); - const net::UploadBytesElementReader* reader = - (*stream->GetElementReaders())[0]->AsBytesReader(); - const int size = reader->length(); - scoped_refptr<net::IOBuffer> buffer(new net::IOBuffer(size)); - const std::string request_body(reader->bytes()); - - { - base::AutoLock auto_lock(interceptor->interceptor_lock_); - interceptor->requests_.push_back( - {request_body, request->extra_request_headers()}); - if (interceptor->expectations_.empty()) - return nullptr; - const auto& expectation = interceptor->expectations_.front(); - if (expectation.first->Match(request_body)) { - const int response_code(expectation.second.response_code); - const std::string response_body(expectation.second.response_body); - interceptor->expectations_.pop(); - ++interceptor->hit_count_; - interceptor->request_job_ = - new URLRequestMockJob(interceptor, request, network_delegate, - response_code, response_body); - if (interceptor->url_job_request_ready_callback_) { - io_task_runner_->PostTask( - FROM_HERE, - std::move(interceptor->url_job_request_ready_callback_)); - } - return interceptor->request_job_; - } - } - - return nullptr; - } - - using InterceptorMap = - std::map<GURL, scoped_refptr<URLRequestPostInterceptor>>; - InterceptorMap interceptors_; - - const std::string scheme_; - const std::string hostname_; - scoped_refptr<base::SequencedTaskRunner> io_task_runner_; - - DISALLOW_COPY_AND_ASSIGN(Delegate); -}; - -URLRequestPostInterceptorFactory::URLRequestPostInterceptorFactory( - const std::string& scheme, - const std::string& hostname, - scoped_refptr<base::SequencedTaskRunner> io_task_runner) - : scheme_(scheme), - hostname_(hostname), - io_task_runner_(io_task_runner), - delegate_(new URLRequestPostInterceptor::Delegate(scheme, - hostname, - io_task_runner)) { - io_task_runner_->PostTask( - FROM_HERE, base::BindOnce(&URLRequestPostInterceptor::Delegate::Register, - base::Unretained(delegate_))); -} - -URLRequestPostInterceptorFactory::~URLRequestPostInterceptorFactory() { - io_task_runner_->PostTask( - FROM_HERE, - base::BindOnce(&URLRequestPostInterceptor::Delegate::Unregister, - base::Unretained(delegate_))); -} - -scoped_refptr<URLRequestPostInterceptor> -URLRequestPostInterceptorFactory::CreateInterceptor( - const base::FilePath& filepath) { - const GURL base_url( - base::StringPrintf("%s://%s", scheme_.c_str(), hostname_.c_str())); - GURL absolute_url(base_url.Resolve(filepath.MaybeAsASCII())); - auto interceptor = scoped_refptr<URLRequestPostInterceptor>( - new URLRequestPostInterceptor(absolute_url, io_task_runner_)); - bool res = io_task_runner_->PostTask( - FROM_HERE, - base::BindOnce(&URLRequestPostInterceptor::Delegate::OnCreateInterceptor, - base::Unretained(delegate_), interceptor)); - return res ? interceptor : nullptr; -} - -bool PartialMatch::Match(const std::string& actual) const { - return actual.find(expected_) != std::string::npos; -} - -bool AnyMatch::Match(const std::string&) const { - return true; -} - -InterceptorFactory::InterceptorFactory( - scoped_refptr<base::SequencedTaskRunner> io_task_runner) - : URLRequestPostInterceptorFactory(POST_INTERCEPT_SCHEME, - POST_INTERCEPT_HOSTNAME, - io_task_runner) {} - -InterceptorFactory::~InterceptorFactory() { -} - -scoped_refptr<URLRequestPostInterceptor> -InterceptorFactory::CreateInterceptor() { - return CreateInterceptorForPath(POST_INTERCEPT_PATH); -} - -scoped_refptr<URLRequestPostInterceptor> -InterceptorFactory::CreateInterceptorForPath(const char* url_path) { - return URLRequestPostInterceptorFactory::CreateInterceptor( - base::FilePath::FromUTF8Unsafe(url_path)); -} - -} // namespace update_client diff --git a/chromium/components/update_client/utils.cc b/chromium/components/update_client/utils.cc index 1213c1f5b57..85003a2808e 100644 --- a/chromium/components/update_client/utils.cc +++ b/chromium/components/update_client/utils.cc @@ -20,7 +20,6 @@ #include "base/strings/string_number_conversions.h" #include "base/strings/string_piece.h" #include "base/strings/string_util.h" -#include "base/strings/stringprintf.h" #include "base/values.h" #include "components/crx_file/id_util.h" #include "components/data_use_measurement/core/data_use_user_data.h" @@ -33,18 +32,19 @@ #include "net/base/load_flags.h" #include "net/traffic_annotation/network_traffic_annotation.h" #include "net/url_request/url_fetcher.h" -#include "net/url_request/url_request_context_getter.h" #include "net/url_request/url_request_status.h" +#include "services/network/public/cpp/resource_request.h" +#include "services/network/public/cpp/simple_url_loader.h" #include "url/gurl.h" namespace update_client { -std::unique_ptr<net::URLFetcher> SendProtocolRequest( +std::unique_ptr<network::SimpleURLLoader> SendProtocolRequest( const GURL& url, const std::map<std::string, std::string>& protocol_request_extra_headers, const std::string& protocol_request, - net::URLFetcherDelegate* url_fetcher_delegate, - scoped_refptr<net::URLRequestContextGetter> url_request_context_getter) { + network::SimpleURLLoader::BodyAsStringCallback callback, + scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) { net::NetworkTrafficAnnotationTag traffic_annotation = net::DefineNetworkTrafficAnnotation("component_updater_utils", R"( semantics { @@ -73,26 +73,26 @@ std::unique_ptr<net::URLFetcher> SendProtocolRequest( } } })"); - std::unique_ptr<net::URLFetcher> url_fetcher = net::URLFetcher::Create( - 0, url, net::URLFetcher::POST, url_fetcher_delegate, traffic_annotation); - if (!url_fetcher.get()) - return url_fetcher; - - data_use_measurement::DataUseUserData::AttachToFetcher( - url_fetcher.get(), data_use_measurement::DataUseUserData::UPDATE_CLIENT); - url_fetcher->SetUploadData("application/xml", protocol_request); - url_fetcher->SetRequestContext(url_request_context_getter.get()); - url_fetcher->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES | - net::LOAD_DO_NOT_SAVE_COOKIES | - net::LOAD_DISABLE_CACHE); - url_fetcher->SetAutomaticallyRetryOn5xx(false); - url_fetcher->SetAutomaticallyRetryOnNetworkChanges(3); + // Create and initialize URL loader. + auto resource_request = std::make_unique<network::ResourceRequest>(); + resource_request->url = url; + resource_request->method = "POST"; + resource_request->load_flags = net::LOAD_DO_NOT_SEND_COOKIES | + net::LOAD_DO_NOT_SAVE_COOKIES | + net::LOAD_DISABLE_CACHE; for (const auto& header : protocol_request_extra_headers) - url_fetcher->AddExtraRequestHeader(base::StringPrintf( - "%s: %s", header.first.c_str(), header.second.c_str())); - - url_fetcher->Start(); - return url_fetcher; + resource_request->headers.SetHeader(header.first, header.second); + + auto simple_loader = network::SimpleURLLoader::Create( + std::move(resource_request), traffic_annotation); + const int max_retry_on_network_change = 3; + simple_loader->SetRetryOptions( + max_retry_on_network_change, + network::SimpleURLLoader::RETRY_ON_NETWORK_CHANGE); + simple_loader->AttachStringForUpload(protocol_request, "application/xml"); + simple_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie( + url_loader_factory.get(), std::move(callback)); + return simple_loader; } bool FetchSuccess(const net::URLFetcher& fetcher) { @@ -249,7 +249,7 @@ std::unique_ptr<base::DictionaryValue> ReadManifest( JSONFileValueDeserializer deserializer(manifest); std::string error; std::unique_ptr<base::Value> root = deserializer.Deserialize(nullptr, &error); - if (!root.get()) + if (!root) return std::unique_ptr<base::DictionaryValue>(); if (!root->is_dict()) return std::unique_ptr<base::DictionaryValue>(); diff --git a/chromium/components/update_client/utils.h b/chromium/components/update_client/utils.h index e2840f9dcbb..525b7ed8a60 100644 --- a/chromium/components/update_client/utils.h +++ b/chromium/components/update_client/utils.h @@ -24,10 +24,13 @@ class FilePath; namespace net { class URLFetcher; -class URLFetcherDelegate; -class URLRequestContextGetter; } +namespace network { +class SharedURLLoaderFactory; +class SimpleURLLoader; +} // namespace network + namespace update_client { class Component; @@ -38,15 +41,18 @@ struct CrxComponent; // in an update check request. using InstallerAttribute = std::pair<std::string, std::string>; +using LoadCompleteCallback = + base::OnceCallback<void(std::unique_ptr<std::string> response_body)>; + // Sends a protocol request to the the service endpoint specified by |url|. // The body of the request is provided by |protocol_request| and it is // expected to contain XML data. The caller owns the returned object. -std::unique_ptr<net::URLFetcher> SendProtocolRequest( +std::unique_ptr<network::SimpleURLLoader> SendProtocolRequest( const GURL& url, const std::map<std::string, std::string>& protocol_request_extra_headers, const std::string& protocol_request, - net::URLFetcherDelegate* url_fetcher_delegate, - scoped_refptr<net::URLRequestContextGetter> url_request_context_getter); + LoadCompleteCallback callback, + scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory); // Returns true if the url request of |fetcher| was succesful. bool FetchSuccess(const net::URLFetcher& fetcher); |