summaryrefslogtreecommitdiffstats
path: root/chrome/browser/component_updater
diff options
context:
space:
mode:
authorcpu@chromium.org <cpu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-08-09 20:20:12 +0000
committercpu@chromium.org <cpu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-08-09 20:20:12 +0000
commitcf44261b7ddbb9d5849ed49e918ea82179118e1e (patch)
tree51e4c71cd35d99e1839384c7901fa4d10100d38d /chrome/browser/component_updater
parentefa5d93850c47d34931c89533ec3f480938e056a (diff)
downloadchromium_src-cf44261b7ddbb9d5849ed49e918ea82179118e1e.zip
chromium_src-cf44261b7ddbb9d5849ed49e918ea82179118e1e.tar.gz
chromium_src-cf44261b7ddbb9d5849ed49e918ea82179118e1e.tar.bz2
Component updater eight piece
"in which we beef up the unit tests and find several bugs" 1- When parsing the xml response components not mentioned there were left in "updating" state so they got stranded there. 2- Wrong condition : if ((item->status != CrxUpdateItem::kNoUpdate) || (item->status != CrxUpdateItem::kUpToDate)) So again some components are stranded in the kNoUpdate state. Also a new precondition ScheduleNextRun() cannot be called if a url_fetcher is in progress. BUG=61602 TEST=included Review URL: http://codereview.chromium.org/7601019 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@96048 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/component_updater')
-rw-r--r--chrome/browser/component_updater/component_updater_interceptor.cc4
-rw-r--r--chrome/browser/component_updater/component_updater_interceptor.h6
-rw-r--r--chrome/browser/component_updater/component_updater_service.cc34
-rw-r--r--chrome/browser/component_updater/component_updater_service_unittest.cc57
4 files changed, 85 insertions, 16 deletions
diff --git a/chrome/browser/component_updater/component_updater_interceptor.cc b/chrome/browser/component_updater/component_updater_interceptor.cc
index d6cb88c..21695f1 100644
--- a/chrome/browser/component_updater/component_updater_interceptor.cc
+++ b/chrome/browser/component_updater/component_updater_interceptor.cc
@@ -9,7 +9,8 @@
#include "net/url_request/url_request_test_job.h"
#include "testing/gtest/include/gtest/gtest.h"
-ComponentUpdateInterceptor::ComponentUpdateInterceptor() {
+ComponentUpdateInterceptor::ComponentUpdateInterceptor()
+ : hit_count_(0) {
net::URLRequest::Deprecated::RegisterRequestInterceptor(this);
}
@@ -34,6 +35,7 @@ net::URLRequestJob* ComponentUpdateInterceptor::MaybeIntercept(
return NULL;
}
const Response& response = it->second;
+ ++hit_count_;
std::string contents;
EXPECT_TRUE(file_util::ReadFileToString(response.data_path, &contents));
diff --git a/chrome/browser/component_updater/component_updater_interceptor.h b/chrome/browser/component_updater/component_updater_interceptor.h
index d008133..1949844 100644
--- a/chrome/browser/component_updater/component_updater_interceptor.h
+++ b/chrome/browser/component_updater/component_updater_interceptor.h
@@ -33,8 +33,11 @@ class ComponentUpdateInterceptor
const std::string& headers,
const FilePath& path);
+ // Returns how many requests have been issued that have a stored reply.
+ int hit_count() const { return hit_count_; }
+
private:
- // When computing matches, this ignores the query parameters of the url.
+ // When computing matches, this ignores the query parameters of the url.
virtual net::URLRequestJob* MaybeIntercept(net::URLRequest* request) OVERRIDE;
friend class base::RefCountedThreadSafe<ComponentUpdateInterceptor>;
@@ -48,6 +51,7 @@ class ComponentUpdateInterceptor
typedef std::map<GURL, Response> ResponseMap;
ResponseMap responses_;
+ int hit_count_;
DISALLOW_COPY_AND_ASSIGN(ComponentUpdateInterceptor);
};
diff --git a/chrome/browser/component_updater/component_updater_service.cc b/chrome/browser/component_updater/component_updater_service.cc
index 3dbf5a7..4e07cb2 100644
--- a/chrome/browser/component_updater/component_updater_service.cc
+++ b/chrome/browser/component_updater/component_updater_service.cc
@@ -344,6 +344,7 @@ ComponentUpdateService::Status CrxUpdateService::Stop() {
// long one.
void CrxUpdateService::ScheduleNextRun(bool step_delay) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ DCHECK(url_fetcher_.get() == NULL);
CHECK(!timer_.IsRunning());
// It could be the case that Stop() had been called while a url request
// or unpacking was in flight, if so we arrive here but |running_| is
@@ -351,17 +352,18 @@ void CrxUpdateService::ScheduleNextRun(bool step_delay) {
if (!running_)
return;
+ int64 delay = step_delay ? config_->StepDelay() : config_->NextCheckDelay();
+
if (!step_delay) {
NotificationService::current()->Notify(
chrome::NOTIFICATION_COMPONENT_UPDATER_SLEEPING,
Source<ComponentUpdateService>(this),
NotificationService::NoDetails());
// Zero is only used for unit tests.
- if (0 == config_->NextCheckDelay())
+ if (0 == delay)
return;
}
- int64 delay = step_delay ? config_->StepDelay() : config_->NextCheckDelay();
timer_.Start(base::TimeDelta::FromSeconds(delay),
this, &CrxUpdateService::ProcessPendingItems);
}
@@ -489,7 +491,7 @@ void CrxUpdateService::ProcessPendingItems() {
for (UpdateItems::const_iterator it = work_items_.begin();
it != work_items_.end(); ++it) {
CrxUpdateItem* item = *it;
- if ((item->status != CrxUpdateItem::kNoUpdate) ||
+ if ((item->status != CrxUpdateItem::kNoUpdate) &&
(item->status != CrxUpdateItem::kUpToDate))
continue;
base::TimeDelta delta = base::Time::Now() - item->last_check;
@@ -532,11 +534,12 @@ void CrxUpdateService::OnURLFetchComplete(const URLFetcher* source,
if (FetchSuccess(*source)) {
std::string xml;
source->GetResponseAsString(&xml);
+ url_fetcher_.reset();
ParseManifest(xml);
} else {
+ url_fetcher_.reset();
CrxUpdateService::OnParseUpdateManifestFailed("network error");
}
- url_fetcher_.reset();
}
// Parsing the manifest is either done right now for tests or in a sandboxed
@@ -577,16 +580,21 @@ void CrxUpdateService::OnParseUpdateManifestSucceeded(
continue; // Not updating this component now.
if (it->version.empty()) {
+ // No version means no update available.
crx->status = CrxUpdateItem::kNoUpdate;
- continue; // No version means no update available.
+ continue;
}
if (!IsVersionNewer(crx->component.version, it->version)) {
+ // Our component is up to date.
crx->status = CrxUpdateItem::kUpToDate;
- continue; // Our component is up to date.
+ continue;
}
if (!it->browser_min_version.empty()) {
- if (IsVersionNewer(chrome_version_, it->browser_min_version))
- continue; // Does not apply for this version.
+ if (IsVersionNewer(chrome_version_, it->browser_min_version)) {
+ // Does not apply for this chrome version.
+ crx->status = CrxUpdateItem::kNoUpdate;
+ continue;
+ }
}
// All test passed. Queue an upgrade for this component and fire the
// notifications.
@@ -599,6 +607,11 @@ void CrxUpdateService::OnParseUpdateManifestSucceeded(
Source<std::string>(&crx->id),
NotificationService::NoDetails());
}
+
+ // All the components that are not mentioned in the manifest we
+ // consider them up to date.
+ ChangeItemStatus(CrxUpdateItem::kChecking, CrxUpdateItem::kUpToDate);
+
// If there are updates pending we do a short wait.
ScheduleNextRun(update_pending ? true : false);
}
@@ -624,6 +637,7 @@ void CrxUpdateService::OnURLFetchComplete(const URLFetcher* source,
size_t count = ChangeItemStatus(CrxUpdateItem::kDownloading,
CrxUpdateItem::kNoUpdate);
DCHECK_EQ(count, 1ul);
+ url_fetcher_.reset();
ScheduleNextRun(false);
} else {
FilePath temp_crx_path;
@@ -631,6 +645,8 @@ void CrxUpdateService::OnURLFetchComplete(const URLFetcher* source,
size_t count = ChangeItemStatus(CrxUpdateItem::kDownloading,
CrxUpdateItem::kUpdating);
DCHECK_EQ(count, 1ul);
+ url_fetcher_.reset();
+
NotificationService::current()->Notify(
chrome::NOTIFICATION_COMPONENT_UPDATE_READY,
Source<std::string>(&context->id),
@@ -642,8 +658,6 @@ void CrxUpdateService::OnURLFetchComplete(const URLFetcher* source,
temp_crx_path),
config_->StepDelay());
}
-
- url_fetcher_.reset();
}
// Install consists of digital signature verification, unpacking and then
diff --git a/chrome/browser/component_updater/component_updater_service_unittest.cc b/chrome/browser/component_updater/component_updater_service_unittest.cc
index 83cda97..69f4eb0 100644
--- a/chrome/browser/component_updater/component_updater_service_unittest.cc
+++ b/chrome/browser/component_updater/component_updater_service_unittest.cc
@@ -30,6 +30,9 @@ namespace {
// and loops faster. In actual usage it takes hours do to a full cycle.
class TestConfigurator : public ComponentUpdateService::Configurator {
public:
+ TestConfigurator() : times_(1) {
+ }
+
virtual int InitialDelay() OVERRIDE { return 0; }
virtual int NextCheckDelay() OVERRIDE {
@@ -37,6 +40,9 @@ class TestConfigurator : public ComponentUpdateService::Configurator {
// to happen. In test we normally only test one cycle so it is a good
// time to break from the test messageloop Run() method so the test can
// finish.
+ if (--times_ > 0)
+ return 1;
+
MessageLoop::current()->Quit();
return 0;
}
@@ -59,6 +65,12 @@ class TestConfigurator : public ComponentUpdateService::Configurator {
// Don't use the utility process to decode files.
virtual bool InProcess() OVERRIDE { return true; }
+
+ // Set how many update checks are called, the default value is just once.
+ void SetLoopCount(int times) { times_ = times; }
+
+ private:
+ int times_;
};
class TestInstaller : public ComponentInstaller {
@@ -110,10 +122,10 @@ const char header_ok_reply[] =
// Common fixture for all the component updater tests.
class ComponentUpdaterTest : public TestingBrowserProcessTest {
public:
- ComponentUpdaterTest() : component_updater_(NULL) {
+ ComponentUpdaterTest() : component_updater_(NULL), test_config_(NULL) {
// The component updater instance under test.
- component_updater_.reset(
- ComponentUpdateServiceFactory(new TestConfigurator));
+ test_config_ = new TestConfigurator;
+ component_updater_.reset(ComponentUpdateServiceFactory(test_config_));
// The test directory is chrome/test/data/components.
PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir_);
test_data_dir_ = test_data_dir_.AppendASCII("components");
@@ -150,10 +162,15 @@ class ComponentUpdaterTest : public TestingBrowserProcessTest {
return notification_tracker_;
}
+ TestConfigurator* test_configurator() {
+ return test_config_;
+ }
+
private:
scoped_ptr<ComponentUpdateService> component_updater_;
FilePath test_data_dir_;
TestNotificationTracker notification_tracker_;
+ TestConfigurator* test_config_;
};
// Verify that our test fixture work and the component updater can
@@ -208,6 +225,8 @@ TEST_F(ComponentUpdaterTest, CheckCrxSleep) {
header_ok_reply,
test_file("updatecheck_reply_1.xml"));
+ // We loop twice, but there are no updates so we expect two sleep messages.
+ test_configurator()->SetLoopCount(2);
component_updater()->Start();
ASSERT_EQ(1ul, notification_tracker().size());
@@ -216,9 +235,39 @@ TEST_F(ComponentUpdaterTest, CheckCrxSleep) {
message_loop.Run();
- ASSERT_EQ(2ul, notification_tracker().size());
+ ASSERT_EQ(3ul, notification_tracker().size());
TestNotificationTracker::Event ev2 = notification_tracker().at(1);
EXPECT_EQ(chrome::NOTIFICATION_COMPONENT_UPDATER_SLEEPING, ev2.type);
+ TestNotificationTracker::Event ev3 = notification_tracker().at(2);
+ EXPECT_EQ(chrome::NOTIFICATION_COMPONENT_UPDATER_SLEEPING, ev2.type);
+ EXPECT_EQ(2, interceptor->hit_count());
+
+ EXPECT_EQ(0, static_cast<TestInstaller*>(com.installer)->error());
+ EXPECT_EQ(0, static_cast<TestInstaller*>(com.installer)->install_count());
+
+ component_updater()->Stop();
+
+ // Loop twice again but this case we simulate a server error by returning
+ // an empty file.
+
+ interceptor->SetResponse(expected_update_url,
+ header_ok_reply,
+ test_file("updatecheck_reply_empty"));
+
+ notification_tracker().Reset();
+ test_configurator()->SetLoopCount(2);
+ component_updater()->Start();
+
+ message_loop.Run();
+
+ ASSERT_EQ(3ul, notification_tracker().size());
+ ev1 = notification_tracker().at(0);
+ EXPECT_EQ(chrome::NOTIFICATION_COMPONENT_UPDATER_STARTED, ev1.type);
+ ev2 = notification_tracker().at(1);
+ EXPECT_EQ(chrome::NOTIFICATION_COMPONENT_UPDATER_SLEEPING, ev2.type);
+ ev3 = notification_tracker().at(2);
+ EXPECT_EQ(chrome::NOTIFICATION_COMPONENT_UPDATER_SLEEPING, ev2.type);
+ EXPECT_EQ(4, interceptor->hit_count());
EXPECT_EQ(0, static_cast<TestInstaller*>(com.installer)->error());
EXPECT_EQ(0, static_cast<TestInstaller*>(com.installer)->install_count());
generated by cgit v1.1 at 2025-03-26 05:10:09 +0000