Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1050)

Issue 2745533005: Show notification during ARC managed provision (Closed)

Created:
1 year, 3 months ago by emaxx
Modified:
1 year, 2 months ago
CC:
chromium-reviews, elijahtaylor+arcwatch_chromium.org, oshima+watch_chromium.org, hidehiko+watch_chromium.org, lhchavez+watch_chromium.org, victorhsieh+watch_chromium.org, davemoore+watch_chromium.org, yusukes+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Show notification during ARC managed provision Display a notification during ARC provisioning when the opt-in flow happens silently (without showing the Terms of Service screen) due to enterprise policies. The notification is removed either when provisioning finishes successfully, or when it fails and some error is displayed. BUG=700609 BUG=b/35865652 TEST=New unit tests; Manual test: set up enterprise policies ArcEnabled, ArcBackupRestoreEnabled, ArcLocationServiceEnabled, log into a fresh profile, check that the notification about ARC being installed is shown, and after 1-2 minutes the notification goes away and Play Store is available. Review-Url: https://codereview.chromium.org/2745533005 Cr-Commit-Position: refs/heads/master@{#464707} Committed: https://chromium.googlesource.com/chromium/src/+/65e89bdb39e1f22f3032cfafdd3f2da1ba64a9b9

Patch Set 1 #

Patch Set 2 : Add tests, clean up #

Total comments: 16

Patch Set 3 : Renaming, nit fixes #

Total comments: 6

Patch Set 4 : Rebase #

Patch Set 5 : Address feedback. New icon. #

Total comments: 8

Patch Set 6 : Fix nits. Update notification message. #

Patch Set 7 : Update message #

Messages

Total messages: 63 (49 generated)
emaxx
Hidehiko, PTAL.
1 year, 3 months ago (2017-03-11 02:37:35 UTC) #14
hidehiko
https://codereview.chromium.org/2745533005/diff/10008/chrome/browser/chromeos/arc/arc_session_manager.cc File chrome/browser/chromeos/arc/arc_session_manager.cc (right): https://codereview.chromium.org/2745533005/diff/10008/chrome/browser/chromeos/arc/arc_session_manager.cc#newcode561 chrome/browser/chromeos/arc/arc_session_manager.cc:561: // TODO(hidehiko): DCHECK the previous state. This is called ...
1 year, 3 months ago (2017-03-13 05:10:47 UTC) #17
Luis Héctor Chávez
drive-by https://codereview.chromium.org/2745533005/diff/10008/chrome/browser/chromeos/arc/arc_session_manager.h File chrome/browser/chromeos/arc/arc_session_manager.h (right): https://codereview.chromium.org/2745533005/diff/10008/chrome/browser/chromeos/arc/arc_session_manager.h#newcode99 chrome/browser/chromeos/arc/arc_session_manager.h:99: virtual void OnArcStartOptInAndroidManagementCheck() {} nit: Events should be ...
1 year, 3 months ago (2017-03-13 15:50:41 UTC) #19
emaxx
Updated the CL, thanks for the comments Hidehiko and Luis Héctor. Note that I'll have ...
1 year, 3 months ago (2017-03-16 18:32:12 UTC) #26
Luis Héctor Chávez
https://codereview.chromium.org/2745533005/diff/10008/chrome/browser/chromeos/arc/notification/arc_provision_notification_service.h File chrome/browser/chromeos/arc/notification/arc_provision_notification_service.h (right): https://codereview.chromium.org/2745533005/diff/10008/chrome/browser/chromeos/arc/notification/arc_provision_notification_service.h#newcode28 chrome/browser/chromeos/arc/notification/arc_provision_notification_service.h:28: }; On 2017/03/16 18:32:12, emaxx wrote: > On 2017/03/13 ...
1 year, 3 months ago (2017-03-16 19:28:14 UTC) #27
emaxx
Hidehiko, Luis Héctor, PTAL at this review again: there have been no objections on this ...
1 year, 3 months ago (2017-03-24 14:13:15 UTC) #28
Luis Héctor Chávez
https://codereview.chromium.org/2745533005/diff/50001/chrome/browser/chromeos/arc/arc_session_manager.cc File chrome/browser/chromeos/arc/arc_session_manager.cc (right): https://codereview.chromium.org/2745533005/diff/50001/chrome/browser/chromeos/arc/arc_session_manager.cc#newcode531 chrome/browser/chromeos/arc/arc_session_manager.cc:531: // ArcPlayStoreEnabledPreferenceHandler Thanks for fixing the typo in L529, ...
1 year, 3 months ago (2017-03-24 17:39:16 UTC) #29
emaxx
Incorporated the latest requirements from the UI team - now the notification uses the special ...
1 year, 2 months ago (2017-04-11 02:09:49 UTC) #44
Luis Héctor Chávez
lgtm with one more nit, but please wait for hidehiko@'s sign-off. https://codereview.chromium.org/2745533005/diff/130001/chrome/browser/chromeos/arc/notification/arc_provision_notification_service.h File chrome/browser/chromeos/arc/notification/arc_provision_notification_service.h (right): ...
1 year, 2 months ago (2017-04-11 16:02:53 UTC) #45
hidehiko
LGTM with minor comments. https://codereview.chromium.org/2745533005/diff/130001/chrome/browser/chromeos/arc/arc_session_manager.h File chrome/browser/chromeos/arc/arc_session_manager.h (right): https://codereview.chromium.org/2745533005/diff/130001/chrome/browser/chromeos/arc/arc_session_manager.h#newcode114 chrome/browser/chromeos/arc/arc_session_manager.h:114: // the support app switches ...
1 year, 2 months ago (2017-04-12 04:51:34 UTC) #46
emaxx
+oshima@: Please review the changes under chrome/app/. Note that the icon and the notification text ...
1 year, 2 months ago (2017-04-12 13:06:46 UTC) #50
oshima
c/a/t lgtm
1 year, 2 months ago (2017-04-12 20:23:28 UTC) #57
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2745533005/170001
1 year, 2 months ago (2017-04-14 08:27:53 UTC) #60
commit-bot: I haz the power
1 year, 2 months ago (2017-04-14 09:24:32 UTC) #63
Message was sent while issue was closed.
Committed patchset #7 (id:170001) as
https://chromium.googlesource.com/chromium/src/+/65e89bdb39e1f22f3032cfafdd3f...

Powered by Google App Engine
This is Rietveld 408576698