Component updater eight piece

chrome/browser/component_updater/component_updater_service_unittest.cc

 // Copyright (c) 2011 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 "chrome/browser/component_updater/component_updater_service.h"
 
 #include "base/compiler_specific.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/path_service.h"
@@ skipping 12 matching lines @@
 #include "googleurl/src/gurl.h"
 #include "libxml/globals.h"
 
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 // Overrides some of the component updater behaviors so it is easier to test
 // 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 {
     // This is called when a new full cycle of checking for updates is going
     // 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)

[asargent_no_longer_on_chrome, 2011/08/09 17:56:19]

Should this be "> 0" to protect against off-by-one errors when times becomes
negative?

[cpu_(ooo_6.6-7.5), 2011/08/09 20:21:53]

Done.

+      return 1;
+
     MessageLoop::current()->Quit();
     return 0;
   }
 
   virtual int StepDelay() OVERRIDE {
     return 0;
   }
 
   virtual int MinimumReCheckWait() OVERRIDE {
     return 0;
   }
 
   virtual GURL UpdateUrl() OVERRIDE { return GURL("http://localhost/upd"); }
 
   virtual size_t UrlSizeLimit() OVERRIDE { return 256; }
 
   virtual net::URLRequestContextGetter* RequestContext() OVERRIDE {
     return new TestURLRequestContextGetter();
   }
 
   // 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 {
  public :
   explicit TestInstaller()
       : error_(0), install_count_(0) {
   }
 
   virtual void OnUpdateError(int error) OVERRIDE {
     EXPECT_NE(0, error);
@@ skipping 31 matching lines @@
 
 const char header_ok_reply[] =
     "HTTP/1.1 200 OK\0"
     "Content-type: text/html\0"
     "\0";
 }  // namespace
 
 // 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;

[asargent_no_longer_on_chrome, 2011/08/09 17:56:19]

Are you leaking test_config_ here? I don't see it getting cleaned up anywhere.
Also, could the member variable just be a full object instead of a pointer? It
doesn't look like you need to pass any arguments to the constructor or anything.

[cpu_(ooo_6.6-7.5), 2011/08/09 20:21:53]

The config is owned by the component update service
scoped_ptr<Config> config_

On 2011/08/09 17:56:19, Antony Sargent wrote:

+    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");
 
     // Subscribe to all component updater notifications.
     const int notifications[] = {
       chrome::NOTIFICATION_COMPONENT_UPDATER_STARTED,
       chrome::NOTIFICATION_COMPONENT_UPDATER_SLEEPING,
       chrome::NOTIFICATION_COMPONENT_UPDATE_FOUND,
       chrome::NOTIFICATION_COMPONENT_UPDATE_READY
@@ skipping 16 matching lines @@
 
   // Makes the full path to a component updater test file.
   const FilePath test_file(const char* file) {
     return test_data_dir_.AppendASCII(file);
   }
 
   TestNotificationTracker& notification_tracker() {
     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
 // be created and destroyed with no side effects.
 TEST_F(ComponentUpdaterTest, VerifyFixture) {
   EXPECT_TRUE(component_updater() != NULL);
   EXPECT_EQ(0ul, notification_tracker().size());
 }
 
 // Verify that the component updater can be caught in a quick
@@ skipping 34 matching lines @@
   component_updater()->RegisterComponent(com);
 
   const char expected_update_url[] =
       "http://localhost/upd?x=id%3D"
       "abagagagagagagagagagagagagagagag%26v%3D1.1%26uc";
 
   interceptor->SetResponse(expected_update_url,
                            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());
   TestNotificationTracker::Event ev1 = notification_tracker().at(0);
   EXPECT_EQ(chrome::NOTIFICATION_COMPONENT_UPDATER_STARTED, ev1.type);
 
   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());
 
   component_updater()->Stop();
 
   delete com.installer;
   xmlCleanupGlobals();
 }
 
@@ skipping 55 matching lines @@
   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());
 
   component_updater()->Stop();
   delete com1.installer;
   delete com2.installer;
   xmlCleanupGlobals();
 }