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

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

Created:
9 months ago by emaxx
Modified:
8 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 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+561 lines, -18 lines) Patch
M chrome/app/chromeos_strings.grdp View 1 2 3 4 5 6 1 chunk +6 lines, -0 lines 0 comments Download
A chrome/app/theme/default_100_percent/cros/notification_play_store_optin_in_progress.png View 1 2 3 4 Binary file 0 comments Download
A chrome/app/theme/default_200_percent/cros/notification_play_store_optin_in_progress.png View 1 2 3 4 Binary file 0 comments Download
M chrome/app/theme/theme_resources.grd View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/chromeos/BUILD.gn View 1 2 3 4 5 2 chunks +3 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/arc/arc_service_launcher.cc View 1 2 3 2 chunks +3 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/arc/arc_session_manager.h View 1 2 3 4 5 3 chunks +15 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/arc/arc_session_manager.cc View 1 2 3 4 8 chunks +22 lines, -18 lines 0 comments Download
A chrome/browser/chromeos/arc/notification/arc_provision_notification_service.h View 1 2 3 4 5 1 chunk +64 lines, -0 lines 0 comments Download
A chrome/browser/chromeos/arc/notification/arc_provision_notification_service.cc View 1 2 3 4 5 1 chunk +148 lines, -0 lines 0 comments Download
A chrome/browser/chromeos/arc/notification/arc_provision_notification_service_unittest.cc View 1 2 3 4 5 1 chunk +299 lines, -0 lines 0 comments Download
Trybot results:  win_clang   win_chromium_x64_rel_ng   win_chromium_compile_dbg_ng   win_chromium_rel_ng   mac_chromium_rel_ng   mac_chromium_compile_dbg_ng   ios-simulator-xcode-clang   ios-device-xcode-clang   ios-simulator   linux_chromium_tsan_rel_ng   linux_chromium_rel_ng   ios-device   linux_chromium_chromeos_rel_ng   linux_chromium_compile_dbg_ng   chromium_presubmit   linux_chromium_asan_rel_ng   linux_chromium_chromeos_ozone_rel_ng   cast_shell_linux   chromeos_daisy_chromium_compile_only_ng   chromeos_amd64-generic_chromium_compile_only_ng   linux_android_rel_ng   cast_shell_android   android_compile_dbg   android_cronet   android_n5x_swarming_rel   android_clang_dbg_recipe   android_arm64_dbg_recipe   win_clang   win_chromium_x64_rel_ng   win_chromium_rel_ng   win_chromium_compile_dbg_ng   mac_chromium_rel_ng   mac_chromium_compile_dbg_ng   ios-simulator-xcode-clang   ios-device   ios-device-xcode-clang   ios-simulator   linux_chromium_compile_dbg_ng   linux_chromium_tsan_rel_ng   linux_chromium_rel_ng   linux_chromium_chromeos_rel_ng   linux_chromium_asan_rel_ng   chromium_presubmit   chromeos_daisy_chromium_compile_only_ng   linux_chromium_chromeos_ozone_rel_ng   cast_shell_linux   linux_android_rel_ng   chromeos_amd64-generic_chromium_compile_only_ng   cast_shell_android   android_n5x_swarming_rel   android_cronet   android_clang_dbg_recipe   android_compile_dbg   android_arm64_dbg_recipe 

Messages

Total messages: 63 (49 generated)
emaxx
Hidehiko, PTAL.
9 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 ...
9 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 ...
9 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 ...
8 months, 4 weeks 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 ...
8 months, 4 weeks 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 ...
8 months, 3 weeks 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, ...
8 months, 3 weeks ago (2017-03-24 17:39:16 UTC) #29
emaxx
Incorporated the latest requirements from the UI team - now the notification uses the special ...
8 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): ...
8 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 ...
8 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 ...
8 months ago (2017-04-12 13:06:46 UTC) #50
oshima
c/a/t lgtm
8 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
8 months ago (2017-04-14 08:27:53 UTC) #60
commit-bot: I haz the power
8 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 0eb02b776