[cros] update screen re-factoring pt.1

chrome/browser/chromeos/login/update_screen_browsertest.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/chromeos/cros/mock_login_library.h"
 #include "chrome/browser/chromeos/cros/mock_network_library.h"
 #include "chrome/browser/chromeos/cros/mock_update_library.h"
 #include "chrome/browser/chromeos/login/mock_screen_observer.h"
 #include "chrome/browser/chromeos/login/update_screen.h"
 #include "chrome/browser/chromeos/login/wizard_controller.h"
@@ skipping 52 matching lines @@
         .RetiresOnSaturation();
     EXPECT_CALL(*mock_network_library_, AddNetworkManagerObserver(_))
         .Times(1)
         .RetiresOnSaturation();
     EXPECT_CALL(*mock_network_library_, FindWifiDevice())
         .Times(AnyNumber());
     EXPECT_CALL(*mock_network_library_, FindEthernetDevice())
         .Times(AnyNumber());
   }
 
+  virtual void SetUpOnMainThread() {
+    mock_screen_observer_.reset(new MockScreenObserver());
+    ASSERT_TRUE(controller() != NULL);
+    controller()->set_observer(mock_screen_observer_.get());
+    update_screen_ = controller()->GetUpdateScreen();
+    ASSERT_TRUE(update_screen_ != NULL);
+    ASSERT_EQ(controller()->current_screen(), update_screen_);
+  }
+
   virtual void TearDownInProcessBrowserTestFixture() {
+    controller()->set_observer(NULL);
     cros_mock_->test_api()->SetUpdateLibrary(NULL, true);
     WizardInProcessBrowserTest::TearDownInProcessBrowserTestFixture();
   }
 
+  void UpdateStatusChanged() {
+    static_cast<UpdateLibrary::Observer*>(update_screen_)->

[Paweł Hajdan Jr., 2011/05/19 14:23:42]

This is worrying - UpdateScreen inherits from UpdateLibrary::Observer. If
casting to the parent class results in a different behavior, it's a maintenance
problem. Otherwise you shouldn't need a cast.

[altimofeev, 2011/05/19 14:58:41]

I don't want this method to be used by anyone, except Notification service,
which treats the class as an Observer. Is it forbidden in our style guide?

[Paweł Hajdan Jr., 2011/05/19 16:22:07]

I think we generally don't bother with making interface-inherited methods
private. But that's something for the original reviewer to decide.

For the test, if you still want to keep that method private, please friend the
test class instead of using a cast. "friend UpdateScreenTest" should work fine.

[altimofeev, 2011/05/20 09:22:41]

Returned back to the public variant. Sorry for disturbing.

+        UpdateStatusChanged(mock_update_library_);
+  }
+
   MockLoginLibrary* mock_login_library_;
   MockUpdateLibrary* mock_update_library_;
   MockNetworkLibrary* mock_network_library_;
 
+  scoped_ptr<MockScreenObserver> mock_screen_observer_;
+  UpdateScreen* update_screen_;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(UpdateScreenTest);
 };
 
 IN_PROC_BROWSER_TEST_F(UpdateScreenTest, TestBasic) {
-  ASSERT_TRUE(controller() != NULL);
-  scoped_ptr<MockScreenObserver> mock_screen_observer(new MockScreenObserver());
-  controller()->set_observer(mock_screen_observer.get());
-  UpdateScreen* update_screen = controller()->GetUpdateScreen();
-  ASSERT_TRUE(update_screen != NULL);
-  ASSERT_EQ(controller()->current_screen(), update_screen);
-  UpdateView* update_view = update_screen->view();
-  ASSERT_TRUE(update_view != NULL);
-  controller()->set_observer(NULL);
+  ASSERT_TRUE(update_screen_->actor_.get() != NULL);
 }
 
 IN_PROC_BROWSER_TEST_F(UpdateScreenTest, TestNoUpdate) {
-  ASSERT_TRUE(controller() != NULL);
-  scoped_ptr<MockScreenObserver> mock_screen_observer(new MockScreenObserver());
-  controller()->set_observer(mock_screen_observer.get());
-  UpdateScreen* update_screen = controller()->GetUpdateScreen();
-  ASSERT_TRUE(update_screen != NULL);
-  ASSERT_EQ(controller()->current_screen(), update_screen);
-
   UpdateLibrary::Status status;
   status.status = UPDATE_STATUS_IDLE;
   EXPECT_CALL(*mock_update_library_, status())
       .Times(AtLeast(1))
       .WillRepeatedly(ReturnRef(status));
-  EXPECT_CALL(*mock_screen_observer, OnExit(ScreenObserver::UPDATE_NOUPDATE))
+  EXPECT_CALL(*mock_screen_observer_, OnExit(ScreenObserver::UPDATE_NOUPDATE))
       .Times(1);
-  update_screen->UpdateStatusChanged(mock_update_library_);
-
-  controller()->set_observer(NULL);
+  UpdateStatusChanged();
 }
 
 IN_PROC_BROWSER_TEST_F(UpdateScreenTest, TestUpdateAvailable) {
-  ASSERT_TRUE(controller() != NULL);
-  scoped_ptr<MockScreenObserver> mock_screen_observer(new MockScreenObserver());
-  controller()->set_observer(mock_screen_observer.get());
-  UpdateScreen* update_screen = controller()->GetUpdateScreen();
-  ASSERT_TRUE(update_screen != NULL);
-  ASSERT_EQ(controller()->current_screen(), update_screen);
-  update_screen->SetAllUpdatesCritical(true);
+  update_screen_->SetAllUpdatesCritical(true);
 
   UpdateLibrary::Status status;
 
   status.status = UPDATE_STATUS_UPDATE_AVAILABLE;
   status.new_version = "latest and greatest";
   EXPECT_CALL(*mock_update_library_, status())
       .Times(AtLeast(1))
       .WillRepeatedly(ReturnRef(status));
-  update_screen->UpdateStatusChanged(mock_update_library_);
+  UpdateStatusChanged();
 
   status.status = UPDATE_STATUS_DOWNLOADING;
   status.download_progress = 0.0;
   EXPECT_CALL(*mock_update_library_, status())
       .Times(AtLeast(1))
       .WillRepeatedly(ReturnRef(status));
-  update_screen->UpdateStatusChanged(mock_update_library_);
+  UpdateStatusChanged();
 
   status.download_progress = 0.5;
   EXPECT_CALL(*mock_update_library_, status())
       .Times(AtLeast(1))
       .WillRepeatedly(ReturnRef(status));
-  update_screen->UpdateStatusChanged(mock_update_library_);
+  UpdateStatusChanged();
 
   status.download_progress = 1.0;
   EXPECT_CALL(*mock_update_library_, status())
       .Times(AtLeast(1))
       .WillRepeatedly(ReturnRef(status));
-  update_screen->UpdateStatusChanged(mock_update_library_);
+  UpdateStatusChanged();
 
   status.status = UPDATE_STATUS_VERIFYING;
   EXPECT_CALL(*mock_update_library_, status())
       .Times(AtLeast(1))
       .WillRepeatedly(ReturnRef(status));
-  update_screen->UpdateStatusChanged(mock_update_library_);
+  UpdateStatusChanged();
 
   status.status = UPDATE_STATUS_FINALIZING;
   EXPECT_CALL(*mock_update_library_, status())
       .Times(AtLeast(1))
       .WillRepeatedly(ReturnRef(status));
-  update_screen->UpdateStatusChanged(mock_update_library_);
+  UpdateStatusChanged();
 
   status.status = UPDATE_STATUS_UPDATED_NEED_REBOOT;
   EXPECT_CALL(*mock_update_library_, status())
       .Times(AtLeast(1))
       .WillRepeatedly(ReturnRef(status));
   EXPECT_CALL(*mock_update_library_, RebootAfterUpdate())
       .Times(1);
-  update_screen->UpdateStatusChanged(mock_update_library_);
-
-  controller()->set_observer(NULL);
+  UpdateStatusChanged();
 }
 
 static void RequestUpdateCheckFail(UpdateCallback callback, void* userdata) {
   callback(userdata, chromeos::UPDATE_RESULT_FAILED, NULL);
 }
 
 IN_PROC_BROWSER_TEST_F(UpdateScreenTest, TestErrorIssuingUpdateCheck) {
-  ASSERT_TRUE(controller() != NULL);
-  scoped_ptr<MockScreenObserver> mock_screen_observer(new MockScreenObserver());
-  controller()->set_observer(mock_screen_observer.get());
-  UpdateScreen* update_screen = controller()->GetUpdateScreen();
-  ASSERT_TRUE(update_screen != NULL);
-  ASSERT_EQ(controller()->current_screen(), update_screen);
-
   // First, cancel the update that is already in progress.
-  EXPECT_CALL(*mock_screen_observer,
+  EXPECT_CALL(*mock_screen_observer_,
               OnExit(ScreenObserver::UPDATE_NOUPDATE))
       .Times(1);
-  update_screen->CancelUpdate();
+  update_screen_->CancelUpdate();
 
   // Run UpdateScreen::StartUpdate() again, but CheckForUpdate() will fail
   // issuing the update check this time.
   EXPECT_CALL(*mock_update_library_, AddObserver(_))
       .Times(1);
   EXPECT_CALL(*mock_update_library_, RemoveObserver(_))
       .Times(AtLeast(1));
   EXPECT_CALL(*mock_update_library_, RequestUpdateCheck(_,_))
       .Times(1)
       .WillOnce(Invoke(RequestUpdateCheckFail));
-  EXPECT_CALL(*mock_screen_observer,
+  EXPECT_CALL(*mock_screen_observer_,
               OnExit(ScreenObserver::UPDATE_ERROR_CHECKING_FOR_UPDATE))
       .Times(1);
-  update_screen->StartUpdate();
-
-  controller()->set_observer(NULL);
+  update_screen_->StartUpdate();
 }
 
 IN_PROC_BROWSER_TEST_F(UpdateScreenTest, TestErrorCheckingForUpdate) {
-  ASSERT_TRUE(controller() != NULL);
-  scoped_ptr<MockScreenObserver> mock_screen_observer(new MockScreenObserver());
-  controller()->set_observer(mock_screen_observer.get());
-  UpdateScreen* update_screen = controller()->GetUpdateScreen();
-  ASSERT_TRUE(update_screen != NULL);
-  ASSERT_EQ(controller()->current_screen(), update_screen);
-
   UpdateLibrary::Status status;
   status.status = UPDATE_STATUS_ERROR;
   EXPECT_CALL(*mock_update_library_, status())
       .Times(AtLeast(1))
       .WillRepeatedly(ReturnRef(status));
-  EXPECT_CALL(*mock_screen_observer,
+  EXPECT_CALL(*mock_screen_observer_,
               OnExit(ScreenObserver::UPDATE_ERROR_CHECKING_FOR_UPDATE))
       .Times(1);
-  update_screen->UpdateStatusChanged(mock_update_library_);
-
-  controller()->set_observer(NULL);
+  UpdateStatusChanged();
 }
 
 IN_PROC_BROWSER_TEST_F(UpdateScreenTest, TestErrorUpdating) {
-  ASSERT_TRUE(controller() != NULL);
-  scoped_ptr<MockScreenObserver> mock_screen_observer(new MockScreenObserver());
-  controller()->set_observer(mock_screen_observer.get());
-  UpdateScreen* update_screen = controller()->GetUpdateScreen();
-  ASSERT_TRUE(update_screen != NULL);
-  ASSERT_EQ(controller()->current_screen(), update_screen);
-
   UpdateLibrary::Status status;
-
   status.status = UPDATE_STATUS_UPDATE_AVAILABLE;
   status.new_version = "latest and greatest";
   EXPECT_CALL(*mock_update_library_, status())
       .Times(AtLeast(1))
       .WillRepeatedly(ReturnRef(status));
-  update_screen->UpdateStatusChanged(mock_update_library_);
+  UpdateStatusChanged();
 
   status.status = UPDATE_STATUS_ERROR;
   EXPECT_CALL(*mock_update_library_, status())
       .Times(AtLeast(1))
       .WillRepeatedly(ReturnRef(status));
-  EXPECT_CALL(*mock_screen_observer,
+  EXPECT_CALL(*mock_screen_observer_,
               OnExit(ScreenObserver::UPDATE_ERROR_UPDATING))
       .Times(1);
-  update_screen->UpdateStatusChanged(mock_update_library_);
-
-  controller()->set_observer(NULL);
+  UpdateStatusChanged();
 }
 
 }  // namespace chromeos