cros: Refactor of BaseScreen::GetName to BaseScreen::screen_id.

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

 // Copyright (c) 2012 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.
 
 #ifndef CHROME_BROWSER_CHROMEOS_LOGIN_SCREENS_BASE_SCREEN_H_
 #define CHROME_BROWSER_CHROMEOS_LOGIN_SCREENS_BASE_SCREEN_H_
 
 #include <string>
 
 #include "base/gtest_prod_util.h"
@@ skipping 10 matching lines @@
 
 class ModelViewChannel;
 
 // Base class for the all OOBE/login/before-session screens.
 // Screens are identified by ID, screen and it's JS counterpart must have same
 // id.
 // Most of the screens will be re-created for each appearance with Initialize()
 // method called just once.
 class BaseScreen {
  public:
-  explicit BaseScreen(BaseScreenDelegate* base_screen_delegate);
+  explicit BaseScreen(BaseScreenDelegate* base_screen_delegate,
+                      const std::string& screen_id);
   virtual ~BaseScreen();
 
   // ---- Old implementation ----
 
   // Makes wizard screen visible.
   virtual void Show() = 0;
 
   // Makes wizard screen invisible.
   virtual void Hide() = 0;
 
-  // Returns the screen name.
-  virtual std::string GetName() const = 0;
-
   // ---- New Implementation ----
 
   // Called to perform initialization of the screen. UI is guaranteed to exist
   // at this point. Screen can alter context, resulting context will be passed
   // to JS. This method will be called once per instance of the Screen object.
   virtual void Initialize(::login::ScreenContext* context);
 
   // Called when screen appears.
   virtual void OnShow();
   // Called when screen disappears, either because it finished it's work, or
   // because some other screen pops up.
   virtual void OnHide();
 
   // Called when we navigate from screen so that we will never return to it.
   // This is a last chance to call JS counterpart, this object will be deleted
   // soon.
   virtual void OnClose();
 
   // Indicates whether status area should be displayed while this screen is
   // displayed.
   virtual bool IsStatusAreaDisplayed();
 
   // Returns the identifier of the screen.
-  virtual std::string GetID() const;
+  std::string screen_id() const { return screen_id_; }

[achuithb, 2017/01/03 23:20:30]

return const reference.

Why screen_id instead of screen_name? screen_name seems more consistent with the
constants here:
https://cs.chromium.org/chromium/src/chrome/browser/chromeos/login/wizard_con...

[jdufault, 2017/01/04 21:49:27]

Done.
All of those constants are going away in
https://codereview.chromium.org/2566443005/.

Re id vs name, the previous WIP refactor (that was never finished) implied id as
the new name, which I think is reasonable because these strings all act as
unique identifiers; name does not imply the uniqueness.

 
   // Called when user action event with |event_id|
   // happened. Notification about this event comes from the JS
   // counterpart.
   virtual void OnUserAction(const std::string& action_id);
 
   void set_model_view_channel(ModelViewChannel* channel) { channel_ = channel; }
 
  protected:
   // Scoped context editor, which automatically commits all pending
@@ skipping 63 matching lines @@
   friend class ScreenManager;
   friend class UpdateScreenTest;
 
   void SetContext(::login::ScreenContext* context);
 
   // Called when context for the current screen was
   // changed. Notification about this event comes from the JS
   // counterpart.
   void OnContextChanged(const base::DictionaryValue& diff);
 
-  ModelViewChannel* channel_;
+  ModelViewChannel* channel_ = nullptr;
 
-  BaseScreenDelegate* base_screen_delegate_;
+  BaseScreenDelegate* base_screen_delegate_ = nullptr;
+
+  std::string screen_id_;

[achuithb, 2017/01/03 23:20:30]

make this const as well?

[jdufault, 2017/01/04 21:49:27]

Done.

 
   DISALLOW_COPY_AND_ASSIGN(BaseScreen);
 };
 
 }  // namespace chromeos
 
 #endif  // CHROME_BROWSER_CHROMEOS_LOGIN_SCREENS_BASE_SCREEN_H_