Supervised users: When an extension requires new permissions, send request to custodian

chrome/browser/supervised_user/supervised_user_service.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/supervised_user/supervised_user_service.h"
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/prefs/pref_service.h"
@@ skipping 207 matching lines @@
     const scoped_refptr<net::URLRequestContextGetter>& context,
     const std::string& cx) {
   ui_url_filter_->InitAsyncURLChecker(context.get(), cx);
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
       base::Bind(&SupervisedUserURLFilter::InitAsyncURLChecker,
                  io_url_filter_, context, cx));
 }
 
+class SupervisedUserService::SendPermissionRequestHelper {
+ public:
+  virtual ~SendPermissionRequestHelper() {}
+
+  virtual void SendPermissionRequest(PermissionRequestCreator* creator,

[Bernhard Bauer, 2015/03/02 23:09:59]

Nice! Now that I see this, I'm wondering though whether this would be even
cleaner with callbacks? That would remove a lot of the boilerplate here.

[Marc Treib, 2015/03/03 10:04:50]

You're right, it's much less boilerplate with callbacks. Done, thanks! (How
didn't I think of that?!)

+                                     const SuccessCallback& callback) = 0;
+};
+
+class SupervisedUserService::SendURLAccessRequestHelper
+    : public SupervisedUserService::SendPermissionRequestHelper {
+ public:
+  explicit SendURLAccessRequestHelper(const GURL& url) : url_(url) {}
+
+  void SendPermissionRequest(PermissionRequestCreator* creator,
+                             const SuccessCallback& callback) override {
+    creator->CreateURLAccessRequest(url_, callback);
+  }
+
+ private:
+  const GURL url_;
+};
+
+class SupervisedUserService::SendExtensionUpdateRequestHelper
+    : public SupervisedUserService::SendPermissionRequestHelper {
+ public:
+  explicit SendExtensionUpdateRequestHelper(const std::string& id) : id_(id) {}
+
+  void SendPermissionRequest(PermissionRequestCreator* creator,
+                             const SuccessCallback& callback) override {
+    creator->CreateExtensionUpdateRequest(id_, callback);
+  }
+
+ private:
+  const std::string id_;
+};
+
 SupervisedUserService::SupervisedUserService(Profile* profile)
     : includes_sync_sessions_type_(true),
       profile_(profile),
       active_(false),
       delegate_(NULL),
       waiting_for_sync_initialization_(false),
       is_profile_active_(false),
       did_init_(false),
       did_shutdown_(false),
       weak_ptr_factory_(this) {
@@ skipping 286 matching lines @@
 
 size_t SupervisedUserService::FindEnabledPermissionRequestCreator(
     size_t start) {
   for (size_t i = start; i < permissions_creators_.size(); ++i) {
     if (permissions_creators_[i]->IsEnabled())
       return i;
   }
   return permissions_creators_.size();
 }
 
-void SupervisedUserService::AddAccessRequestInternal(
-    const GURL& url,
+void SupervisedUserService::AddPermissionRequestInternal(
+    scoped_ptr<SendPermissionRequestHelper> helper,
     const SuccessCallback& callback,
     size_t index) {
   // Find a permission request creator that is enabled.
   size_t next_index = FindEnabledPermissionRequestCreator(index);
   if (next_index >= permissions_creators_.size()) {
     callback.Run(false);
     return;
   }
 
-  permissions_creators_[next_index]->CreatePermissionRequest(
-      url,
+  helper->SendPermissionRequest(
+      permissions_creators_[next_index],
       base::Bind(&SupervisedUserService::OnPermissionRequestIssued,
-                 weak_ptr_factory_.GetWeakPtr(), url, callback, next_index));
+                 weak_ptr_factory_.GetWeakPtr(), base::Passed(&helper),

[Marc Treib, 2015/03/02 18:26:00]

This is ugly: The base::Passed nulls the helper, but apparently the
helper->SendPermissionRequest is resolved first. Don't know if that's
well-defined though - do you know? I guess the alternative would be to grab a
raw pointer to the helper before the Bind...

[Bernhard Bauer, 2015/03/02 23:09:59]

Wait, how would we call the method before its arguments are fully resolved? Or
do we dereference the scoped_ptr, *then* resolve the arguments, then call it?
That seems indeed kind of scary to me, and yes, I would in that case get a raw
pointer (which really just makes the order explicit in which things happen).

[Marc Treib, 2015/03/03 10:04:50]

Thinking about this again, it probably depends on the order of evaluation of
method arguments: "helper->" is basically the first argument, and apparently
that's evaluated before the "base::Passed(&helper)". That's not well-defined
though.
Luckily, the version with callbacks makes this unnecessary :)

+                 callback, next_index));
 }
 
 void SupervisedUserService::OnPermissionRequestIssued(
-    const GURL& url,
+    scoped_ptr<SendPermissionRequestHelper> helper,
     const SuccessCallback& callback,
     size_t index,
     bool success) {
   if (success) {
     callback.Run(true);
     return;
   }
 
-  AddAccessRequestInternal(url, callback, index + 1);
+  AddPermissionRequestInternal(helper.Pass(), callback, index + 1);
 }
 
 void SupervisedUserService::OnSupervisedUserIdChanged() {
   SetActive(ProfileIsSupervised());
 }
 
 void SupervisedUserService::OnDefaultFilteringBehaviorChanged() {
   DCHECK(ProfileIsSupervised());
 
   int behavior_value = profile_->GetPrefs()->GetInteger(
@@ skipping 52 matching lines @@
 
 void SupervisedUserService::OnBlacklistLoaded() {
   FOR_EACH_OBSERVER(
       SupervisedUserServiceObserver, observer_list_, OnURLFilterChanged());
 }
 
 bool SupervisedUserService::AccessRequestsEnabled() {
   return FindEnabledPermissionRequestCreator(0) < permissions_creators_.size();
 }
 
-void SupervisedUserService::AddAccessRequest(const GURL& url,
-                                             const SuccessCallback& callback) {
-  AddAccessRequestInternal(SupervisedUserURLFilter::Normalize(url), callback,
-                           0);
+void SupervisedUserService::AddURLAccessRequest(
+    const GURL& url,
+    const SuccessCallback& callback) {
+  AddPermissionRequestInternal(
+      make_scoped_ptr(new SendURLAccessRequestHelper(
+          SupervisedUserURLFilter::Normalize(url))),
+      callback, 0);
+}
+
+void SupervisedUserService::AddExtensionUpdateRequest(
+    const std::string& extension_id,
+    const SuccessCallback& callback) {
+  AddPermissionRequestInternal(
+      make_scoped_ptr(new SendExtensionUpdateRequestHelper(extension_id)),
+      callback, 0);
 }
 
 void SupervisedUserService::InitSync(const std::string& refresh_token) {
   StartSetupSync();
 
   ProfileOAuth2TokenService* token_service =
       ProfileOAuth2TokenServiceFactory::GetForProfile(profile_);
   token_service->UpdateCredentials(supervised_users::kSupervisedUserPseudoEmail,
                                    refresh_token);
 
@@ skipping 265 matching lines @@
   // The active user can be NULL in unit tests.
   if (user_manager::UserManager::Get()->GetActiveUser()) {
     return UTF16ToUTF8(user_manager::UserManager::Get()->GetUserDisplayName(
         user_manager::UserManager::Get()->GetActiveUser()->GetUserID()));
   }
   return std::string();
 #else
   return profile_->GetPrefs()->GetString(prefs::kProfileName);
 #endif
 }