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

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

This removes the virtual method GetName in favor of just storing the name directly on the BaseScreen instance and returning it via a new method screen_id. Previously, all of the GetName overrides just returned a value directly.

BUG=672142
Review-Url: https://codereview.chromium.org/2549373005
Cr-Commit-Position: refs/heads/master@{#441742}
Committed: https://chromium.googlesource.com/chromium/src/+/453b8869061af7fb18fc773dc4e585229bad6bb9

[jdufault, 2016-12-06 23:55:55]

The CQ bit was checked by jdufault@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-06 23:56:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-07 00:09:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-07 00:09:42]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[jdufault, 2016-12-07 19:10:58]

Description was changed from

==========
cros: GetName => GetId

BUG=
==========

to

==========
cros: Finish refactor of BaseScreen::GetName to BaseScreen::GetId.

BUG=672142
==========

[jdufault, 2016-12-07 19:10:58]

jdufault@chromium.org changed reviewers:
+ achuith@chromium.org

[jdufault, 2016-12-07 19:11:29]

achuith@, PTAL. Low priority. This CL does not contain any real changes besides
renames.

[jdufault, 2016-12-08 21:39:54]

On 2016/12/07 19:11:29, jdufault wrote:
> achuith@, PTAL. Low priority. This CL does not contain any real changes
besides
> renames.

Ignore CL for time being. I think there is going to be a better name than
GetId()

[jdufault, 2016-12-08 23:39:24]

The CQ bit was checked by jdufault@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-08 23:40:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-09 00:54:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-09 00:54:40]

Dry run: This issue passed the CQ dry run.

[jdufault, 2016-12-09 20:09:20]

The CQ bit was checked by jdufault@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 20:10:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jdufault, 2016-12-09 20:37:46]

The CQ bit was checked by jdufault@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 20:38:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jdufault, 2016-12-09 20:42:02]

jdufault@chromium.org changed reviewers:
+ alemate@chromium.org

[jdufault, 2016-12-09 20:42:15]

alemate@ PTAL.

[commit-bot: I haz the power, 2016-12-09 21:58:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-09 21:58:56]

Dry run: This issue passed the CQ dry run.

[jdufault, 2016-12-14 20:29:28]

Description was changed from

==========
cros: Finish refactor of BaseScreen::GetName to BaseScreen::GetId.

BUG=672142
==========

to

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

This removes the virtual method GetName in favor of just storing the name
directly on the BaseScreen instance and returning it via a new method screen_id.
Previously, all of the GetName overrides just returned a value directly.

BUG=672142
==========

[jdufault, 2017-01-03 19:17:25]

On 2016/12/09 20:42:15, jdufault wrote:
> alemate@ PTAL.

Ping

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

https://codereview.chromium.org/2549373005/diff/50001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/screens/base_screen.h (right):

https://codereview.chromium.org/2549373005/diff/50001/chrome/browser/chromeos...
chrome/browser/chromeos/login/screens/base_screen.h:66: std::string screen_id()
const { return screen_id_; }
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...

https://codereview.chromium.org/2549373005/diff/50001/chrome/browser/chromeos...
chrome/browser/chromeos/login/screens/base_screen.h:154: std::string screen_id_;
make this const as well?

[jdufault, 2017-01-04 20:24:38]

The CQ bit was checked by jdufault@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-04 20:25:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-04 20:53:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-04 20:53:52]

Dry run: This issue passed the CQ dry run.

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

https://codereview.chromium.org/2549373005/diff/50001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/screens/base_screen.h (right):

https://codereview.chromium.org/2549373005/diff/50001/chrome/browser/chromeos...
chrome/browser/chromeos/login/screens/base_screen.h:66: std::string screen_id()
const { return screen_id_; }
On 2017/01/03 23:20:30, achuithb wrote:
> return const reference.
Done.

> 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...

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.

https://codereview.chromium.org/2549373005/diff/50001/chrome/browser/chromeos...
chrome/browser/chromeos/login/screens/base_screen.h:154: std::string screen_id_;
On 2017/01/03 23:20:30, achuithb wrote:
> make this const as well?

Done.

[achuithb, 2017-01-04 22:29:03]

lgtm

[achuithb, 2017-01-04 22:29:42]

Feel free to ignore since this isn't your code.

https://codereview.chromium.org/2549373005/diff/70001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/wizard_controller.cc (right):

https://codereview.chromium.org/2549373005/diff/70001/chrome/browser/chromeos...
chrome/browser/chromeos/login/wizard_controller.cc:984: std::string screen_id =
new_current->screen_id();
nit: const

https://codereview.chromium.org/2549373005/diff/70001/chrome/browser/chromeos...
chrome/browser/chromeos/login/wizard_controller.cc:1088: std::string
previous_screen_id = current_screen_->screen_id();
nit: const

[jdufault, 2017-01-05 20:09:47]

The CQ bit was checked by jdufault@chromium.org

[jdufault, 2017-01-05 20:09:47]

The patchset sent to the CQ was uploaded after l-g-t-m from achuith@chromium.org
Link to the patchset: https://codereview.chromium.org/2549373005/#ps90001
(title: "Nits")

[jdufault, 2017-01-05 20:10:04]

https://codereview.chromium.org/2549373005/diff/70001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/wizard_controller.cc (right):

https://codereview.chromium.org/2549373005/diff/70001/chrome/browser/chromeos...
chrome/browser/chromeos/login/wizard_controller.cc:984: std::string screen_id =
new_current->screen_id();
On 2017/01/04 22:29:41, achuithb wrote:
> nit: const

Done.

https://codereview.chromium.org/2549373005/diff/70001/chrome/browser/chromeos...
chrome/browser/chromeos/login/wizard_controller.cc:1088: std::string
previous_screen_id = current_screen_->screen_id();
On 2017/01/04 22:29:41, achuithb wrote:
> nit: const

Done.

[commit-bot: I haz the power, 2017-01-05 20:10:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-05 20:41:40]

CQ is committing da patch.

Bot data: {"patchset_id": 90001, "attempt_start_ts": 1483646987193490,
"parent_rev": "9624455dac5989ff22a4a26021b1344f6b7e3e27", "commit_rev":
"453b8869061af7fb18fc773dc4e585229bad6bb9"}

[commit-bot: I haz the power, 2017-01-05 20:42:14]

Description was changed from

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

This removes the virtual method GetName in favor of just storing the name
directly on the BaseScreen instance and returning it via a new method screen_id.
Previously, all of the GetName overrides just returned a value directly.

BUG=672142
==========

to

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

This removes the virtual method GetName in favor of just storing the name
directly on the BaseScreen instance and returning it via a new method screen_id.
Previously, all of the GetName overrides just returned a value directly.

BUG=672142

Review-Url: https://codereview.chromium.org/2549373005
Cr-Commit-Position: refs/heads/master@{#441742}
Committed:
https://chromium.googlesource.com/chromium/src/+/453b8869061af7fb18fc773dc4e5...
==========

[commit-bot: I haz the power, 2017-01-05 20:42:15]

Committed patchset #6 (id:90001) as
https://chromium.googlesource.com/chromium/src/+/453b8869061af7fb18fc773dc4e5...