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

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

Created:
1 year, 1 month ago by emaxx
Modified:
1 year 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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, 1 month 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 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 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 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 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 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 ago (2017-04-12 13:06:46 UTC) #50
oshima
c/a/t lgtm
1 year 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 ago (2017-04-14 08:27:53 UTC) #60
commit-bot: I haz the power
1 year 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