Add value proposition entrypoint from Voice Interaction shortcut

Add value proposition entrypoint from Voice Interaction shortcut

- Voice interaction keyboard shortcut should bring up voice interaction
  value prop screen if voice interaction is disabled.
- If playstore is already set up, start voice interaction setup wizard.
- Otherwise show playstore terms, and start voice interaction setup
  wizard afterwards if user accept.

BUG=b/38263948

Review-Url: https://codereview.chromium.org/2929963002
Cr-Commit-Position: refs/heads/master@{#478763}
Committed: https://chromium.googlesource.com/chromium/src/+/8dbd1920a8016c49d059d852fb55c03f716257ad

[yueli, 2017-06-08 23:26:56]

updowndota@chromium.org changed reviewers:
+ xiaohuic@chromium.org, xiyuan@chromium.org

[yueli, 2017-06-08 23:26:57]

PTAL

[xc, 2017-06-09 18:00:14]

https://codereview.chromium.org/2929963002/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/wizard_controller.cc (right):

https://codereview.chromium.org/2929963002/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/wizard_controller.cc:1564: if
(!device_marked_registered_ &&
Add comment on why we need to do these check this way, it is not obvious.  This
is essentially adding an alternative flow into SUW.

[Luis Héctor Chávez, 2017-06-09 18:15:06]

lhchavez@chromium.org changed reviewers:
+ lhchavez@chromium.org

[Luis Héctor Chávez, 2017-06-09 18:15:07]

drive-by

https://codereview.chromium.org/2929963002/diff/1/chrome/browser/chromeos/arc...
File
chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc
(right):

https://codereview.chromium.org/2929963002/diff/1/chrome/browser/chromeos/arc...
chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc:374:
new chromeos::LoginDisplayHostImpl(screen_bounds);
Please add a comment about the lifetime of this object. Just by reading this
code it appears as if you were leaking memory.

[xiyuan, 2017-06-12 17:19:56]

https://codereview.chromium.org/2929963002/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/wizard_controller.cc (right):

https://codereview.chromium.org/2929963002/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/wizard_controller.cc:258:
device_marked_registered_ = StartupUtils::IsDeviceRegistered();
|device_marked_registered_| does not look like related to voice interaction
feature. Can you use an explicit flag instead?

https://codereview.chromium.org/2929963002/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/wizard_controller.cc:877:
host_->Finalize(std::move(empty_callback));
Since you don't care about the callback, you can just do
  host_->Finalize(base::OnceClosure());

[yueli, 2017-06-12 20:10:08]

https://codereview.chromium.org/2929963002/diff/1/chrome/browser/chromeos/arc...
File
chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc
(right):

https://codereview.chromium.org/2929963002/diff/1/chrome/browser/chromeos/arc...
chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc:374:
new chromeos::LoginDisplayHostImpl(screen_bounds);
On 2017/06/09 18:15:07, Luis Héctor Chávez wrote:
> Please add a comment about the lifetime of this object. Just by reading this
> code it appears as if you were leaking memory.

Done.

https://codereview.chromium.org/2929963002/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/wizard_controller.cc (right):

https://codereview.chromium.org/2929963002/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/wizard_controller.cc:258:
device_marked_registered_ = StartupUtils::IsDeviceRegistered();
On 2017/06/12 17:19:56, xiyuan wrote:
> |device_marked_registered_| does not look like related to voice interaction
> feature. Can you use an explicit flag instead?

How about use IsSessionStarted instead and add comments about it?

https://codereview.chromium.org/2929963002/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/wizard_controller.cc:877:
host_->Finalize(std::move(empty_callback));
On 2017/06/12 17:19:56, xiyuan wrote:
> Since you don't care about the callback, you can just do
>   host_->Finalize(base::OnceClosure());

Done.

https://codereview.chromium.org/2929963002/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/wizard_controller.cc:1564: if
(!device_marked_registered_ &&
On 2017/06/09 18:00:14, xc wrote:
> Add comment on why we need to do these check this way, it is not obvious. 
This
> is essentially adding an alternative flow into SUW.

Done.

[xiyuan, 2017-06-12 20:18:12]

lgtm

[yueli, 2017-06-12 20:22:11]

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

[commit-bot: I haz the power, 2017-06-12 20:22:35]

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-06-12 21:04:14]

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

[commit-bot: I haz the power, 2017-06-12 21:04:15]

Dry run: This issue passed the CQ dry run.

[yueli, 2017-06-12 21:05:17]

The CQ bit was checked by updowndota@chromium.org

[commit-bot: I haz the power, 2017-06-12 21:06:03]

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-06-12 21:09:53]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1497301517967150,
"parent_rev": "3f99a04b7cba08ec751f2e646a8180bbf7f870af", "commit_rev":
"8dbd1920a8016c49d059d852fb55c03f716257ad"}

[commit-bot: I haz the power, 2017-06-12 21:10:06]

Description was changed from

==========
Add value proposition entrypoint from Voice Interaction shortcut

- Voice interaction keyboard shortcut should bring up voice interaction
  value prop screen if voice interaction is disabled.
- If playstore is already set up, start voice interaction setup wizard.
- Otherwise show playstore terms, and start voice interaction setup
  wizard afterwards if user accept.

BUG=b/38263948
==========

to

==========
Add value proposition entrypoint from Voice Interaction shortcut

- Voice interaction keyboard shortcut should bring up voice interaction
  value prop screen if voice interaction is disabled.
- If playstore is already set up, start voice interaction setup wizard.
- Otherwise show playstore terms, and start voice interaction setup
  wizard afterwards if user accept.

BUG=b/38263948

Review-Url: https://codereview.chromium.org/2929963002
Cr-Commit-Position: refs/heads/master@{#478763}
Committed:
https://chromium.googlesource.com/chromium/src/+/8dbd1920a8016c49d059d852fb55...
==========

[commit-bot: I haz the power, 2017-06-12 21:10:08]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/8dbd1920a8016c49d059d852fb55...