Disable adb sideloading settings if powerwash is needed

On device initialized before M74 (when boot lockbox was first
introduced), it requires a powerwash first to use boot lockbox. In such
case, disable the toggle and show sublabel to explain the situation.

      1) still see login screen warning indicator
      2) see settings toggle disabled, with a sublabel

Test: Force to return NEED_POWERWASH from session_manager
Test: No change if not forced
Bug: chromium:893332
Change-Id: Idf75884404769ff724be0225aa9a674e03d978ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1918335
Commit-Queue: Victor Hsieh <victorhsieh@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715877}

chrome/app/settings_strings.grdp

@@ -662,6 +662,9 @@
     <message name="IDS_SETTINGS_CROSTINI_ARC_ADB_LABEL" desc="Label for enabling ADB in ARC.">
       Enable on this device
     </message>
+    <message name="IDS_SETTINGS_CROSTINI_ARC_ADB_POWERWASH_REQUIRED_SUBLABEL" desc="Sublabel notifying user of powerwash requirement before enabling ARC ADB sideloading.">
+            A factory reset of this Chromebook is required to enable ADB debugging. <ph name="BEGIN_LINK_LEARN_MORE">&lt;a target="_blank" href="$1"&gt;</ph>Learn more<ph name="END_LINK_LEARN_MORE">&lt;/a&gt;</ph>
+    </message>
     <message name="IDS_SETTINGS_CROSTINI_ARC_ADB_CONFIRMATION_TITLE_ENABLE" desc="Confirmation dialog title to restart for enabling ADB.">
       Enable ADB?
     </message>

chrome/browser/chromeos/login/screens/enable_adb_sideloading_screen.cc

@@ -10,7 +10,6 @@
 #include "chrome/browser/chromeos/login/ui/login_display_host.h"
 #include "chrome/browser/chromeos/login/wizard_controller.h"
 #include "chrome/common/pref_names.h"
-#include "chromeos/dbus/session_manager/session_manager_client.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 
@@ -30,7 +29,8 @@ enum class AdbSideloadingPromptEvent {
   kEnabled = 3,
   kFailedToDisplay = 4,
   kFailedToEnable = 5,
-  kMaxValue = kFailedToEnable,
+  kFailedToDisplay_NeedPowerwash = 6,
+  kMaxValue = kFailedToDisplay_NeedPowerwash,
 };
 
 void LogEvent(AdbSideloadingPromptEvent action) {
@@ -79,32 +79,44 @@ void EnableAdbSideloadingScreen::Show() {
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
-void EnableAdbSideloadingScreen::OnQueryAdbSideload(bool success,
-                                                    bool enabled) {
+void EnableAdbSideloadingScreen::OnQueryAdbSideload(
+    SessionManagerClient::AdbSideloadResponseCode response_code,
+    bool enabled) {
   DVLOG(1) << "EnableAdbSideloadingScreen::OnQueryAdbSideload"
-           << ", success=" << success << ", enabled=" << enabled;
-  DCHECK(view_);
-  bool already_enabled = success && enabled;
-  if (already_enabled) {
-    LogEvent(AdbSideloadingPromptEvent::kSkipped);
-    exit_callback_.Run();
-  } else {
-    if (success) {
-      LogEvent(AdbSideloadingPromptEvent::kPromptShown);
-      view_->SetScreenState(
-          EnableAdbSideloadingScreenView::UIState::UI_STATE_SETUP);
-    } else {
-      LogEvent(AdbSideloadingPromptEvent::kFailedToDisplay);
-      view_->SetScreenState(
-          EnableAdbSideloadingScreenView::UIState::UI_STATE_ERROR);
-    }
-    view_->Show();
-  }
+           << ", response_code=" << static_cast<int>(response_code)
+           << ", enabled=" << enabled;
 
   // Clear prefs so that the screen won't be triggered again.
   PrefService* prefs = g_browser_process->local_state();
   prefs->ClearPref(prefs::kEnableAdbSideloadingRequested);
   prefs->CommitPendingWrite();
+
+  if (enabled) {
+    DCHECK_EQ(response_code,
+              SessionManagerClient::AdbSideloadResponseCode::SUCCESS);
+    LogEvent(AdbSideloadingPromptEvent::kSkipped);
+    exit_callback_.Run();
+    return;
+  }
+
+  DCHECK(view_);
+  EnableAdbSideloadingScreenView::UIState ui_state;
+  switch (response_code) {
+    case SessionManagerClient::AdbSideloadResponseCode::SUCCESS:
+      LogEvent(AdbSideloadingPromptEvent::kPromptShown);
+      ui_state = EnableAdbSideloadingScreenView::UIState::UI_STATE_SETUP;
+      break;
+    case SessionManagerClient::AdbSideloadResponseCode::NEED_POWERWASH:
+      LogEvent(AdbSideloadingPromptEvent::kFailedToDisplay_NeedPowerwash);
+      ui_state = EnableAdbSideloadingScreenView::UIState::UI_STATE_ERROR;
+      break;
+    case SessionManagerClient::AdbSideloadResponseCode::FAILED:
+      LogEvent(AdbSideloadingPromptEvent::kFailedToDisplay);
+      ui_state = EnableAdbSideloadingScreenView::UIState::UI_STATE_ERROR;
+      break;
+  }
+  view_->SetScreenState(ui_state);
+  view_->Show();
 }
 
 void EnableAdbSideloadingScreen::Hide() {
@@ -125,15 +137,20 @@ void EnableAdbSideloadingScreen::OnEnable() {
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
-void EnableAdbSideloadingScreen::OnEnableAdbSideload(bool success) {
-  if (success) {
-    LogEvent(AdbSideloadingPromptEvent::kEnabled);
-    exit_callback_.Run();
-  } else {
-    LogEvent(AdbSideloadingPromptEvent::kFailedToEnable);
-    DCHECK(view_);
-    view_->SetScreenState(
-        EnableAdbSideloadingScreenView::UIState::UI_STATE_ERROR);
+void EnableAdbSideloadingScreen::OnEnableAdbSideload(
+    SessionManagerClient::AdbSideloadResponseCode response_code) {
+  switch (response_code) {
+    case SessionManagerClient::AdbSideloadResponseCode::SUCCESS:
+      LogEvent(AdbSideloadingPromptEvent::kEnabled);
+      exit_callback_.Run();
+      break;
+    case SessionManagerClient::AdbSideloadResponseCode::NEED_POWERWASH:
+    case SessionManagerClient::AdbSideloadResponseCode::FAILED:
+      LogEvent(AdbSideloadingPromptEvent::kFailedToEnable);
+      DCHECK(view_);
+      view_->SetScreenState(
+          EnableAdbSideloadingScreenView::UIState::UI_STATE_ERROR);
+      break;
   }
 }
 

chrome/browser/chromeos/login/screens/enable_adb_sideloading_screen.h

@@ -13,6 +13,7 @@
 #include "chrome/browser/chromeos/login/help_app_launcher.h"
 #include "chrome/browser/chromeos/login/screens/base_screen.h"
 #include "chrome/browser/ui/webui/chromeos/login/enable_adb_sideloading_screen_handler.h"
+#include "chromeos/dbus/session_manager/session_manager_client.h"
 
 class PrefRegistrySimple;
 
@@ -41,8 +42,11 @@ class EnableAdbSideloadingScreen : public BaseScreen {
   base::RepeatingClosure* exit_callback() { return &exit_callback_; }
 
  private:
-  void OnQueryAdbSideload(bool success, bool enabled);
-  void OnEnableAdbSideload(bool success);
+  void OnQueryAdbSideload(
+      SessionManagerClient::AdbSideloadResponseCode response_code,
+      bool enabled);
+  void OnEnableAdbSideload(
+      SessionManagerClient::AdbSideloadResponseCode response_code);
 
   void OnEnable();
   void OnCancel();

chrome/browser/chromeos/login/version_info_updater.cc

@@ -19,7 +19,6 @@
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
-#include "chromeos/dbus/session_manager/session_manager_client.h"
 #include "chromeos/dbus/util/version_loader.h"
 #include "chromeos/settings/cros_settings_names.h"
 #include "chromeos/strings/grit/chromeos_strings.h"
@@ -182,9 +181,11 @@ void VersionInfoUpdater::OnStoreError(policy::CloudPolicyStore* store) {
   UpdateEnterpriseInfo();
 }
 
-void VersionInfoUpdater::OnQueryAdbSideload(bool success, bool enabled) {
-  if (!success) {
-    LOG(ERROR) << "Failed to query adb sideload status";
+void VersionInfoUpdater::OnQueryAdbSideload(
+    SessionManagerClient::AdbSideloadResponseCode response_code,
+    bool enabled) {
+  if (response_code != SessionManagerClient::AdbSideloadResponseCode::SUCCESS) {
+    LOG(WARNING) << "Failed to query adb sideload status";
     // Pretend to be enabled to show warning at login screen conservatively.
     enabled = true;
   }

chrome/browser/chromeos/login/version_info_updater.h

@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
+#include "chromeos/dbus/session_manager/session_manager_client.h"
 #include "components/policy/core/common/cloud/cloud_policy_store.h"
 
 namespace device {
@@ -83,7 +84,9 @@ class VersionInfoUpdater : public policy::CloudPolicyStore::Observer {
   void OnGetAdapter(scoped_refptr<device::BluetoothAdapter> adapter);
 
   // Callback from SessionManagerClient::QueryAdbSideload.
-  void OnQueryAdbSideload(bool success, bool enabled);
+  void OnQueryAdbSideload(
+      SessionManagerClient::AdbSideloadResponseCode response_code,
+      bool enabled);
 
   // Information pieces for version label.
   std::string version_text_;

chrome/browser/resources/settings/crostini_page/BUILD.gn

@@ -21,6 +21,7 @@ js_library("crostini_arc_adb") {
     ":crostini_browser_proxy",
     "..:route",
     "//ui/webui/resources/cr_elements/policy:cr_policy_indicator",
+    "//ui/webui/resources/js:i18n_behavior",
     "//ui/webui/resources/js:web_ui_listener_behavior",
   ]
 }

chrome/browser/resources/settings/crostini_page/crostini_arc_adb.html

@@ -18,12 +18,17 @@
     <div class="settings-box">
       <div id="enableArcAdbLabel" class="start">
         $i18n{crostiniArcAdbLabel}
+        <div class="secondary" hidden="[[!arcAdbNeedPowerwash_]]"
+            inner-h-t-m-l="[[i18nAdvanced(
+                'crostiniArcAdbPowerwashRequiredSublabel')]]">
+        </div>
       </div>
       <cr-policy-indicator indicator-type="[[getPolicyIndicatorType_(
           isOwnerProfile_, isEnterpriseManaged_)]]"></cr-policy-indicator>
       <cr-toggle id="arcAdbEnabledButton" aria-labelledby="enableArcAdbLabel"
           checked$="[[arcAdbEnabled_]]"
-          disabled="[[shouldDisable_(isOwnerProfile_, isEnterpriseManaged_)]]"
+          disabled="[[shouldDisable_(isOwnerProfile_, isEnterpriseManaged_,
+                      arcAdbNeedPowerwash_)]]"
           on-change="onArcAdbToggleChanged_">
       </cr-toggle>
     </div>

chrome/browser/resources/settings/crostini_page/crostini_arc_adb.js

@@ -10,7 +10,7 @@
 Polymer({
   is: 'settings-crostini-arc-adb',
 
-  behaviors: [WebUIListenerBehavior],
+  behaviors: [I18nBehavior, WebUIListenerBehavior],
 
   properties: {
     /** @private {boolean} */
@@ -19,6 +19,17 @@ Polymer({
       value: false,
     },
 
+    /**
+     * Whether the device requires a powerwash first (to define nvram for boot
+     * lockbox). This happens to devices initialized through OOBE flow before
+     * M74.
+     * @private {boolean}
+     */
+    arcAdbNeedPowerwash_: {
+      type: Boolean,
+      value: false,
+    },
+
     /** @private {boolean} */
     isOwnerProfile_: {
       type: Boolean,
@@ -44,8 +55,10 @@ Polymer({
 
   attached: function() {
     this.addWebUIListener(
-        'crostini-arc-adb-sideload-status-changed', (enabled) => {
+        'crostini-arc-adb-sideload-status-changed',
+        (enabled, need_powerwash) => {
           this.arcAdbEnabled_ = enabled;
+          this.arcAdbNeedPowerwash_ = need_powerwash;
         });
     settings.CrostiniBrowserProxyImpl.getInstance()
         .requestArcAdbSideloadStatus();
@@ -58,8 +71,9 @@ Polymer({
    * developer tool.
    * @private
    */
-  shouldDisable_: function(isOwnerProfile, isEnterpriseManaged) {
-    return !isOwnerProfile || isEnterpriseManaged;
+  shouldDisable_: function(
+      isOwnerProfile, isEnterpriseManaged, arcAdbNeedPowerwash) {
+    return !isOwnerProfile || isEnterpriseManaged || arcAdbNeedPowerwash;
   },
 
   /** @private */

chrome/browser/ui/webui/chromeos/login/enable_adb_sideloading_screen_handler.h

@@ -56,9 +56,6 @@ class EnableAdbSideloadingScreenHandler : public EnableAdbSideloadingScreenView,
   void Initialize() override;
 
  private:
-  // Callback for SessionManagerClient::OnQueryAdbSideload
-  void OnQueryAdbSideload(bool success, bool is_allowed);
-
   EnableAdbSideloadingScreen* screen_ = nullptr;
 
   // Keeps whether screen should be shown right after initialization.

chrome/browser/ui/webui/settings/chromeos/crostini_handler.cc

@@ -21,7 +21,6 @@
 #include "chrome/browser/policy/profile_policy_connector.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
-#include "chromeos/dbus/session_manager/session_manager_client.h"
 #include "components/prefs/pref_service.h"
 #include "components/user_manager/user_manager.h"
 #include "content/public/browser/browser_thread.h"
@@ -302,14 +301,19 @@ void CrostiniHandler::HandleQueryArcAdbRequest(const base::ListValue* args) {
                                       weak_ptr_factory_.GetWeakPtr()));
 }
 
-void CrostiniHandler::OnQueryAdbSideload(bool success, bool enabled) {
-  if (!success) {
+void CrostiniHandler::OnQueryAdbSideload(
+    SessionManagerClient::AdbSideloadResponseCode response_code,
+    bool enabled) {
+  if (response_code != SessionManagerClient::AdbSideloadResponseCode::SUCCESS) {
     LOG(ERROR) << "Failed to query adb sideload status";
     enabled = false;
   }
+  bool need_powerwash =
+      response_code ==
+      SessionManagerClient::AdbSideloadResponseCode::NEED_POWERWASH;
   // Other side listens with cr.addWebUIListener
   FireWebUIListener("crostini-arc-adb-sideload-status-changed",
-                    base::Value(enabled));
+                    base::Value(enabled), base::Value(need_powerwash));
 }
 
 void CrostiniHandler::HandleEnableArcAdbRequest(const base::ListValue* args) {

chrome/browser/ui/webui/settings/chromeos/crostini_handler.h

@@ -12,6 +12,7 @@
 #include "chrome/browser/chromeos/crostini/crostini_manager.h"
 #include "chrome/browser/chromeos/usb/cros_usb_detector.h"
 #include "chrome/browser/ui/webui/settings/settings_page_ui_handler.h"
+#include "chromeos/dbus/session_manager/session_manager_client.h"
 
 class Profile;
 
@@ -70,7 +71,9 @@ class CrostiniHandler : public ::settings::SettingsPageUIHandler,
   // Handle a request for disabling adb sideloading in ARC.
   void HandleDisableArcAdbRequest(const base::ListValue* args);
   // Callback of HandleQueryArcAdbRequest.
-  void OnQueryAdbSideload(bool success, bool enabled);
+  void OnQueryAdbSideload(
+      SessionManagerClient::AdbSideloadResponseCode response_code,
+      bool enabled);
   // Returns whether the current user can change adb sideloading configuration
   // on current device.
   bool CheckEligibilityToChangeArcAdbSideloading() const;

chrome/browser/ui/webui/settings/settings_localized_strings_provider.cc

@@ -596,6 +596,12 @@ void AddCrostiniStrings(content::WebUIDataSource* html_source,
       l10n_util::GetStringFUTF16(
           IDS_SETTINGS_CROSTINI_SUBTEXT, ui::GetChromeOSDeviceName(),
           GetHelpUrlWithBoard(chrome::kLinuxAppsLearnMoreURL)));
+  // TODO(crbug.com/893332): replace with the final URL
+  html_source->AddString(
+      "crostiniArcAdbPowerwashRequiredSublabel",
+      l10n_util::GetStringFUTF16(
+          IDS_SETTINGS_CROSTINI_ARC_ADB_POWERWASH_REQUIRED_SUBLABEL,
+          GetHelpUrlWithBoard(chrome::kLinuxAppsLearnMoreURL)));
   html_source->AddString("crostiniRemove", l10n_util::GetStringFUTF16(
                                                IDS_SETTINGS_CROSTINI_REMOVE,
                                                ui::GetChromeOSDeviceName()));

chromeos/dbus/session_manager/session_manager_client.cc

@@ -1006,7 +1006,11 @@ class SessionManagerClientImpl : public SessionManagerClient {
     if (!response) {
       LOG(ERROR) << "Failed to call EnableAdbSideload: "
                  << (error ? error->ToString() : "(null)");
-      std::move(callback).Run(false);
+      if (error && error->GetErrorName() == DBUS_ERROR_NOT_SUPPORTED) {
+        std::move(callback).Run(AdbSideloadResponseCode::NEED_POWERWASH);
+      } else {
+        std::move(callback).Run(AdbSideloadResponseCode::FAILED);
+      }
       return;
     }
 
@@ -1014,10 +1018,10 @@ class SessionManagerClientImpl : public SessionManagerClient {
     dbus::MessageReader reader(response);
     if (!reader.PopBool(&succeeded)) {
       LOG(ERROR) << "Failed to enable sideloading";
-      std::move(callback).Run(false);
+      std::move(callback).Run(AdbSideloadResponseCode::FAILED);
       return;
     }
-    std::move(callback).Run(true);
+    std::move(callback).Run(AdbSideloadResponseCode::SUCCESS);
   }
 
   void OnQueryAdbSideload(QueryAdbSideloadCallback callback,
@@ -1026,7 +1030,11 @@ class SessionManagerClientImpl : public SessionManagerClient {
     if (!response) {
       LOG(ERROR) << "Failed to call QueryAdbSideload: "
                  << (error ? error->ToString() : "(null)");
-      std::move(callback).Run(false, false);
+      if (error && error->GetErrorName() == DBUS_ERROR_NOT_SUPPORTED) {
+        std::move(callback).Run(AdbSideloadResponseCode::NEED_POWERWASH, false);
+      } else {
+        std::move(callback).Run(AdbSideloadResponseCode::FAILED, false);
+      }
       return;
     }
 
@@ -1034,9 +1042,10 @@ class SessionManagerClientImpl : public SessionManagerClient {
     dbus::MessageReader reader(response);
     if (!reader.PopBool(&is_allowed)) {
       LOG(ERROR) << "Failed to interpret the response";
-      std::move(callback).Run(false, false);
+      std::move(callback).Run(AdbSideloadResponseCode::FAILED, false);
+      return;
     }
-    std::move(callback).Run(true, is_allowed);
+    std::move(callback).Run(AdbSideloadResponseCode::SUCCESS, is_allowed);
   }
 
   dbus::ObjectProxy* session_manager_proxy_ = nullptr;

chromeos/dbus/session_manager/session_manager_client.h

@@ -63,6 +63,15 @@ class COMPONENT_EXPORT(SESSION_MANAGER) SessionManagerClient {
     COUNT
   };
 
+  enum class AdbSideloadResponseCode {
+    // ADB sideload operation has finished successfully.
+    SUCCESS = 1,
+    // ADB sideload operation has failed.
+    FAILED = 2,
+    // ADB sideload requires a powerwash to unblock (to define nvram).
+    NEED_POWERWASH = 3,
+  };
+
   // Interface for observing changes from the session manager.
   class Observer {
    public:
@@ -421,7 +430,8 @@ class COMPONENT_EXPORT(SESSION_MANAGER) SessionManagerClient {
   virtual void GetArcStartTime(
       DBusMethodCallback<base::TimeTicks> callback) = 0;
 
-  using EnableAdbSideloadCallback = base::OnceCallback<void(bool succeeded)>;
+  using EnableAdbSideloadCallback =
+      base::OnceCallback<void(AdbSideloadResponseCode response_code)>;
 
   // Asynchronously attempts to enable ARC APK Sideloading. Upon completion,
   // invokes |callback| with the result; true on success, false on failure of
@@ -429,7 +439,8 @@ class COMPONENT_EXPORT(SESSION_MANAGER) SessionManagerClient {
   virtual void EnableAdbSideload(EnableAdbSideloadCallback callback) = 0;
 
   using QueryAdbSideloadCallback =
-      base::OnceCallback<void(bool succeeded, bool is_allowed)>;
+      base::OnceCallback<void(AdbSideloadResponseCode response_code,
+                              bool is_allowed)>;
 
   // Asynchronously queries for the current status of ARC APK Sideloading. Upon
   // completion, invokes |callback| with |succeeded| indicating if the query

tools/metrics/histograms/enums.xml

@@ -1036,6 +1036,8 @@ Unknown properties are collapsed to zero. -->
   <int value="3" label="Enabled by user"/>
   <int value="4" label="Failed to display the prompt"/>
   <int value="5" label="Failed to enable"/>
+  <int value="6" label="Failed to display the prompt (need powerwash)"/>
+  <int value="7" label="Failed to enable (need powerwash)"/>
 </enum>
 
 <enum name="AddIceCandidateResult">