Fix up permission API in not fully active documents
Changes in this CL to be aligned with permission API specs:
- Throw InvalidStateError DOMException for |query| in not fully active document.
- Ignore dispatching change event if document is null or document is not fully active. If the event is dropped while the page is in BackForwardCache, a single event would be dispatched when the page is restored from BackForwardCache
Bug: 1238709
Change-Id: Id88d39a2671e051dd18e6184bb57c538cf5c79fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4065979
Commit-Queue: Thomas Nguyen <tungnh@google.com>
Reviewed-by: Illia Klimov <elklm@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1083661}
diff --git a/content/browser/permissions/permission_controller_impl.cc b/content/browser/permissions/permission_controller_impl.cc
index 53b0598..7bd3d47 100644
--- a/content/browser/permissions/permission_controller_impl.cc
+++ b/content/browser/permissions/permission_controller_impl.cc
@@ -2,10 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#ifndef CONTENT_BROWSER_PERMISSIONS_PERMISSION_CONTROLLER_IMPL_CC_
-#define CONTENT_BROWSER_PERMISSIONS_PERMISSION_CONTROLLER_IMPL_CC_
-
#include "content/browser/permissions/permission_controller_impl.h"
+
#include "base/bind.h"
#include "content/browser/permissions/permission_service_context.h"
#include "content/browser/permissions/permission_util.h"
@@ -28,6 +26,15 @@
namespace {
+constexpr char kPermissionBlockedPortalsMessage[] =
+ "%s permission has been blocked because it was requested inside a "
+ "portal. "
+ "Portals don't currently support permission requests.";
+
+constexpr char kPermissionBlockedFencedFrameMessage[] =
+ "%s permission has been blocked because it was requested inside a fenced "
+ "frame. Fenced frames don't currently support permission requests.";
+
absl::optional<blink::scheduler::WebSchedulerTrackedFeature>
PermissionToSchedulingFeature(PermissionType permission_name) {
switch (permission_name) {
@@ -73,15 +80,6 @@
}
}
-const char kPermissionBlockedPortalsMessage[] =
- "%s permission has been blocked because it was requested inside a "
- "portal. "
- "Portals don't currently support permission requests.";
-
-const char kPermissionBlockedFencedFrameMessage[] =
- "%s permission has been blocked because it was requested inside a fenced "
- "frame. Fenced frames don't currently support permission requests.";
-
void LogPermissionBlockedMessage(PermissionType permission,
content::RenderFrameHost* rfh,
const char* message) {
@@ -225,7 +223,8 @@
: browser_context_(browser_context) {}
// TODO(https://crbug.com/1271543): Remove this method and use
-// `PermissionController` instead. static
+// `PermissionController` instead.
+// static
PermissionControllerImpl* PermissionControllerImpl::FromBrowserContext(
BrowserContext* browser_context) {
return static_cast<PermissionControllerImpl*>(
@@ -717,6 +716,4 @@
}
}
-} // namespace content
-
-#endif // CONTENT_BROWSER_PERMISSIONS_PERMISSION_CONTROLLER_IMPL_CC_
+} // namespace content
\ No newline at end of file
diff --git a/content/browser/permissions/permission_service_context.cc b/content/browser/permissions/permission_service_context.cc
index 011b9c4..30744374 100644
--- a/content/browser/permissions/permission_service_context.cc
+++ b/content/browser/permissions/permission_service_context.cc
@@ -18,6 +18,7 @@
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/web_contents.h"
#include "mojo/public/cpp/bindings/remote.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/blink/public/common/permissions/permission_utils.h"
#include "url/origin.h"
@@ -45,9 +46,12 @@
class PermissionServiceContext::PermissionSubscription {
public:
PermissionSubscription(
+ blink::mojom::PermissionStatus last_known_status,
PermissionServiceContext* context,
mojo::PendingRemote<blink::mojom::PermissionObserver> observer)
- : context_(context), observer_(std::move(observer)) {
+ : last_known_status_(last_known_status),
+ context_(context),
+ observer_(std::move(observer)) {
observer_.set_disconnect_handler(base::BindOnce(
&PermissionSubscription::OnConnectionError, base::Unretained(this)));
}
@@ -68,9 +72,32 @@
context_->ObserverHadConnectionError(id_);
}
+ void StoreStatusAtBFCacheEntry() {
+ status_at_bf_cache_entry_ =
+ absl::make_optional<blink::mojom::PermissionStatus>(last_known_status_);
+ }
+
+ void NotifyPermissionStatusChangedIfNeeded() {
+ DCHECK(status_at_bf_cache_entry_.has_value());
+ if (status_at_bf_cache_entry_.value() != last_known_status_) {
+ observer_->OnPermissionStatusChange(last_known_status_);
+ }
+ status_at_bf_cache_entry_.reset();
+ }
+
void OnPermissionStatusChanged(blink::mojom::PermissionStatus status) {
- if (observer_.is_connected())
+ if (!observer_.is_connected()) {
+ return;
+ }
+
+ last_known_status_ = status;
+
+ // Dispatching events while in BFCache is redundant. Permissions code in
+ // renderer process would decide to drop the event by looking at document's
+ // active status.
+ if (!status_at_bf_cache_entry_.has_value()) {
observer_->OnPermissionStatusChange(status);
+ }
}
void set_id(PermissionController::SubscriptionId id) { id_ = id; }
@@ -80,10 +107,17 @@
}
private:
+ blink::mojom::PermissionStatus last_known_status_ =
+ blink::mojom::PermissionStatus::LAST;
const raw_ptr<PermissionServiceContext> context_;
mojo::Remote<blink::mojom::PermissionObserver> observer_;
PermissionController::SubscriptionId id_;
+ // Optional variable to store the last status before the corresponding
+ // RenderFrameHost enters BFCache, and will be cleared when the
+ // RenderFrameHost is restored from BFCache. Non-empty value indicates that
+ // the RenderFrameHost is in BFCache.
+ absl::optional<blink::mojom::PermissionStatus> status_at_bf_cache_entry_;
base::WeakPtrFactory<PermissionSubscription> weak_ptr_factory_{this};
};
@@ -98,6 +132,7 @@
PermissionServiceContext::PermissionServiceContext(
RenderFrameHost* render_frame_host)
: render_frame_host_(render_frame_host), render_process_host_(nullptr) {
+ render_frame_host->AddObserver(this);
render_frame_host->GetProcess()->AddObserver(this);
}
@@ -106,8 +141,10 @@
: render_frame_host_(nullptr), render_process_host_(render_process_host) {}
PermissionServiceContext::~PermissionServiceContext() {
- if (render_frame_host_)
+ if (render_frame_host_) {
+ render_frame_host_->RemoveObserver(this);
render_frame_host_->GetProcess()->RemoveObserver(this);
+ }
}
void PermissionServiceContext::CreateService(
@@ -134,15 +171,15 @@
blink::mojom::PermissionStatus last_known_status,
mojo::PendingRemote<blink::mojom::PermissionObserver> observer) {
BrowserContext* browser_context = GetBrowserContext();
- if (!browser_context)
+ if (!browser_context) {
return;
+ }
- auto subscription =
- std::make_unique<PermissionSubscription>(this, std::move(observer));
+ auto subscription = std::make_unique<PermissionSubscription>(
+ last_known_status, this, std::move(observer));
if (current_status != last_known_status) {
subscription->OnPermissionStatusChanged(current_status);
- last_known_status = current_status;
}
GURL requesting_origin(origin.Serialize());
@@ -188,6 +225,19 @@
// earlier so we need to listen to this event so we can do our clean up as
// well.
host->RemoveObserver(this);
+ render_frame_host_->RemoveObserver(this);
+}
+
+void PermissionServiceContext::DidEnterBackForwardCache() {
+ for (auto& iter : subscriptions_) {
+ iter.second->StoreStatusAtBFCacheEntry();
+ }
+}
+
+void PermissionServiceContext::DidRestoreFromBackForwardCache() {
+ for (auto& iter : subscriptions_) {
+ iter.second->NotifyPermissionStatusChangedIfNeeded();
+ }
}
} // namespace content
diff --git a/content/browser/permissions/permission_service_context.h b/content/browser/permissions/permission_service_context.h
index 04aa3fb..a2f6d861 100644
--- a/content/browser/permissions/permission_service_context.h
+++ b/content/browser/permissions/permission_service_context.h
@@ -11,6 +11,7 @@
#include "base/memory/raw_ptr.h"
#include "content/public/browser/document_user_data.h"
#include "content/public/browser/permission_controller.h"
+#include "content/public/browser/render_frame_host_observer.h"
#include "content/public/browser/render_process_host_observer.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
@@ -29,6 +30,7 @@
namespace content {
class BrowserContext;
+class PermissionServiceContextTest;
class RenderFrameHost;
class RenderProcessHost;
@@ -40,8 +42,10 @@
//
// PermissionServiceContext instances associated with a RenderFrameHost must be
// created via the DocumentUserData static factories, as these
-// instances are deleted when a new document is commited.
-class PermissionServiceContext : public RenderProcessHostObserver {
+// instances are deleted when a new document is committed.
+class CONTENT_EXPORT PermissionServiceContext
+ : public RenderProcessHostObserver,
+ public RenderFrameHostObserver {
public:
explicit PermissionServiceContext(RenderProcessHost* render_process_host);
PermissionServiceContext(const PermissionServiceContext&) = delete;
@@ -83,6 +87,10 @@
// RenderProcessHostObserver:
void RenderProcessHostDestroyed(RenderProcessHost* host) override;
+ // RenderFrameHostObserver:
+ void DidEnterBackForwardCache() override;
+ void DidRestoreFromBackForwardCache() override;
+
std::set<blink::PermissionType>& GetOnchangeEventListeners() {
return onchange_event_listeners_;
}
@@ -90,6 +98,7 @@
private:
class PermissionSubscription;
struct DocumentPermissionServiceContextHolder;
+ friend class PermissionServiceContextTest;
// Use DocumentUserData static methods to create instances attached
// to a RenderFrameHost.
diff --git a/content/browser/permissions/permission_service_context_unittest.cc b/content/browser/permissions/permission_service_context_unittest.cc
new file mode 100644
index 0000000..07ec8ef
--- /dev/null
+++ b/content/browser/permissions/permission_service_context_unittest.cc
@@ -0,0 +1,275 @@
+// Copyright 2022 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/browser/permissions/permission_service_context.h"
+
+#include <memory>
+
+#include "base/memory/ptr_util.h"
+#include "base/memory/raw_ptr.h"
+#include "content/browser/permissions/permission_controller_impl.h"
+#include "content/browser/renderer_host/render_frame_host_impl.h"
+#include "content/public/browser/weak_document_ptr.h"
+#include "content/public/test/test_renderer_host.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/public/common/permissions/permission_utils.h"
+#include "third_party/blink/public/mojom/permissions/permission.mojom.h"
+#include "url/origin.h"
+
+namespace content {
+
+namespace {
+constexpr char kTestUrl[] = "https://google.com";
+}
+
+class TestPermissionObserver : public blink::mojom::PermissionObserver {
+ public:
+ TestPermissionObserver() = default;
+
+ TestPermissionObserver(const TestPermissionObserver&) = delete;
+ TestPermissionObserver& operator=(const TestPermissionObserver&) = delete;
+
+ ~TestPermissionObserver() override = default;
+
+ // Closes the bindings associated with this observer.
+ void Close() { receiver_.reset(); }
+
+ // Returns a pipe to this observer.
+ mojo::PendingRemote<blink::mojom::PermissionObserver> GetRemote() {
+ mojo::PendingRemote<blink::mojom::PermissionObserver> remote;
+ receiver_.Bind(remote.InitWithNewPipeAndPassReceiver());
+ return remote;
+ }
+
+ // Returns the number of events received by this observer.
+ size_t change_event_count() const { return change_event_count_; }
+
+ // blink::mojom::PermissionObserver implementation.
+ void OnPermissionStatusChange(
+ blink::mojom::PermissionStatus status) override {
+ change_event_count_++;
+ }
+
+ private:
+ size_t change_event_count_ = 0;
+ mojo::Receiver<blink::mojom::PermissionObserver> receiver_{this};
+};
+
+class PermissionServiceContextTest : public RenderViewHostTestHarness {
+ public:
+ PermissionServiceContextTest() = default;
+ PermissionServiceContextTest(const PermissionServiceContextTest&) = delete;
+ PermissionServiceContextTest& operator=(const PermissionServiceContextTest&) =
+ delete;
+ ~PermissionServiceContextTest() override = default;
+
+ void SetUp() override {
+ RenderViewHostTestHarness::SetUp();
+ origin_ = url::Origin::Create(GURL(kTestUrl));
+ NavigateAndCommit(origin_.GetURL());
+ permission_controller_ =
+ PermissionControllerImpl::FromBrowserContext(browser_context());
+ auto* render_frame_host = main_rfh();
+ render_frame_host_impl_ =
+ static_cast<RenderFrameHostImpl*>(render_frame_host);
+ permission_service_context_ =
+ PermissionServiceContext::GetForCurrentDocument(render_frame_host);
+ }
+
+ std::unique_ptr<TestPermissionObserver> CreateSubscription(
+ PermissionType type,
+ blink::mojom::PermissionStatus last_status,
+ blink::mojom::PermissionStatus current_status) {
+ permission_controller()->SetOverrideForDevTools(origin_, type, last_status);
+ auto observer = std::make_unique<TestPermissionObserver>();
+ permission_service_context()->CreateSubscription(
+ type, origin_, current_status, last_status, observer->GetRemote());
+ WaitForAsyncTasksToComplete();
+ return observer;
+ }
+
+ void SimulatePermissionChangedEvent(PermissionType type,
+ blink::mojom::PermissionStatus status) {
+ permission_controller()->SetOverrideForDevTools(origin_, type, status);
+ WaitForAsyncTasksToComplete();
+ }
+
+ // Waits until the Mojo task (async) has finished.
+ void WaitForAsyncTasksToComplete() { task_environment()->RunUntilIdle(); }
+
+ PermissionControllerImpl* permission_controller() {
+ return permission_controller_;
+ }
+
+ PermissionServiceContext* permission_service_context() {
+ return permission_service_context_;
+ }
+
+ RenderFrameHostImpl* render_frame_host() { return render_frame_host_impl_; }
+
+ private:
+ url::Origin origin_;
+ raw_ptr<PermissionControllerImpl> permission_controller_;
+ raw_ptr<RenderFrameHostImpl> render_frame_host_impl_;
+ raw_ptr<PermissionServiceContext> permission_service_context_;
+};
+
+TEST_F(PermissionServiceContextTest, DispatchPermissionChangeEvent) {
+ EXPECT_TRUE(render_frame_host()->IsInLifecycleState(
+ RenderFrameHost::LifecycleState::kActive));
+ auto observer = CreateSubscription(PermissionType::GEOLOCATION,
+ blink::mojom::PermissionStatus::ASK,
+ blink::mojom::PermissionStatus::ASK);
+ EXPECT_EQ(observer->change_event_count(), 0U);
+ SimulatePermissionChangedEvent(blink::PermissionType::GEOLOCATION,
+ blink::mojom::PermissionStatus::DENIED);
+ EXPECT_EQ(observer->change_event_count(), 1U);
+}
+
+TEST_F(PermissionServiceContextTest,
+ DispatchPermissionChangeEventInBackForwardCache) {
+ EXPECT_TRUE(render_frame_host()->IsInLifecycleState(
+ RenderFrameHost::LifecycleState::kActive));
+ auto observer = CreateSubscription(PermissionType::GEOLOCATION,
+ blink::mojom::PermissionStatus::ASK,
+ blink::mojom::PermissionStatus::ASK);
+ EXPECT_EQ(observer->change_event_count(), 0U);
+ SimulatePermissionChangedEvent(blink::PermissionType::GEOLOCATION,
+ blink::mojom::PermissionStatus::DENIED);
+
+ // After dispatching changed events when the render frame host is active,
+ // the event counter should increment as expected.
+ EXPECT_EQ(observer->change_event_count(), 1U);
+ SimulatePermissionChangedEvent(blink::PermissionType::GEOLOCATION,
+ blink::mojom::PermissionStatus::ASK);
+ EXPECT_EQ(observer->change_event_count(), 2U);
+
+ // Simulate the render frame host is put into the back/forward cache
+ render_frame_host()->DidEnterBackForwardCache();
+ EXPECT_TRUE(render_frame_host()->IsInLifecycleState(
+ RenderFrameHost::LifecycleState::kInBackForwardCache));
+ SimulatePermissionChangedEvent(blink::PermissionType::GEOLOCATION,
+ blink::mojom::PermissionStatus::DENIED);
+
+ // Trigger a permission status change event.
+ SimulatePermissionChangedEvent(blink::PermissionType::GEOLOCATION,
+ blink::mojom::PermissionStatus::ASK);
+
+ // Now the change events should not should increment the counter.
+ EXPECT_EQ(observer->change_event_count(), 2U);
+
+ // Simulate the render frame host is back to active state by setting the
+ // lifecycle state.
+ render_frame_host()->SetLifecycleState(
+ RenderFrameHostImpl::LifecycleStateImpl::kActive);
+ EXPECT_TRUE(render_frame_host()->IsInLifecycleState(
+ RenderFrameHost::LifecycleState::kActive));
+ SimulatePermissionChangedEvent(blink::PermissionType::GEOLOCATION,
+ blink::mojom::PermissionStatus::DENIED);
+
+ // Since the render frame host is active, the dispatched events should
+ // increment the counter.
+ EXPECT_EQ(observer->change_event_count(), 3U);
+}
+
+TEST_F(PermissionServiceContextTest,
+ DispatchMultiplePermissionChangeEventsInBackForwardCache) {
+ EXPECT_TRUE(render_frame_host()->IsInLifecycleState(
+ RenderFrameHost::LifecycleState::kActive));
+ auto observer = CreateSubscription(PermissionType::GEOLOCATION,
+ blink::mojom::PermissionStatus::ASK,
+ blink::mojom::PermissionStatus::ASK);
+ EXPECT_EQ(observer->change_event_count(), 0U);
+
+ // Simulate the render frame host is put into the back/forward cache.
+ // Trigger a permission status change event, the event should not should
+ // increment the counter.
+ render_frame_host()->DidEnterBackForwardCache();
+ EXPECT_TRUE(render_frame_host()->IsInLifecycleState(
+ RenderFrameHost::LifecycleState::kInBackForwardCache));
+
+ for (size_t i = 0; i < 10; ++i) {
+ SimulatePermissionChangedEvent(blink::PermissionType::GEOLOCATION,
+ blink::mojom::PermissionStatus::ASK);
+ EXPECT_EQ(observer->change_event_count(), 0U);
+ SimulatePermissionChangedEvent(blink::PermissionType::GEOLOCATION,
+ blink::mojom::PermissionStatus::DENIED);
+ EXPECT_EQ(observer->change_event_count(), 0U);
+ }
+
+ // Simulate the render frame host is back to active state by setting the
+ // lifecycle state. The last event should be dispatched and increment the
+ // counter.
+ render_frame_host()->SetLifecycleState(
+ RenderFrameHostImpl::LifecycleStateImpl::kActive);
+ EXPECT_TRUE(render_frame_host()->IsInLifecycleState(
+ RenderFrameHost::LifecycleState::kActive));
+ WaitForAsyncTasksToComplete();
+ EXPECT_EQ(observer->change_event_count(), 1U);
+}
+
+TEST_F(PermissionServiceContextTest,
+ DispatchSameStatusAfterLeaveBackForwardCache) {
+ EXPECT_TRUE(render_frame_host()->IsInLifecycleState(
+ RenderFrameHost::LifecycleState::kActive));
+ auto observer = CreateSubscription(PermissionType::GEOLOCATION,
+ blink::mojom::PermissionStatus::ASK,
+ blink::mojom::PermissionStatus::ASK);
+ EXPECT_EQ(observer->change_event_count(), 0U);
+
+ // Simulate the render frame host is put into the back/forward cache.
+ // Trigger a permission status change event, the event should not should
+ // increment the counter.
+ render_frame_host()->DidEnterBackForwardCache();
+ EXPECT_TRUE(render_frame_host()->IsInLifecycleState(
+ RenderFrameHost::LifecycleState::kInBackForwardCache));
+ SimulatePermissionChangedEvent(blink::PermissionType::GEOLOCATION,
+ blink::mojom::PermissionStatus::DENIED);
+ EXPECT_EQ(observer->change_event_count(), 0U);
+
+ // Permission status changes back to the status at BFCache entry
+ SimulatePermissionChangedEvent(blink::PermissionType::GEOLOCATION,
+ blink::mojom::PermissionStatus::ASK);
+ EXPECT_EQ(observer->change_event_count(), 0U);
+
+ // Simulate the render frame host is back to active state by setting the
+ // lifecycle state. No event should be dispatched.
+ render_frame_host()->SetLifecycleState(
+ RenderFrameHostImpl::LifecycleStateImpl::kActive);
+ EXPECT_TRUE(render_frame_host()->IsInLifecycleState(
+ RenderFrameHost::LifecycleState::kActive));
+ WaitForAsyncTasksToComplete();
+ EXPECT_EQ(observer->change_event_count(), 0U);
+}
+
+TEST_F(PermissionServiceContextTest,
+ DispatchDifferentStatusAfterLeaveBackForwardCache) {
+ EXPECT_TRUE(render_frame_host()->IsInLifecycleState(
+ RenderFrameHost::LifecycleState::kActive));
+ auto observer = CreateSubscription(PermissionType::GEOLOCATION,
+ blink::mojom::PermissionStatus::ASK,
+ blink::mojom::PermissionStatus::ASK);
+ EXPECT_EQ(observer->change_event_count(), 0U);
+
+ // Simulate the render frame host is put into the back/forward cache.
+ // Trigger a permission status change event, the event should not should
+ // increment the counter.
+ render_frame_host()->DidEnterBackForwardCache();
+ EXPECT_TRUE(render_frame_host()->IsInLifecycleState(
+ RenderFrameHost::LifecycleState::kInBackForwardCache));
+ SimulatePermissionChangedEvent(blink::PermissionType::GEOLOCATION,
+ blink::mojom::PermissionStatus::DENIED);
+ EXPECT_EQ(observer->change_event_count(), 0U);
+
+ // Simulate the render frame host is back to active state by setting the
+ // lifecycle state. The last event should be dispatched.
+ render_frame_host()->SetLifecycleState(
+ RenderFrameHostImpl::LifecycleStateImpl::kActive);
+ EXPECT_TRUE(render_frame_host()->IsInLifecycleState(
+ RenderFrameHost::LifecycleState::kActive));
+ WaitForAsyncTasksToComplete();
+ EXPECT_EQ(observer->change_event_count(), 1U);
+}
+
+} // namespace content
diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
index fba64c4..dc7425f 100644
--- a/content/browser/renderer_host/render_frame_host_impl.cc
+++ b/content/browser/renderer_host/render_frame_host_impl.cc
@@ -170,6 +170,7 @@
#include "content/public/browser/document_service_internal.h"
#include "content/public/browser/download_manager.h"
#include "content/public/browser/global_routing_id.h"
+#include "content/public/browser/render_frame_host_observer.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_widget_host_view.h"
#include "content/public/browser/shared_cors_origin_access_list.h"
@@ -1852,6 +1853,10 @@
#if BUILDFLAG(IS_P2P_ENABLED)
GetProcess()->PauseSocketManagerForRenderFrameHost(GetGlobalId());
#endif // BUILDFLAG(IS_P2P_ENABLED)
+
+ for (auto& observer : observers_) {
+ observer.DidEnterBackForwardCache();
+ }
}
// The frame as been restored from the BackForwardCache.
@@ -3595,6 +3600,14 @@
return policy_container_host_->policies().is_credentialless;
}
+void RenderFrameHostImpl::AddObserver(RenderFrameHostObserver* observer) {
+ observers_.AddObserver(observer);
+}
+
+void RenderFrameHostImpl::RemoveObserver(RenderFrameHostObserver* observer) {
+ observers_.RemoveObserver(observer);
+}
+
void RenderFrameHostImpl::OnCreateChildFrame(
int new_routing_id,
mojo::PendingAssociatedRemote<mojom::Frame> frame_remote,
@@ -13759,6 +13772,12 @@
}
}
}
+
+ if (lifecycle_state_ == LifecycleStateImpl::kInBackForwardCache) {
+ for (auto& observer : observers_) {
+ observer.DidRestoreFromBackForwardCache();
+ }
+ }
}
if (lifecycle_state() == LifecycleStateImpl::kInBackForwardCache)
diff --git a/content/browser/renderer_host/render_frame_host_impl.h b/content/browser/renderer_host/render_frame_host_impl.h
index 21201bc..c6c4a77 100644
--- a/content/browser/renderer_host/render_frame_host_impl.h
+++ b/content/browser/renderer_host/render_frame_host_impl.h
@@ -2068,6 +2068,9 @@
bool IsCredentialless() const override;
+ void AddObserver(RenderFrameHostObserver* observer) override;
+ void RemoveObserver(RenderFrameHostObserver* observer) override;
+
bool is_fenced_frame_root_originating_from_opaque_url() const {
return is_fenced_frame_root_originating_from_opaque_url_;
}
@@ -4713,6 +4716,9 @@
// to change across MPArch activations like prerendering.
const base::UnguessableToken devtools_frame_token_;
+ // The observers watching our state changed event.
+ base::ObserverList<RenderFrameHostObserver> observers_;
+
// BrowserInterfaceBroker implementation through which this
// RenderFrameHostImpl exposes document-scoped Mojo services to the currently
// active document in the corresponding RenderFrame.
diff --git a/content/public/browser/BUILD.gn b/content/public/browser/BUILD.gn
index e8fe0f9a..ed8fd40a 100644
--- a/content/public/browser/BUILD.gn
+++ b/content/public/browser/BUILD.gn
@@ -319,6 +319,8 @@
"reduce_accept_language_controller_delegate.h",
"reload_type.h",
"render_frame_host.h",
+ "render_frame_host_observer.cc",
+ "render_frame_host_observer.h",
"render_frame_host_receiver_set.h",
"render_frame_metadata_provider.h",
"render_process_host.h",
diff --git a/content/public/browser/render_frame_host.h b/content/public/browser/render_frame_host.h
index fe97a5f..6b34918 100644
--- a/content/public/browser/render_frame_host.h
+++ b/content/public/browser/render_frame_host.h
@@ -108,6 +108,7 @@
class BrowserContext;
class DocumentRef;
struct GlobalRenderFrameHostId;
+class RenderFrameHostObserver;
class RenderProcessHost;
class RenderViewHost;
class RenderWidgetHost;
@@ -1061,6 +1062,11 @@
// Updated on every cross-document navigation.
virtual bool IsCredentialless() const = 0;
+ // Add and remove observers for state changed events. Observers are
+ // responsible to remove themselves from observer list before they go away.
+ virtual void AddObserver(RenderFrameHostObserver* observer) = 0;
+ virtual void RemoveObserver(RenderFrameHostObserver* observer) = 0;
+
private:
// This interface should only be implemented inside content.
friend class RenderFrameHostImpl;
diff --git a/content/public/browser/render_frame_host_observer.cc b/content/public/browser/render_frame_host_observer.cc
new file mode 100644
index 0000000..822c4c8
--- /dev/null
+++ b/content/public/browser/render_frame_host_observer.cc
@@ -0,0 +1,14 @@
+// Copyright 2022 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/public/browser/render_frame_host_observer.h"
+#include "base/check.h"
+
+namespace content {
+
+RenderFrameHostObserver::~RenderFrameHostObserver() {
+ DCHECK(!IsInObserverList());
+}
+
+} // namespace content
\ No newline at end of file
diff --git a/content/public/browser/render_frame_host_observer.h b/content/public/browser/render_frame_host_observer.h
new file mode 100644
index 0000000..6e55994
--- /dev/null
+++ b/content/public/browser/render_frame_host_observer.h
@@ -0,0 +1,36 @@
+// Copyright 2022 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_PUBLIC_BROWSER_RENDER_FRAME_HOST_OBSERVER_H_
+#define CONTENT_PUBLIC_BROWSER_RENDER_FRAME_HOST_OBSERVER_H_
+
+#include "base/observer_list_types.h"
+#include "content/common/content_export.h"
+
+namespace content {
+
+// An observer API implemented by classes which would like to observe
+// RenderFrameHost state changed events.
+//
+// This API is appropriate for observer classes extending
+// |content::DocumentUserData| (which have a 1-1 relationship and are owned by
+// RenderFrameHost) to track the state of a single RenderFrameHost instead of
+// the whole frame tree (see WebContentsObserver::RenderFrameHostStateChanged)
+class CONTENT_EXPORT RenderFrameHostObserver : public base::CheckedObserver {
+ public:
+ // This method is invoked whenever the RenderFrameHost enters
+ // BackForwardCache.
+ virtual void DidEnterBackForwardCache() {}
+
+ // This method is invoked whenever the RenderFrameHost is restored from
+ // BackForwardCache.
+ virtual void DidRestoreFromBackForwardCache() {}
+
+ protected:
+ ~RenderFrameHostObserver() override;
+};
+
+} // namespace content
+
+#endif // CONTENT_PUBLIC_BROWSER_RENDER_FRAME_HOST_OBSERVER_H_
\ No newline at end of file
diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn
index 27228180..7e16705d 100644
--- a/content/test/BUILD.gn
+++ b/content/test/BUILD.gn
@@ -2336,6 +2336,7 @@
"../browser/payments/payment_app_provider_impl_unittest.cc",
"../browser/payments/payment_manager_unittest.cc",
"../browser/permissions/permission_controller_impl_unittest.cc",
+ "../browser/permissions/permission_service_context_unittest.cc",
"../browser/permissions/permission_util_unittest.cc",
"../browser/picture_in_picture/picture_in_picture_service_impl_unittest.cc",
"../browser/preloading/anchor_element_interaction_host_impl_unittest.cc",
diff --git a/third_party/blink/renderer/modules/permissions/permission_status.cc b/third_party/blink/renderer/modules/permissions/permission_status.cc
index 110cd27d..49cdeeb 100644
--- a/third_party/blink/renderer/modules/permissions/permission_status.cc
+++ b/third_party/blink/renderer/modules/permissions/permission_status.cc
@@ -8,6 +8,7 @@
#include "third_party/blink/public/platform/platform.h"
#include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
#include "third_party/blink/renderer/core/dom/events/event.h"
+#include "third_party/blink/renderer/core/frame/local_dom_window.h"
#include "third_party/blink/renderer/modules/event_target_modules_names.h"
#include "third_party/blink/renderer/modules/permissions/permission_status_listener.h"
@@ -109,6 +110,19 @@
}
void PermissionStatus::OnPermissionStatusChange(MojoPermissionStatus status) {
+ // https://www.w3.org/TR/permissions/#onchange-attribute
+ // 1. If this's relevant global object is a Window object, then:
+ // - Let document be status's relevant global object's associated Document.
+ // - If document is null or document is not fully active, terminate this
+ // algorithm.
+ if (auto* window = DynamicTo<LocalDOMWindow>(GetExecutionContext())) {
+ auto* document = window->document();
+ if (!document || !document->IsActive()) {
+ // Note: if the event is dropped out while in BFCache, one single change
+ // event might be dispatched later when the page is restored from BFCache.
+ return;
+ }
+ }
DispatchEvent(*Event::Create(event_type_names::kChange));
}
diff --git a/third_party/blink/renderer/modules/permissions/permissions.cc b/third_party/blink/renderer/modules/permissions/permissions.cc
index 35454956..49125b7 100644
--- a/third_party/blink/renderer/modules/permissions/permissions.cc
+++ b/third_party/blink/renderer/modules/permissions/permissions.cc
@@ -20,8 +20,10 @@
#include "third_party/blink/renderer/core/frame/frame.h"
#include "third_party/blink/renderer/core/frame/local_dom_window.h"
#include "third_party/blink/renderer/core/frame/local_frame.h"
+#include "third_party/blink/renderer/core/page/page.h"
#include "third_party/blink/renderer/modules/permissions/permission_status.h"
#include "third_party/blink/renderer/modules/permissions/permission_utils.h"
+#include "third_party/blink/renderer/platform/bindings/exception_code.h"
#include "third_party/blink/renderer/platform/wtf/functional.h"
#include "third_party/blink/renderer/platform/wtf/vector.h"
#include "third_party/blink/renderer/platform/wtf/wtf_size_t.h"
@@ -54,6 +56,26 @@
ScriptPromise Permissions::query(ScriptState* script_state,
const ScriptValue& raw_permission,
ExceptionState& exception_state) {
+ // https://www.w3.org/TR/permissions/#query-method
+ // If this's relevant global object is a Window object, and if the current
+ // settings object's associated Document is not fully active, return a promise
+ // rejected with an "InvalidStateError" DOMException.
+ auto* context = ExecutionContext::From(script_state);
+ if (auto* window = DynamicTo<LocalDOMWindow>(context)) {
+ auto* document = window->document();
+ if (document && !document->IsActive()) {
+ // It's impossible for Permissions.query to occur while in BFCache.
+ if (document->GetPage()) {
+ DCHECK(!document->GetPage()
+ ->GetPageLifecycleState()
+ ->is_in_back_forward_cache);
+ }
+ exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
+ "The document is not active");
+ return ScriptPromise();
+ }
+ }
+
PermissionDescriptorPtr descriptor =
ParsePermissionDescriptor(script_state, raw_permission, exception_state);
if (exception_state.HadException())
@@ -67,11 +89,10 @@
// permission prompt will be shown even if the returned permission will most
// likely be "prompt".
PermissionDescriptorPtr descriptor_copy = descriptor->Clone();
- GetService(ExecutionContext::From(script_state))
- ->HasPermission(
- std::move(descriptor),
- WTF::BindOnce(&Permissions::TaskComplete, WrapPersistent(this),
- WrapPersistent(resolver), std::move(descriptor_copy)));
+ GetService(context)->HasPermission(
+ std::move(descriptor),
+ WTF::BindOnce(&Permissions::TaskComplete, WrapPersistent(this),
+ WrapPersistent(resolver), std::move(descriptor_copy)));
return promise;
}
diff --git a/third_party/blink/web_tests/external/wpt/permissions/non-fully-active.https-expected.txt b/third_party/blink/web_tests/external/wpt/permissions/non-fully-active.https-expected.txt
index cf5b4c0a..ef59c33 100644
--- a/third_party/blink/web_tests/external/wpt/permissions/non-fully-active.https-expected.txt
+++ b/third_party/blink/web_tests/external/wpt/permissions/non-fully-active.https-expected.txt
@@ -1,5 +1,5 @@
This is a testharness.js-based test.
-FAIL Trying to query() a non-fully active document rejects with a InvalidStateError promise_rejects_dom: must reject in the right global when the document is not fully active function "function() { throw e }" threw object "TypeError: Failed to execute 'query' on 'Permissions': Failed to read the 'name' property from 'PermissionDescriptor': The provided value 'whatever' is not a valid enum value of type PermissionName." that is not a DOMException InvalidStateError: property "code" is equal to undefined, expected 11
+PASS Trying to query() a non-fully active document rejects with a InvalidStateError
PASS Permission change events shouldn't fire on non-fully active document
Harness: the test ran to completion.
diff --git a/third_party/blink/web_tests/external/wpt/permissions/non-fully-active.https.html.ini b/third_party/blink/web_tests/external/wpt/permissions/non-fully-active.https.html.ini
index 909bd793..e12b6ff 100644
--- a/third_party/blink/web_tests/external/wpt/permissions/non-fully-active.https.html.ini
+++ b/third_party/blink/web_tests/external/wpt/permissions/non-fully-active.https.html.ini
@@ -1,7 +1,7 @@
[non-fully-active.https.html]
expected: [OK, TIMEOUT]
[Trying to query() a non-fully active document rejects with a InvalidStateError]
- expected: FAIL
+ expected: PASS
[Permission change events shouldn't fire on non-fully active document]
expected: [PASS, TIMEOUT]