cros: Break BaseScreenHandler into two classes.

cros: Break BaseScreenHandler into two classes.

Not every type which derived from BaseScreenHandler was actually a
screen handler. Try to encapsulate that in the type hierarchy.

Store the screen type the handler models inside the handler to allow
dynamic fetching of the handler based on an OobeScreen instance.

Also try to make it clear when a Handler calls ShowScreen or similar
with it's own OobeScreen.

BUG=685287
Review-Url: https://codereview.chromium.org/2713513009
Cr-Commit-Position: refs/heads/master@{#454766}
Committed: https://chromium.googlesource.com/chromium/src/+/6dfeb5c0053434339445c146105c756334502ea9

[jdufault, 2017-02-25 01:25:49]

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

[commit-bot: I haz the power, 2017-02-25 01:26: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, 2017-02-25 01:28:41]

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

[commit-bot: I haz the power, 2017-02-25 01:28:42]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jdufault, 2017-02-25 01:30:33]

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

[commit-bot: I haz the power, 2017-02-25 01:30:42]

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-02-25 01:33:47]

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

[commit-bot: I haz the power, 2017-02-25 01:33:48]

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

[jdufault, 2017-02-27 21:19:17]

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

[commit-bot: I haz the power, 2017-02-27 21:20:17]

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-02-27 22:40:46]

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

[commit-bot: I haz the power, 2017-02-27 22:40:47]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jdufault, 2017-02-28 21:31:16]

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

[commit-bot: I haz the power, 2017-02-28 21:32:49]

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-02-28 22:49:02]

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

[commit-bot: I haz the power, 2017-02-28 22:49:02]

Dry run: This issue passed the CQ dry run.

[jdufault, 2017-02-28 22:51:00]

Patchset #1 (id:1) has been deleted

[jdufault, 2017-02-28 22:51:21]

Patchset #1 (id:20001) has been deleted

[jdufault, 2017-02-28 22:51:29]

Patchset #1 (id:40001) has been deleted

[jdufault, 2017-02-28 22:57:04]

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

[jdufault, 2017-02-28 22:57:05]

alemate@ PTAL.

[Alexander Alekseev, 2017-03-01 01:53:40]

https://codereview.chromium.org/2713513009/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/base_screen_handler.h (right):

https://codereview.chromium.org/2713513009/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/base_screen_handler.h:21: OobeScreen
oobe_screen() const { return oobe_screen_; }
It looks like BaseScreenHandler is now empty and can be removed.
So you are probably renaming it to BaseWebUiHandler, aren't you?

[jdufault, 2017-03-01 17:53:51]

https://codereview.chromium.org/2713513009/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/base_screen_handler.h (right):

https://codereview.chromium.org/2713513009/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/base_screen_handler.h:21: OobeScreen
oobe_screen() const { return oobe_screen_; }
On 2017/03/01 01:53:40, Alexander Alekseev wrote:
> It looks like BaseScreenHandler is now empty and can be removed.
> So you are probably renaming it to BaseWebUiHandler, aren't you?

I'd like to move more logic from BaseWebUIHandler to BaseScreenHandler, but it
is very challenging to do so at the moment.

The idea is that BaseScreenHandler is derived by any WebUI class directly
associated with an OobeScreen. Other WebUI classes (like the one for oobe or the
login page) are not associated with a specific OobeScreen so they derive from
BaseWebUIHandler.

[jdufault, 2017-03-02 21:06:43]

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

[commit-bot: I haz the power, 2017-03-02 21:07:14]

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-03-02 22:41:42]

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

[commit-bot: I haz the power, 2017-03-02 22:41:43]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[jdufault, 2017-03-02 22:43:33]

PTAL

[jdufault, 2017-03-02 22:44:02]

On 2017/03/02 22:43:33, jdufault wrote:
> PTAL

(addressed comments discussed offline)

[Alexander Alekseev, 2017-03-04 05:46:02]

The CQ bit was checked by alemate@chromium.org

[Alexander Alekseev, 2017-03-04 05:46:02]

lgtm

[commit-bot: I haz the power, 2017-03-04 05:46:07]

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-03-04 06:47:58]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1488606362377490,
"parent_rev": "52c98e8a1b2a5100406e2bdd5d4dc2f444e061c5", "commit_rev":
"6dfeb5c0053434339445c146105c756334502ea9"}

[commit-bot: I haz the power, 2017-03-04 06:49:15]

Description was changed from

==========
cros: Break BaseScreenHandler into two classes.

Not every type which derived from BaseScreenHandler was actually a
screen handler. Try to encapsulate that in the type hierarchy.

Store the screen type the handler models inside the handler to allow
dynamic fetching of the handler based on an OobeScreen instance.

Also try to make it clear when a Handler calls ShowScreen or similar
with it's own OobeScreen.

BUG=685287
==========

to

==========
cros: Break BaseScreenHandler into two classes.

Not every type which derived from BaseScreenHandler was actually a
screen handler. Try to encapsulate that in the type hierarchy.

Store the screen type the handler models inside the handler to allow
dynamic fetching of the handler based on an OobeScreen instance.

Also try to make it clear when a Handler calls ShowScreen or similar
with it's own OobeScreen.

BUG=685287

Review-Url: https://codereview.chromium.org/2713513009
Cr-Commit-Position: refs/heads/master@{#454766}
Committed:
https://chromium.googlesource.com/chromium/src/+/6dfeb5c0053434339445c146105c...
==========

[commit-bot: I haz the power, 2017-03-04 06:49:16]

Committed patchset #3 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/6dfeb5c0053434339445c146105c...