Use IDType for permission change subscriptions.
Bug: 1025683
Change-Id: I3b44ba7833138e8a657a4192e1a36c978695db32
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2791431
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Yuchen Liu <yucliu@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Fabrice de Gans-Riberi <fdegans@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Illia Klimov <elklm@google.com>
Auto-Submit: Balazs Engedy <engedy@chromium.org>
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#867999}
diff --git a/android_webview/browser/aw_permission_manager.cc b/android_webview/browser/aw_permission_manager.cc
index bb90990..b7d1c4e 100644
--- a/android_webview/browser/aw_permission_manager.cc
+++ b/android_webview/browser/aw_permission_manager.cc
@@ -470,16 +470,17 @@
.GetOrigin());
}
-int AwPermissionManager::SubscribePermissionStatusChange(
+AwPermissionManager::SubscriptionId
+AwPermissionManager::SubscribePermissionStatusChange(
PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(PermissionStatus)> callback) {
- return content::PermissionController::kNoPendingOperation;
+ return SubscriptionId();
}
void AwPermissionManager::UnsubscribePermissionStatusChange(
- int subscription_id) {}
+ SubscriptionId subscription_id) {}
void AwPermissionManager::CancelPermissionRequest(int request_id) {
PendingRequest* pending_request = pending_requests_.Lookup(request_id);
diff --git a/android_webview/browser/aw_permission_manager.h b/android_webview/browser/aw_permission_manager.h
index d9670ca..7439e78 100644
--- a/android_webview/browser/aw_permission_manager.h
+++ b/android_webview/browser/aw_permission_manager.h
@@ -49,13 +49,14 @@
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin) override;
- int SubscribePermissionStatusChange(
+ SubscriptionId SubscribePermissionStatusChange(
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(blink::mojom::PermissionStatus)> callback)
override;
- void UnsubscribePermissionStatusChange(int subscription_id) override;
+ void UnsubscribePermissionStatusChange(
+ SubscriptionId subscription_id) override;
protected:
void CancelPermissionRequest(int request_id);
diff --git a/chrome/browser/permissions/permission_manager_browsertest.cc b/chrome/browser/permissions/permission_manager_browsertest.cc
index a6c31af..35773077 100644
--- a/chrome/browser/permissions/permission_manager_browsertest.cc
+++ b/chrome/browser/permissions/permission_manager_browsertest.cc
@@ -49,13 +49,13 @@
callback_ = std::move(callback);
}
- int SubscribePermissionStatusChange(
+ SubscriptionId SubscribePermissionStatusChange(
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(blink::mojom::PermissionStatus)> callback)
override {
- int result =
+ SubscriptionId result =
permissions::PermissionManager::SubscribePermissionStatusChange(
permission, render_frame_host, requesting_origin, callback);
std::move(callback_).Run();
diff --git a/chromecast/browser/cast_permission_manager.cc b/chromecast/browser/cast_permission_manager.cc
index b358fc3b..b3a226d 100644
--- a/chromecast/browser/cast_permission_manager.cc
+++ b/chromecast/browser/cast_permission_manager.cc
@@ -63,17 +63,17 @@
return blink::mojom::PermissionStatus::GRANTED;
}
-int CastPermissionManager::SubscribePermissionStatusChange(
+CastPermissionManager::SubscriptionId
+CastPermissionManager::SubscribePermissionStatusChange(
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(blink::mojom::PermissionStatus)> callback) {
- return content::PermissionController::kNoPendingOperation;
+ return SubscriptionId();
}
void CastPermissionManager::UnsubscribePermissionStatusChange(
- int subscription_id) {
-}
+ SubscriptionId subscription_id) {}
} // namespace shell
} // namespace chromecast
diff --git a/chromecast/browser/cast_permission_manager.h b/chromecast/browser/cast_permission_manager.h
index 564ac2b..f9da64c6 100644
--- a/chromecast/browser/cast_permission_manager.h
+++ b/chromecast/browser/cast_permission_manager.h
@@ -43,13 +43,14 @@
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin) override;
- int SubscribePermissionStatusChange(
+ SubscriptionId SubscribePermissionStatusChange(
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(blink::mojom::PermissionStatus)> callback)
override;
- void UnsubscribePermissionStatusChange(int subscription_id) override;
+ void UnsubscribePermissionStatusChange(
+ SubscriptionId subscription_id) override;
private:
DISALLOW_COPY_AND_ASSIGN(CastPermissionManager);
diff --git a/components/permissions/permission_manager.cc b/components/permissions/permission_manager.cc
index 76eddc0..a5dab29 100644
--- a/components/permissions/permission_manager.cc
+++ b/components/permissions/permission_manager.cc
@@ -537,14 +537,15 @@
origin->GetURL());
}
-int PermissionManager::SubscribePermissionStatusChange(
+PermissionManager::SubscriptionId
+PermissionManager::SubscribePermissionStatusChange(
PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(PermissionStatus)> callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (is_shutting_down_)
- return 0;
+ return SubscriptionId();
if (subscriptions_.IsEmpty())
PermissionsClient::Get()
@@ -581,16 +582,20 @@
subscription->callback =
base::BindRepeating(&SubscriptionCallbackWrapper, std::move(callback));
- return subscriptions_.Add(std::move(subscription));
+ auto id = subscription_id_generator_.GenerateNextId();
+ subscriptions_.AddWithID(std::move(subscription), id);
+ return id;
}
-void PermissionManager::UnsubscribePermissionStatusChange(int subscription_id) {
+void PermissionManager::UnsubscribePermissionStatusChange(
+ SubscriptionId subscription_id) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (is_shutting_down_)
return;
- // Whether |subscription_id| is known will be checked by the Remove() call.
- subscriptions_.Remove(subscription_id);
+ if (subscriptions_.Lookup(subscription_id)) {
+ subscriptions_.Remove(subscription_id);
+ }
if (subscriptions_.IsEmpty()) {
PermissionsClient::Get()
diff --git a/components/permissions/permission_manager.h b/components/permissions/permission_manager.h
index 47931b9..d3e71c1 100644
--- a/components/permissions/permission_manager.h
+++ b/components/permissions/permission_manager.h
@@ -114,13 +114,14 @@
bool IsPermissionOverridableByDevTools(
content::PermissionType permission,
const base::Optional<url::Origin>& origin) override;
- int SubscribePermissionStatusChange(
+ SubscriptionId SubscribePermissionStatusChange(
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(blink::mojom::PermissionStatus)> callback)
override;
- void UnsubscribePermissionStatusChange(int subscription_id) override;
+ void UnsubscribePermissionStatusChange(
+ SubscriptionId subscription_id) override;
// TODO(raymes): Rather than exposing this, use the denial reason from
// GetPermissionStatus in callers to determine whether a permission is
@@ -153,7 +154,8 @@
class PermissionResponseCallback;
struct Subscription;
- using SubscriptionsMap = base::IDMap<std::unique_ptr<Subscription>>;
+ using SubscriptionsMap =
+ base::IDMap<std::unique_ptr<Subscription>, SubscriptionId>;
PermissionContextBase* GetPermissionContext(ContentSettingsType type);
@@ -185,6 +187,7 @@
content::BrowserContext* browser_context_;
PendingRequestsMap pending_requests_;
SubscriptionsMap subscriptions_;
+ SubscriptionId::Generator subscription_id_generator_;
PermissionContextMap permission_contexts_;
using ContentSettingsTypeOverrides =
diff --git a/components/permissions/permission_manager_unittest.cc b/components/permissions/permission_manager_unittest.cc
index 1946121..f390e32 100644
--- a/components/permissions/permission_manager_unittest.cc
+++ b/components/permissions/permission_manager_unittest.cc
@@ -343,7 +343,7 @@
}
TEST_F(PermissionManagerTest, SubscribeUnsubscribeAfterShutdown) {
- int subscription_id =
+ content::PermissionControllerDelegate::SubscriptionId subscription_id =
GetPermissionControllerDelegate()->SubscribePermissionStatusChange(
PermissionType::GEOLOCATION, main_rfh(), url(),
base::BindRepeating(&PermissionManagerTest::OnPermissionChange,
@@ -358,7 +358,7 @@
subscription_id);
// Check that subscribe/unsubscribe after shutdown don't crash.
- int subscription2_id =
+ content::PermissionControllerDelegate::SubscriptionId subscription2_id =
GetPermissionControllerDelegate()->SubscribePermissionStatusChange(
PermissionType::GEOLOCATION, main_rfh(), url(),
base::BindRepeating(&PermissionManagerTest::OnPermissionChange,
@@ -369,7 +369,7 @@
}
TEST_F(PermissionManagerTest, SameTypeChangeNotifies) {
- int subscription_id =
+ content::PermissionControllerDelegate::SubscriptionId subscription_id =
GetPermissionControllerDelegate()->SubscribePermissionStatusChange(
PermissionType::GEOLOCATION, main_rfh(), url(),
base::BindRepeating(&PermissionManagerTest::OnPermissionChange,
@@ -386,7 +386,7 @@
}
TEST_F(PermissionManagerTest, DifferentTypeChangeDoesNotNotify) {
- int subscription_id =
+ content::PermissionControllerDelegate::SubscriptionId subscription_id =
GetPermissionControllerDelegate()->SubscribePermissionStatusChange(
PermissionType::GEOLOCATION, main_rfh(), url(),
base::BindRepeating(&PermissionManagerTest::OnPermissionChange,
@@ -402,7 +402,7 @@
}
TEST_F(PermissionManagerTest, ChangeAfterUnsubscribeDoesNotNotify) {
- int subscription_id =
+ content::PermissionControllerDelegate::SubscriptionId subscription_id =
GetPermissionControllerDelegate()->SubscribePermissionStatusChange(
PermissionType::GEOLOCATION, main_rfh(), url(),
base::BindRepeating(&PermissionManagerTest::OnPermissionChange,
@@ -418,7 +418,7 @@
}
TEST_F(PermissionManagerTest, DifferentPrimaryUrlDoesNotNotify) {
- int subscription_id =
+ content::PermissionControllerDelegate::SubscriptionId subscription_id =
GetPermissionControllerDelegate()->SubscribePermissionStatusChange(
PermissionType::GEOLOCATION, main_rfh(), url(),
base::BindRepeating(&PermissionManagerTest::OnPermissionChange,
@@ -435,7 +435,7 @@
}
TEST_F(PermissionManagerTest, DifferentSecondaryUrlDoesNotNotify) {
- int subscription_id =
+ content::PermissionControllerDelegate::SubscriptionId subscription_id =
GetPermissionControllerDelegate()->SubscribePermissionStatusChange(
PermissionType::STORAGE_ACCESS_GRANT, main_rfh(), url(),
base::BindRepeating(&PermissionManagerTest::OnPermissionChange,
@@ -452,7 +452,7 @@
}
TEST_F(PermissionManagerTest, WildCardPatternNotifies) {
- int subscription_id =
+ content::PermissionControllerDelegate::SubscriptionId subscription_id =
GetPermissionControllerDelegate()->SubscribePermissionStatusChange(
PermissionType::GEOLOCATION, main_rfh(), url(),
base::BindRepeating(&PermissionManagerTest::OnPermissionChange,
@@ -472,7 +472,7 @@
GetHostContentSettingsMap()->SetContentSettingDefaultScope(
url(), url(), ContentSettingsType::GEOLOCATION, CONTENT_SETTING_ALLOW);
- int subscription_id =
+ content::PermissionControllerDelegate::SubscriptionId subscription_id =
GetPermissionControllerDelegate()->SubscribePermissionStatusChange(
PermissionType::GEOLOCATION, main_rfh(), url(),
base::BindRepeating(&PermissionManagerTest::OnPermissionChange,
@@ -489,7 +489,7 @@
}
TEST_F(PermissionManagerTest, NewValueCorrectlyPassed) {
- int subscription_id =
+ content::PermissionControllerDelegate::SubscriptionId subscription_id =
GetPermissionControllerDelegate()->SubscribePermissionStatusChange(
PermissionType::GEOLOCATION, main_rfh(), url(),
base::BindRepeating(&PermissionManagerTest::OnPermissionChange,
@@ -509,7 +509,7 @@
GetHostContentSettingsMap()->SetContentSettingDefaultScope(
url(), url(), ContentSettingsType::GEOLOCATION, CONTENT_SETTING_ALLOW);
- int subscription_id =
+ content::PermissionControllerDelegate::SubscriptionId subscription_id =
GetPermissionControllerDelegate()->SubscribePermissionStatusChange(
PermissionType::GEOLOCATION, main_rfh(), url(),
base::BindRepeating(&PermissionManagerTest::OnPermissionChange,
@@ -528,7 +528,7 @@
GetHostContentSettingsMap()->SetContentSettingDefaultScope(
url(), url(), ContentSettingsType::GEOLOCATION, CONTENT_SETTING_ASK);
- int subscription_id =
+ content::PermissionControllerDelegate::SubscriptionId subscription_id =
GetPermissionControllerDelegate()->SubscribePermissionStatusChange(
PermissionType::GEOLOCATION, main_rfh(), url(),
base::BindRepeating(&PermissionManagerTest::OnPermissionChange,
@@ -556,7 +556,7 @@
GetHostContentSettingsMap()->SetContentSettingDefaultScope(
url(), url(), ContentSettingsType::GEOLOCATION, CONTENT_SETTING_ASK);
- int subscription_id =
+ content::PermissionControllerDelegate::SubscriptionId subscription_id =
GetPermissionControllerDelegate()->SubscribePermissionStatusChange(
PermissionType::GEOLOCATION, nullptr, url(),
base::BindRepeating(&PermissionManagerTest::OnPermissionChange,
@@ -581,7 +581,7 @@
}
TEST_F(PermissionManagerTest, SubscribeMIDIPermission) {
- int subscription_id =
+ content::PermissionControllerDelegate::SubscriptionId subscription_id =
GetPermissionControllerDelegate()->SubscribePermissionStatusChange(
PermissionType::MIDI, main_rfh(), url(),
base::BindRepeating(&PermissionManagerTest::OnPermissionChange,
@@ -793,7 +793,7 @@
content::RenderFrameHost* parent = main_rfh();
content::RenderFrameHost* child = AddChildRFH(parent, kOrigin2);
- int subscription_id =
+ content::PermissionControllerDelegate::SubscriptionId subscription_id =
GetPermissionControllerDelegate()->SubscribePermissionStatusChange(
PermissionType::GEOLOCATION, child, GURL(kOrigin2),
base::BindRepeating(&PermissionManagerTest::OnPermissionChange,
diff --git a/content/browser/android/nfc_host.cc b/content/browser/android/nfc_host.cc
index 6f6ca797..0642493 100644
--- a/content/browser/android/nfc_host.cc
+++ b/content/browser/android/nfc_host.cc
@@ -50,7 +50,7 @@
return;
}
- if (subscription_id_ == PermissionController::kNoPendingOperation) {
+ if (!subscription_id_) {
// base::Unretained() is safe here because the subscription is canceled when
// this object is destroyed.
subscription_id_ = permission_controller_->SubscribePermissionStatusChange(
@@ -106,10 +106,8 @@
void NFCHost::Close() {
nfc_provider_.reset();
- if (subscription_id_ != PermissionController::kNoPendingOperation) {
- permission_controller_->UnsubscribePermissionStatusChange(subscription_id_);
- subscription_id_ = PermissionController::kNoPendingOperation;
- }
+ permission_controller_->UnsubscribePermissionStatusChange(subscription_id_);
+ subscription_id_ = PermissionController::SubscriptionId();
}
} // namespace content
diff --git a/content/browser/android/nfc_host.h b/content/browser/android/nfc_host.h
index 25af96d..f881abf 100644
--- a/content/browser/android/nfc_host.h
+++ b/content/browser/android/nfc_host.h
@@ -44,7 +44,7 @@
mojo::Remote<device::mojom::NFCProvider> nfc_provider_;
// Permission change subscription ID provided by |permission_controller_|.
- int subscription_id_ = PermissionController::kNoPendingOperation;
+ PermissionController::SubscriptionId subscription_id_;
DISALLOW_COPY_AND_ASSIGN(NFCHost);
};
diff --git a/content/browser/android/nfc_host_unittest.cc b/content/browser/android/nfc_host_unittest.cc
index cac4625..f40b16d 100644
--- a/content/browser/android/nfc_host_unittest.cc
+++ b/content/browser/android/nfc_host_unittest.cc
@@ -48,7 +48,7 @@
};
TEST_F(NFCHostTest, GetNFCTwice) {
- constexpr int kSubscriptionId = 42;
+ constexpr MockPermissionManager::SubscriptionId kSubscriptionId(42);
NavigateAndCommit(GURL(kTestUrl));
diff --git a/content/browser/permissions/permission_controller_impl.cc b/content/browser/permissions/permission_controller_impl.cc
index 98c77388..655066a 100644
--- a/content/browser/permissions/permission_controller_impl.cc
+++ b/content/browser/permissions/permission_controller_impl.cc
@@ -132,7 +132,8 @@
int render_frame_id = -1;
int render_process_id = -1;
base::RepeatingCallback<void(blink::mojom::PermissionStatus)> callback;
- int delegate_subscription_id;
+ // This is default-initialized to an invalid ID.
+ PermissionControllerDelegate::SubscriptionId delegate_subscription_id;
};
PermissionControllerImpl::~PermissionControllerImpl() {
@@ -388,7 +389,8 @@
subscription->callback.Run(status);
}
-int PermissionControllerImpl::SubscribePermissionStatusChange(
+PermissionControllerImpl::SubscriptionId
+PermissionControllerImpl::SubscribePermissionStatusChange(
PermissionType permission,
RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
@@ -422,21 +424,21 @@
base::BindRepeating(
&PermissionControllerImpl::OnDelegatePermissionStatusChange,
base::Unretained(this), subscription.get()));
- } else {
- subscription->delegate_subscription_id = kNoPendingOperation;
}
- return subscriptions_.Add(std::move(subscription));
+
+ auto id = subscription_id_generator_.GenerateNextId();
+ subscriptions_.AddWithID(std::move(subscription), id);
+ return id;
}
void PermissionControllerImpl::UnsubscribePermissionStatusChange(
- int subscription_id) {
+ SubscriptionId subscription_id) {
Subscription* subscription = subscriptions_.Lookup(subscription_id);
if (!subscription)
return;
PermissionControllerDelegate* delegate =
browser_context_->GetPermissionControllerDelegate();
- if (delegate &&
- subscription->delegate_subscription_id != kNoPendingOperation) {
+ if (delegate) {
delegate->UnsubscribePermissionStatusChange(
subscription->delegate_subscription_id);
}
diff --git a/content/browser/permissions/permission_controller_impl.h b/content/browser/permissions/permission_controller_impl.h
index 7ebf3c4..d857888 100644
--- a/content/browser/permissions/permission_controller_impl.h
+++ b/content/browser/permissions/permission_controller_impl.h
@@ -72,18 +72,19 @@
const GURL& requesting_origin,
const GURL& embedding_origin);
- int SubscribePermissionStatusChange(
+ SubscriptionId SubscribePermissionStatusChange(
PermissionType permission,
RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
const base::RepeatingCallback<void(blink::mojom::PermissionStatus)>&
callback);
- void UnsubscribePermissionStatusChange(int subscription_id);
+ void UnsubscribePermissionStatusChange(SubscriptionId subscription_id);
private:
struct Subscription;
- using SubscriptionsMap = base::IDMap<std::unique_ptr<Subscription>>;
+ using SubscriptionsMap =
+ base::IDMap<std::unique_ptr<Subscription>, SubscriptionId>;
using SubscriptionsStatusMap =
base::flat_map<SubscriptionsMap::KeyType, blink::mojom::PermissionStatus>;
@@ -98,7 +99,13 @@
const base::Optional<url::Origin>& origin);
DevToolsPermissionOverrides devtools_permission_overrides_;
+
+ // Note that SubscriptionId is distinct from
+ // PermissionControllerDelegate::SubscriptionId, and the concrete ID values
+ // may be different as well.
SubscriptionsMap subscriptions_;
+ SubscriptionId::Generator subscription_id_generator_;
+
BrowserContext* browser_context_;
DISALLOW_COPY_AND_ASSIGN(PermissionControllerImpl);
diff --git a/content/browser/permissions/permission_service_context.cc b/content/browser/permissions/permission_service_context.cc
index 0e4ed37..756a16a2 100644
--- a/content/browser/permissions/permission_service_context.cc
+++ b/content/browser/permissions/permission_service_context.cc
@@ -11,7 +11,6 @@
#include "content/browser/permissions/permission_service_impl.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/navigation_handle.h"
-#include "content/public/browser/permission_controller.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/web_contents.h"
@@ -32,7 +31,7 @@
PermissionSubscription& operator=(const PermissionSubscription&) = delete;
~PermissionSubscription() {
- DCHECK_NE(id_, 0);
+ DCHECK(id_);
BrowserContext* browser_context = context_->GetBrowserContext();
if (browser_context) {
PermissionControllerImpl::FromBrowserContext(browser_context)
@@ -41,7 +40,7 @@
}
void OnConnectionError() {
- DCHECK_NE(id_, 0);
+ DCHECK(id_);
context_->ObserverHadConnectionError(id_);
}
@@ -49,12 +48,12 @@
observer_->OnPermissionStatusChange(status);
}
- void set_id(int id) { id_ = id; }
+ void set_id(PermissionController::SubscriptionId id) { id_ = id; }
private:
PermissionServiceContext* const context_;
mojo::Remote<blink::mojom::PermissionObserver> observer_;
- int id_ = 0;
+ PermissionController::SubscriptionId id_;
};
RENDER_DOCUMENT_HOST_USER_DATA_KEY_IMPL(PermissionServiceContext)
@@ -104,7 +103,7 @@
}
GURL requesting_origin(origin.Serialize());
- int subscription_id =
+ auto subscription_id =
PermissionControllerImpl::FromBrowserContext(browser_context)
->SubscribePermissionStatusChange(
permission_type, render_frame_host_, requesting_origin,
@@ -115,7 +114,8 @@
subscriptions_[subscription_id] = std::move(subscription);
}
-void PermissionServiceContext::ObserverHadConnectionError(int subscription_id) {
+void PermissionServiceContext::ObserverHadConnectionError(
+ PermissionController::SubscriptionId subscription_id) {
size_t erased = subscriptions_.erase(subscription_id);
DCHECK_EQ(1u, erased);
}
diff --git a/content/browser/permissions/permission_service_context.h b/content/browser/permissions/permission_service_context.h
index 3feec2c7..c2ef1db 100644
--- a/content/browser/permissions/permission_service_context.h
+++ b/content/browser/permissions/permission_service_context.h
@@ -9,6 +9,7 @@
#include <unordered_map>
#include "content/common/content_export.h"
+#include "content/public/browser/permission_controller.h"
#include "content/public/browser/permission_type.h"
#include "content/public/browser/render_document_host_user_data.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
@@ -58,7 +59,8 @@
mojo::PendingRemote<blink::mojom::PermissionObserver> observer);
// Called when the connection to a PermissionObserver has an error.
- void ObserverHadConnectionError(int subscription_id);
+ void ObserverHadConnectionError(
+ PermissionController::SubscriptionId subscription_id);
// May return nullptr during teardown, or when showing an interstitial.
BrowserContext* GetBrowserContext() const;
@@ -81,7 +83,8 @@
RenderFrameHost* const render_frame_host_;
RenderProcessHost* const render_process_host_;
mojo::UniqueReceiverSet<blink::mojom::PermissionService> services_;
- std::unordered_map<int, std::unique_ptr<PermissionSubscription>>
+ std::unordered_map<PermissionController::SubscriptionId,
+ std::unique_ptr<PermissionSubscription>>
subscriptions_;
};
diff --git a/content/browser/renderer_host/media/media_stream_manager.cc b/content/browser/renderer_host/media/media_stream_manager.cc
index 124454a..01285e4c 100644
--- a/content/browser/renderer_host/media/media_stream_manager.cc
+++ b/content/browser/renderer_host/media/media_stream_manager.cc
@@ -676,9 +676,9 @@
std::string tab_capture_device_id;
- int audio_subscription_id = PermissionControllerImpl::kNoPendingOperation;
+ PermissionController::SubscriptionId audio_subscription_id;
- int video_subscription_id = PermissionControllerImpl::kNoPendingOperation;
+ PermissionController::SubscriptionId video_subscription_id;
private:
std::vector<MediaRequestState> state_;
@@ -2733,8 +2733,8 @@
if (!controller)
return;
- int audio_subscription_id = PermissionControllerImpl::kNoPendingOperation;
- int video_subscription_id = PermissionControllerImpl::kNoPendingOperation;
+ PermissionController::SubscriptionId audio_subscription_id;
+ PermissionController::SubscriptionId video_subscription_id;
if (is_audio_request) {
// It is safe to bind base::Unretained(this) because MediaStreamManager is
@@ -2776,8 +2776,8 @@
const std::string& label,
int requesting_process_id,
int requesting_frame_id,
- int audio_subscription_id,
- int video_subscription_id) {
+ PermissionController::SubscriptionId audio_subscription_id,
+ PermissionController::SubscriptionId video_subscription_id) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
DeviceRequest* const request = FindRequest(label);
@@ -2804,8 +2804,8 @@
void MediaStreamManager::UnsubscribeFromPermissionControllerOnUIThread(
int requesting_process_id,
int requesting_frame_id,
- int audio_subscription_id,
- int video_subscription_id) {
+ PermissionController::SubscriptionId audio_subscription_id,
+ PermissionController::SubscriptionId video_subscription_id) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
PermissionControllerImpl* controller =
diff --git a/content/browser/renderer_host/media/media_stream_manager.h b/content/browser/renderer_host/media/media_stream_manager.h
index ccc85b9..0fc874bf 100644
--- a/content/browser/renderer_host/media/media_stream_manager.h
+++ b/content/browser/renderer_host/media/media_stream_manager.h
@@ -50,6 +50,7 @@
#include "content/public/browser/desktop_media_id.h"
#include "content/public/browser/media_request_state.h"
#include "content/public/browser/media_stream_request.h"
+#include "content/public/browser/permission_controller.h"
#include "media/base/video_facing.h"
#include "third_party/blink/public/common/mediastream/media_devices.h"
#include "third_party/blink/public/common/mediastream/media_stream_controls.h"
@@ -570,19 +571,20 @@
// Store the subscription ids on a DeviceRequest in order to allow
// unsubscribing when the request is deleted.
- void SetPermissionSubscriptionIDs(const std::string& label,
- int requesting_process_id,
- int requesting_frame_id,
- int audio_subscription_id,
- int video_subscription_id);
+ void SetPermissionSubscriptionIDs(
+ const std::string& label,
+ int requesting_process_id,
+ int requesting_frame_id,
+ PermissionController::SubscriptionId audio_subscription_id,
+ PermissionController::SubscriptionId video_subscription_id);
// Unsubscribe from following permission updates for the two specified
// subscription IDs. Called when a request is deleted.
static void UnsubscribeFromPermissionControllerOnUIThread(
int requesting_process_id,
int requesting_frame_id,
- int audio_subscription_id,
- int video_subscription_id);
+ PermissionController::SubscriptionId audio_subscription_id,
+ PermissionController::SubscriptionId video_subscription_id);
// Callback that the PermissionController calls when a permission is updated.
void PermissionChangedCallback(int requesting_process_id,
diff --git a/content/public/browser/permission_controller.h b/content/public/browser/permission_controller.h
index b9b42def..77fe96a 100644
--- a/content/public/browser/permission_controller.h
+++ b/content/public/browser/permission_controller.h
@@ -6,6 +6,7 @@
#define CONTENT_PUBLIC_BROWSER_PERMISSION_CONTROLLER_H_
#include "base/supports_user_data.h"
+#include "base/util/type_safety/id_type.h"
#include "content/common/content_export.h"
#include "content/public/browser/permission_type.h"
#include "third_party/blink/public/mojom/permissions/permission_status.mojom.h"
@@ -20,8 +21,13 @@
class CONTENT_EXPORT PermissionController
: public base::SupportsUserData::Data {
public:
- // Constant retured when registering and subscribing if
- // cancelling/unsubscribing at a later stage would have no effect.
+ // Identifier for an active subscription. This is intentionally a distinct
+ // type from PermissionControllerDelegate::SubscriptionId as the concrete
+ // identifier values may be different.
+ using SubscriptionId = util::IdType64<PermissionController>;
+
+ // Constant returned when requesting a permission if cancelling at a later
+ // stage would have no effect.
static const int kNoPendingOperation = -1;
~PermissionController() override {}
@@ -48,4 +54,17 @@
} // namespace content
+namespace std {
+
+template <>
+struct hash<content::PermissionController::SubscriptionId> {
+ std::size_t operator()(
+ const content::PermissionController::SubscriptionId& v) const {
+ content::PermissionController::SubscriptionId::Hasher hasher;
+ return hasher(v);
+ }
+};
+
+} // namespace std
+
#endif // CONTENT_PUBLIC_BROWSER_PERMISSION_CONTROLLER_H_
diff --git a/content/public/browser/permission_controller_delegate.h b/content/public/browser/permission_controller_delegate.h
index e47de2a..82a1d4f 100644
--- a/content/public/browser/permission_controller_delegate.h
+++ b/content/public/browser/permission_controller_delegate.h
@@ -5,6 +5,7 @@
#ifndef CONTENT_PUBLIC_BROWSER_PERMISSION_CONTROLLER_DELEGATE_H_
#define CONTENT_PUBLIC_BROWSER_PERMISSION_CONTROLLER_DELEGATE_H_
+#include "base/util/type_safety/id_type.h"
#include "content/common/content_export.h"
#include "content/public/browser/devtools_permission_overrides.h"
#include "third_party/blink/public/mojom/permissions/permission_status.mojom.h"
@@ -18,6 +19,10 @@
class CONTENT_EXPORT PermissionControllerDelegate {
public:
using PermissionOverrides = DevToolsPermissionOverrides::PermissionOverrides;
+
+ // Identifier for an active subscription.
+ using SubscriptionId = util::IdType64<PermissionControllerDelegate>;
+
virtual ~PermissionControllerDelegate() = default;
// Requests a permission on behalf of a frame identified by
@@ -80,21 +85,21 @@
// Runs the given |callback| whenever the |permission| associated with the
// given RenderFrameHost changes. A nullptr should be passed if the request
- // is from a worker. Returns the subscription_id to be used to unsubscribe.
- // Can be kNoPendingOperation if the subscribe was not successful.
- virtual int SubscribePermissionStatusChange(
+ // is from a worker. Returns the ID to be used to unsubscribe, which can be
+ // `is_null()` if the subscribe was not successful.
+ virtual SubscriptionId SubscribePermissionStatusChange(
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(blink::mojom::PermissionStatus)>
callback) = 0;
- // Unregisters from permission status change notifications.
- // The |subscription_id| must match the value returned by the
- // SubscribePermissionStatusChange call. Unsubscribing
- // an already unsubscribed |subscription_id| or providing the
- // |subscription_id| kNoPendingOperation is a no-op.
- virtual void UnsubscribePermissionStatusChange(int subscription_id) = 0;
+ // Unregisters from permission status change notifications. The
+ // |subscription_id| must match the value returned by the
+ // SubscribePermissionStatusChange call. Unsubscribing an already
+ // unsubscribed |subscription_id| or an `is_null()` ID is a no-op.
+ virtual void UnsubscribePermissionStatusChange(
+ SubscriptionId subscription_id) = 0;
// Manually overrides default permission settings of delegate, if overrides
// are tracked by the delegate. This method should only be called by the
@@ -116,4 +121,17 @@
} // namespace content
+namespace std {
+
+template <>
+struct hash<content::PermissionControllerDelegate::SubscriptionId> {
+ std::size_t operator()(
+ const content::PermissionControllerDelegate::SubscriptionId& v) const {
+ content::PermissionControllerDelegate::SubscriptionId::Hasher hasher;
+ return hasher(v);
+ }
+};
+
+} // namespace std
+
#endif // CONTENT_PUBLIC_BROWSER_PERMISSION_CONTROLLER_DELEGATE_H_
diff --git a/content/public/test/mock_permission_manager.h b/content/public/test/mock_permission_manager.h
index aaaf623..287fc01 100644
--- a/content/public/test/mock_permission_manager.h
+++ b/content/public/test/mock_permission_manager.h
@@ -50,12 +50,14 @@
const GURL& requesting_origin,
const GURL& embedding_origin) override {}
MOCK_METHOD4(SubscribePermissionStatusChange,
- int(PermissionType permission,
+ SubscriptionId(
+ PermissionType permission,
RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(blink::mojom::PermissionStatus)>
callback));
- MOCK_METHOD1(UnsubscribePermissionStatusChange, void(int subscription_id));
+ MOCK_METHOD1(UnsubscribePermissionStatusChange,
+ void(SubscriptionId subscription_id));
private:
DISALLOW_COPY_AND_ASSIGN(MockPermissionManager);
diff --git a/content/shell/browser/shell_permission_manager.cc b/content/shell/browser/shell_permission_manager.cc
index a490106a..df6d342 100644
--- a/content/shell/browser/shell_permission_manager.cc
+++ b/content/shell/browser/shell_permission_manager.cc
@@ -134,16 +134,16 @@
.GetOrigin());
}
-int ShellPermissionManager::SubscribePermissionStatusChange(
+ShellPermissionManager::SubscriptionId
+ShellPermissionManager::SubscribePermissionStatusChange(
PermissionType permission,
RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(blink::mojom::PermissionStatus)> callback) {
- return PermissionController::kNoPendingOperation;
+ return SubscriptionId();
}
void ShellPermissionManager::UnsubscribePermissionStatusChange(
- int subscription_id) {
-}
+ SubscriptionId subscription_id) {}
} // namespace content
diff --git a/content/shell/browser/shell_permission_manager.h b/content/shell/browser/shell_permission_manager.h
index 8547766..ecda464 100644
--- a/content/shell/browser/shell_permission_manager.h
+++ b/content/shell/browser/shell_permission_manager.h
@@ -42,13 +42,14 @@
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin) override;
- int SubscribePermissionStatusChange(
+ SubscriptionId SubscribePermissionStatusChange(
PermissionType permission,
RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(blink::mojom::PermissionStatus)> callback)
override;
- void UnsubscribePermissionStatusChange(int subscription_id) override;
+ void UnsubscribePermissionStatusChange(
+ SubscriptionId subscription_id) override;
private:
DISALLOW_COPY_AND_ASSIGN(ShellPermissionManager);
diff --git a/content/web_test/browser/web_test_permission_manager.cc b/content/web_test/browser/web_test_permission_manager.cc
index 64d118f..bfa725f 100644
--- a/content/web_test/browser/web_test_permission_manager.cc
+++ b/content/web_test/browser/web_test_permission_manager.cc
@@ -147,7 +147,8 @@
.GetOrigin());
}
-int WebTestPermissionManager::SubscribePermissionStatusChange(
+WebTestPermissionManager::SubscriptionId
+WebTestPermissionManager::SubscribePermissionStatusChange(
PermissionType permission,
RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
@@ -170,14 +171,18 @@
GetPermissionStatus(permission, subscription->permission.origin,
subscription->permission.embedding_origin);
- return subscriptions_.Add(std::move(subscription));
+ auto id = subscription_id_generator_.GenerateNextId();
+ subscriptions_.AddWithID(std::move(subscription), id);
+ return id;
}
void WebTestPermissionManager::UnsubscribePermissionStatusChange(
- int subscription_id) {
+ SubscriptionId subscription_id) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
- // Whether |subscription_id| is known will be checked by the Remove() call.
+ if (!subscriptions_.Lookup(subscription_id))
+ return;
+
subscriptions_.Remove(subscription_id);
}
diff --git a/content/web_test/browser/web_test_permission_manager.h b/content/web_test/browser/web_test_permission_manager.h
index 5eaede2..c9da1aa4 100644
--- a/content/web_test/browser/web_test_permission_manager.h
+++ b/content/web_test/browser/web_test_permission_manager.h
@@ -52,13 +52,14 @@
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin) override;
- int SubscribePermissionStatusChange(
+ SubscriptionId SubscribePermissionStatusChange(
PermissionType permission,
RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(blink::mojom::PermissionStatus)> callback)
override;
- void UnsubscribePermissionStatusChange(int subscription_id) override;
+ void UnsubscribePermissionStatusChange(
+ SubscriptionId subscription_id) override;
void SetPermission(PermissionType permission,
blink::mojom::PermissionStatus status,
@@ -98,7 +99,8 @@
};
struct Subscription;
- using SubscriptionsMap = base::IDMap<std::unique_ptr<Subscription>>;
+ using SubscriptionsMap =
+ base::IDMap<std::unique_ptr<Subscription>, SubscriptionId>;
using PermissionsMap = std::unordered_map<PermissionDescription,
blink::mojom::PermissionStatus,
PermissionDescription::Hash>;
@@ -116,6 +118,7 @@
// List of subscribers currently listening to permission changes.
SubscriptionsMap subscriptions_;
+ SubscriptionId::Generator subscription_id_generator_;
mojo::ReceiverSet<blink::test::mojom::PermissionAutomation> receivers_;
diff --git a/fuchsia/engine/browser/web_engine_permission_delegate.cc b/fuchsia/engine/browser/web_engine_permission_delegate.cc
index 3ad0ada2..589cfd5 100644
--- a/fuchsia/engine/browser/web_engine_permission_delegate.cc
+++ b/fuchsia/engine/browser/web_engine_permission_delegate.cc
@@ -85,20 +85,21 @@
permission, url::Origin::Create(requesting_origin));
}
-int WebEnginePermissionDelegate::SubscribePermissionStatusChange(
+WebEnginePermissionDelegate::SubscriptionId
+WebEnginePermissionDelegate::SubscribePermissionStatusChange(
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(blink::mojom::PermissionStatus)> callback) {
// TODO(crbug.com/1063094): Implement permission status subscription. It's
// used in blink to emit PermissionStatus.onchange notifications.
- NOTIMPLEMENTED() << ": " << static_cast<int>(permission);
- return content::PermissionController::kNoPendingOperation;
+ NOTIMPLEMENTED_LOG_ONCE() << ": " << static_cast<int>(permission);
+ return SubscriptionId();
}
void WebEnginePermissionDelegate::UnsubscribePermissionStatusChange(
- int subscription_id) {
+ SubscriptionId subscription_id) {
// TODO(crbug.com/1063094): Implement permission status subscription. It's
// used in blink to emit PermissionStatus.onchange notifications.
- NOTREACHED();
+ NOTIMPLEMENTED_LOG_ONCE();
}
diff --git a/fuchsia/engine/browser/web_engine_permission_delegate.h b/fuchsia/engine/browser/web_engine_permission_delegate.h
index 036207b..c39989b 100644
--- a/fuchsia/engine/browser/web_engine_permission_delegate.h
+++ b/fuchsia/engine/browser/web_engine_permission_delegate.h
@@ -45,13 +45,14 @@
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin) override;
- int SubscribePermissionStatusChange(
+ SubscriptionId SubscribePermissionStatusChange(
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(blink::mojom::PermissionStatus)> callback)
override;
- void UnsubscribePermissionStatusChange(int subscription_id) override;
+ void UnsubscribePermissionStatusChange(
+ SubscriptionId subscription_id) override;
};
#endif // FUCHSIA_ENGINE_BROWSER_WEB_ENGINE_PERMISSION_DELEGATE_H_
diff --git a/headless/lib/browser/headless_permission_manager.cc b/headless/lib/browser/headless_permission_manager.cc
index 5d4d609f..359ecdc 100644
--- a/headless/lib/browser/headless_permission_manager.cc
+++ b/headless/lib/browser/headless_permission_manager.cc
@@ -71,15 +71,16 @@
return blink::mojom::PermissionStatus::ASK;
}
-int HeadlessPermissionManager::SubscribePermissionStatusChange(
+HeadlessPermissionManager::SubscriptionId
+HeadlessPermissionManager::SubscribePermissionStatusChange(
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(blink::mojom::PermissionStatus)> callback) {
- return content::PermissionController::kNoPendingOperation;
+ return SubscriptionId();
}
void HeadlessPermissionManager::UnsubscribePermissionStatusChange(
- int subscription_id) {}
+ SubscriptionId subscription_id) {}
} // namespace headless
diff --git a/headless/lib/browser/headless_permission_manager.h b/headless/lib/browser/headless_permission_manager.h
index 4b83309a..ac30670 100644
--- a/headless/lib/browser/headless_permission_manager.h
+++ b/headless/lib/browser/headless_permission_manager.h
@@ -46,13 +46,14 @@
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin) override;
- int SubscribePermissionStatusChange(
+ SubscriptionId SubscribePermissionStatusChange(
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(blink::mojom::PermissionStatus)> callback)
override;
- void UnsubscribePermissionStatusChange(int subscription_id) override;
+ void UnsubscribePermissionStatusChange(
+ SubscriptionId subscription_id) override;
private:
content::BrowserContext* browser_context_;