Fix ActionUpdate::DownloadComplete crash.
We changed the way download callbacks are called and how the instance
of the CRX downloader is owned.
BUG=495426
Review URL: https://codereview.chromium.org/1156893007
Cr-Commit-Position: refs/heads/master@{#332505}
diff --git a/components/update_client/update_client_unittest.cc b/components/update_client/update_client_unittest.cc
index 4440d95..0ae4cc2 100644
--- a/components/update_client/update_client_unittest.cc
+++ b/components/update_client/update_client_unittest.cc
@@ -715,6 +715,245 @@
StopWorkerPool();
}
+// Tests the scenario where there is a download timeout for the first
+// CRX. The update for the first CRX fails. The update client waits before
+// attempting the update for the second CRX. This update succeeds.
+TEST_F(UpdateClientTest, TwoCrxUpdateDownloadTimeout) {
+ class DataCallbackFake {
+ public:
+ static void Callback(const std::vector<std::string>& ids,
+ std::vector<CrxComponent>* components) {
+ CrxComponent crx1;
+ crx1.name = "test_jebg";
+ crx1.pk_hash.assign(jebg_hash, jebg_hash + arraysize(jebg_hash));
+ crx1.version = Version("0.9");
+ crx1.installer = new TestInstaller;
+
+ CrxComponent crx2;
+ crx2.name = "test_ihfo";
+ crx2.pk_hash.assign(ihfo_hash, ihfo_hash + arraysize(ihfo_hash));
+ crx2.version = Version("0.8");
+ crx2.installer = new TestInstaller;
+
+ components->push_back(crx1);
+ components->push_back(crx2);
+ }
+ };
+
+ class CompletionCallbackFake {
+ public:
+ static void Callback(const base::Closure& quit_closure, int error) {
+ EXPECT_EQ(0, error);
+ quit_closure.Run();
+ }
+ };
+
+ class FakeUpdateChecker : public UpdateChecker {
+ public:
+ static scoped_ptr<UpdateChecker> Create(const Configurator& config) {
+ return scoped_ptr<UpdateChecker>(new FakeUpdateChecker());
+ }
+
+ bool CheckForUpdates(
+ const std::vector<CrxUpdateItem*>& items_to_check,
+ const std::string& additional_attributes,
+ const UpdateCheckCallback& update_check_callback) override {
+ /*
+ Fake the following response:
+
+ <?xml version='1.0' encoding='UTF-8'?>
+ <response protocol='3.0'>
+ <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'/>
+ </packages>
+ </manifest>
+ </updatecheck>
+ </app>
+ <app appid='ihfokbkgjpifnbbojhneepfflplebdkc'>
+ <updatecheck status='ok'>
+ <urls>
+ <url codebase='http://localhost/download/'/>
+ </urls>
+ <manifest version='1.0' prodversionmin='11.0.1.0'>
+ <packages>
+ <package name='ihfokbkgjpifnbbojhneepfflplebdkc_1.crx'/>
+ </packages>
+ </manifest>
+ </updatecheck>
+ </app>
+ </response>
+ */
+ UpdateResponse::Result::Manifest::Package package1;
+ package1.name = "jebgalgnebhfojomionfpkfelancnnkf.crx";
+
+ UpdateResponse::Result result1;
+ result1.extension_id = "jebgalgnebhfojomionfpkfelancnnkf";
+ result1.crx_urls.push_back(GURL("http://localhost/download/"));
+ result1.manifest.version = "1.0";
+ result1.manifest.browser_min_version = "11.0.1.0";
+ result1.manifest.packages.push_back(package1);
+
+ UpdateResponse::Result::Manifest::Package package2;
+ package2.name = "ihfokbkgjpifnbbojhneepfflplebdkc_1.crx";
+
+ UpdateResponse::Result result2;
+ result2.extension_id = "ihfokbkgjpifnbbojhneepfflplebdkc";
+ result2.crx_urls.push_back(GURL("http://localhost/download/"));
+ result2.manifest.version = "1.0";
+ result2.manifest.browser_min_version = "11.0.1.0";
+ result2.manifest.packages.push_back(package2);
+
+ UpdateResponse::Results results;
+ results.list.push_back(result1);
+ results.list.push_back(result2);
+
+ base::ThreadTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE, base::Bind(update_check_callback, GURL(), 0, "", results));
+ return true;
+ }
+ };
+
+ class FakeCrxDownloader : public CrxDownloader {
+ public:
+ static scoped_ptr<CrxDownloader> Create(
+ bool is_background_download,
+ net::URLRequestContextGetter* context_getter,
+ const scoped_refptr<base::SequencedTaskRunner>& url_fetcher_task_runner,
+ const scoped_refptr<base::SingleThreadTaskRunner>&
+ background_task_runner) {
+ return scoped_ptr<CrxDownloader>(new FakeCrxDownloader());
+ }
+
+ private:
+ FakeCrxDownloader() : CrxDownloader(scoped_ptr<CrxDownloader>().Pass()) {}
+ ~FakeCrxDownloader() override {}
+
+ void DoStartDownload(const GURL& url) override {
+ DownloadMetrics download_metrics;
+ FilePath path;
+ Result result;
+ if (url.path() == "/download/jebgalgnebhfojomionfpkfelancnnkf.crx") {
+ download_metrics.url = url;
+ download_metrics.downloader = DownloadMetrics::kNone;
+ download_metrics.error = -118;
+ download_metrics.downloaded_bytes = 0;
+ download_metrics.total_bytes = 0;
+ download_metrics.download_time_ms = 1000;
+
+ EXPECT_TRUE(MakeTestFile(
+ TestFilePath("jebgalgnebhfojomionfpkfelancnnkf.crx"), &path));
+
+ result.error = -118;
+ result.response = path;
+ result.downloaded_bytes = 0;
+ result.total_bytes = 0;
+ } else if (url.path() ==
+ "/download/ihfokbkgjpifnbbojhneepfflplebdkc_1.crx") {
+ download_metrics.url = url;
+ download_metrics.downloader = DownloadMetrics::kNone;
+ download_metrics.error = 0;
+ download_metrics.downloaded_bytes = 53638;
+ download_metrics.total_bytes = 53638;
+ download_metrics.download_time_ms = 2000;
+
+ EXPECT_TRUE(MakeTestFile(
+ TestFilePath("ihfokbkgjpifnbbojhneepfflplebdkc_1.crx"), &path));
+
+ result.error = 0;
+ result.response = path;
+ result.downloaded_bytes = 53638;
+ result.total_bytes = 53638;
+ } else {
+ NOTREACHED();
+ }
+
+ base::ThreadTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE, base::Bind(&FakeCrxDownloader::OnDownloadProgress,
+ base::Unretained(this), result));
+
+ base::ThreadTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE,
+ base::Bind(&FakeCrxDownloader::OnDownloadComplete,
+ base::Unretained(this), true, result, download_metrics));
+ }
+ };
+
+ class FakePingManager : public FakePingManagerImpl {
+ public:
+ explicit FakePingManager(const Configurator& config)
+ : FakePingManagerImpl(config) {}
+ ~FakePingManager() override {
+ const auto& ping_items = items();
+ EXPECT_EQ(2U, ping_items.size());
+ EXPECT_EQ("jebgalgnebhfojomionfpkfelancnnkf", ping_items[0].id);
+ EXPECT_TRUE(base::Version("0.9").Equals(ping_items[0].previous_version));
+ EXPECT_TRUE(base::Version("1.0").Equals(ping_items[0].next_version));
+ EXPECT_EQ(1, ping_items[0].error_category); // Network error.
+ EXPECT_EQ(-118, ping_items[0].error_code);
+ EXPECT_EQ("ihfokbkgjpifnbbojhneepfflplebdkc", ping_items[1].id);
+ EXPECT_TRUE(base::Version("0.8").Equals(ping_items[1].previous_version));
+ EXPECT_TRUE(base::Version("1.0").Equals(ping_items[1].next_version));
+ EXPECT_EQ(0, ping_items[1].error_category);
+ EXPECT_EQ(0, ping_items[1].error_code);
+ }
+ };
+
+ scoped_ptr<FakePingManager> ping_manager(new FakePingManager(*config()));
+ scoped_refptr<UpdateClient> update_client(new UpdateClientImpl(
+ config(), ping_manager.Pass(), &FakeUpdateChecker::Create,
+ &FakeCrxDownloader::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(1);
+ EXPECT_CALL(observer, OnEvent(Events::COMPONENT_NOT_UPDATED,
+ "jebgalgnebhfojomionfpkfelancnnkf")).Times(1);
+ }
+ {
+ InSequence seq;
+ EXPECT_CALL(observer, OnEvent(Events::COMPONENT_CHECKING_FOR_UPDATES,
+ "ihfokbkgjpifnbbojhneepfflplebdkc")).Times(1);
+ EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_FOUND,
+ "ihfokbkgjpifnbbojhneepfflplebdkc")).Times(1);
+ EXPECT_CALL(observer, OnEvent(Events::COMPONENT_WAIT,
+ "ihfokbkgjpifnbbojhneepfflplebdkc")).Times(1);
+ EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_DOWNLOADING,
+ "ihfokbkgjpifnbbojhneepfflplebdkc")).Times(1);
+ EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATE_READY,
+ "ihfokbkgjpifnbbojhneepfflplebdkc")).Times(1);
+ EXPECT_CALL(observer, OnEvent(Events::COMPONENT_UPDATED,
+ "ihfokbkgjpifnbbojhneepfflplebdkc")).Times(1);
+ }
+
+ update_client->AddObserver(&observer);
+
+ std::vector<std::string> ids;
+ ids.push_back(std::string("jebgalgnebhfojomionfpkfelancnnkf"));
+ ids.push_back(std::string("ihfokbkgjpifnbbojhneepfflplebdkc"));
+
+ update_client->Update(
+ ids, base::Bind(&DataCallbackFake::Callback),
+ base::Bind(&CompletionCallbackFake::Callback, quit_closure()));
+
+ RunThreads();
+
+ update_client->RemoveObserver(&observer);
+
+ StopWorkerPool();
+}
+
// Tests the differential update scenario for one CRX.
TEST_F(UpdateClientTest, OneCrxDiffUpdate) {
class DataCallbackFake {
