Apps&Extensions for Supervised Users: send permission request on outdated re-enables

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/files/file_util.h"
 #include "base/memory/ref_counted.h"
@@ skipping 91 matching lines @@
   creator->CreateURLAccessRequest(url, callback);
 }
 
 void CreateExtensionUpdateRequest(
     const std::string& id,
     PermissionRequestCreator* creator,
     const SupervisedUserService::SuccessCallback& callback) {
   creator->CreateExtensionUpdateRequest(id, callback);
 }
 
+// Default callback for AddExtensionUpdateRequest.
+void ExtensionUpdateRequestSent(const std::string& id, bool success) {
+  LOG_IF(WARNING, !success) << "Failed sending update request for " << id;

[Devlin, 2015/10/16 02:40:24]

LOGs are annoying.  We never read them, except when it's something random that
happens and no one understands it.  When can this happen?

If it shouldn't (ever), (D)CHECK.  If it can and we care about metrics/knowing
about it, UMA it.  If it can and we just don't know what to do, but feel bad
doing nothing, make it a VLOG.

[Marc Treib, 2015/10/16 09:22:20]

It can happen when the server we talk to is down, etc. I think it's rare enough
that it won't be annoying, but eh. It's now a VLOG.

+}
+
 base::FilePath GetBlacklistPath() {
   base::FilePath blacklist_dir;
   PathService::Get(chrome::DIR_USER_DATA, &blacklist_dir);
   return blacklist_dir.AppendASCII(kBlacklistFilename);
 }
 
 #if defined(ENABLE_EXTENSIONS)
 enum ExtensionState {
   EXTENSION_FORCED,
   EXTENSION_BLOCKED,
@@ skipping 123 matching lines @@
 void SupervisedUserService::AddExtensionUpdateRequest(
     const std::string& extension_id,
     const base::Version& version,
     const SuccessCallback& callback) {
   std::string id = extension_id + ":" + version.GetString();
   AddPermissionRequestInternal(
       base::Bind(CreateExtensionUpdateRequest, id),
       callback, 0);
 }
 
+void SupervisedUserService::AddExtensionUpdateRequest(
+    const std::string& extension_id,
+    const base::Version& version) {
+  AddExtensionUpdateRequest(
+      extension_id, version,
+      base::Bind(ExtensionUpdateRequestSent,
+                 extension_id + ":" + version.GetString()));

[Devlin, 2015/10/16 02:40:24]

nitty nit: prefer base::StringPrintf for times like this.

[Marc Treib, 2015/10/16 09:22:20]

Done.

+}
+
 std::string SupervisedUserService::GetCustodianEmailAddress() const {
   std::string email = profile_->GetPrefs()->GetString(
       prefs::kSupervisedUserCustodianEmail);
 #if defined(OS_CHROMEOS)
   // |GetActiveUser()| can return null in unit tests.
   if (email.empty() && !!user_manager::UserManager::Get()->GetActiveUser()) {
     email = chromeos::ChromeUserManager::Get()
         ->GetSupervisedUserManager()
         ->GetManagerDisplayEmail(
             user_manager::UserManager::Get()->GetActiveUser()->email());
@@ skipping 709 matching lines @@
 
   is_profile_active_ = profile_became_active;
 }
 #endif  // !defined(OS_ANDROID) && !defined(OS_IOS)
 
 void SupervisedUserService::OnSiteListUpdated() {
   FOR_EACH_OBSERVER(
       SupervisedUserServiceObserver, observer_list_, OnURLFilterChanged());
 }