Skip to content

Commit

Permalink
Revert "Use base::apple::Owned[frameworktype] for ui::PlatformEvent"
Browse files Browse the repository at this point in the history
This reverts commit 326a3f7.

Reason for revert: Might be cause of https://crbug.com/1449023

Original change's description:
> Use base::apple::Owned[frameworktype] for ui::PlatformEvent
>
> base::apple::Owned[frameworktype] is a new way to easily generate
> owning wrapper classes for framework types. This CL switches
> ui::PlatformEvent to use it, and does cleanup.
>
> This unlocks the ability to convert ui/events/ to use ARC.
>
> Bug: 1443009
> Bug: 1433041
> Bug: 1280317
> Change-Id: Ib1ffd2f54aa1ba641eebbabe3ed4349b026650f3
> Include-Ci-Only-Tests: true
> Cq-Include-Trybots: luci.chromium.try:ios-blink-dbg-fyi
> Cq-Include-Trybots: luci.chrome.try:mac-chrome
> Validate-Test-Flakiness: skip
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4562796
> Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
> Auto-Submit: Avi Drissman <avi@chromium.org>
> Reviewed-by: Mark Mentovai <mark@chromium.org>
> Commit-Queue: Avi Drissman <avi@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1149240}

Bug: 1443009
Bug: 1433041
Bug: 1280317
Bug: 1449023
Change-Id: I555fed7f7e764b0721275f9224f6721b02c219eb
Cq-Include-Trybots: luci.chromium.try:ios-blink-dbg-fyi
Cq-Include-Trybots: luci.chrome.try:mac-chrome
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4566548
Auto-Submit: Avi Drissman <avi@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1149427}
  • Loading branch information
Avi Drissman authored and Chromium LUCI CQ committed May 25, 2023
1 parent 29eeb6a commit 8b7cdbc
Show file tree
Hide file tree
Showing 25 changed files with 279 additions and 322 deletions.
9 changes: 3 additions & 6 deletions chrome/browser/image_editor/event_capture_mac.mm
Expand Up @@ -8,7 +8,6 @@

#include <memory>

#include "base/apple/owned_objc.h"
#include "base/check.h"
#include "base/functional/callback.h"
#include "base/memory/ptr_util.h"
Expand Down Expand Up @@ -51,8 +50,7 @@ void Reset() {
private:
// remote_cocoa::CocoaMouseCaptureDelegate:
bool PostCapturedEvent(NSEvent* event) override {
std::unique_ptr<ui::Event> ui_event =
ui::EventFromNative(base::apple::OwnedNSEvent(event));
std::unique_ptr<ui::Event> ui_event = ui::EventFromNative(event);
if (!ui_event) {
return false;
}
Expand Down Expand Up @@ -123,9 +121,8 @@ void OnMouseCaptureLost() override {
return event;
}

if (!target_window || event.window == target_window) {
std::unique_ptr<ui::Event> ui_event =
ui::EventFromNative(base::apple::OwnedNSEvent(event));
if (!target_window || [event window] == target_window) {
std::unique_ptr<ui::Event> ui_event = ui::EventFromNative(event);
if (!ui_event) {
return event;
}
Expand Down
1 change: 0 additions & 1 deletion components/remote_cocoa/app_shim/BUILD.gn
Expand Up @@ -64,7 +64,6 @@ component("app_shim") {
defines = [ "REMOTE_COCOA_APP_SHIM_IMPLEMENTATION" ]
deps = [
"//base",
"//base:base_arc",
"//base:i18n",
"//components/crash/core/common",
"//components/remote_cocoa/common:mojo",
Expand Down
38 changes: 16 additions & 22 deletions components/remote_cocoa/app_shim/bridged_content_view.mm
Expand Up @@ -6,7 +6,6 @@

#include <limits>

#include "base/apple/owned_objc.h"
#include "base/check_op.h"
#import "base/mac/foundation_util.h"
#import "base/mac/mac_util.h"
Expand Down Expand Up @@ -327,26 +326,24 @@ - (void)processCapturedMouseEvent:(NSEvent*)theEvent {
if (remote_cocoa::IsNSToolbarFullScreenWindow(target)) {
// We are in immersive fullscreen. This event is generated from the overlay
// window which sits atop the toolbar. Convert the event location to the
// content view coordinate, which should have the same bounds as the overlay
// content view coordiate, which should have the same bounds as the overlay
// window.
// This is to handle the case that `target` may contain the titlebar which
// the overlay window does not contain. Without this, buttons in the toolbar
// are not clickable when the titlebar is revealed.
event_location = MovePointToView(theEvent.locationInWindow, source, self);
event_location = MovePointToView([theEvent locationInWindow], source, self);
} else {
event_location =
MovePointToWindow(theEvent.locationInWindow, source, target);
MovePointToWindow([theEvent locationInWindow], source, target);
}
[self updateTooltipIfRequiredAt:event_location];

if (isScrollEvent) {
auto event =
std::make_unique<ui::ScrollEvent>(base::apple::OwnedNSEvent(theEvent));
auto event = std::make_unique<ui::ScrollEvent>(theEvent);
event->set_location(event_location);
_bridge->host()->OnScrollEvent(std::move(event));
} else {
auto event =
std::make_unique<ui::MouseEvent>(base::apple::OwnedNSEvent(theEvent));
auto event = std::make_unique<ui::MouseEvent>(theEvent);
event->set_location(event_location);
_bridge->host()->OnMouseEvent(std::move(event));
}
Expand Down Expand Up @@ -434,7 +431,7 @@ - (BOOL)handleUnhandledKeyDownAsKeyEvent {
if (!_hasUnhandledKeyDownEvent)
return NO;

ui::KeyEvent event((base::apple::OwnedNSEvent(_keyDownEvent)));
ui::KeyEvent event(_keyDownEvent);
[self handleKeyEvent:&event];
_hasUnhandledKeyDownEvent = NO;
return event.handled();
Expand Down Expand Up @@ -462,7 +459,7 @@ - (void)handleAction:(ui::TextEditCommand)command
// key events!
if (NSApp.currentEvent.type == NSEventTypeKeyDown ||
NSApp.currentEvent.type == NSEventTypeKeyUp) {
event.SetNativeEvent(base::apple::OwnedNSEvent(NSApp.currentEvent));
event.SetNativeEvent(NSApp.currentEvent);
}

if ([self dispatchKeyEventToMenuController:&event])
Expand Down Expand Up @@ -647,8 +644,7 @@ - (void)mouseEvent:(NSEvent*)theEvent {
return;

DCHECK([theEvent type] != NSEventTypeScrollWheel);
auto event =
std::make_unique<ui::MouseEvent>(base::apple::OwnedNSEvent(theEvent));
auto event = std::make_unique<ui::MouseEvent>(theEvent);
[self adjustUiEventLocation:event.get() fromNativeEvent:theEvent];

// Aura updates tooltips with the help of aura::Window::AddPreTargetHandler().
Expand Down Expand Up @@ -838,7 +834,7 @@ - (void)keyDown:(NSEvent*)theEvent {
}

- (void)keyUp:(NSEvent*)theEvent {
ui::KeyEvent event((base::apple::OwnedNSEvent(theEvent)));
ui::KeyEvent event(theEvent);
[self handleKeyEvent:&event];
}

Expand All @@ -850,16 +846,15 @@ - (void)flagsChanged:(NSEvent*)theEvent {
// is also sent, so we should drop this one. See https://crbug.com/889618
return;
}
ui::KeyEvent event((base::apple::OwnedNSEvent(theEvent)));
ui::KeyEvent event(theEvent);
[self handleKeyEvent:&event];
}

- (void)scrollWheel:(NSEvent*)theEvent {
if (!_bridge)
return;

auto event =
std::make_unique<ui::ScrollEvent>(base::apple::OwnedNSEvent(theEvent));
auto event = std::make_unique<ui::ScrollEvent>(theEvent);
[self adjustUiEventLocation:event.get() fromNativeEvent:theEvent];

// Aura updates tooltips with the help of aura::Window::AddPreTargetHandler().
Expand All @@ -885,23 +880,22 @@ - (void)swipeWithEvent:(NSEvent*)event {
swipeDetails.set_device_type(ui::GestureDeviceType::DEVICE_TOUCHPAD);
swipeDetails.set_touch_points(3);

base::apple::OwnedNSEvent owned_event(event);
gfx::PointF location = ui::EventLocationFromNative(owned_event);
gfx::PointF location = ui::EventLocationFromNative(event);
// Note this uses the default unique_touch_event_id of 0 (Swipe events do not
// support -[NSEvent eventNumber]). This doesn't seem like a real omission
// because the three-finger swipes are instant and can't be tracked anyway.
auto gestureEvent = std::make_unique<ui::GestureEvent>(
location.x(), location.y(), ui::EventFlagsFromNative(owned_event),
ui::EventTimeFromNative(owned_event), swipeDetails);
location.x(), location.y(), ui::EventFlagsFromNative(event),
ui::EventTimeFromNative(event), swipeDetails);
_bridge->host()->OnGestureEvent(std::move(gestureEvent));
}

- (void)quickLookWithEvent:(NSEvent*)theEvent {
if (!_bridge)
return;

const gfx::Point locationInContent = gfx::ToFlooredPoint(
ui::EventLocationFromNative(base::apple::OwnedNSEvent(theEvent)));
const gfx::Point locationInContent =
gfx::ToFlooredPoint(ui::EventLocationFromNative(theEvent));

bool foundWord = false;
gfx::DecoratedText decoratedWord;
Expand Down
Expand Up @@ -861,8 +861,7 @@ NSUInteger CountBridgedWindows(NSArray* child_windows) {
return event;
}

std::unique_ptr<ui::Event> ui_event =
ui::EventFromNative(base::apple::OwnedNSEvent(event));
std::unique_ptr<ui::Event> ui_event = ui::EventFromNative(event);
bool event_handled = false;
weak_ptr->host_->DispatchMonitorEvent(std::move(ui_event),
&event_handled);
Expand Down
20 changes: 10 additions & 10 deletions content/app_shim_remote_cocoa/render_widget_host_view_cocoa.mm
Expand Up @@ -708,7 +708,7 @@ - (void)mouseEvent:(NSEvent*)theEvent {
// and the following NSEventTypeMouseExited event should have the same pointer
// type. For NSEventTypeMouseExited and NSEventTypeMouseEntered events, they
// do not have a subtype. We decide their pointer types by checking if we
// received a NSEventTypeTabletProximity event.
// recevied a NSEventTypeTabletProximity event.
NSEventType type = [theEvent type];
if (type == NSEventTypeMouseEntered || type == NSEventTypeMouseExited) {
_pointerType = _isStylusEnteringProximity
Expand Down Expand Up @@ -1102,7 +1102,7 @@ - (void)keyEvent:(NSEvent*)theEvent wasKeyEquivalent:(BOOL)equiv {

// Indicates if we should send the key event and corresponding editor commands
// after processing the input method result.
BOOL delayEventUntilAfterImeComposition = NO;
BOOL delayEventUntilAfterImeCompostion = NO;

// To emulate Windows, over-write |event.windowsKeyCode| to VK_PROCESSKEY
// while an input method is composing or inserting a text.
Expand All @@ -1123,7 +1123,7 @@ - (void)keyEvent:(NSEvent*)theEvent wasKeyEquivalent:(BOOL)equiv {
// We shouldn't do this if a new marked text was set by the input method,
// otherwise the new marked text might be cancelled by webkit.
if (_hasEditCommands && !_hasMarkedText)
delayEventUntilAfterImeComposition = YES;
delayEventUntilAfterImeCompostion = YES;
} else {
_hostHelper->ForwardKeyboardEventWithCommands(event, latency_info,
std::move(_editCommands));
Expand Down Expand Up @@ -1178,10 +1178,10 @@ - (void)keyEvent:(NSEvent*)theEvent wasKeyEquivalent:(BOOL)equiv {
// edit commands here. This usually occurs when the input method wants to
// finish current composition session but still wants the application to
// handle the key event. See http://crbug.com/48161 for reference.
if (delayEventUntilAfterImeComposition) {
// If |delayEventUntilAfterImeComposition| is YES, then a fake key down
// event with windowsKeyCode == 0xE5 has already been sent to webkit. So
// before sending the real key down event, we need to send a fake key up
if (delayEventUntilAfterImeCompostion) {
// If |delayEventUntilAfterImeCompostion| is YES, then a fake key down event
// with windowsKeyCode == 0xE5 has already been sent to webkit.
// So before sending the real key down event, we need to send a fake key up
// event to balance it.
NativeWebKeyboardEvent fakeEvent = event;
fakeEvent.SetType(blink::WebInputEvent::Type::kKeyUp);
Expand All @@ -1205,7 +1205,7 @@ - (void)keyEvent:(NSEvent*)theEvent wasKeyEquivalent:(BOOL)equiv {
event.text[1] = 0;
event.skip_in_browser = true;
_hostHelper->ForwardKeyboardEvent(event, latency_info);
} else if ((!textInserted || delayEventUntilAfterImeComposition) &&
} else if ((!textInserted || delayEventUntilAfterImeCompostion) &&
event.text[0] != '\0' &&
((modifierFlags & kCtrlCmdKeyMask) ||
(_hasEditCommands && _editCommands.empty()))) {
Expand Down Expand Up @@ -1555,7 +1555,7 @@ - (BOOL)becomeFirstResponder {

_host->OnFirstResponderChanged(true);

// Cancel any ongoing composition text which was left before we lost focus.
// Cancel any onging composition text which was left before we lost focus.
// TODO(suzhe): We should do it in -resignFirstResponder: method, but
// somehow that method won't be called when switching among different tabs.
// See http://crbug.com/47209
Expand All @@ -1580,7 +1580,7 @@ - (BOOL)resignFirstResponder {
_host->RequestShutdown();
}

// We should cancel any ongoing composition whenever RWH's Blur() method gets
// We should cancel any onging composition whenever RWH's Blur() method gets
// called, because in this case, webkit will confirm the ongoing composition
// internally.
[self cancelComposition];
Expand Down
Expand Up @@ -35,7 +35,6 @@

#include <stdint.h>

#include "base/apple/owned_objc.h"
#include "base/mac/mac_util.h"
#include "base/metrics/histogram_macros.h"
#include "base/strings/string_util.h"
Expand Down Expand Up @@ -251,20 +250,19 @@ bool IsSystemKeyEvent(const blink::WebKeyboardEvent& event) {
} // namespace

blink::WebKeyboardEvent WebKeyboardEventBuilder::Build(NSEvent* event) {
ui::ComputeEventLatencyOS(base::apple::OwnedNSEvent(event));
ui::ComputeEventLatencyOS(event);

ui::DomCode dom_code = ui::DomCodeFromNSEvent(event);
int modifiers =
ModifiersFromEvent(event) | ui::DomCodeToWebInputEventModifiers(dom_code);

if ((event.type != NSEventTypeFlagsChanged) && event.ARepeat) {
if (([event type] != NSEventTypeFlagsChanged) && [event isARepeat])
modifiers |= blink::WebInputEvent::kIsAutoRepeat;
}

blink::WebKeyboardEvent result(
ui::IsKeyUpEvent(event) ? blink::WebInputEvent::Type::kKeyUp
: blink::WebInputEvent::Type::kRawKeyDown,
modifiers, ui::EventTimeStampFromSeconds(event.timestamp));
modifiers, ui::EventTimeStampFromSeconds([event timestamp]));

// Some keys have the same meaning but different locations on the keyboard:
// the left and right shift keys; the numeric keypad keys and their
Expand Down Expand Up @@ -333,7 +331,7 @@ bool IsSystemKeyEvent(const blink::WebKeyboardEvent& event) {
NSView* view,
blink::WebPointerProperties::PointerType pointerType,
bool unacceleratedMovement) {
ui::ComputeEventLatencyOS(base::apple::OwnedNSEvent(event));
ui::ComputeEventLatencyOS(event);
blink::WebInputEvent::Type event_type =
blink::WebInputEvent::Type::kUndefined;
int click_count = 0;
Expand Down Expand Up @@ -451,7 +449,7 @@ bool IsSystemKeyEvent(const blink::WebKeyboardEvent& event) {
blink::WebMouseWheelEvent WebMouseWheelEventBuilder::Build(
NSEvent* event,
NSView* view) {
ui::ComputeEventLatencyOS(base::apple::OwnedNSEvent(event));
ui::ComputeEventLatencyOS(event);
blink::WebMouseWheelEvent result(
blink::WebInputEvent::Type::kMouseWheel, ModifiersFromEvent(event),
ui::EventTimeStampFromSeconds([event timestamp]));
Expand Down Expand Up @@ -683,7 +681,7 @@ bool IsSystemKeyEvent(const blink::WebKeyboardEvent& event) {

blink::WebTouchEvent result(event_type, ModifiersFromEvent(event),
ui::EventTimeStampFromSeconds([event timestamp]));
ui::ComputeEventLatencyOS(base::apple::OwnedNSEvent(event));
ui::ComputeEventLatencyOS(event);
result.hovering = event_type == blink::WebInputEvent::Type::kTouchEnd;
result.unique_touch_event_id = ui::GetNextTouchEventId();
result.touches_length = 1;
Expand Down
Expand Up @@ -8,7 +8,6 @@
#import <Cocoa/Cocoa.h>
#include <stddef.h>

#include "base/apple/owned_objc.h"
#include "base/mac/mac_util.h"
#include "base/mac/scoped_cftyperef.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -676,7 +675,7 @@
blink::WebMouseWheelEvent web_event =
content::WebMouseWheelEventBuilder::Build(mac_event,
[window contentView]);
ui::MouseWheelEvent ui_event((base::apple::OwnedNSEvent(mac_event)));
ui::MouseWheelEvent ui_event(mac_event);

EXPECT_EQ(delta_x * ui::kScrollbarPixelsPerCocoaTick, web_event.delta_x);
EXPECT_EQ(web_event.delta_x, ui_event.x_offset());
Expand Down
5 changes: 1 addition & 4 deletions ui/base/BUILD.gn
Expand Up @@ -566,10 +566,7 @@ component("base") {

if (is_mac) {
public_deps += [ "//ui/base/cursor" ]
deps += [
"//base:base_arc",
"//ui/resources:ui_resources_grd",
]
deps += [ "//ui/resources:ui_resources_grd" ]
}

if (toolkit_views) {
Expand Down
6 changes: 2 additions & 4 deletions ui/base/cocoa/menu_controller.mm
Expand Up @@ -4,7 +4,6 @@

#import "ui/base/cocoa/menu_controller.h"

#include "base/apple/owned_objc.h"
#include "base/check_op.h"
#include "base/functional/bind.h"
#include "base/mac/foundation_util.h"
Expand Down Expand Up @@ -327,9 +326,8 @@ - (void)itemSelected:(id)sender {
ui::ElementTrackerMac::GetInstance()->NotifyMenuItemActivated([sender menu],
identifier);
}
model->ActivatedAt(
modelIndex,
ui::EventFlagsFromNative(base::apple::OwnedNSEvent(NSApp.currentEvent)));
model->ActivatedAt(modelIndex,
ui::EventFlagsFromNative([NSApp currentEvent]));
// Note: |self| may be destroyed by the call to ActivatedAt().
}

Expand Down
5 changes: 0 additions & 5 deletions ui/events/BUILD.gn
Expand Up @@ -376,7 +376,6 @@ component("events") {
}

if (is_mac) {
deps += [ "//base:base_arc" ]
frameworks = [ "AppKit.framework" ]
}

Expand Down Expand Up @@ -724,10 +723,6 @@ if (use_blink) {
deps += [ "//ui/events:keyboard_hook" ]
}

if (is_mac) {
deps += [ "//base:base_arc" ]
}

if (is_castos) {
sources += [ "chromecast/scroller_unittest.cc" ]
}
Expand Down

0 comments on commit 8b7cdbc

Please sign in to comment.