Add a PermissionContext subclass for the midi permission
This adds a PermissionContext subclass for the midi permission, removing the
special casing from PermissionManager. This is necessary because the midi
permission will have a feature policy associated with it and feature policy
will take effect in the PermissionContext subclass in
GetPermissionStatusInternal. It needs to live here so that the permission
status checks in PermissionContextBase::RequestPermission return the
correct values. It is also just where all other permission checks live (like
secure origin, kill switch, etc.).
BUG=689802
Review-Url: https://codereview.chromium.org/2897833002
Cr-Commit-Position: refs/heads/master@{#475272}
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index c66a68d1..b259642 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -577,6 +577,8 @@
"media/media_url_constants.h",
"media/midi_permission_context.cc",
"media/midi_permission_context.h",
+ "media/midi_sysex_permission_context.cc",
+ "media/midi_sysex_permission_context.h",
"media/output_protection_proxy.cc",
"media/output_protection_proxy.h",
"media/router/media_router_feature.cc",
diff --git a/chrome/browser/media/midi_permission_context.cc b/chrome/browser/media/midi_permission_context.cc
index 218f9b9..7e880138 100644
--- a/chrome/browser/media/midi_permission_context.cc
+++ b/chrome/browser/media/midi_permission_context.cc
@@ -4,35 +4,17 @@
#include "chrome/browser/media/midi_permission_context.h"
-#include "chrome/browser/content_settings/tab_specific_content_settings.h"
-#include "chrome/browser/permissions/permission_request_id.h"
-#include "content/public/browser/child_process_security_policy.h"
-#include "url/gurl.h"
-
MidiPermissionContext::MidiPermissionContext(Profile* profile)
- : PermissionContextBase(profile,
- CONTENT_SETTINGS_TYPE_MIDI_SYSEX) {}
+ : PermissionContextBase(profile, CONTENT_SETTINGS_TYPE_MIDI) {}
MidiPermissionContext::~MidiPermissionContext() {
}
-void MidiPermissionContext::UpdateTabContext(const PermissionRequestID& id,
- const GURL& requesting_frame,
- bool allowed) {
- TabSpecificContentSettings* content_settings =
- TabSpecificContentSettings::GetForFrame(id.render_process_id(),
- id.render_frame_id());
- if (!content_settings)
- return;
-
- if (allowed) {
- content_settings->OnMidiSysExAccessed(requesting_frame);
-
- content::ChildProcessSecurityPolicy::GetInstance()->
- GrantSendMidiSysExMessage(id.render_process_id());
- } else {
- content_settings->OnMidiSysExAccessBlocked(requesting_frame);
- }
+ContentSetting MidiPermissionContext::GetPermissionStatusInternal(
+ content::RenderFrameHost* render_frame_host,
+ const GURL& requesting_origin,
+ const GURL& embedding_origin) const {
+ return CONTENT_SETTING_ALLOW;
}
bool MidiPermissionContext::IsRestrictedToSecureOrigins() const {
diff --git a/chrome/browser/media/midi_permission_context.h b/chrome/browser/media/midi_permission_context.h
index fe94d4b..5282655 100644
--- a/chrome/browser/media/midi_permission_context.h
+++ b/chrome/browser/media/midi_permission_context.h
@@ -8,9 +8,6 @@
#include "base/macros.h"
#include "chrome/browser/permissions/permission_context_base.h"
-class GURL;
-class PermissionRequestID;
-
class MidiPermissionContext : public PermissionContextBase {
public:
explicit MidiPermissionContext(Profile* profile);
@@ -18,9 +15,10 @@
private:
// PermissionContextBase:
- void UpdateTabContext(const PermissionRequestID& id,
- const GURL& requesting_frame,
- bool allowed) override;
+ ContentSetting GetPermissionStatusInternal(
+ content::RenderFrameHost* render_frame_host,
+ const GURL& requesting_origin,
+ const GURL& embedding_origin) const override;
bool IsRestrictedToSecureOrigins() const override;
DISALLOW_COPY_AND_ASSIGN(MidiPermissionContext);
diff --git a/chrome/browser/media/midi_sysex_permission_context.cc b/chrome/browser/media/midi_sysex_permission_context.cc
new file mode 100644
index 0000000..689e141a
--- /dev/null
+++ b/chrome/browser/media/midi_sysex_permission_context.cc
@@ -0,0 +1,38 @@
+// Copyright 2017 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/media/midi_sysex_permission_context.h"
+
+#include "chrome/browser/content_settings/tab_specific_content_settings.h"
+#include "chrome/browser/permissions/permission_request_id.h"
+#include "content/public/browser/child_process_security_policy.h"
+#include "url/gurl.h"
+
+MidiSysexPermissionContext::MidiSysexPermissionContext(Profile* profile)
+ : PermissionContextBase(profile, CONTENT_SETTINGS_TYPE_MIDI_SYSEX) {}
+
+MidiSysexPermissionContext::~MidiSysexPermissionContext() {}
+
+void MidiSysexPermissionContext::UpdateTabContext(const PermissionRequestID& id,
+ const GURL& requesting_frame,
+ bool allowed) {
+ TabSpecificContentSettings* content_settings =
+ TabSpecificContentSettings::GetForFrame(id.render_process_id(),
+ id.render_frame_id());
+ if (!content_settings)
+ return;
+
+ if (allowed) {
+ content_settings->OnMidiSysExAccessed(requesting_frame);
+
+ content::ChildProcessSecurityPolicy::GetInstance()
+ ->GrantSendMidiSysExMessage(id.render_process_id());
+ } else {
+ content_settings->OnMidiSysExAccessBlocked(requesting_frame);
+ }
+}
+
+bool MidiSysexPermissionContext::IsRestrictedToSecureOrigins() const {
+ return true;
+}
diff --git a/chrome/browser/media/midi_sysex_permission_context.h b/chrome/browser/media/midi_sysex_permission_context.h
new file mode 100644
index 0000000..f7aa8cf
--- /dev/null
+++ b/chrome/browser/media/midi_sysex_permission_context.h
@@ -0,0 +1,29 @@
+// Copyright 2017 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.
+
+#ifndef CHROME_BROWSER_MEDIA_MIDI_SYSEX_PERMISSION_CONTEXT_H_
+#define CHROME_BROWSER_MEDIA_MIDI_SYSEX_PERMISSION_CONTEXT_H_
+
+#include "base/macros.h"
+#include "chrome/browser/permissions/permission_context_base.h"
+
+class GURL;
+class PermissionRequestID;
+
+class MidiSysexPermissionContext : public PermissionContextBase {
+ public:
+ explicit MidiSysexPermissionContext(Profile* profile);
+ ~MidiSysexPermissionContext() override;
+
+ private:
+ // PermissionContextBase:
+ void UpdateTabContext(const PermissionRequestID& id,
+ const GURL& requesting_frame,
+ bool allowed) override;
+ bool IsRestrictedToSecureOrigins() const override;
+
+ DISALLOW_COPY_AND_ASSIGN(MidiSysexPermissionContext);
+};
+
+#endif // CHROME_BROWSER_MEDIA_MIDI_SYSEX_PERMISSION_CONTEXT_H_
diff --git a/chrome/browser/media/midi_permission_context_unittest.cc b/chrome/browser/media/midi_sysex_permission_context_unittest.cc
similarity index 70%
rename from chrome/browser/media/midi_permission_context_unittest.cc
rename to chrome/browser/media/midi_sysex_permission_context_unittest.cc
index 9b90980f..72e3c1b 100644
--- a/chrome/browser/media/midi_permission_context_unittest.cc
+++ b/chrome/browser/media/midi_sysex_permission_context_unittest.cc
@@ -1,13 +1,12 @@
-// Copyright 2015 The Chromium Authors. All rights reserved.
+// Copyright 2017 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/media/midi_permission_context.h"
-
#include "base/bind.h"
#include "base/macros.h"
#include "build/build_config.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
+#include "chrome/browser/media/midi_permission_context.h"
#include "chrome/browser/permissions/permission_request_id.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "chrome/test/base/testing_profile.h"
@@ -30,24 +29,18 @@
class TestPermissionContext : public MidiPermissionContext {
public:
explicit TestPermissionContext(Profile* profile)
- : MidiPermissionContext(profile),
- permission_set_(false),
- permission_granted_(false),
- tab_context_updated_(false) {}
+ : MidiPermissionContext(profile),
+ permission_set_(false),
+ permission_granted_(false),
+ tab_context_updated_(false) {}
~TestPermissionContext() override {}
- bool permission_granted() {
- return permission_granted_;
- }
+ bool permission_granted() { return permission_granted_; }
- bool permission_set() {
- return permission_set_;
- }
+ bool permission_set() { return permission_set_; }
- bool tab_context_updated() {
- return tab_context_updated_;
- }
+ bool tab_context_updated() { return tab_context_updated_; }
void TrackPermissionDecision(ContentSetting content_setting) {
permission_set_ = true;
@@ -62,9 +55,9 @@
}
private:
- bool permission_set_;
- bool permission_granted_;
- bool tab_context_updated_;
+ bool permission_set_;
+ bool permission_granted_;
+ bool tab_context_updated_;
};
} // anonymous namespace
@@ -93,13 +86,11 @@
GURL url("http://www.example.com");
content::WebContentsTester::For(web_contents())->NavigateAndCommit(url);
- const PermissionRequestID id(
- web_contents()->GetRenderProcessHost()->GetID(),
- web_contents()->GetMainFrame()->GetRoutingID(),
- -1);
+ const PermissionRequestID id(web_contents()->GetRenderProcessHost()->GetID(),
+ web_contents()->GetMainFrame()->GetRoutingID(),
+ -1);
permission_context.RequestPermission(
- web_contents(),
- id, url, true,
+ web_contents(), id, url, true,
base::Bind(&TestPermissionContext::TrackPermissionDecision,
base::Unretained(&permission_context)));
@@ -109,10 +100,8 @@
ContentSetting setting =
HostContentSettingsMapFactory::GetForProfile(profile())
- ->GetContentSetting(url.GetOrigin(),
- url.GetOrigin(),
- CONTENT_SETTINGS_TYPE_MIDI_SYSEX,
- std::string());
+ ->GetContentSetting(url.GetOrigin(), url.GetOrigin(),
+ CONTENT_SETTINGS_TYPE_MIDI_SYSEX, std::string());
EXPECT_EQ(CONTENT_SETTING_ASK, setting);
}
@@ -124,23 +113,20 @@
// Check that there is no saved content settings.
EXPECT_EQ(CONTENT_SETTING_ASK,
+ HostContentSettingsMapFactory::GetForProfile(profile())
+ ->GetContentSetting(
+ insecure_url.GetOrigin(), insecure_url.GetOrigin(),
+ CONTENT_SETTINGS_TYPE_MIDI_SYSEX, std::string()));
+ EXPECT_EQ(
+ CONTENT_SETTING_ASK,
HostContentSettingsMapFactory::GetForProfile(profile())
- ->GetContentSetting(insecure_url.GetOrigin(),
- insecure_url.GetOrigin(),
- CONTENT_SETTINGS_TYPE_MIDI_SYSEX,
- std::string()));
- EXPECT_EQ(CONTENT_SETTING_ASK,
+ ->GetContentSetting(secure_url.GetOrigin(), insecure_url.GetOrigin(),
+ CONTENT_SETTINGS_TYPE_MIDI_SYSEX, std::string()));
+ EXPECT_EQ(
+ CONTENT_SETTING_ASK,
HostContentSettingsMapFactory::GetForProfile(profile())
- ->GetContentSetting(secure_url.GetOrigin(),
- insecure_url.GetOrigin(),
- CONTENT_SETTINGS_TYPE_MIDI_SYSEX,
- std::string()));
- EXPECT_EQ(CONTENT_SETTING_ASK,
- HostContentSettingsMapFactory::GetForProfile(profile())
- ->GetContentSetting(insecure_url.GetOrigin(),
- secure_url.GetOrigin(),
- CONTENT_SETTINGS_TYPE_MIDI_SYSEX,
- std::string()));
+ ->GetContentSetting(insecure_url.GetOrigin(), secure_url.GetOrigin(),
+ CONTENT_SETTINGS_TYPE_MIDI_SYSEX, std::string()));
EXPECT_EQ(CONTENT_SETTING_BLOCK,
permission_context
diff --git a/chrome/browser/permissions/permission_manager.cc b/chrome/browser/permissions/permission_manager.cc
index fc69174..bed2a61 100644
--- a/chrome/browser/permissions/permission_manager.cc
+++ b/chrome/browser/permissions/permission_manager.cc
@@ -13,6 +13,7 @@
#include "chrome/browser/background_sync/background_sync_permission_context.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/media/midi_permission_context.h"
+#include "chrome/browser/media/midi_sysex_permission_context.h"
#include "chrome/browser/media/webrtc/media_stream_device_permission_context.h"
#include "chrome/browser/notifications/notification_permission_context.h"
#include "chrome/browser/permissions/permission_context_base.h"
@@ -111,14 +112,6 @@
return CONTENT_SETTINGS_TYPE_DEFAULT;
}
-// Returns whether the permission has a constant ContentSetting value (i.e.
-// always approved or always denied)
-// The ContentSettingsTypes for which true is returned will also return nullptr
-// in PermissionManager::GetPermissionContext since they don't have a context.
-bool IsConstantPermission(ContentSettingsType type) {
- return type == CONTENT_SETTINGS_TYPE_MIDI;
-}
-
void SubscriptionCallbackWrapper(
const base::Callback<void(PermissionStatus)>& callback,
ContentSetting content_setting) {
@@ -149,22 +142,6 @@
callback.Run(vector[0]);
}
-// Function used for handling permission types which do not change their
-// value i.e. they are always approved or always denied etc.
-// CONTENT_SETTING_DEFAULT is returned if the permission needs further handling.
-// This function should only be called when IsConstantPermission has returned
-// true for the PermissionType.
-ContentSetting GetContentSettingForConstantPermission(
- ContentSettingsType type) {
- DCHECK(IsConstantPermission(type));
- switch (type) {
- case CONTENT_SETTINGS_TYPE_MIDI:
- return CONTENT_SETTING_ALLOW;
- default:
- return CONTENT_SETTING_BLOCK;
- }
-}
-
} // anonymous namespace
class PermissionManager::PendingRequest {
@@ -231,6 +208,8 @@
: profile_(profile),
weak_ptr_factory_(this) {
permission_contexts_[CONTENT_SETTINGS_TYPE_MIDI_SYSEX] =
+ base::MakeUnique<MidiSysexPermissionContext>(profile);
+ permission_contexts_[CONTENT_SETTINGS_TYPE_MIDI] =
base::MakeUnique<MidiPermissionContext>(profile);
permission_contexts_[CONTENT_SETTINGS_TYPE_PUSH_MESSAGING] =
base::MakeUnique<NotificationPermissionContext>(
@@ -313,16 +292,8 @@
for (size_t i = 0; i < permissions.size(); ++i) {
const ContentSettingsType permission = permissions[i];
- if (IsConstantPermission(permission) || !GetPermissionContext(permission)) {
- // Track permission request usages even for constant permissions.
- PermissionUmaUtil::PermissionRequested(permission, requesting_origin,
- embedding_origin, profile_);
- OnPermissionsRequestResponseStatus(
- request_id, i, GetContentSettingForConstantPermission(permission));
- continue;
- }
-
PermissionContextBase* context = GetPermissionContext(permission);
+ DCHECK(context);
context->RequestPermission(
web_contents, request, requesting_origin, user_gesture,
base::Bind(&PermissionManager::OnPermissionsRequestResponseStatus,
@@ -515,8 +486,6 @@
for (SubscriptionsMap::iterator iter(&subscriptions_);
!iter.IsAtEnd(); iter.Advance()) {
Subscription* subscription = iter.GetCurrentValue();
- if (IsConstantPermission(subscription->permission))
- continue;
if (subscription->permission != content_type)
continue;
@@ -551,13 +520,9 @@
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
const GURL& embedding_origin) {
- if (IsConstantPermission(permission)) {
- return PermissionResult(GetContentSettingForConstantPermission(permission),
- PermissionStatusSource::UNSPECIFIED);
- }
PermissionContextBase* context = GetPermissionContext(permission);
PermissionResult result = context->GetPermissionStatus(
- nullptr /* render_frame_host */, requesting_origin.GetOrigin(),
+ render_frame_host, requesting_origin.GetOrigin(),
embedding_origin.GetOrigin());
DCHECK(result.content_setting == CONTENT_SETTING_ALLOW ||
result.content_setting == CONTENT_SETTING_ASK ||
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index 2026fd6..ae524d7 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -3093,7 +3093,7 @@
"../browser/media/android/router/media_router_android_unittest.cc",
"../browser/media/cast_remoting_connector_unittest.cc",
"../browser/media/media_engagement_contents_observer_unittest.cc",
- "../browser/media/midi_permission_context_unittest.cc",
+ "../browser/media/midi_sysex_permission_context_unittest.cc",
"../browser/media/router/browser_presentation_connection_proxy_unittest.cc",
"../browser/media/router/create_presentation_connection_request_unittest.cc",
"../browser/media/router/issue_manager_unittest.cc",
