Create base::apple::Owned[frameworktype], use it for gfx::NativeEvent
base::apple::Owned[frameworktype] is a new way to easily generate
owning wrapper classes for framework types. This CL creates it,
switches gfx::NativeEvent to use it, and does cleanup.
This will also be used for ui::PlatformEvent in a followup.
Bug: 1443009
Bug: 1433041
Bug: 1280317
Change-Id: I2777864f7da1b146c4027f9ef14ad749c5bc1ccb
Include-Ci-Only-Tests: true
Cq-Include-Trybots: luci.chrome.try:mac-chrome
Validate-Test-Flakiness: skip
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4545385
Commit-Queue: Victor Miura <vmiura@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Auto-Submit: Avi Drissman <avi@chromium.org>
Reviewed-by: Victor Miura <vmiura@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1148763}
diff --git a/base/BUILD.gn b/base/BUILD.gn
index 4925a75..a2f4d91b 100644
--- a/base/BUILD.gn
+++ b/base/BUILD.gn
@@ -205,6 +205,18 @@
flags = [ "ENABLE_MESSAGE_PUMP_EPOLL=$enable_message_pump_epoll" ]
}
+if (is_apple) {
+ # TODO(https://crbug.com/1280317): Merge back into the `base` target once all
+ # .mm files are ARCed.
+ source_set("base_arc") {
+ sources = [
+ "apple/owned_objc.h",
+ "apple/owned_objc.mm",
+ ]
+ configs += [ "//build/config/compiler:enable_arc" ]
+ }
+}
+
# Base and everything it depends on should be a static library rather than
# a source set. Base is more of a "library" in the classic sense in that many
# small parts of it are used in many different contexts. This combined with a
@@ -2092,6 +2104,7 @@
"time/time_mac.mm",
]
frameworks += [ "Security.framework" ]
+ public_deps += [ ":base_arc" ]
}
# Linux.
diff --git a/base/apple/owned_objc.h b/base/apple/owned_objc.h
new file mode 100644
index 0000000..852199b8
--- /dev/null
+++ b/base/apple/owned_objc.h
@@ -0,0 +1,70 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef BASE_APPLE_OWNED_OBJC_H_
+#define BASE_APPLE_OWNED_OBJC_H_
+
+#include <memory>
+
+#include "build/build_config.h"
+
+// This file defines wrappers to allow C++ code to own Objective-C objects
+// without being Objective-C++ code themselves. These should not be used for
+// pure Objective-C++ code, in which the underlying Objective-C types should be
+// used, nor should they be used for the case where the pimpl idiom would work
+// (https://chromium.googlesource.com/chromium/src/+/main/docs/mac/mixing_cpp_and_objc.md).
+
+#if __OBJC__
+
+#define GENERATE_OWNED_OBJC_TYPE(name) @class name;
+#define GENERATE_OWNED_OBJC_PROTOCOL(name) @protocol name;
+#include "base/apple/owned_objc_types.h"
+#undef GENERATE_OWNED_OBJC_TYPE
+#undef GENERATE_OWNED_OBJC_PROTOCOL
+
+#endif // __OBJC__
+
+// Define this class two ways: the full-fledged way that allows Objective-C code
+// to fully construct and access the inner Objective-C object, and a
+// C++-compatible way that does not expose any Objective-C code and only allows
+// default construction and validity checking.
+#if __OBJC__
+#define OWNED_TYPE_DECL_OBJC_ADDITIONS(name, objctype) \
+ explicit Owned##name(objctype obj); \
+ objctype Get() const;
+#else
+#define OWNED_TYPE_DECL_OBJC_ADDITIONS(name, objctype)
+#endif // __OBJC__
+
+#define OWNED_OBJC_DECL(name, objctype) \
+ namespace base::apple { \
+ class Owned##name { \
+ public: \
+ /* Default-construct in a null state. */ \
+ Owned##name(); \
+ ~Owned##name(); \
+ Owned##name(const Owned##name&); \
+ Owned##name& operator=(const Owned##name&); \
+ /* Returns whether the object contains a valid object.*/ \
+ bool IsValid() const; \
+ /* Objective-C-only constructor and getter. */ \
+ OWNED_TYPE_DECL_OBJC_ADDITIONS(name, objctype) \
+ \
+ private: \
+ struct ObjCStorage; \
+ std::unique_ptr<ObjCStorage> objc_storage_; \
+ }; \
+ } // namespace base::apple
+
+#define GENERATE_OWNED_OBJC_TYPE(name) OWNED_OBJC_DECL(name, name*)
+#define GENERATE_OWNED_OBJC_PROTOCOL(name) OWNED_OBJC_DECL(name, id<name>)
+
+#include "base/apple/owned_objc_types.h"
+
+#undef GENERATE_OWNED_OBJC_TYPE
+#undef GENERATE_OWNED_OBJC_PROTOCOL
+#undef OWNED_OBJC_DECL
+#undef OWNED_TYPE_DECL_OBJC_ADDITIONS
+
+#endif // BASE_APPLE_OWNED_OBJC_H_
diff --git a/base/apple/owned_objc.mm b/base/apple/owned_objc.mm
new file mode 100644
index 0000000..f919df9
--- /dev/null
+++ b/base/apple/owned_objc.mm
@@ -0,0 +1,48 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/apple/owned_objc.h"
+
+#include <MacTypes.h> // For nil, to avoid having to bring in frameworks.
+
+#include "build/build_config.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+#define OWNED_OBJC_IMPL(name, objctype) \
+ namespace base::apple { \
+ struct Owned##name::ObjCStorage { \
+ objctype __strong obj; \
+ }; \
+ Owned##name::Owned##name() \
+ : objc_storage_(std::make_unique<ObjCStorage>()) {} \
+ Owned##name::~Owned##name() = default; \
+ Owned##name::Owned##name(objctype obj) : Owned##name() { \
+ objc_storage_->obj = obj; \
+ } \
+ Owned##name::Owned##name(const Owned##name& other) : Owned##name() { \
+ objc_storage_->obj = other.objc_storage_->obj; \
+ } \
+ Owned##name& Owned##name::operator=(const Owned##name& other) { \
+ objc_storage_->obj = other.objc_storage_->obj; \
+ return *this; \
+ } \
+ bool Owned##name::IsValid() const { \
+ return objc_storage_->obj != nil; \
+ } \
+ objctype Owned##name::Get() const { \
+ return objc_storage_->obj; \
+ } \
+ } // namespace base::apple
+
+#define GENERATE_OWNED_OBJC_TYPE(name) OWNED_OBJC_IMPL(name, name*)
+#define GENERATE_OWNED_OBJC_PROTOCOL(name) OWNED_OBJC_IMPL(name, id<name>)
+
+#include "base/apple/owned_objc_types.h"
+
+#undef GENERATE_OWNED_OBJC_TYPE
+#undef GENERATE_OWNED_OBJC_PROTOCOL
+#undef OWNED_OBJC_IMPL
diff --git a/base/apple/owned_objc_types.h b/base/apple/owned_objc_types.h
new file mode 100644
index 0000000..68be95d
--- /dev/null
+++ b/base/apple/owned_objc_types.h
@@ -0,0 +1,22 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// This file intentionally does not have header guards; it's included inside
+// macros to generate classes. The following lines silence a presubmit and
+// Tricium warning that would otherwise be triggered by this:
+//
+// no-include-guard-because-multiply-included
+// NOLINT(build/header_guard)
+
+#if !defined(BASE_APPLE_OWNED_OBJC_H_)
+#error Please #include "base/apple/owned_objc.h" instead of this file
+#endif
+
+#if BUILDFLAG(IS_MAC)
+GENERATE_OWNED_OBJC_TYPE(NSEvent)
+#elif BUILDFLAG(IS_IOS)
+GENERATE_OWNED_OBJC_TYPE(UIEvent)
+#endif
+GENERATE_OWNED_OBJC_PROTOCOL(MTLDevice)
+GENERATE_OWNED_OBJC_PROTOCOL(MTLSharedEvent)
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
index 8d4b50a..11b7622 100644
--- a/chrome/browser/ui/BUILD.gn
+++ b/chrome/browser/ui/BUILD.gn
@@ -4115,6 +4115,7 @@
deps += [
":ui_arc",
+ "//base:base_arc",
"//chrome/browser/apps/app_shim",
"//chrome/browser/mac:keystone_glue",
"//chrome/browser/updater:browser_updater_client",
diff --git a/chrome/browser/ui/cocoa/chrome_command_dispatcher_delegate.mm b/chrome/browser/ui/cocoa/chrome_command_dispatcher_delegate.mm
index b104d9b6..a39e875 100644
--- a/chrome/browser/ui/cocoa/chrome_command_dispatcher_delegate.mm
+++ b/chrome/browser/ui/cocoa/chrome_command_dispatcher_delegate.mm
@@ -4,6 +4,7 @@
#import "chrome/browser/ui/cocoa/chrome_command_dispatcher_delegate.h"
+#include "base/apple/owned_objc.h"
#include "base/check.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/global_keyboard_shortcuts_mac.h"
@@ -43,7 +44,8 @@
// when the focused view is not a RenderWidgetHostView, which is why this
// logic is necessary. Duplicating the logic adds a bit of redundant work,
// but doesn't cause problems.
- content::NativeWebKeyboardEvent keyboard_event(event);
+ content::NativeWebKeyboardEvent keyboard_event(
+ (base::apple::OwnedNSEvent(event)));
ui::Accelerator accelerator =
ui::GetAcceleratorFromNativeWebKeyboardEvent(keyboard_event);
auto* bridge =
@@ -110,7 +112,7 @@
bool will_execute = false;
const bool kIsBeforeFirstResponder = true;
- // See if this command will excute on the window bridge side.
+ // See if this command will execute on the window bridge side.
bridge->host()->WillExecuteCommand(result.chrome_command,
WindowOpenDisposition::CURRENT_TAB,
kIsBeforeFirstResponder, &will_execute);
diff --git a/chrome/browser/ui/views/frame/browser_frame_mac.mm b/chrome/browser/ui/views/frame/browser_frame_mac.mm
index eb1570b..4811b2a 100644
--- a/chrome/browser/ui/views/frame/browser_frame_mac.mm
+++ b/chrome/browser/ui/views/frame/browser_frame_mac.mm
@@ -66,10 +66,10 @@
return false;
// If the event was not synthesized it should have an os_event.
- DCHECK(event.os_event);
+ DCHECK(event.os_event.IsValid());
// Do not fire shortcuts on key up.
- return [event.os_event type] == NSEventTypeKeyDown;
+ return event.os_event.Get().type == NSEventTypeKeyDown;
}
} // namespace
@@ -471,12 +471,15 @@
// -[CommandDispatcher performKeyEquivalent:]. If this logic is being hit,
// it means that the event was not handled, so we must return either
// NOT_HANDLED or NOT_HANDLED_IS_SHORTCUT.
- if (EventUsesPerformKeyEquivalent(event.os_event)) {
- int command_id = CommandForKeyEvent(event.os_event).chrome_command;
- if (command_id == -1)
- command_id = DelayedWebContentsCommandForKeyEvent(event.os_event);
- if (command_id != -1)
+ NSEvent* ns_event = event.os_event.Get();
+ if (EventUsesPerformKeyEquivalent(ns_event)) {
+ int command_id = CommandForKeyEvent(ns_event).chrome_command;
+ if (command_id == -1) {
+ command_id = DelayedWebContentsCommandForKeyEvent(ns_event);
+ }
+ if (command_id != -1) {
return content::KeyboardEventProcessingResult::NOT_HANDLED_IS_SHORTCUT;
+ }
}
return content::KeyboardEventProcessingResult::NOT_HANDLED;
@@ -484,12 +487,13 @@
bool BrowserFrameMac::HandleKeyboardEvent(
const content::NativeWebKeyboardEvent& event) {
- if (!ShouldHandleKeyboardEvent(event))
+ if (!ShouldHandleKeyboardEvent(event)) {
return false;
+ }
// Redispatch the event. If it's a keyEquivalent:, this gives
// CommandDispatcher the opportunity to finish passing the event to consumers.
- return GetNSWindowHost()->RedispatchKeyEvent(event.os_event);
+ return GetNSWindowHost()->RedispatchKeyEvent(event.os_event.Get());
}
bool BrowserFrameMac::ShouldRestorePreviousBrowserWidgetState() const {
diff --git a/content/app_shim_remote_cocoa/ns_view_bridge_factory_impl.mm b/content/app_shim_remote_cocoa/ns_view_bridge_factory_impl.mm
index c87ae83..34f794f 100644
--- a/content/app_shim_remote_cocoa/ns_view_bridge_factory_impl.mm
+++ b/content/app_shim_remote_cocoa/ns_view_bridge_factory_impl.mm
@@ -108,7 +108,7 @@
std::vector<std::unique_ptr<blink::WebInputEvent>>{},
std::vector<std::unique_ptr<blink::WebInputEvent>>{}, latency_info);
std::vector<uint8_t> native_event_data =
- ui::EventToData(key_event.os_event);
+ ui::EventToData(key_event.os_event.Get());
host_->ForwardKeyboardEventWithCommands(
std::move(input_event), native_event_data, key_event.skip_in_browser,
std::move(edit_commands));
diff --git a/content/app_shim_remote_cocoa/render_widget_host_view_cocoa.mm b/content/app_shim_remote_cocoa/render_widget_host_view_cocoa.mm
index b95ab55..ed4e4ad 100644
--- a/content/app_shim_remote_cocoa/render_widget_host_view_cocoa.mm
+++ b/content/app_shim_remote_cocoa/render_widget_host_view_cocoa.mm
@@ -11,6 +11,7 @@
#include <tuple>
#include <utility>
+#include "base/apple/owned_objc.h"
#include "base/containers/contains.h"
#include "base/debug/crash_logging.h"
#import "base/mac/foundation_util.h"
@@ -956,7 +957,7 @@
// Don't cancel child popups; the key events are probably what's triggering
// the popup in the first place.
- NativeWebKeyboardEvent event(theEvent);
+ NativeWebKeyboardEvent event((base::apple::OwnedNSEvent(theEvent)));
ui::LatencyInfo latency_info;
if (event.GetType() == blink::WebInputEvent::Type::kRawKeyDown ||
event.GetType() == blink::WebInputEvent::Type::kChar) {
diff --git a/content/browser/BUILD.gn b/content/browser/BUILD.gn
index 0813b457c..dc056f3 100644
--- a/content/browser/BUILD.gn
+++ b/content/browser/BUILD.gn
@@ -2622,6 +2622,7 @@
]
deps += [
":mac_helpers",
+ "//base:base_arc",
"//components/remote_cocoa/app_shim",
"//components/remote_cocoa/browser",
"//components/remote_cocoa/common:mojo",
@@ -2642,6 +2643,7 @@
} else if (is_ios) {
sources += [
"child_process_launcher_helper_ios.cc",
+ "devtools/protocol/native_input_event_builder_ios.mm",
"renderer_host/browser_compositor_ios.h",
"renderer_host/browser_compositor_ios.mm",
"renderer_host/delegated_frame_host_client_ios.cc",
diff --git a/content/browser/devtools/protocol/native_input_event_builder.h b/content/browser/devtools/protocol/native_input_event_builder.h
index e522d6b..c72650a 100644
--- a/content/browser/devtools/protocol/native_input_event_builder.h
+++ b/content/browser/devtools/protocol/native_input_event_builder.h
@@ -8,13 +8,11 @@
#include "build/build_config.h"
#include "content/public/browser/native_web_keyboard_event.h"
-namespace content {
-namespace protocol {
+namespace content::protocol {
class NativeInputEventBuilder {
public:
-#if BUILDFLAG(IS_MAC)
- // This returned object has a retain count of 1.
+#if BUILDFLAG(IS_APPLE)
static gfx::NativeEvent CreateEvent(const NativeWebKeyboardEvent& event);
#else
// We only need this for Macs because they require an OS event to process
@@ -25,7 +23,6 @@
#endif
};
-} // namespace protocol
-} // namespace content
+} // namespace content::protocol
#endif // CONTENT_BROWSER_DEVTOOLS_PROTOCOL_NATIVE_INPUT_EVENT_BUILDER_H_
diff --git a/content/browser/devtools/protocol/native_input_event_builder_ios.mm b/content/browser/devtools/protocol/native_input_event_builder_ios.mm
new file mode 100644
index 0000000..16726f56
--- /dev/null
+++ b/content/browser/devtools/protocol/native_input_event_builder_ios.mm
@@ -0,0 +1,19 @@
+// Copyright 2023 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/devtools/protocol/native_input_event_builder.h"
+
+#include "base/apple/owned_objc.h"
+
+namespace content::protocol {
+
+gfx::NativeEvent NativeInputEventBuilder::CreateEvent(
+ const NativeWebKeyboardEvent& event) {
+ // We only need this for Macs because they require an OS event to process
+ // some keyboard events in browser (see: https://crbug.com/667387). TODO: Does
+ // this hold true for iOS Blink?
+ return base::apple::OwnedUIEvent();
+}
+
+} // namespace content::protocol
diff --git a/content/browser/devtools/protocol/native_input_event_builder_mac.mm b/content/browser/devtools/protocol/native_input_event_builder_mac.mm
index a8f08dd9..04a58f51 100644
--- a/content/browser/devtools/protocol/native_input_event_builder_mac.mm
+++ b/content/browser/devtools/protocol/native_input_event_builder_mac.mm
@@ -6,16 +6,15 @@
#include <algorithm>
+#include "base/apple/owned_objc.h"
#include "base/strings/sys_string_conversions.h"
#include "content/browser/devtools/protocol/native_input_event_builder.h"
#include "third_party/blink/public/common/input/web_input_event.h"
-namespace content {
-namespace protocol {
+namespace content::protocol {
// Mac requires a native event to emulate key events. This method gives
// only crude capabilities (see: crbug.com/667387).
-// The returned object has a retain count of 1.
gfx::NativeEvent NativeInputEventBuilder::CreateEvent(
const NativeWebKeyboardEvent& event) {
NSEventType type = NSEventTypeKeyUp;
@@ -41,17 +40,17 @@
(modifiers & blink::WebInputEvent::kMetaKey ? NSEventModifierFlagCommand
: 0);
- return [[NSEvent keyEventWithType:type
- location:NSZeroPoint
- modifierFlags:flags
- timestamp:0
- windowNumber:0
- context:nil
- characters:character
- charactersIgnoringModifiers:character
- isARepeat:NO
- keyCode:event.native_key_code] retain];
+ return base::apple::OwnedNSEvent([NSEvent
+ keyEventWithType:type
+ location:NSZeroPoint
+ modifierFlags:flags
+ timestamp:0
+ windowNumber:0
+ context:nil
+ characters:character
+ charactersIgnoringModifiers:character
+ isARepeat:NO
+ keyCode:event.native_key_code]);
}
-} // namespace protocol
-} // namespace content
+} // namespace content::protocol
diff --git a/content/browser/renderer_host/native_web_keyboard_event_ios.mm b/content/browser/renderer_host/native_web_keyboard_event_ios.mm
index 35cfbcb..0b3a084 100644
--- a/content/browser/renderer_host/native_web_keyboard_event_ios.mm
+++ b/content/browser/renderer_host/native_web_keyboard_event_ios.mm
@@ -13,44 +13,39 @@
NativeWebKeyboardEvent::NativeWebKeyboardEvent(blink::WebInputEvent::Type type,
int modifiers,
base::TimeTicks timestamp)
- : WebKeyboardEvent(type, modifiers, timestamp),
- os_event(NULL),
- skip_in_browser(false) {}
+ : WebKeyboardEvent(type, modifiers, timestamp), skip_in_browser(false) {}
NativeWebKeyboardEvent::NativeWebKeyboardEvent(
const blink::WebKeyboardEvent& web_event,
gfx::NativeView native_view)
- : WebKeyboardEvent(web_event), os_event(nullptr), skip_in_browser(false) {}
+ : WebKeyboardEvent(web_event), skip_in_browser(false) {}
NativeWebKeyboardEvent::NativeWebKeyboardEvent(gfx::NativeEvent native_event)
- : WebKeyboardEvent(WebKeyboardEventBuilder::Build(native_event)),
- os_event([native_event retain]),
+ : WebKeyboardEvent(WebKeyboardEventBuilder::Build(native_event.Get())),
+ os_event(native_event),
skip_in_browser(false) {}
NativeWebKeyboardEvent::NativeWebKeyboardEvent(const ui::KeyEvent& key_event)
- : NativeWebKeyboardEvent(key_event.native_event()) {}
+ : NativeWebKeyboardEvent(
+ base::apple::OwnedUIEvent(key_event.native_event())) {}
NativeWebKeyboardEvent::NativeWebKeyboardEvent(
const NativeWebKeyboardEvent& other)
: WebKeyboardEvent(other),
- os_event([other.os_event retain]),
+ os_event(other.os_event),
skip_in_browser(other.skip_in_browser) {}
NativeWebKeyboardEvent& NativeWebKeyboardEvent::operator=(
const NativeWebKeyboardEvent& other) {
WebKeyboardEvent::operator=(other);
- UIEvent* previous = os_event;
- os_event = [other.os_event retain];
- [previous release];
+ os_event = other.os_event;
skip_in_browser = other.skip_in_browser;
return *this;
}
-NativeWebKeyboardEvent::~NativeWebKeyboardEvent() {
- [os_event release];
-}
+NativeWebKeyboardEvent::~NativeWebKeyboardEvent() = default;
} // namespace content
diff --git a/content/browser/renderer_host/native_web_keyboard_event_mac.mm b/content/browser/renderer_host/native_web_keyboard_event_mac.mm
index 8e2f262..c04ec7a 100644
--- a/content/browser/renderer_host/native_web_keyboard_event_mac.mm
+++ b/content/browser/renderer_host/native_web_keyboard_event_mac.mm
@@ -38,19 +38,17 @@
return text_length;
}
-} // namepsace
+} // namespace
NativeWebKeyboardEvent::NativeWebKeyboardEvent(blink::WebInputEvent::Type type,
int modifiers,
base::TimeTicks timestamp)
- : WebKeyboardEvent(type, modifiers, timestamp),
- os_event(NULL),
- skip_in_browser(false) {}
+ : WebKeyboardEvent(type, modifiers, timestamp), skip_in_browser(false) {}
NativeWebKeyboardEvent::NativeWebKeyboardEvent(
const blink::WebKeyboardEvent& web_event,
gfx::NativeView native_view)
- : WebKeyboardEvent(web_event), os_event(nullptr), skip_in_browser(false) {
+ : WebKeyboardEvent(web_event), skip_in_browser(false) {
NSEventType type = NSEventTypeKeyUp;
int flags = modifiersForEvent(web_event.GetModifiers());
if (web_event.GetType() == blink::WebInputEvent::Type::kChar ||
@@ -77,57 +75,52 @@
web_event.unmodified_text)
length:unmod_text_length] autorelease];
- os_event = [[NSEvent keyEventWithType:type
- location:NSZeroPoint
- modifierFlags:flags
- timestamp:ui::EventTimeStampToSeconds(
- web_event.TimeStamp())
- windowNumber:[[native_view.GetNativeNSView() window]
- windowNumber]
- context:nil
- characters:text
- charactersIgnoringModifiers:unmodified_text
- isARepeat:NO
- keyCode:web_event.native_key_code] retain];
+ os_event = base::apple::OwnedNSEvent([NSEvent
+ keyEventWithType:type
+ location:NSZeroPoint
+ modifierFlags:flags
+ timestamp:ui::EventTimeStampToSeconds(
+ web_event.TimeStamp())
+ windowNumber:[[native_view.GetNativeNSView() window]
+ windowNumber]
+ context:nil
+ characters:text
+ charactersIgnoringModifiers:unmodified_text
+ isARepeat:NO
+ keyCode:web_event.native_key_code]);
// The eventRef is necessary for MacOS code (like NSMenu) to work later in the
// pipeline. As per documentation:
// https://developer.apple.com/documentation/appkit/nsevent/1525143-eventref
// "Other NSEvent objects create an EventRef when this property is first
// accessed, if possible".
- [os_event eventRef];
+ [os_event.Get() eventRef];
}
NativeWebKeyboardEvent::NativeWebKeyboardEvent(gfx::NativeEvent native_event)
- : WebKeyboardEvent(WebKeyboardEventBuilder::Build(native_event)),
- os_event([native_event retain]),
+ : WebKeyboardEvent(WebKeyboardEventBuilder::Build(native_event.Get())),
+ os_event(native_event),
skip_in_browser(false) {}
NativeWebKeyboardEvent::NativeWebKeyboardEvent(const ui::KeyEvent& key_event)
- : NativeWebKeyboardEvent(key_event.native_event()) {
-}
+ : NativeWebKeyboardEvent(
+ base::apple::OwnedNSEvent(key_event.native_event())) {}
NativeWebKeyboardEvent::NativeWebKeyboardEvent(
const NativeWebKeyboardEvent& other)
: WebKeyboardEvent(other),
- os_event([other.os_event retain]),
- skip_in_browser(other.skip_in_browser) {
-}
+ os_event(other.os_event),
+ skip_in_browser(other.skip_in_browser) {}
NativeWebKeyboardEvent& NativeWebKeyboardEvent::operator=(
const NativeWebKeyboardEvent& other) {
WebKeyboardEvent::operator=(other);
- NSObject* previous = os_event;
- os_event = [other.os_event retain];
- [previous release];
-
+ os_event = other.os_event;
skip_in_browser = other.skip_in_browser;
return *this;
}
-NativeWebKeyboardEvent::~NativeWebKeyboardEvent() {
- [os_event release];
-}
+NativeWebKeyboardEvent::~NativeWebKeyboardEvent() = default;
} // namespace content
diff --git a/content/browser/renderer_host/native_web_keyboard_event_mac_unittest.mm b/content/browser/renderer_host/native_web_keyboard_event_mac_unittest.mm
index 64e19bc..f9ebad4 100644
--- a/content/browser/renderer_host/native_web_keyboard_event_mac_unittest.mm
+++ b/content/browser/renderer_host/native_web_keyboard_event_mac_unittest.mm
@@ -31,8 +31,8 @@
content::WebKeyboardEventBuilder::Build(ns_event);
content::NativeWebKeyboardEvent native_event(web_event, gfx::NativeView());
- NSEvent* round_trip_ns_event = native_event.os_event;
- EXPECT_EQ([round_trip_ns_event type], [ns_event type]);
- EXPECT_EQ([round_trip_ns_event modifierFlags], [ns_event modifierFlags]);
- EXPECT_EQ([round_trip_ns_event keyCode], [ns_event keyCode]);
+ NSEvent* round_trip_ns_event = native_event.os_event.Get();
+ EXPECT_EQ(round_trip_ns_event.type, ns_event.type);
+ EXPECT_EQ(round_trip_ns_event.modifierFlags, ns_event.modifierFlags);
+ EXPECT_EQ(round_trip_ns_event.keyCode, ns_event.keyCode);
}
diff --git a/content/browser/renderer_host/render_widget_host_view_mac.mm b/content/browser/renderer_host/render_widget_host_view_mac.mm
index a7cbd153..6fe4b7b3 100644
--- a/content/browser/renderer_host/render_widget_host_view_mac.mm
+++ b/content/browser/renderer_host/render_widget_host_view_mac.mm
@@ -11,6 +11,7 @@
#include <tuple>
#include <utility>
+#include "base/apple/owned_objc.h"
#include "base/auto_reset.h"
#include "base/command_line.h"
#include "base/feature_list.h"
@@ -2146,8 +2147,8 @@
// close to the original NSEvent, resulting in all sorts of bugs. Use the
// native event serialization to reconstruct the NSEvent.
// https://crbug.com/919167,943197,964052
- [native_event.os_event release];
- native_event.os_event = [ui::EventFromData(native_event_data) retain];
+ native_event.os_event =
+ base::apple::OwnedNSEvent(ui::EventFromData(native_event_data));
ForwardKeyboardEventWithCommands(native_event, input_event->latency_info(),
std::move(edit_commands));
}
diff --git a/content/renderer/render_view_browsertest_mac.mm b/content/renderer/render_view_browsertest_mac.mm
index fbdd782..4b0ab7e 100644
--- a/content/renderer/render_view_browsertest_mac.mm
+++ b/content/renderer/render_view_browsertest_mac.mm
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "base/apple/owned_objc.h"
#include "base/run_loop.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
@@ -97,7 +98,8 @@
const char* kArrowDownScrollDown = "40,false,false,true,false\n9844";
blink_widget->AddEditCommandForNextKeyEvent(
blink::WebString::FromLatin1("moveToEndOfDocument"), blink::WebString());
- SendNativeKeyEvent(NativeWebKeyboardEvent(arrowDownKeyDown));
+ SendNativeKeyEvent(
+ NativeWebKeyboardEvent(base::apple::OwnedNSEvent(arrowDownKeyDown)));
base::RunLoop().RunUntilIdle();
ExecuteJavaScriptForTests("scroll.textContent = window.pageYOffset");
output = TestWebFrameContentDumper::DumpWebViewAsText(web_view_,
@@ -109,7 +111,8 @@
blink_widget->AddEditCommandForNextKeyEvent(
blink::WebString::FromLatin1("moveToBeginningOfDocument"),
blink::WebString());
- SendNativeKeyEvent(NativeWebKeyboardEvent(arrowUpKeyDown));
+ SendNativeKeyEvent(
+ NativeWebKeyboardEvent(base::apple::OwnedNSEvent(arrowUpKeyDown)));
base::RunLoop().RunUntilIdle();
ExecuteJavaScriptForTests("scroll.textContent = window.pageYOffset");
output = TestWebFrameContentDumper::DumpWebViewAsText(web_view_,
@@ -125,7 +128,8 @@
const char* kArrowDownNoScroll = "40,false,false,true,false\n100";
blink_widget->AddEditCommandForNextKeyEvent(
blink::WebString::FromLatin1("moveToEndOfDocument"), blink::WebString());
- SendNativeKeyEvent(NativeWebKeyboardEvent(arrowDownKeyDown));
+ SendNativeKeyEvent(
+ NativeWebKeyboardEvent(base::apple::OwnedNSEvent(arrowDownKeyDown)));
base::RunLoop().RunUntilIdle();
ExecuteJavaScriptForTests("scroll.textContent = window.pageYOffset");
output = TestWebFrameContentDumper::DumpWebViewAsText(web_view_,
@@ -137,7 +141,8 @@
blink_widget->AddEditCommandForNextKeyEvent(
blink::WebString::FromLatin1("moveToBeginningOfDocument"),
blink::WebString());
- SendNativeKeyEvent(NativeWebKeyboardEvent(arrowUpKeyDown));
+ SendNativeKeyEvent(
+ NativeWebKeyboardEvent(base::apple::OwnedNSEvent(arrowUpKeyDown)));
base::RunLoop().RunUntilIdle();
ExecuteJavaScriptForTests("scroll.textContent = window.pageYOffset");
output = TestWebFrameContentDumper::DumpWebViewAsText(web_view_,
diff --git a/content/shell/BUILD.gn b/content/shell/BUILD.gn
index 1e96da7..d50bb63 100644
--- a/content/shell/BUILD.gn
+++ b/content/shell/BUILD.gn
@@ -367,7 +367,10 @@
}
if (is_mac) {
- deps += [ "//ui/display:test_support" ]
+ deps += [
+ "//base:base_arc",
+ "//ui/display:test_support",
+ ]
}
if (is_ios) {
diff --git a/content/shell/browser/shell_platform_delegate_mac.mm b/content/shell/browser/shell_platform_delegate_mac.mm
index df2594685..29faa9d 100644
--- a/content/shell/browser/shell_platform_delegate_mac.mm
+++ b/content/shell/browser/shell_platform_delegate_mac.mm
@@ -345,14 +345,15 @@
// The event handling to get this strictly right is a tangle; cheat here a bit
// by just letting the menus have a chance at it.
- if (event.os_event.type == NSEventTypeKeyDown) {
- if ((event.os_event.modifierFlags & NSEventModifierFlagCommand) &&
- [event.os_event.characters isEqual:@"l"]) {
+ NSEvent* ns_event = event.os_event.Get();
+ if (ns_event.type == NSEventTypeKeyDown) {
+ if ((ns_event.modifierFlags & NSEventModifierFlagCommand) &&
+ [ns_event.characters isEqual:@"l"]) {
[shell_data.delegate.window makeFirstResponder:shell_data.url_edit_view];
return true;
}
- [NSApp.mainMenu performKeyEquivalent:event.os_event];
+ [NSApp.mainMenu performKeyEquivalent:ns_event];
return true;
}
return false;
diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn
index c304b8d..eb3e2ac 100644
--- a/content/test/BUILD.gn
+++ b/content/test/BUILD.gn
@@ -2974,6 +2974,7 @@
if (is_mac) {
data_deps += [ "//device/fido/strings:fido_test_strings" ]
+ deps += [ "//base:base_arc" ]
}
if (is_posix) {
diff --git a/gpu/command_buffer/client/BUILD.gn b/gpu/command_buffer/client/BUILD.gn
index 22bf9d6..2d9417a 100644
--- a/gpu/command_buffer/client/BUILD.gn
+++ b/gpu/command_buffer/client/BUILD.gn
@@ -90,11 +90,14 @@
deps = [
":gles2_interface",
"//gpu/command_buffer/common:common_sources",
- "//gpu/ipc/common:surface_handle_type",
"//ui/gfx:memory_buffer",
"//ui/gfx/geometry",
]
+ if (!is_nacl) {
+ deps += [ "//gpu/ipc/common:surface_handle_type" ]
+ }
+
# These files now rely on Skia which isn't allowed as a dependency in nacl builds.
if (!is_nacl && !is_minimal_toolchain) {
sources += [
diff --git a/ui/events/platform_event.h b/ui/events/platform_event.h
index ea2f727..189a876 100644
--- a/ui/events/platform_event.h
+++ b/ui/events/platform_event.h
@@ -7,6 +7,10 @@
#include "build/build_config.h"
+// TODO(https://crbug.com/1443009): Both gfx::NativeEvent and ui::PlatformEvent
+// are typedefs for native event types on different platforms, but they're
+// slightly different and used in different places. They should be merged.
+
#if BUILDFLAG(IS_WIN)
#include "base/win/windows_types.h"
#elif BUILDFLAG(IS_MAC)
diff --git a/ui/gfx/BUILD.gn b/ui/gfx/BUILD.gn
index c7a777df..80edfe22 100644
--- a/ui/gfx/BUILD.gn
+++ b/ui/gfx/BUILD.gn
@@ -293,7 +293,7 @@
":gfx_skia",
":gfx_switches",
":memory_buffer_sources",
- ":native_widget_types",
+ ":native_widget_types_sources",
":resize_image_dimensions",
":selection_bound_sources",
"//base",
@@ -499,21 +499,36 @@
}
# Depend on this to use native_widget_types.h without pulling in all of gfx.
-source_set("native_widget_types") {
+# The structure here allows native_widget_types to be part of the gfx component
+# in the component build, but be a separate source set in a static build.
+group("native_widget_types") {
+ if (is_component_build) {
+ public_deps = [ ":gfx" ]
+ } else {
+ public_deps = [ ":native_widget_types_sources" ]
+ }
+}
+
+source_set("native_widget_types_sources") {
+ visibility = [ ":*" ] # Depend on through ":native_widget_types".
+
public = [ "native_widget_types.h" ]
+ sources = [ "native_widget_types.cc" ]
public_deps = [
":gfx_export",
"//base",
]
- if (is_mac) {
- sources = [ "native_widget_types.mm" ]
-
- frameworks = [ "AppKit.framework" ]
- }
-
deps = [ "//build:chromeos_buildflags" ]
+
+ defines = [ "GFX_IMPLEMENTATION" ]
+
+ if (is_mac) {
+ sources += [ "native_widget_types_mac.mm" ]
+ frameworks = [ "AppKit.framework" ]
+ deps += [ "//base:base_arc" ]
+ }
}
group("selection_bound") {
@@ -527,7 +542,7 @@
# Depend on this to use selection_bound.h without pulling in all of gfx.
# Cannot be a static_library in component builds due to exported functions
source_set("selection_bound_sources") {
- visibility = [ ":*" ]
+ visibility = [ ":*" ] # Depend on through ":selection_bound".
sources = [
"gfx_export.h",
@@ -584,12 +599,15 @@
"gpu_fence.h",
"gpu_fence_handle.cc",
"gpu_fence_handle.h",
- "native_pixmap.h",
"overlay_priority_hint.h",
"overlay_transform.h",
"surface_origin.h",
]
+ if (!is_nacl) {
+ sources += [ "native_pixmap.h" ]
+ }
+
if (use_blink) {
sources += [
"ca_layer_params.cc",
@@ -607,13 +625,16 @@
deps = [
":gfx_switches",
- ":native_widget_types",
"//base",
"//build:chromecast_buildflags",
"//build:chromeos_buildflags",
"//ui/gfx/geometry",
]
+ if (!is_nacl) {
+ deps += [ ":native_widget_types_sources" ]
+ }
+
if (is_linux || is_chromeos) {
sources += [
"linux/client_native_pixmap_dmabuf.cc",
diff --git a/ui/gfx/native_widget_types.cc b/ui/gfx/native_widget_types.cc
new file mode 100644
index 0000000..89bd8a4
--- /dev/null
+++ b/ui/gfx/native_widget_types.cc
@@ -0,0 +1,20 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ui/gfx/native_widget_types.h"
+
+// TODO(https://crbug.com/1443009): ui::PlatformEvent has its own version of
+// this function. When unifying, remove one of these copies.
+
+namespace gfx {
+
+GFX_EXPORT bool IsNativeEventValid(const NativeEvent& event) {
+#if BUILDFLAG(IS_APPLE)
+ return event.IsValid();
+#else
+ return !!event;
+#endif
+}
+
+} // namespace gfx
diff --git a/ui/gfx/native_widget_types.h b/ui/gfx/native_widget_types.h
index 46a02a3..73283ac 100644
--- a/ui/gfx/native_widget_types.h
+++ b/ui/gfx/native_widget_types.h
@@ -13,9 +13,17 @@
#if BUILDFLAG(IS_ANDROID)
#include "base/android/scoped_java_ref.h"
-#elif BUILDFLAG(IS_MAC)
+#endif
+
+#if BUILDFLAG(IS_APPLE)
+#include "base/apple/owned_objc.h"
+#endif
+
+#if BUILDFLAG(IS_MAC)
#include <string>
-#elif BUILDFLAG(IS_WIN)
+#endif
+
+#if BUILDFLAG(IS_WIN)
#include "base/win/windows_types.h"
#endif
@@ -39,6 +47,10 @@
// The name 'View' here meshes with macOS where the UI elements are called
// 'views' and with our Chrome UI code where the elements are also called
// 'views'.
+//
+// TODO(https://crbug.com/1443009): Both gfx::NativeEvent and ui::PlatformEvent
+// are typedefs for native event types on different platforms, but they're
+// slightly different and used in different places. They should be merged.
#if defined(USE_AURA)
namespace aura {
@@ -59,13 +71,11 @@
#elif BUILDFLAG(IS_IOS)
#ifdef __OBJC__
struct objc_object;
-@class UIEvent;
@class UIImage;
@class UIView;
@class UIWindow;
@class UITextField;
#else
-class UIEvent;
class UIImage;
class UIView;
class UIWindow;
@@ -74,7 +84,6 @@
#elif BUILDFLAG(IS_MAC)
#ifdef __OBJC__
@class NSCursor;
-@class NSEvent;
@class NSImage;
@class NSView;
@class NSWindow;
@@ -82,7 +91,6 @@
#else
struct objc_object;
class NSCursor;
-class NSEvent;
class NSImage;
class NSView;
class NSWindow;
@@ -121,12 +129,12 @@
using NativeCursor = void*;
using NativeView = UIView*;
using NativeWindow = UIWindow*;
-using NativeEvent = UIEvent*;
+using NativeEvent = base::apple::OwnedUIEvent;
constexpr NativeView kNullNativeView = nullptr;
constexpr NativeWindow kNullNativeWindow = nullptr;
#elif BUILDFLAG(IS_MAC)
using NativeCursor = NSCursor*;
-using NativeEvent = NSEvent*;
+using NativeEvent = base::apple::OwnedNSEvent;
// NativeViews and NativeWindows on macOS are not necessarily in the same
// process as the NSViews and NSWindows that they represent. Require an explicit
// function call (GetNativeNSView or GetNativeNSWindow) to retrieve the
@@ -231,12 +239,15 @@
using NativeViewAccessible = UnimplementedNativeViewAccessible*;
#endif
+// Returns if the event is valid.
+GFX_EXPORT bool IsNativeEventValid(const NativeEvent& event);
+
// A constant value to indicate that gfx::NativeCursor refers to no cursor.
#if defined(USE_AURA)
const ui::mojom::CursorType kNullCursor =
static_cast<ui::mojom::CursorType>(-1);
#else
-const gfx::NativeCursor kNullCursor = static_cast<gfx::NativeCursor>(nullptr);
+const NativeCursor kNullCursor = static_cast<NativeCursor>(nullptr);
#endif
// Note: for test_shell we're packing a pointer into the NativeViewId. So, if
diff --git a/ui/gfx/native_widget_types.mm b/ui/gfx/native_widget_types_mac.mm
similarity index 100%
rename from ui/gfx/native_widget_types.mm
rename to ui/gfx/native_widget_types_mac.mm
diff --git a/ui/views/controls/webview/BUILD.gn b/ui/views/controls/webview/BUILD.gn
index f37a5a8..7462baf7 100644
--- a/ui/views/controls/webview/BUILD.gn
+++ b/ui/views/controls/webview/BUILD.gn
@@ -41,6 +41,7 @@
"//ui/content_accelerators",
"//ui/events",
"//ui/events:events_base",
+ "//ui/gfx:native_widget_types",
"//ui/web_dialogs",
"//url",
]
@@ -61,6 +62,10 @@
sources += [ "unhandled_keyboard_event_handler_default.cc" ]
}
}
+
+ if (is_mac) {
+ deps += [ "//base:base_arc" ]
+ }
}
static_library("test_support") {
diff --git a/ui/views/controls/webview/unhandled_keyboard_event_handler.cc b/ui/views/controls/webview/unhandled_keyboard_event_handler.cc
index e520615..badf6378 100644
--- a/ui/views/controls/webview/unhandled_keyboard_event_handler.cc
+++ b/ui/views/controls/webview/unhandled_keyboard_event_handler.cc
@@ -6,6 +6,7 @@
#include "content/public/browser/native_web_keyboard_event.h"
#include "ui/content_accelerators/accelerator_util.h"
+#include "ui/gfx/native_widget_types.h"
#include "ui/views/focus/focus_manager.h"
namespace views {
@@ -49,8 +50,9 @@
ignore_next_char_event_ = false;
}
- if (event.os_event)
+ if (gfx::IsNativeEventValid(event.os_event)) {
return HandleNativeKeyboardEvent(event, focus_manager);
+ }
return false;
}
diff --git a/ui/views/controls/webview/unhandled_keyboard_event_handler_mac.mm b/ui/views/controls/webview/unhandled_keyboard_event_handler_mac.mm
index 66905c2..d067fe6 100644
--- a/ui/views/controls/webview/unhandled_keyboard_event_handler_mac.mm
+++ b/ui/views/controls/webview/unhandled_keyboard_event_handler_mac.mm
@@ -13,14 +13,16 @@
bool UnhandledKeyboardEventHandler::HandleNativeKeyboardEvent(
const content::NativeWebKeyboardEvent& event,
FocusManager* focus_manager) {
- if (event.skip_in_browser)
+ if (event.skip_in_browser) {
return false;
+ }
- auto os_event = event.os_event;
- auto* host = views::NativeWidgetMacNSWindowHost::GetFromNativeWindow(
- [os_event window]);
- if (host)
- return host->RedispatchKeyEvent(os_event);
+ NSEvent* ns_event = event.os_event.Get();
+ auto* host =
+ views::NativeWidgetMacNSWindowHost::GetFromNativeWindow(ns_event.window);
+ if (host) {
+ return host->RedispatchKeyEvent(ns_event);
+ }
return false;
}
diff --git a/ui/views/controls/webview/web_dialog_view.cc b/ui/views/controls/webview/web_dialog_view.cc
index af075e8..d7f5236 100644
--- a/ui/views/controls/webview/web_dialog_view.cc
+++ b/ui/views/controls/webview/web_dialog_view.cc
@@ -20,6 +20,7 @@
#include "ui/events/event.h"
#include "ui/events/keycodes/keyboard_codes.h"
#include "ui/gfx/geometry/rounded_corners_f.h"
+#include "ui/gfx/native_widget_types.h"
#include "ui/views/controls/webview/webview.h"
#include "ui/views/layout/fill_layout.h"
#include "ui/views/widget/native_widget_private.h"
@@ -370,8 +371,9 @@
// they're all browser-specific. (This may change in the future.)
bool WebDialogView::HandleKeyboardEvent(content::WebContents* source,
const NativeWebKeyboardEvent& event) {
- if (!event.os_event)
+ if (!gfx::IsNativeEventValid(event.os_event)) {
return false;
+ }
return unhandled_keyboard_event_handler_.HandleKeyboardEvent(
event, GetFocusManager());
