Implement midi permissions on top of the new common permission classes

chrome/browser/content_settings/permission_context_base.cc

Index: chrome/browser/content_settings/permission_context_base.cc
diff --git a/chrome/browser/services/gcm/permission_context_base.cc b/chrome/browser/content_settings/permission_context_base.cc
similarity index 78%
rename from chrome/browser/services/gcm/permission_context_base.cc
rename to chrome/browser/content_settings/permission_context_base.cc
index f31b77681e28716977b3b129d1d5ee95021c600e..d8e908528dfd0d05bf115b7f9a742dcd7f254d16 100644
--- a/chrome/browser/services/gcm/permission_context_base.cc
+++ b/chrome/browser/content_settings/permission_context_base.cc
@@ -2,16 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/services/gcm/permission_context_base.h"
+#include "chrome/browser/content_settings/permission_context_base.h"
 
 #include "base/logging.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/content_settings/host_content_settings_map.h"
+#include "chrome/browser/content_settings/permission_bubble_request_impl.h"
 #include "chrome/browser/content_settings/permission_queue_controller.h"
 #include "chrome/browser/content_settings/permission_request_id.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/services/gcm/permission_bubble_request_impl.h"
 #include "chrome/browser/ui/website_settings/permission_bubble_manager.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
@@ -19,7 +19,6 @@
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 
-namespace gcm {
 
 PermissionContextBase::PermissionContextBase(
     Profile* profile,
@@ -46,8 +45,8 @@ void PermissionContextBase::RequestPermission(
 
   DecidePermission(web_contents,
                    id,
-                   requesting_frame,
-                   requesting_frame,
+                   requesting_frame.GetOrigin(),
+                   web_contents->GetURL().GetOrigin(),

[Michael van Ouwerkerk, 2014/07/04 10:34:32]

Use GetLastCommittedURL as GetURL is deprecated and does not do what you want!

[Miguel Garcia, 2014/07/04 12:09:52]

True, you pointed this out before, thanks for spotting it!

                    user_gesture,
                    callback);
 }
@@ -55,21 +54,22 @@ void PermissionContextBase::RequestPermission(
 void PermissionContextBase::DecidePermission(
     content::WebContents* web_contents,
     const PermissionRequestID& id,
-    const GURL& requesting_frame,
-    const GURL& embedder,
+    const GURL& requesting_origin,
+    const GURL& embedder_origin,
     bool user_gesture,
     const BrowserPermissionCallback& callback) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   ContentSetting content_setting =
       profile_->GetHostContentSettingsMap()->GetContentSetting(
-          requesting_frame, embedder, permission_type_, std::string());
+          requesting_origin, embedder_origin, permission_type_, std::string());
   switch (content_setting) {
     case CONTENT_SETTING_BLOCK:
-      PermissionDecided(id, requesting_frame, embedder, callback, false);
+      PermissionDecided(
+          id, requesting_origin, embedder_origin, callback, false);
       break;

[Bernhard Bauer, 2014/07/04 10:24:13]

Could you return here, so we can move the ASK case out of this switch statement?

[Miguel Garcia, 2014/07/04 12:09:52]

Done.

     case CONTENT_SETTING_ALLOW:
-      PermissionDecided(id, requesting_frame, embedder, callback, true);
+      PermissionDecided(id, requesting_origin, embedder_origin, callback, true);
       break;
     default:
       if (PermissionBubbleManager::Enabled()) {
@@ -78,15 +78,15 @@ void PermissionContextBase::DecidePermission(
         DCHECK(bubble_manager);
         scoped_ptr<PermissionBubbleRequest> request_ptr(
             new PermissionBubbleRequestImpl(
-                requesting_frame,
+                requesting_origin,
                 user_gesture,
                 permission_type_,
                 profile_->GetPrefs()->GetString(prefs::kAcceptLanguages),
                 base::Bind(&PermissionContextBase::NotifyPermissionSet,
                            weak_factory_.GetWeakPtr(),
                            id,
-                           requesting_frame,
-                           embedder,
+                           requesting_origin,
+                           embedder_origin,
                            callback),
                            base::Bind(&PermissionContextBase::CleanUpBubble,

[Bernhard Bauer, 2014/07/04 10:24:13]

This should be aligned with the previous base::Bind.

[Miguel Garcia, 2014/07/04 12:09:52]

Done.

                                       weak_factory_.GetWeakPtr(), id)));
@@ -103,14 +103,14 @@ void PermissionContextBase::DecidePermission(
       // we're using only bubbles. crbug.com/337458
       GetQueueController()->CreateInfoBarRequest(
           id,
-          requesting_frame,
-          embedder,
+          requesting_origin,
+          embedder_origin,
           std::string(),
           base::Bind(&PermissionContextBase::NotifyPermissionSet,
                      weak_factory_.GetWeakPtr(),
                      id,
-                     requesting_frame,
-                     embedder,
+                     requesting_origin,
+                     embedder_origin,
                      callback,
                      // the queue controller takes care of persisting the
                      // permission
@@ -120,12 +120,13 @@ void PermissionContextBase::DecidePermission(
 
 void PermissionContextBase::PermissionDecided(
     const PermissionRequestID& id,
-    const GURL& requesting_frame,
-    const GURL& embedder,
+    const GURL& requesting_origin,
+    const GURL& embedder_origin,
     const BrowserPermissionCallback& callback,
     bool allowed) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-  NotifyPermissionSet(id, requesting_frame, embedder, callback, false, allowed);
+  NotifyPermissionSet(
+      id, requesting_origin, embedder_origin, callback, false, allowed);
 }
 
 PermissionQueueController* PermissionContextBase::GetQueueController() {
@@ -134,17 +135,16 @@ PermissionQueueController* PermissionContextBase::GetQueueController() {
 
 void PermissionContextBase::NotifyPermissionSet(
     const PermissionRequestID& id,
-    const GURL& requesting_frame,
-    const GURL& embedder,
+    const GURL& requesting_origin,
+    const GURL& embedder_origin,
     const BrowserPermissionCallback& callback,
     bool persist,
     bool allowed) {
   if (persist) {
-    UpdateContentSetting(requesting_frame, embedder, allowed);
+    UpdateContentSetting(requesting_origin, embedder_origin, allowed);

[Bernhard Bauer, 2014/07/04 10:24:13]

Nit: braces are unnecessary for single-line bodies.

[Miguel Garcia, 2014/07/04 12:09:52]

Done.

   }
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-  UpdateTabContext(id, requesting_frame, allowed);
-
+  UpdateTabContext(id, requesting_origin, allowed);
   callback.Run(allowed);
 }
 
@@ -160,17 +160,15 @@ void PermissionContextBase::CleanUpBubble(const PermissionRequestID& id) {
 }
 
 void PermissionContextBase::UpdateContentSetting(
-    const GURL& requesting_frame,
-    const GURL& embedder,
+    const GURL& requesting_origin,
+    const GURL& embedder_origin,
     bool allowed) {
   ContentSetting content_setting =
       allowed ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
   profile_->GetHostContentSettingsMap()->SetContentSetting(
-      ContentSettingsPattern::FromURLNoWildcard(requesting_frame.GetOrigin()),
-      ContentSettingsPattern::FromURLNoWildcard(embedder.GetOrigin()),
+      ContentSettingsPattern::FromURLNoWildcard(requesting_origin.GetOrigin()),

[Michael van Ouwerkerk, 2014/07/04 10:34:32]

origin.GetOrigin() looks redundant. Is any code passing in something other than
the actual origin? Maybe add a DCHECK for origin.spec() !=
origin.GetOrigin().spec() at the entry point.

[Miguel Garcia, 2014/07/04 12:09:52]

Good idea,added the DCHECK

+      ContentSettingsPattern::FromURLNoWildcard(embedder_origin.GetOrigin()),
       permission_type_,
       std::string(),
       content_setting);
 }
-
-}  // namespace gcm