Skip to content

Commit

Permalink
Implement user activation gating for CloseWatcher cancel events
Browse files Browse the repository at this point in the history
This matches the spec after
WICG/close-watcher#8. Note the outstanding
question at WICG/close-watcher#7 about the
Esc key; since Chromium currently does not count the Esc key as user
activation, we have one particular behavior.

The tracking of last user activation time ensures that any user
activation allows the CloseWatcher to, in the future, fire a cancel
event---not just during the transient activation duration time window.
This gives the desired semantics outlined in
https://github.com/WICG/close-watcher#asking-for-confirmation and
https://github.com/WICG/close-watcher#abuse-analysis.

Bug: 1171318

Change-Id: I34212f29539d5796b4a9bdc4c7351b9d8ea5880d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3168969
Commit-Queue: Domenic Denicola <[email protected]>
Reviewed-by: Jeremy Roman <[email protected]>
Cr-Commit-Position: refs/heads/main@{#932757}
  • Loading branch information
domenic authored and Chromium LUCI CQ committed Oct 19, 2021
1 parent c1bc767 commit f81bb8a
Show file tree
Hide file tree
Showing 7 changed files with 209 additions and 43 deletions.
12 changes: 12 additions & 0 deletions third_party/blink/renderer/core/frame/local_dom_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -910,6 +910,11 @@ void LocalDOMWindow::RegisterEventListenerObserver(
event_listener_observers_.insert(event_listener_observer);
}

void LocalDOMWindow::RegisterUserActivationObserver(
UserActivationObserver* user_activation_observer) {
user_activation_observers_.insert(user_activation_observer);
}

void LocalDOMWindow::Reset() {
DCHECK(document());
FrameDestroyed();
Expand Down Expand Up @@ -2120,6 +2125,7 @@ void LocalDOMWindow::Trace(Visitor* visitor) const {
visitor->Trace(external_);
visitor->Trace(visualViewport_);
visitor->Trace(event_listener_observers_);
visitor->Trace(user_activation_observers_);
visitor->Trace(current_event_);
visitor->Trace(trusted_types_map_);
visitor->Trace(input_method_controller_);
Expand Down Expand Up @@ -2158,4 +2164,10 @@ void LocalDOMWindow::SetStorageKey(const BlinkStorageKey& storage_key) {
storage_key_ = storage_key;
}

void LocalDOMWindow::DidReceiveUserActivation() {
for (auto& it : user_activation_observers_) {
it->DidReceiveUserActivation();
}
}

} // namespace blink
8 changes: 8 additions & 0 deletions third_party/blink/renderer/core/frame/local_dom_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ class CORE_EXPORT LocalDOMWindow final : public DOMWindow,
const AtomicString&) = 0;
virtual void DidRemoveAllEventListeners(LocalDOMWindow*) = 0;
};
class CORE_EXPORT UserActivationObserver : public GarbageCollectedMixin {
public:
virtual void DidReceiveUserActivation() = 0;
};

static LocalDOMWindow* From(const ScriptState*);

Expand Down Expand Up @@ -343,6 +347,7 @@ class CORE_EXPORT LocalDOMWindow final : public DOMWindow,
DEFINE_ATTRIBUTE_EVENT_LISTENER(orientationchange, kOrientationchange)

void RegisterEventListenerObserver(EventListenerObserver*);
void RegisterUserActivationObserver(UserActivationObserver*);

void FrameDestroyed();
void Reset();
Expand Down Expand Up @@ -437,6 +442,8 @@ class CORE_EXPORT LocalDOMWindow final : public DOMWindow,
const BlinkStorageKey& GetStorageKey() const { return storage_key_; }
void SetStorageKey(const BlinkStorageKey& storage_key);

void DidReceiveUserActivation();

protected:
// EventTarget overrides.
void AddedEventListener(const AtomicString& event_type,
Expand Down Expand Up @@ -497,6 +504,7 @@ class CORE_EXPORT LocalDOMWindow final : public DOMWindow,
scoped_refptr<SerializedScriptValue> pending_state_object_;

HeapHashSet<WeakMember<EventListenerObserver>> event_listener_observers_;
HeapHashSet<WeakMember<UserActivationObserver>> user_activation_observers_;

// https://dom.spec.whatwg.org/#window-current-event
// We represent the "undefined" value as nullptr.
Expand Down
4 changes: 3 additions & 1 deletion third_party/blink/renderer/core/frame/local_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2316,8 +2316,9 @@ void LocalFrame::NotifyUserActivation(
LocalFrame* frame,
mojom::blink::UserActivationNotificationType notification_type,
bool need_browser_verification) {
if (frame)
if (frame) {
frame->NotifyUserActivation(notification_type, need_browser_verification);
}
}

// static
Expand Down Expand Up @@ -2345,6 +2346,7 @@ void LocalFrame::NotifyUserActivation(
notification_type);
Client()->NotifyUserActivation();
NotifyUserActivationInFrameTree(notification_type);
DomWindow()->DidReceiveUserActivation();
}

bool LocalFrame::ConsumeTransientUserActivation(
Expand Down
52 changes: 36 additions & 16 deletions third_party/blink/renderer/modules/closewatcher/close_watcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,16 @@ const char CloseWatcher::WatcherStack::kSupplementName[] =
CloseWatcher::WatcherStack& CloseWatcher::WatcherStack::From(
LocalDOMWindow& window) {
auto* stack = Supplement<LocalDOMWindow>::From<WatcherStack>(window);
if (!stack) {
stack = MakeGarbageCollected<WatcherStack>(window);
Supplement<LocalDOMWindow>::ProvideTo(window, stack);
}

// Must have been installed by InstallUserActivationObserver.
DCHECK(stack);
return *stack;
}

CloseWatcher::WatcherStack::WatcherStack(LocalDOMWindow& window)
: Supplement<LocalDOMWindow>(window), receiver_(this, &window) {}
: Supplement<LocalDOMWindow>(window), receiver_(this, &window) {
window.RegisterUserActivationObserver(this);
}

void CloseWatcher::WatcherStack::Add(CloseWatcher* watcher) {
if (watchers_.IsEmpty()) {
Expand Down Expand Up @@ -65,6 +66,16 @@ void CloseWatcher::WatcherStack::Invoke(ExecutionContext*, Event* e) {
Signal();
}

bool CloseWatcher::WatcherStack::CheckForCreation() {
if (HasActiveWatcher() && !LocalFrame::ConsumeTransientUserActivation(
GetSupplementable()->GetFrame())) {
return false;
}

ConsumeCloseWatcherCancelability();
return true;
}

CloseWatcher* CloseWatcher::Create(ScriptState* script_state,
ExceptionState& exception_state) {
LocalDOMWindow* window = LocalDOMWindow::From(script_state);
Expand All @@ -76,8 +87,8 @@ CloseWatcher* CloseWatcher::Create(ScriptState* script_state,
}

WatcherStack& stack = WatcherStack::From(*window);
if (stack.HasActiveWatcher() &&
!LocalFrame::HasTransientUserActivation(window->GetFrame())) {

if (!stack.CheckForCreation()) {
exception_state.ThrowDOMException(
DOMExceptionCode::kNotAllowedError,
"Creating more than one CloseWatcher at a time requires a user "
Expand All @@ -94,21 +105,30 @@ CloseWatcher::CloseWatcher(LocalDOMWindow* window)
: ExecutionContextClient(window) {}

void CloseWatcher::signalClosed() {
if (IsClosed() || dispatching_cancel_)
if (IsClosed() || dispatching_cancel_ || !DomWindow())
return;

// TODO(domenic): only dispatch cancel if there's been user activation.
Event& cancel_event = *Event::CreateCancelable(event_type_names::kCancel);
{
base::AutoReset<bool> scoped_committing(&dispatching_cancel_, true);
DispatchEvent(cancel_event);
}
if (cancel_event.defaultPrevented()) {
state_ = State::kModal;
WatcherStack& stack = WatcherStack::From(*DomWindow());

if (stack.CanCloseWatcherFireCancel()) {
stack.ConsumeCloseWatcherCancelability();
Event& cancel_event = *Event::CreateCancelable(event_type_names::kCancel);
{
base::AutoReset<bool> scoped_committing(&dispatching_cancel_, true);
DispatchEvent(cancel_event);
}
if (cancel_event.defaultPrevented())
return;
}
Close();
}

// static
void CloseWatcher::InstallUserActivationObserver(LocalDOMWindow& window) {
Supplement<LocalDOMWindow>::ProvideTo(
window, MakeGarbageCollected<WatcherStack>(window));
}

void CloseWatcher::Close() {
if (IsClosed())
return;
Expand Down
46 changes: 43 additions & 3 deletions third_party/blink/renderer/modules/closewatcher/close_watcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
#ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_CLOSEWATCHER_CLOSE_WATCHER_H_
#define THIRD_PARTY_BLINK_RENDERER_MODULES_CLOSEWATCHER_CLOSE_WATCHER_H_

#include "base/time/time.h"
#include "third_party/blink/public/mojom/close_watcher/close_listener.mojom-blink.h"
#include "third_party/blink/renderer/core/dom/events/event_target.h"
#include "third_party/blink/renderer/core/dom/events/native_event_listener.h"
#include "third_party/blink/renderer/core/execution_context/execution_context_lifecycle_observer.h"
#include "third_party/blink/renderer/core/frame/local_dom_window.h"
#include "third_party/blink/renderer/platform/heap/collection_support/heap_linked_hash_set.h"
#include "third_party/blink/renderer/platform/mojo/heap_mojo_receiver.h"
#include "third_party/blink/renderer/platform/supplementable.h"
Expand All @@ -24,6 +26,8 @@ class CloseWatcher final : public EventTargetWithInlineData,
explicit CloseWatcher(LocalDOMWindow*);
void Trace(Visitor*) const override;

static void InstallUserActivationObserver(LocalDOMWindow&);

bool IsClosed() const { return state_ == State::kClosed; }

void signalClosed();
Expand All @@ -47,7 +51,8 @@ class CloseWatcher final : public EventTargetWithInlineData,
// CloseWatcher when the stack is non-empty requires a user activation.
class WatcherStack final : public NativeEventListener,
public Supplement<LocalDOMWindow>,
public mojom::blink::CloseListener {
public mojom::blink::CloseListener,
public LocalDOMWindow::UserActivationObserver {
public:
static const char kSupplementName[];

Expand All @@ -56,10 +61,39 @@ class CloseWatcher final : public EventTargetWithInlineData,

void Add(CloseWatcher*);
void Remove(CloseWatcher*);
bool HasActiveWatcher() { return !watchers_.IsEmpty(); }
bool HasActiveWatcher() const { return !watchers_.IsEmpty(); }

// This checks whether we can create a new CloseWatcher. Per spec we allow
// one "free" CloseWatcher, but if one already exists in the stack, then
// doing so will consume transient user activation.
//
// So the return values mean:
//
// - true: either we have no close watchers in the stack, and this is our
// free close watcher, or we had other close watchers in the stack, and we
// successfully consumed transient user activation so we can create a new
// one.
//
// - false: we had other close watchers in the stack, and there was no
// transient user activation to consume.
//
// If true is returned, then CanCloseWatcherFireCancel() will flip to false
// until the next user activation; close-watcher-related user activations
// are a shared resource between creation and the cancel event.
bool CheckForCreation();

void Trace(Visitor*) const final;

void DidReceiveUserActivation() override {
user_activation_time_ = base::TimeTicks::Now();
}
void ConsumeCloseWatcherCancelability() {
last_used_user_activation_time_ = user_activation_time_;
}
bool CanCloseWatcherFireCancel() const {
return last_used_user_activation_time_ != user_activation_time_;
}

private:
// NativeEventListener override:
void Invoke(ExecutionContext*, Event*) final;
Expand All @@ -72,9 +106,15 @@ class CloseWatcher final : public EventTargetWithInlineData,
// Holds a pipe which the service uses to notify this object
// when the idle state has changed.
HeapMojoReceiver<mojom::blink::CloseListener, WatcherStack> receiver_;

// Tracks last use activation time consumed by a close watcher vs. last
// time user activation happened:
// https://wicg.github.io/close-watcher/#timestamp-of-last-activation-used-for-close-watchers
base::TimeTicks user_activation_time_;
base::TimeTicks last_used_user_activation_time_;
};

enum class State { kActive, kModal, kClosed };
enum class State { kActive, kClosed };
State state_ = State::kActive;
bool dispatching_cancel_ = false;
};
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/modules/modules_initializer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
#include "third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.h"
#include "third_party/blink/renderer/modules/canvas/imagebitmap/image_bitmap_rendering_context.h"
#include "third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.h"
#include "third_party/blink/renderer/modules/closewatcher/close_watcher.h"
#include "third_party/blink/renderer/modules/csspaint/css_paint_image_generator_impl.h"
#include "third_party/blink/renderer/modules/csspaint/nativepaint/background_color_paint_image_generator_impl.h"
#include "third_party/blink/renderer/modules/csspaint/nativepaint/clip_path_paint_image_generator_impl.h"
Expand Down Expand Up @@ -244,6 +245,7 @@ void ModulesInitializer::InstallSupplements(LocalFrame& frame) const {
InspectorAccessibilityAgent::ProvideTo(&frame);
ImageDownloaderImpl::ProvideTo(frame);
AudioRendererSinkCache::InstallWindowObserver(*frame.DomWindow());
CloseWatcher::InstallUserActivationObserver(*frame.DomWindow());
}

MediaControls* ModulesInitializer::CreateMediaControls(
Expand Down
Loading

0 comments on commit f81bb8a

Please sign in to comment.