Component updater: Fix couple of bugs (part nine)

- The version of a updated componnent was not being updated upon succesful install
- a small memory leak.

BUG=61602
TEST=test included
Review URL: http://codereview.chromium.org/7631018

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@97203 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 35 insertions, 15 deletions

diff --git a/chrome/browser/component_updater/component_updater_service.cc b/chrome/browser/component_updater/component_updater_service.cc
index 4e07cb2..66a2e33 100644
--- a/chrome/browser/component_updater/component_updater_service.cc
+++ b/chrome/browser/component_updater/component_updater_service.cc
@@ -151,6 +151,7 @@ struct CrxUpdateItem {
   std::string id;
   base::Time last_check;
   CrxComponent component;
+  Version next_version;
 
   CrxUpdateItem() : status(kNew) {}
 
@@ -269,7 +270,8 @@ class CrxUpdateService : public ComponentUpdateService {
 
   void Install(const CRXContext* context, const FilePath& crx_path);
 
-  void DoneInstalling(ComponentUnpacker::Error error);
+  void DoneInstalling(const std::string& component_id,
+                      ComponentUnpacker::Error error);
 
   size_t ChangeItemStatus(CrxUpdateItem::Status from,
                           CrxUpdateItem::Status to);
@@ -529,7 +531,7 @@ void CrxUpdateService::ProcessPendingItems() {
 // Caled when we got a response from the update server. It consists of an xml
 // document following the omaha update scheme.
 void CrxUpdateService::OnURLFetchComplete(const URLFetcher* source,
-                                          UpdateContext*) {
+                                          UpdateContext* context) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (FetchSuccess(*source)) {
     std::string xml;
@@ -540,6 +542,7 @@ void CrxUpdateService::OnURLFetchComplete(const URLFetcher* source,
     url_fetcher_.reset();
     CrxUpdateService::OnParseUpdateManifestFailed("network error");
   }
+  delete context;
 }
 
 // Parsing the manifest is either done right now for tests or in a sandboxed
@@ -600,6 +603,7 @@ void CrxUpdateService::OnParseUpdateManifestSucceeded(
     // notifications.
     crx->crx_url = it->crx_url;
     crx->status = CrxUpdateItem::kCanUpdate;
+    crx->next_version = Version(it->version);
     ++update_pending;
 
     NotificationService::current()->Notify(
@@ -670,26 +674,28 @@ void CrxUpdateService::Install(const CRXContext* context,
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   ComponentUnpacker
       unpacker(context->pk_hash, crx_path, context->installer);
-  delete context;
   if (!file_util::Delete(crx_path, false)) {
     NOTREACHED() << crx_path.value();
   }
-
   BrowserThread::PostDelayedTask(BrowserThread::UI, FROM_HERE,
       NewRunnableMethod(this, &CrxUpdateService::DoneInstalling,
-                        unpacker.error()),
+                        context->id, unpacker.error()),
       config_->StepDelay());
+  delete context;
 }
 
 // Installation has been completed. Adjust the component status and
 // schedule the next check.
-void CrxUpdateService::DoneInstalling(ComponentUnpacker::Error error) {
+void CrxUpdateService::DoneInstalling(const std::string& component_id,
+                                      ComponentUnpacker::Error error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  CrxUpdateItem::Status status =
-      (error == ComponentUnpacker::kNone) ? CrxUpdateItem::kUpdated :
-                                            CrxUpdateItem::kNoUpdate;
-  size_t count = ChangeItemStatus(CrxUpdateItem::kUpdating, status);
-  DCHECK_EQ(count, 1ul);
+
+  CrxUpdateItem* item = FindUpdateItemById(component_id);
+  item->status = (error == ComponentUnpacker::kNone) ? CrxUpdateItem::kUpdated :
+                                                       CrxUpdateItem::kNoUpdate;
+  if (item->status == CrxUpdateItem::kUpdated)
+    item->component.version = item->next_version;
+
   ScheduleNextRun(false);
 }
 
diff --git a/chrome/browser/component_updater/component_updater_service_unittest.cc b/chrome/browser/component_updater/component_updater_service_unittest.cc
index df316df..ff7eb80 100644
--- a/chrome/browser/component_updater/component_updater_service_unittest.cc
+++ b/chrome/browser/component_updater/component_updater_service_unittest.cc
@@ -280,7 +280,8 @@ TEST_F(ComponentUpdaterTest, CheckCrxSleep) {
 
 // Verify that we can check for updates and install one component. Besides
 // the notifications above NOTIFICATION_COMPONENT_UPDATE_FOUND and
-// NOTIFICATION_COMPONENT_UPDATE_READY should have been fired.
+// NOTIFICATION_COMPONENT_UPDATE_READY should have been fired. We do two loops
+// so the second time arround there should be nothing left to do.
 TEST_F(ComponentUpdaterTest, InstallCrx) {
   MessageLoop message_loop;
   BrowserThread ui_thread(BrowserThread::UI, &message_loop);
@@ -308,23 +309,32 @@ TEST_F(ComponentUpdaterTest, InstallCrx) {
   component_updater()->RegisterComponent(com1);
   component_updater()->RegisterComponent(com2);
 
-  const char expected_update_url[] =
+  const char expected_update_url_1[] =
       "http://localhost/upd?x=id%3D"
       "jebgalgnebhfojomionfpkfelancnnkf%26v%3D0.9%26uc&x=id%3D"
       "abagagagagagagagagagagagagagagag%26v%3D2.2%26uc";
 
+  const char expected_update_url_2[] =
+      "http://localhost/upd?x=id%3D"
+      "abagagagagagagagagagagagagagagag%26v%3D2.2%26uc&x=id%3D"
+      "jebgalgnebhfojomionfpkfelancnnkf%26v%3D0.9%26uc";
+
   const char expected_crx_url[] =
       "http://localhost/download/jebgalgnebhfojomionfpkfelancnnkf.crx";
 
-  interceptor->SetResponse(expected_update_url, header_ok_reply,
+  interceptor->SetResponse(expected_update_url_1, header_ok_reply,
+                           test_file("updatecheck_reply_1.xml"));
+  interceptor->SetResponse(expected_update_url_2, header_ok_reply,
                            test_file("updatecheck_reply_1.xml"));
   interceptor->SetResponse(expected_crx_url, header_ok_reply,
                            test_file("jebgalgnebhfojomionfpkfelancnnkf.crx"));
 
+  test_configurator()->SetLoopCount(2);
+
   component_updater()->Start();
   message_loop.Run();
 
-  ASSERT_EQ(4ul, notification_tracker().size());
+  ASSERT_EQ(5ul, notification_tracker().size());
 
   TestNotificationTracker::Event ev1 = notification_tracker().at(1);
   EXPECT_EQ(chrome::NOTIFICATION_COMPONENT_UPDATE_FOUND, ev1.type);
@@ -335,8 +345,12 @@ TEST_F(ComponentUpdaterTest, InstallCrx) {
   TestNotificationTracker::Event ev3 = notification_tracker().at(3);
   EXPECT_EQ(chrome::NOTIFICATION_COMPONENT_UPDATER_SLEEPING, ev3.type);
 
+  TestNotificationTracker::Event ev4 = notification_tracker().at(4);
+  EXPECT_EQ(chrome::NOTIFICATION_COMPONENT_UPDATER_SLEEPING, ev3.type);
+
   EXPECT_EQ(0, static_cast<TestInstaller*>(com1.installer)->error());
   EXPECT_EQ(1, static_cast<TestInstaller*>(com1.installer)->install_count());
+  EXPECT_EQ(2, interceptor->hit_count());
 
   component_updater()->Stop();
   delete com1.installer;