Stop sending remote approvals for supervised users' extensions

The feature kSupervisedUserInitiatedExtensionInstall is default-
disabled, but if enabled, allows legacy supervised users to remotely
send approval requests to their "custodian". For
go/unicros-extensions-dd, we will instead use local parental approval.
This CL removes the code from SupervisedUserService that sends the
remote approval request.

Bug: 1004851
Change-Id: Ia30102c301f720cfc609b6aa093285bb4f839f54
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1816450Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Commit-Queue: Toby Huang <tobyhuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701690}

chrome/browser/extensions/extension_service_sync_unittest.cc

@@ -1952,23 +1952,14 @@ class MockPermissionRequestCreator : public PermissionRequestCreator {
     FAIL();
   }
 
-  void CreateExtensionInstallRequest(
-      const std::string& id,
-      SupervisedUserService::SuccessCallback callback) override {
-    CreateExtensionInstallRequestInternal(id);
-  }
-
   void CreateExtensionUpdateRequest(
       const std::string& id,
       SupervisedUserService::SuccessCallback callback) override {
     CreateExtensionUpdateRequestInternal(id);
   }
 
-  // TODO(crbug.com/729950): These two mock methods can be set to direct calls
-  // once gtest supports move-only objects, since SuccessCallback is move only.
-  MOCK_METHOD1(CreateExtensionInstallRequestInternal,
-               void(const std::string& id));
-
+  // TODO(crbug.com/729950): This mock method can be set to direct calls once
+  // gtest supports move-only objects, since SuccessCallback is move only.
   MOCK_METHOD1(CreateExtensionUpdateRequestInternal,
                void(const std::string& id));
 
@@ -2111,20 +2102,12 @@ TEST_F(ExtensionServiceTestSupervised,
   // Make sure it's enabled.
   EXPECT_TRUE(registry()->enabled_extensions().Contains(id));
 
-  MockPermissionRequestCreator* creator = new MockPermissionRequestCreator;
-  supervised_user_service()->AddPermissionRequestCreator(
-      base::WrapUnique(creator));
   const std::string version("1.0.0.0");
 
-  EXPECT_CALL(*creator, CreateExtensionInstallRequestInternal(
-                            RequestId(good_crx, version)));
-
   // Now make the profile supervised.
   profile()->AsTestingProfile()->SetSupervisedUserId(
       supervised_users::kChildAccountSUID);
 
-  Mock::VerifyAndClearExpectations(creator);
-
   // The extension should not be enabled anymore.
   CheckDisabledForCustodianApproval(id);
   EXPECT_TRUE(IsPendingCustodianApproval(id));
@@ -2144,16 +2127,8 @@ TEST_F(ExtensionServiceTestSupervised,
   // Make sure it's enabled.
   EXPECT_TRUE(registry()->enabled_extensions().Contains(id));
 
-  MockPermissionRequestCreator* creator = new MockPermissionRequestCreator;
-  supervised_user_service()->AddPermissionRequestCreator(
-      base::WrapUnique(creator));
   const std::string version("1.0.0.0");
 
-  // No request should be sent because supervised user initiated installs
-  // are disabled.
-  EXPECT_CALL(*creator, CreateExtensionInstallRequestInternal(testing::_))
-      .Times(0);
-
   // Now make the profile supervised.
   profile()->AsTestingProfile()->SetSupervisedUserId(
       supervised_users::kChildAccountSUID);
@@ -2171,10 +2146,6 @@ TEST_F(ExtensionServiceTestSupervised, ExtensionApprovalBeforeInstallation) {
 
   InitServices(true /* profile_is_supervised */);
 
-  MockPermissionRequestCreator* creator = new MockPermissionRequestCreator;
-  supervised_user_service()->AddPermissionRequestCreator(
-      base::WrapUnique(creator));
-
   std::string id = good_crx;
   std::string version("1.0.0.0");
 
@@ -2184,10 +2155,6 @@ TEST_F(ExtensionServiceTestSupervised, ExtensionApprovalBeforeInstallation) {
   base::FilePath path = data_dir().AppendASCII("good.crx");
   InstallCRX(path, INSTALL_NEW);
 
-  // No approval request should be sent.
-  EXPECT_CALL(*creator, CreateExtensionInstallRequestInternal(testing::_))
-      .Times(0);
-
   // Make sure it's enabled.
   EXPECT_TRUE(registry()->enabled_extensions().Contains(id));
   EXPECT_FALSE(IsPendingCustodianApproval(id));
@@ -2350,22 +2317,14 @@ TEST_F(ExtensionServiceTestSupervised, SupervisedUserInitiatedInstalls) {
 
   InitServices(true /* profile_is_supervised */);
 
-  MockPermissionRequestCreator* creator = new MockPermissionRequestCreator;
-  supervised_user_service()->AddPermissionRequestCreator(
-      base::WrapUnique(creator));
-
   base::FilePath path = data_dir().AppendASCII("good.crx");
   std::string version("1.0.0.0");
 
-  EXPECT_CALL(*creator, CreateExtensionInstallRequestInternal(
-                            RequestId(good_crx, version)));
-
   // Should be installed but disabled, a request for approval should be sent.
   const Extension* extension = InstallCRX(path, INSTALL_WITHOUT_LOAD);
   ASSERT_TRUE(extension);
   ASSERT_EQ(extension->id(), good_crx);
   EXPECT_TRUE(registry()->disabled_extensions().Contains(good_crx));
-  Mock::VerifyAndClearExpectations(creator);
   EXPECT_TRUE(IsPendingCustodianApproval(extension->id()));
 
   SimulateCustodianApprovalChangeViaSync(good_crx, version,

chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.cc

@@ -46,7 +46,6 @@ const char kStateKey[] = "state";
 
 // Request values.
 const char kEventTypeURLRequest[] = "PERMISSION_CHROME_URL";
-const char kEventTypeInstallRequest[] = "PERMISSION_CHROME_CWS_ITEM_INSTALL";
 const char kEventTypeUpdateRequest[] = "PERMISSION_CHROME_CWS_ITEM_UPDATE";
 const char kState[] = "PENDING";
 
@@ -111,12 +110,6 @@ void PermissionRequestCreatorApiary::CreateURLAccessRequest(
                 std::move(callback));
 }
 
-void PermissionRequestCreatorApiary::CreateExtensionInstallRequest(
-    const std::string& id,
-    SuccessCallback callback) {
-  CreateRequest(kEventTypeInstallRequest, id, std::move(callback));
-}
-
 void PermissionRequestCreatorApiary::CreateExtensionUpdateRequest(
     const std::string& id,
     SuccessCallback callback) {

chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.h

@@ -40,8 +40,6 @@ class PermissionRequestCreatorApiary : public PermissionRequestCreator {
   bool IsEnabled() const override;
   void CreateURLAccessRequest(const GURL& url_requested,
                               SuccessCallback callback) override;
-  void CreateExtensionInstallRequest(const std::string& id,
-                                     SuccessCallback callback) override;
   void CreateExtensionUpdateRequest(const std::string& id,
                                     SuccessCallback callback) override;
 

chrome/browser/supervised_user/permission_request_creator.h

@@ -28,11 +28,6 @@ class PermissionRequestCreator {
   virtual void CreateURLAccessRequest(const GURL& url_requested,
                                       SuccessCallback callback) = 0;
 
-  // Creates a request to enable the extension with the given |id| (composed
-  // of extension_id:version), which was disabled after an SU initiated install.
-  virtual void CreateExtensionInstallRequest(const std::string& id,
-                                             SuccessCallback callback) = 0;
-
   // Creates a request to re-enable the extension with the given |id| (composed
   // of extension_id:version), which was disabled due to a permission increase.
   virtual void CreateExtensionUpdateRequest(const std::string& id,

chrome/browser/supervised_user/supervised_user_service.cc

@@ -104,13 +104,6 @@ void CreateURLAccessRequest(const GURL& url,
   creator->CreateURLAccessRequest(url, std::move(callback));
 }
 
-void CreateExtensionInstallRequest(
-    const std::string& id,
-    PermissionRequestCreator* creator,
-    SupervisedUserService::SuccessCallback callback) {
-  creator->CreateExtensionInstallRequest(id, std::move(callback));
-}
-
 void CreateExtensionUpdateRequest(
     const std::string& id,
     PermissionRequestCreator* creator,
@@ -118,11 +111,6 @@ void CreateExtensionUpdateRequest(
   creator->CreateExtensionUpdateRequest(id, std::move(callback));
 }
 
-// Default callback for AddExtensionInstallRequest.
-void ExtensionInstallRequestSent(const std::string& id, bool success) {
-  VLOG_IF(1, !success) << "Failed sending install request for " << id;
-}
-
 // Default callback for AddExtensionUpdateRequest.
 void ExtensionUpdateRequestSent(const std::string& id, bool success) {
   VLOG_IF(1, !success) << "Failed sending update request for " << id;
@@ -208,24 +196,6 @@ void SupervisedUserService::AddURLAccessRequest(const GURL& url,
       std::move(callback), 0);
 }
 
-void SupervisedUserService::AddExtensionInstallRequest(
-    const std::string& extension_id,
-    const base::Version& version,
-    SuccessCallback callback) {
-  std::string id = GetExtensionRequestId(extension_id, version);
-  AddPermissionRequestInternal(
-      base::BindRepeating(CreateExtensionInstallRequest, id),
-      std::move(callback), 0);
-}
-
-void SupervisedUserService::AddExtensionInstallRequest(
-    const std::string& extension_id,
-    const base::Version& version) {
-  std::string id = GetExtensionRequestId(extension_id, version);
-  AddExtensionInstallRequest(extension_id, version,
-                             base::BindOnce(ExtensionInstallRequestSent, id));
-}
-
 void SupervisedUserService::AddExtensionUpdateRequest(
     const std::string& extension_id,
     const base::Version& version,
@@ -824,21 +794,6 @@ bool SupervisedUserService::MustRemainDisabled(
     }
     if (reason)
       *reason = extensions::disable_reason::DISABLE_CUSTODIAN_APPROVAL_REQUIRED;
-    if (base::FeatureList::IsEnabled(
-            supervised_users::kSupervisedUserInitiatedExtensionInstall)) {
-      // If the Extension isn't pending a custodian approval already, send
-      // an approval request.
-      if (!extension_prefs->HasDisableReason(
-              extension->id(), extensions::disable_reason::
-                                   DISABLE_CUSTODIAN_APPROVAL_REQUIRED)) {
-        // MustRemainDisabled is a const method and hence cannot call
-        // AddExtensionInstallRequest directly.
-        SupervisedUserService* supervised_user_service =
-            SupervisedUserServiceFactory::GetForProfile(profile_);
-        supervised_user_service->AddExtensionInstallRequest(
-            extension->id(), extension->version());
-      }
-    }
   }
   return must_remain_disabled;
 }

chrome/browser/supervised_user/supervised_user_service.h

@@ -108,15 +108,6 @@ class SupervisedUserService : public KeyedService,
   // Adds an access request for the given URL.
   void AddURLAccessRequest(const GURL& url, SuccessCallback callback);
 
-  // Adds an install request for the given WebStore item (App/Extension).
-  void AddExtensionInstallRequest(const std::string& extension_id,
-                                  const base::Version& version,
-                                  SuccessCallback callback);
-
-  // Same as above, but without a callback, just logging errors on failure.
-  void AddExtensionInstallRequest(const std::string& extension_id,
-                                  const base::Version& version);
-
   // Adds an update request for the given WebStore item (App/Extension).
   void AddExtensionUpdateRequest(const std::string& extension_id,
                                  const base::Version& version,

chrome/browser/supervised_user/supervised_user_service_unittest.cc

@@ -267,11 +267,6 @@ class MockPermissionRequestCreator : public PermissionRequestCreator {
     callbacks_.push_back(std::move(callback));
   }
 
-  void CreateExtensionInstallRequest(const std::string& extension_id,
-                                     SuccessCallback callback) override {
-    FAIL();
-  }
-
   void CreateExtensionUpdateRequest(const std::string& id,
                                     SuccessCallback callback) override {
     FAIL();