ContentBrowserClient::RequestPermission replies with PermissionStatus instead of bool.

chrome/browser/content_settings/permission_context_base.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/content_settings/permission_context_base.h"
 
 #include "base/logging.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/content_settings/permission_bubble_request_impl.h"
 #include "chrome/browser/content_settings/permission_context_uma_util.h"
@@ skipping 81 matching lines @@
     const BrowserPermissionCallback& callback) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   if (!requesting_origin.is_valid() || !embedding_origin.is_valid()) {
     DVLOG(1)
         << "Attempt to use " << content_settings::GetTypeName(permission_type_)
         << " from an invalid URL: " << requesting_origin
         << "," << embedding_origin
         << " (" << content_settings::GetTypeName(permission_type_)
         << " is not supported in popups)";
-    NotifyPermissionSet(id, requesting_origin, embedding_origin,
-                        callback, false /* persist */, false /* granted */);
+    NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
+                        false /* persist */, CONTENT_SETTING_BLOCK);
     return;
   }
 
   ContentSetting content_setting =
       profile_->GetHostContentSettingsMap()
           ->GetContentSettingAndMaybeUpdateLastUsage(
               requesting_origin, embedding_origin, permission_type_,
               std::string());
 
-  switch (content_setting) {
-    case CONTENT_SETTING_BLOCK:
-      NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
-                          false /* persist */, false /* granted */);
-      return;
-    case CONTENT_SETTING_ALLOW:
-      NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
-                          false /* persist */, true /* granted */);
-      return;
-    default:
-      break;
+  if (content_setting == CONTENT_SETTING_ALLOW ||
+      content_setting == CONTENT_SETTING_BLOCK) {
+    NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
+                        false /* persist */, content_setting);
+    return;
   }
 
   PermissionContextUmaUtil::PermissionRequested(
       permission_type_, requesting_origin);
 
   if (PermissionBubbleManager::Enabled()) {
     if (pending_bubbles_.get(id.ToString()) != NULL)
       return;
     PermissionBubbleManager* bubble_manager =
         PermissionBubbleManager::FromWebContents(web_contents);
@@ skipping 27 matching lines @@
                  // permission
                  false));
 }
 
 void PermissionContextBase::PermissionDecided(
     const PermissionRequestID& id,
     const GURL& requesting_origin,
     const GURL& embedding_origin,
     const BrowserPermissionCallback& callback,
     bool persist,
-    bool allowed) {
+    ContentSetting content_setting) {
   // Infobar persistance and its related UMA is tracked on the infobar
   // controller directly.
   if (PermissionBubbleManager::Enabled()) {
     if (persist) {
-      if (allowed)
+      DCHECK(content_setting == CONTENT_SETTING_ALLOW ||
+             content_setting == CONTENT_SETTING_BLOCK);
+      if (CONTENT_SETTING_ALLOW)

[dgrogan, 2015/05/14 03:48:28]

strictly by code inspection, but won't this block always be entered? Did you
mean content_setting == CONTENT_SETTING_ALLOW

[Miguel Garcia, 2015/05/14 11:10:53]

You are totally right I uploaded a fix in
https://codereview.chromium.org/1130253010

         PermissionContextUmaUtil::PermissionGranted(permission_type_,
                                                     requesting_origin);
       else
         PermissionContextUmaUtil::PermissionDenied(permission_type_,
                                                    requesting_origin);
     } else {
+      DCHECK_EQ(content_setting, CONTENT_SETTING_DEFAULT);
       PermissionContextUmaUtil::PermissionDismissed(permission_type_,
                                                     requesting_origin);
     }
   }
 
   NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
-                      persist, allowed);
+                      persist, content_setting);
 }
 
 PermissionQueueController* PermissionContextBase::GetQueueController() {
   return permission_queue_controller_.get();
 }
 
 Profile* PermissionContextBase::profile() const {
   return profile_;
 }
 
 void PermissionContextBase::NotifyPermissionSet(
     const PermissionRequestID& id,
     const GURL& requesting_origin,
     const GURL& embedding_origin,
     const BrowserPermissionCallback& callback,
     bool persist,
-    bool allowed) {
+    ContentSetting content_setting) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
   if (persist)
-    UpdateContentSetting(requesting_origin, embedding_origin, allowed);
+    UpdateContentSetting(requesting_origin, embedding_origin, content_setting);
 
-  UpdateTabContext(id, requesting_origin, allowed);
-  callback.Run(allowed);
+  UpdateTabContext(id, requesting_origin,
+                   content_setting == CONTENT_SETTING_ALLOW);
+
+  if (content_setting == CONTENT_SETTING_DEFAULT) {
+    content_setting =
+        profile_->GetHostContentSettingsMap()->GetDefaultContentSetting(
+            permission_type_, nullptr);
+  }
+
+  DCHECK_NE(content_setting, CONTENT_SETTING_DEFAULT);
+  callback.Run(content_setting);
 }
 
 void PermissionContextBase::CleanUpBubble(const PermissionRequestID& id) {
   size_t success = pending_bubbles_.erase(id.ToString());
   DCHECK(success == 1) << "Missing request " << id.ToString();
 }
 
-void PermissionContextBase::UpdateContentSetting(const GURL& requesting_origin,
-                                                 const GURL& embedding_origin,
-                                                 bool allowed) {
+void PermissionContextBase::UpdateContentSetting(
+    const GURL& requesting_origin,
+    const GURL& embedding_origin,
+    ContentSetting content_setting) {
   DCHECK_EQ(requesting_origin, requesting_origin.GetOrigin());
   DCHECK_EQ(embedding_origin, embedding_origin.GetOrigin());
-  ContentSetting content_setting =
-      allowed ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
+  DCHECK(content_setting == CONTENT_SETTING_ALLOW ||
+         content_setting == CONTENT_SETTING_BLOCK);
+
   profile_->GetHostContentSettingsMap()->SetContentSetting(
       ContentSettingsPattern::FromURLNoWildcard(requesting_origin),
       ContentSettingsPattern::FromURLNoWildcard(embedding_origin),
       permission_type_, std::string(), content_setting);
 }