arc: Restore Arc OptIn notification.

arc: Restore Arc OptIn notification.

This is partical revert of commit f5deac28f3d4415546a424fd32c6e805aba7f56c.
There is new requirement to restore OptIn prompt notification for
M56 build. This CL partly reverts cut functionality and adopts it
to new code structure.

BUG=676039
TEST=Manually on device.

Committed: https://crrev.com/b0c29c8cb78921717f6f60f7cbf1c7c109670756
Cr-Commit-Position: refs/heads/master@{#439896}

[khmel, 2016-12-20 18:48:44]

khmel@chromium.org changed reviewers:
+ xiyuan@chromium.org

[khmel, 2016-12-20 18:48:44]

Hi Xiyuan,

PTAL

https://codereview.chromium.org/2593753002/diff/1/chrome/browser/chromeos/arc...
File chrome/browser/chromeos/arc/arc_session_manager.cc (right):

https://codereview.chromium.org/2593753002/diff/1/chrome/browser/chromeos/arc...
chrome/browser/chromeos/arc/arc_session_manager.cc:482:
!base::CommandLine::ForCurrentProcess()->HasSwitch(
New in this CL, don't show notification if Arc OOBE OptIn is active. This means
user was already prompted.

https://codereview.chromium.org/2593753002/diff/1/chrome/browser/chromeos/arc...
chrome/browser/chromeos/arc/arc_session_manager.cc:491: if
(!g_disable_ui_for_testing)
There is no CloseUI equivalent that was in previous impl. Make sure notification
is closed on shutdown.

https://codereview.chromium.org/2593753002/diff/1/chrome/browser/chromeos/arc...
chrome/browser/chromeos/arc/arc_session_manager.cc:555:
profile_->GetPrefs()->HasPrefPath(prefs::kArcEnabled)) {
There is no CloseUI equivalent that was in previous impl. Make sure notification
is closed on explicit user's action.

[xiyuan, 2016-12-20 19:00:06]

lgtm

https://codereview.chromium.org/2593753002/diff/1/chrome/browser/chromeos/arc...
File chrome/browser/chromeos/arc/arc_session_manager.cc (right):

https://codereview.chromium.org/2593753002/diff/1/chrome/browser/chromeos/arc...
chrome/browser/chromeos/arc/arc_session_manager.cc:491: if
(!g_disable_ui_for_testing)
nit: Do we need this condition? Think we can always call
ArcAuthNotification::Hide() here.

[khmel, 2016-12-20 19:13:12]

https://codereview.chromium.org/2593753002/diff/1/chrome/browser/chromeos/arc...
File chrome/browser/chromeos/arc/arc_session_manager.cc (right):

https://codereview.chromium.org/2593753002/diff/1/chrome/browser/chromeos/arc...
chrome/browser/chromeos/arc/arc_session_manager.cc:491: if
(!g_disable_ui_for_testing)
On 2016/12/20 19:00:06, xiyuan wrote:
> nit: Do we need this condition? Think we can always call
> ArcAuthNotification::Hide() here.

Thank you for pointing this. Agree that there is a temptation to remove this.
However message_center::MessageCenter is not available by default   in
unit_tests. We can address this issue later and probably add some unit tests
(however making Arc OptIn OOBE persistent would deprecate this). Considering
this urgent for M56 let merge it now.

[xiyuan, 2016-12-20 19:14:23]

https://codereview.chromium.org/2593753002/diff/1/chrome/browser/chromeos/arc...
File chrome/browser/chromeos/arc/arc_session_manager.cc (right):

https://codereview.chromium.org/2593753002/diff/1/chrome/browser/chromeos/arc...
chrome/browser/chromeos/arc/arc_session_manager.cc:491: if
(!g_disable_ui_for_testing)
On 2016/12/20 19:13:12, khmel wrote:
> On 2016/12/20 19:00:06, xiyuan wrote:
> > nit: Do we need this condition? Think we can always call
> > ArcAuthNotification::Hide() here.
> 
> Thank you for pointing this. Agree that there is a temptation to remove this.
> However message_center::MessageCenter is not available by default   in
> unit_tests. We can address this issue later and probably add some unit tests
> (however making Arc OptIn OOBE persistent would deprecate this). Considering
> this urgent for M56 let merge it now.

Acknowledged.

[khmel, 2016-12-20 19:14:56]

The CQ bit was checked by khmel@chromium.org

[commit-bot: I haz the power, 2016-12-20 19:15:31]

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-20 22:46:50]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1482261296940480, "parent_rev":
"45440bb3c14c0dc44a06d704142924a95d062e1f", "commit_rev":
"db81410a50a3514d4ae1185d33d576c35bfc9b31"}

[commit-bot: I haz the power, 2016-12-20 22:47:16]

Description was changed from

==========
arc: Restore Arc OptIn notification.

This is partical revert of commit f5deac28f3d4415546a424fd32c6e805aba7f56c.
There is new requirement to restore OptIn prompt notification for
M56 build. This CL partly reverts cut functionality and adopts it
to new code structure.

BUG=b/676039
TEST=Manually on device.
==========

to

==========
arc: Restore Arc OptIn notification.

This is partical revert of commit f5deac28f3d4415546a424fd32c6e805aba7f56c.
There is new requirement to restore OptIn prompt notification for
M56 build. This CL partly reverts cut functionality and adopts it
to new code structure.

BUG=b/676039
TEST=Manually on device.

Review-Url: https://codereview.chromium.org/2593753002
==========

[commit-bot: I haz the power, 2016-12-20 22:47:17]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-12-20 22:50:19]

Description was changed from

==========
arc: Restore Arc OptIn notification.

This is partical revert of commit f5deac28f3d4415546a424fd32c6e805aba7f56c.
There is new requirement to restore OptIn prompt notification for
M56 build. This CL partly reverts cut functionality and adopts it
to new code structure.

BUG=b/676039
TEST=Manually on device.

Review-Url: https://codereview.chromium.org/2593753002
==========

to

==========
arc: Restore Arc OptIn notification.

This is partical revert of commit f5deac28f3d4415546a424fd32c6e805aba7f56c.
There is new requirement to restore OptIn prompt notification for
M56 build. This CL partly reverts cut functionality and adopts it
to new code structure.

BUG=b/676039
TEST=Manually on device.

Committed: https://crrev.com/b0c29c8cb78921717f6f60f7cbf1c7c109670756
Cr-Commit-Position: refs/heads/master@{#439896}
==========

[commit-bot: I haz the power, 2016-12-20 22:50:20]

Patchset 1 (id:??) landed as
https://crrev.com/b0c29c8cb78921717f6f60f7cbf1c7c109670756
Cr-Commit-Position: refs/heads/master@{#439896}

[khmel, 2016-12-20 23:35:37]

Description was changed from

==========
arc: Restore Arc OptIn notification.

This is partical revert of commit f5deac28f3d4415546a424fd32c6e805aba7f56c.
There is new requirement to restore OptIn prompt notification for
M56 build. This CL partly reverts cut functionality and adopts it
to new code structure.

BUG=b/676039
TEST=Manually on device.

Committed: https://crrev.com/b0c29c8cb78921717f6f60f7cbf1c7c109670756
Cr-Commit-Position: refs/heads/master@{#439896}
==========

to

==========
arc: Restore Arc OptIn notification.

This is partical revert of commit f5deac28f3d4415546a424fd32c6e805aba7f56c.
There is new requirement to restore OptIn prompt notification for
M56 build. This CL partly reverts cut functionality and adopts it
to new code structure.

BUG=676039
TEST=Manually on device.

Committed: https://crrev.com/b0c29c8cb78921717f6f60f7cbf1c7c109670756
Cr-Commit-Position: refs/heads/master@{#439896}
==========