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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 months, 1 week ago by emaxx
Modified:
4 months, 1 week 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 
Commit queue not available (can’t edit this change).

Messages

Total messages: 63 (49 generated)
emaxx
Hidehiko, PTAL.
5 months, 1 week 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 ...
5 months, 1 week 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 ...
5 months, 1 week 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 ...
5 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 ...
5 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 ...
4 months, 4 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, ...
4 months, 4 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 ...
4 months, 1 week 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): ...
4 months, 1 week 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 ...
4 months, 1 week 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 ...
4 months, 1 week ago (2017-04-12 13:06:46 UTC) #50
oshima
c/a/t lgtm
4 months, 1 week 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
4 months, 1 week ago (2017-04-14 08:27:53 UTC) #60
commit-bot: I haz the power
4 months, 1 week 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b