Add a way to properly disable ARC when the user postponed doing ext4-crypto migration. |
||||||||||
Issue descriptionWe'll give a choice for the user to do the migration later, but in that case we temporarily want to suppress ARC from running. We need a way to cleanly achieve that. @hidehiko, can I discuss on this some time later?
,
Mar 29 2017
Updates after discussion with hidehiko@ and investigation after that: The thing is, if we can let arc::IsArcAllowedForProfile() https://cs.chromium.org/chromium/src/chrome/browser/chromeos/arc/arc_util.cc?q=IsArcAllowedForProfil+package:%5Echromium$&l=28 return false, all ARC related stuff can be gracefully disabled. And probably this is the "only if" condition. The question is how to do this. There are two approaches: 1) Pass some flag from the login manager to here when the user postponed migration. The problem of this approach is the case of Chrome browser crash and restart. When it happens, the session is restarted without going through the login manager. Since an on-memory flag does not survive the browser crash, to cover this case we probably need to notify the situation from session-manager to Chrome as well. 2) Let the check function itself detect the bad combination (= "N" on "ecryptfs"), independent from the migration wizard. The problem of this approach is that it requires disk IO or Dbus call to cryptohome. Since we need to detect the situation in a relatively early stage of Chrome startup, before KeyedServices of Profiles are initialized, it's not clear where to insert this async task ping-pong. I'm continuing to investigate both approaches.
,
Apr 5 2017
Update: CL for completely disabling ARC https://codereview.chromium.org/2788383003/ is getting to be in a good shape. I'll land this first. Request from UX/PM/Leads is that we still want to keep ARC app launcher/taskbar icons (and inform about migration when they are clicked.) I'll proceed to this direction next.
,
Apr 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/715a7d91b74fce5547c7a643a092898a7abebe50 commit 715a7d91b74fce5547c7a643a092898a7abebe50 Author: kinaba <kinaba@chromium.org> Date: Thu Apr 06 03:13:45 2017 ChromeOS: Disable ARC when incompatible filesystem is detected. Checks both the user data dir's filesystem and Android version to determine the status. The tricky case is when the browser is crashed and restarted. In that case Profile creation does not go through UserSessionManager. There are three cases to consider: * If the browser has crashed after flushing the pref, restart session correctly reads the value. * If the browser has crashed before flushing the pref in the first sign-in after getting this change, the default value "false" is read. * If the browser has crashed before flushing the pref in subsequent runs, it reads the previously set value--which is safe. Since we never do backward (true->false) migration. We don't see false-positive even though we may encounter false-negative (incorrectly disabling ARC.) Note that the assumption of the 3rd case is valid because we do the ARC version check separately. We may do ARC uprev that can turn the disabling condition true->false as a whole, but that does not revert the filesystem check result that is stored in pref. BUG= 699850 TEST=Manually tested all fs+version combinations Review-Url: https://codereview.chromium.org/2788383003 Cr-Commit-Position: refs/heads/master@{#462339} [modify] https://crrev.com/715a7d91b74fce5547c7a643a092898a7abebe50/chrome/browser/chromeos/arc/arc_session_manager.cc [modify] https://crrev.com/715a7d91b74fce5547c7a643a092898a7abebe50/chrome/browser/chromeos/arc/arc_util.cc [modify] https://crrev.com/715a7d91b74fce5547c7a643a092898a7abebe50/chrome/browser/chromeos/arc/arc_util.h [modify] https://crrev.com/715a7d91b74fce5547c7a643a092898a7abebe50/chrome/browser/chromeos/login/session/user_session_manager.cc [modify] https://crrev.com/715a7d91b74fce5547c7a643a092898a7abebe50/chrome/browser/chromeos/login/session/user_session_manager.h [modify] https://crrev.com/715a7d91b74fce5547c7a643a092898a7abebe50/chrome/common/pref_names.cc [modify] https://crrev.com/715a7d91b74fce5547c7a643a092898a7abebe50/chrome/common/pref_names.h
,
Apr 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/21c492878576af55fb89bf56efd9cbed20ef4eaa commit 21c492878576af55fb89bf56efd9cbed20ef4eaa Author: kinaba <kinaba@chromium.org> Date: Mon Apr 10 14:18:27 2017 arc: Add IsArcAllowedInAppListForProfile. This will be used to keep app list even when ARC is disabled due to filesystem incompatibility. This CL just splits IsArcAllowedForProfile for use in subsequent changes. BUG= 699850 TEST=ChromeArcUtilTest Review-Url: https://codereview.chromium.org/2806443002 Cr-Commit-Position: refs/heads/master@{#463255} [modify] https://crrev.com/21c492878576af55fb89bf56efd9cbed20ef4eaa/chrome/browser/chromeos/arc/arc_util.cc [modify] https://crrev.com/21c492878576af55fb89bf56efd9cbed20ef4eaa/chrome/browser/chromeos/arc/arc_util.h [modify] https://crrev.com/21c492878576af55fb89bf56efd9cbed20ef4eaa/chrome/browser/chromeos/arc/arc_util_unittest.cc
,
Apr 11 2017
,
Apr 11 2017
,
Apr 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6cb1c9a147eff32e3bacec76f0700c6a401c44d2 commit 6cb1c9a147eff32e3bacec76f0700c6a401c44d2 Author: kinaba <kinaba@chromium.org> Date: Tue Apr 11 02:23:27 2017 arc: Notification for guiding the user to filesystem migration. When ARC is temporarily disabled due to filesystem incompatibility, we want to keep the launcher/tray icons for ARC apps displayed, and when the icons are clicked we want to guide the user for proper reconfiguration step (i.e., sign back in and complete migration.) This CL implements the latter "show notificaiton" part. No actual code should hit this notification yet--after this change I'll change several conditions in the app list code to make them alive. BUG= 699850 Review-Url: https://codereview.chromium.org/2813633002 Cr-Commit-Position: refs/heads/master@{#463499} [modify] https://crrev.com/6cb1c9a147eff32e3bacec76f0700c6a401c44d2/chrome/app/chromeos_strings.grdp [modify] https://crrev.com/6cb1c9a147eff32e3bacec76f0700c6a401c44d2/chrome/browser/chromeos/BUILD.gn [add] https://crrev.com/6cb1c9a147eff32e3bacec76f0700c6a401c44d2/chrome/browser/chromeos/arc/arc_migration_guide_notification.cc [add] https://crrev.com/6cb1c9a147eff32e3bacec76f0700c6a401c44d2/chrome/browser/chromeos/arc/arc_migration_guide_notification.h [modify] https://crrev.com/6cb1c9a147eff32e3bacec76f0700c6a401c44d2/chrome/browser/ui/app_list/arc/arc_app_utils.cc
,
Apr 24 2017
,
May 16 2017
I'll rebooting the work on this from tomorrow. Update (or clarification from some peoples point of view) after the UI review meetup: we *want* the ARC related section in the chrome://settings as well in the skipped-session, not just app icons.
,
May 16 2017
,
May 24 2017
Now you'll see the icons and the notifications.
,
May 25 2017
Verification steps: 1. Add a user to M59 Kevin. AU to M60. 2. Log-in to the user and skip "critical update" migration wizard. 3. Play store icon should still stay in the launcher. Expected: Clicking the Play Store icon pops up a notification to suggest user to update & sign-out & in. Expected: Task manager (Shift-Esc) doesn't list any Android process like /system/bin/surfaceflinger
,
Jan 22 2018
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by kinaba@chromium.org
, Mar 13 2017