OOBE: Refactor DeviceDisabledScreen/Handler API

1) Rename SetDelegate to Bind in accordance with other handlers.
2) Remove unnecessary page_is_ready()/show_on_init_ logic: calls
to ShowScreen / JSCall are automatically deferred until JS side is
initialized.
3) Pass screen parameters as the part of the Show method.

Bug: 1084928
Change-Id: I9c33b8b7330a04db430ead770eae8c347c95a5cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2210440
Commit-Queue: Denis Kuznetsov [CET] <antrim@chromium.org>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771000}

chrome/browser/chromeos/login/screens/device_disabled_screen.cc

@@ -23,12 +23,12 @@ DeviceDisabledScreen::DeviceDisabledScreen(DeviceDisabledScreenView* view)
     : BaseScreen(DeviceDisabledScreenView::kScreenId,
                  OobeScreenPriority::SCREEN_DEVICE_DISABLED),
       view_(view) {
-  view_->SetDelegate(this);
+  view_->Bind(this);
 }
 
 DeviceDisabledScreen::~DeviceDisabledScreen() {
   if (view_)
-    view_->SetDelegate(nullptr);
+    view_->Bind(nullptr);
 }
 
 void DeviceDisabledScreen::OnViewDestroyed(DeviceDisabledScreenView* view) {
@@ -36,26 +36,14 @@ void DeviceDisabledScreen::OnViewDestroyed(DeviceDisabledScreenView* view) {
     view_ = nullptr;
 }
 
-const std::string& DeviceDisabledScreen::GetEnrollmentDomain() const {
-  return DeviceDisablingManager()->enrollment_domain();
-}
-
-const std::string& DeviceDisabledScreen::GetMessage() const {
-  return DeviceDisablingManager()->disabled_message();
-}
-
-const std::string& DeviceDisabledScreen::GetSerialNumber() const {
-  return DeviceDisablingManager()->serial_number();
-}
-
 void DeviceDisabledScreen::ShowImpl() {
   if (!view_ || !is_hidden())
     return;
 
-  view_->Show();
+  view_->Show(DeviceDisablingManager()->serial_number(),
+              DeviceDisablingManager()->enrollment_domain(),
+              DeviceDisablingManager()->disabled_message());
   DeviceDisablingManager()->AddObserver(this);
-  if (!DeviceDisablingManager()->disabled_message().empty())
-    view_->UpdateMessage(DeviceDisablingManager()->disabled_message());
 }
 
 void DeviceDisabledScreen::HideImpl() {

chrome/browser/chromeos/login/screens/device_disabled_screen.h

@@ -28,15 +28,6 @@ class DeviceDisabledScreen : public BaseScreen,
   // destroyed first, it must call SetDelegate(nullptr).
   void OnViewDestroyed(DeviceDisabledScreenView* view);
 
-  // Returns the domain that owns the device.
-  const std::string& GetEnrollmentDomain() const;
-
-  // Returns the message that should be shown to the user.
-  const std::string& GetMessage() const;
-
-  // Returns the device serial number that should be shown to the user.
-  const std::string& GetSerialNumber() const;
-
   // system::DeviceDisablingManager::Observer:
   void OnDisabledMessageChanged(const std::string& disabled_message) override;
 

chrome/browser/chromeos/login/screens/mock_device_disabled_screen_view.cc

@@ -13,19 +13,19 @@ using ::testing::NotNull;
 namespace chromeos {
 
 MockDeviceDisabledScreenView::MockDeviceDisabledScreenView()
-    : delegate_(nullptr) {
-  EXPECT_CALL(*this, MockSetDelegate(NotNull())).Times(AtLeast(1));
-  EXPECT_CALL(*this, MockSetDelegate(nullptr)).Times(AtMost(1));
+    : screen_(nullptr) {
+  EXPECT_CALL(*this, MockBind(NotNull())).Times(AtLeast(1));
+  EXPECT_CALL(*this, MockBind(nullptr)).Times(AtMost(1));
 }
 
 MockDeviceDisabledScreenView::~MockDeviceDisabledScreenView() {
-  if (delegate_)
-    delegate_->OnViewDestroyed(this);
+  if (screen_)
+    screen_->OnViewDestroyed(this);
 }
 
-void MockDeviceDisabledScreenView::SetDelegate(DeviceDisabledScreen* delegate) {
-  delegate_ = delegate;
-  MockSetDelegate(delegate);
+void MockDeviceDisabledScreenView::Bind(DeviceDisabledScreen* screen) {
+  screen_ = screen;
+  MockBind(screen);
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/login/screens/mock_device_disabled_screen_view.h

@@ -15,16 +15,19 @@ class MockDeviceDisabledScreenView : public DeviceDisabledScreenView {
   MockDeviceDisabledScreenView();
   ~MockDeviceDisabledScreenView() override;
 
-  void SetDelegate(DeviceDisabledScreen* delegate) override;
+  void Bind(DeviceDisabledScreen* screen) override;
 
-  MOCK_METHOD0(Show, void());
+  MOCK_METHOD3(Show,
+               void(const std::string&,
+                    const std::string&,
+                    const std::string&));
   MOCK_METHOD0(Hide, void());
   MOCK_METHOD1(UpdateMessage, void(const std::string& message));
 
  private:
-  MOCK_METHOD1(MockSetDelegate, void(DeviceDisabledScreen* delegate));
+  MOCK_METHOD1(MockBind, void(DeviceDisabledScreen* delegate));
 
-  DeviceDisabledScreen* delegate_;
+  DeviceDisabledScreen* screen_;
 };
 
 }  // namespace chromeos

chrome/browser/chromeos/login/wizard_controller_browsertest.cc

@@ -525,7 +525,7 @@ class WizardControllerFlowTest : public WizardControllerTest {
         std::make_unique<MockDeviceDisabledScreenView>();
     MockScreen(std::make_unique<DeviceDisabledScreen>(
         device_disabled_screen_view_.get()));
-    EXPECT_CALL(*device_disabled_screen_view_, Show()).Times(0);
+    EXPECT_CALL(*device_disabled_screen_view_, Show(_, _, _)).Times(0);
 
     mock_network_screen_view_ = std::make_unique<MockNetworkScreenView>();
     mock_network_screen_ =
@@ -1237,9 +1237,8 @@ IN_PROC_BROWSER_TEST_F(WizardControllerDeviceStateTest,
   device_state.SetString(policy::kDeviceStateDisabledMessage, kDisabledMessage);
   g_browser_process->local_state()->Set(prefs::kServerBackedDeviceState,
                                         device_state);
-  EXPECT_CALL(*device_disabled_screen_view_, UpdateMessage(kDisabledMessage))
+  EXPECT_CALL(*device_disabled_screen_view_, Show(_, _, kDisabledMessage))
       .Times(1);
-  EXPECT_CALL(*device_disabled_screen_view_, Show()).Times(1);
   mock_auto_enrollment_check_screen_->ExitScreen();
 
   base::RunLoop().RunUntilIdle();
@@ -2776,7 +2775,7 @@ IN_PROC_BROWSER_TEST_F(WizardControllerDemoSetupDeviceDisabledTest,
   g_browser_process->local_state()->Set(prefs::kServerBackedDeviceState,
                                         device_state);
 
-  EXPECT_CALL(*device_disabled_screen_view_, Show()).Times(1);
+  EXPECT_CALL(*device_disabled_screen_view_, Show(_, _, _)).Times(1);
   mock_auto_enrollment_check_screen_->ExitScreen();
 
   base::RunLoop().RunUntilIdle();

chrome/browser/resources/chromeos/login/debug/debug.js

@@ -141,23 +141,33 @@ cr.define('cr.ui.login.debug', function() {
       id: 'device-disabled',
       kind: ScreenKind.ERROR,
       suffix: 'E',
+      data: {
+        serial: '1234567890',
+        domain: '',
+        message: 'Some custom message provided by org admin.',
+      },
       states: [
         {
           // No enrollment domain specified
           id: 'no-domain',
           trigger: (screen) => {
-            screen.setSerialNumberAndEnrollmentDomain('123456789', '');
-            screen.setMessage('Some custom message provided by org admin.');
-          }
+            screen.onBeforeShow({
+              serial: '1234567890',
+              domain: '',
+              message: 'Some custom message provided by org admin.',
+            });
+          },
         },
         {
           // Enrollment domain was specified
           id: 'has-domain',
           trigger: (screen) => {
-            screen.setSerialNumberAndEnrollmentDomain(
-                '123456789', 'example.com');
-            screen.setMessage('Please return this device to the techstop.');
-          }
+            screen.onBeforeShow({
+              serial: '1234567890',
+              domain: 'example.com',
+              message: 'Please return this device to the techstop.',
+            });
+          },
         },
       ]
     },

chrome/browser/resources/chromeos/login/device_disabled.js

@@ -11,11 +11,12 @@ Polymer({
 
   behaviors: [OobeI18nBehavior, OobeDialogHostBehavior, LoginScreenBehavior],
 
-  EXTERNAL_API: ['setSerialNumberAndEnrollmentDomain', 'setMessage'],
+  EXTERNAL_API: ['setMessage'],
 
   properties: {
     /**
      * Device serial number
+     * The serial number of the device.
      */
     serial_: {
       type: String,
@@ -23,7 +24,7 @@ Polymer({
     },
 
     /**
-     * Enrollment domain (can be empty)
+     * The domain that owns the device (can be empty).
      */
     enrollmentDomain_: {
       type: String,
@@ -37,7 +38,6 @@ Polymer({
       type: String,
       value: '',
     },
-
   },
 
 
@@ -60,17 +60,14 @@ Polymer({
     return this.$.dialog;
   },
 
-  /**
-   * Updates the explanation shown to the user. The explanation will indicate
-   * the device serial number and that it is owned by |enrollment_domain|. If
-   * |enrollment_domain| is null or empty, a generic explanation will be used
-   * instead that does not reference any domain.
-   * @param {string} serial_number The serial number of the device.
-   * @param {string} enrollment_domain The domain that owns the device.
-   */
-  setSerialNumberAndEnrollmentDomain(serial_number, enrollment_domain) {
-    this.serial_ = serial_number;
-    this.enrollmentDomain_ = enrollment_domain;
+  onBeforeShow(data) {
+    this.behaviors.forEach((behavior) => {
+      if (behavior.onBeforeShow)
+        behavior.onBeforeShow.call(this, data);
+    });
+    this.serial_ = data.serial;
+    this.enrollmentDomain_ = data.domain;
+    this.message_ = data.message;
   },
 
   /**
@@ -81,6 +78,12 @@ Polymer({
     this.message_ = message;
   },
 
+  /**
+   * Updates the explanation shown to the user. The explanation will indicate
+   * the device serial number and that it is owned by |domain|. If |domain| is
+   * null or empty, a generic explanation will be used instead that does not
+   * reference any domain.
+   */
   disabledText_(locale, serial, domain) {
     if (domain) {
       return this.i18n('deviceDisabledExplanationWithDomain', serial, domain);

chrome/browser/ui/webui/chromeos/login/device_disabled_screen_handler.cc

@@ -20,37 +20,30 @@ DeviceDisabledScreenHandler::DeviceDisabledScreenHandler(
 }
 
 DeviceDisabledScreenHandler::~DeviceDisabledScreenHandler() {
-  if (delegate_)
-    delegate_->OnViewDestroyed(this);
+  if (screen_)
+    screen_->OnViewDestroyed(this);
 }
 
-void DeviceDisabledScreenHandler::Show() {
-  if (!page_is_ready()) {
-    show_on_init_ = true;
-    return;
-  }
-
-  if (delegate_) {
-    CallJS("login.DeviceDisabledScreen.setSerialNumberAndEnrollmentDomain",
-           delegate_->GetSerialNumber(), delegate_->GetEnrollmentDomain());
-    CallJS("login.DeviceDisabledScreen.setMessage", delegate_->GetMessage());
-  }
-  ShowScreen(kScreenId);
+void DeviceDisabledScreenHandler::Show(const std::string& serial,
+                                       const std::string& domain,
+                                       const std::string& message) {
+  base::DictionaryValue screen_data;
+  screen_data.SetStringPath("serial", serial);
+  screen_data.SetStringPath("domain", domain);
+  screen_data.SetStringPath("message", message);
+  ShowScreenWithData(kScreenId, &screen_data);
 }
 
 void DeviceDisabledScreenHandler::Hide() {
-  show_on_init_ = false;
+  NOTREACHED() << "Device should reboot upon removing device disabled flag";
 }
 
-void DeviceDisabledScreenHandler::SetDelegate(DeviceDisabledScreen* delegate) {
-  delegate_ = delegate;
-  if (page_is_ready())
-    Initialize();
+void DeviceDisabledScreenHandler::Bind(DeviceDisabledScreen* screen) {
+  screen_ = screen;
 }
 
 void DeviceDisabledScreenHandler::UpdateMessage(const std::string& message) {
-  if (page_is_ready())
-    CallJS("login.DeviceDisabledScreen.setMessage", message);
+  CallJS("login.DeviceDisabledScreen.setMessage", message);
 }
 
 void DeviceDisabledScreenHandler::DeclareLocalizedValues(
@@ -63,13 +56,6 @@ void DeviceDisabledScreenHandler::DeclareLocalizedValues(
 }
 
 void DeviceDisabledScreenHandler::Initialize() {
-  if (!page_is_ready() || !delegate_)
-    return;
-
-  if (show_on_init_) {
-    Show();
-    show_on_init_ = false;
-  }
 }
 
 void DeviceDisabledScreenHandler::RegisterMessages() {

chrome/browser/ui/webui/chromeos/login/device_disabled_screen_handler.h

@@ -19,9 +19,11 @@ class DeviceDisabledScreenView {
 
   virtual ~DeviceDisabledScreenView() {}
 
-  virtual void Show() = 0;
+  virtual void Show(const std::string& serial,
+                    const std::string& domain,
+                    const std::string& message) = 0;
   virtual void Hide() = 0;
-  virtual void SetDelegate(DeviceDisabledScreen* delegate) = 0;
+  virtual void Bind(DeviceDisabledScreen* screen) = 0;
   virtual void UpdateMessage(const std::string& message) = 0;
 };
 
@@ -35,9 +37,11 @@ class DeviceDisabledScreenHandler : public DeviceDisabledScreenView,
   ~DeviceDisabledScreenHandler() override;
 
   // DeviceDisabledScreenActor:
-  void Show() override;
+  void Show(const std::string& serial,
+            const std::string& domain,
+            const std::string& message) override;
   void Hide() override;
-  void SetDelegate(DeviceDisabledScreen* delegate) override;
+  void Bind(DeviceDisabledScreen* screen) override;
   void UpdateMessage(const std::string& message) override;
 
   // BaseScreenHandler:
@@ -49,10 +53,7 @@ class DeviceDisabledScreenHandler : public DeviceDisabledScreenView,
   // WebUIMessageHandler:
   void RegisterMessages() override;
 
-  DeviceDisabledScreen* delegate_ = nullptr;
-
-  // Indicates whether the screen should be shown right after initialization.
-  bool show_on_init_ = false;
+  DeviceDisabledScreen* screen_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(DeviceDisabledScreenHandler);
 };