MidiPermissionContext::GetPermission() returns BLOCK for insecure origins.

chrome/browser/media/midi_permission_context.cc

Index: chrome/browser/media/midi_permission_context.cc
diff --git a/chrome/browser/media/midi_permission_context.cc b/chrome/browser/media/midi_permission_context.cc
index 715447d53907ff50c8b9855aa2ca408e9164eb5c..95cffbb0fa7a99d0e5f27f806492e75380598baf 100644
--- a/chrome/browser/media/midi_permission_context.cc
+++ b/chrome/browser/media/midi_permission_context.cc
@@ -7,6 +7,7 @@
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "components/content_settings/core/common/permission_request_id.h"
 #include "content/public/browser/child_process_security_policy.h"
+#include "content/public/common/origin_util.h"
 #include "url/gurl.h"
 
 MidiPermissionContext::MidiPermissionContext(Profile* profile)
@@ -16,6 +17,36 @@ MidiPermissionContext::MidiPermissionContext(Profile* profile)
 MidiPermissionContext::~MidiPermissionContext() {
 }
 
+ContentSetting MidiPermissionContext::GetPermissionStatus(
+    const GURL& requesting_origin,
+    const GURL& embedding_origin) const {
+  if (requesting_origin.is_valid() &&
+      !content::IsOriginSecure(requesting_origin)) {

[ddorwin, 2015/05/22 16:18:59]

This is something we'll probably have to deal with for multiple permissions in
the future.

Would it be better to put this logic in the base and add an
AreInsecureOriginsAllowed() pure virtual function that children must implement?
That way, each child implements a single bool and all will have the same
behavior (with no risk of forgetting to add it).

If Base needs to be concrete, the method could return false by default.

[mlamouri (slow - plz ping), 2015/05/22 18:15:59]

I would happily do that when there will be more than one context doing that kind
of logic. It's actually something I was thinking about. However, at the moment,
Midi is the only one because the other ones check for the security origin at the
Blink level. I'm not a big fan of doing design for things that might happen in
the future.

[ddorwin, 2015/05/22 19:57:53]

OOC, what other permissions are checked at the Blink level? Via the individual
API paths or the Permissions API? If we make that assumption here, we should
probably have DCHECKs for it.

It's potentially not obvious to someone working on a feature that these methods
support the Permissions API and would need a check similar to the one in Blink.
An explicit pure virtual method would require that this be considered.

I understand the desire to avoid over-engineering for what might be, but a) we
do know that more features will require secure origins
(https://www.chromium.org/Home/chromium-security/deprecating-powerful-features...)
and b) the current solution uses inheritance when all that is needed is a
Boolean to choose the behavior. Even if YAGNI for other permissions, it seems
that avoiding the overrides would be the better option.

+    return CONTENT_SETTING_BLOCK;
+  }
+
+  return PermissionContextBase::GetPermissionStatus(requesting_origin,
+                                                    embedding_origin);
+}
+
+void MidiPermissionContext::DecidePermission(
+    content::WebContents* web_contents,
+    const PermissionRequestID& id,
+    const GURL& requesting_origin,
+    const GURL& embedding_origin,
+    bool user_gesture,
+    const BrowserPermissionCallback& callback) {
+  if (requesting_origin.is_valid() &&
+      !content::IsOriginSecure(requesting_origin)) {
+    NotifyPermissionSet(id, requesting_origin, embedding_origin, callback,
+                        false /* persist */, CONTENT_SETTING_BLOCK);
+    return;
+  }
+
+  PermissionContextBase::DecidePermission(
+      web_contents, id, requesting_origin,
+      embedding_origin, user_gesture, callback);
+}
 
 void MidiPermissionContext::UpdateTabContext(const PermissionRequestID& id,
                                              const GURL& requesting_frame,