diff --git a/base/BUILD.gn b/base/BUILD.gn index 4925a756be531..a2f4d91b2aea6 100644 --- a/base/BUILD.gn +++ b/base/BUILD.gn @@ -205,6 +205,18 @@ buildflag_header("message_pump_buildflags") { 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 @@ component("base") { "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 0000000000000..852199b8a7272 --- /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 0000000000000..f919df9295d8e --- /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 0000000000000..68be95d9a52d7 --- /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 8d4b50a250a31..11b76223c6128 100644 --- a/chrome/browser/ui/BUILD.gn +++ b/chrome/browser/ui/BUILD.gn @@ -4115,6 +4115,7 @@ static_library("ui") { 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 b104d9b6e91a6..a39e875a9ca2d 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 eb1570b99202a..4811b2a3ecdc4 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 @@ bool ShouldHandleKeyboardEvent(const content::NativeWebKeyboardEvent& event) { 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 @@ content::KeyboardEventProcessingResult BrowserFrameMac::PreHandleKeyboardEvent( // -[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 @@ content::KeyboardEventProcessingResult BrowserFrameMac::PreHandleKeyboardEvent( 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 c87ae83d0a975..34f794f084bb4 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 @@ class RenderWidgetHostNSViewBridgeOwner 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 b95ab55cbffb4..ed4e4ad238008 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 @@ void ExtractUnderlines(NSAttributedString* string, // 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 0813b457cdfe2..dc056f396cf12 100644 --- a/content/browser/BUILD.gn +++ b/content/browser/BUILD.gn @@ -2622,6 +2622,7 @@ source_set("browser") { ] deps += [ ":mac_helpers", + "//base:base_arc", "//components/remote_cocoa/app_shim", "//components/remote_cocoa/browser", "//components/remote_cocoa/common:mojo", @@ -2642,6 +2643,7 @@ source_set("browser") { } 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 e522d6b2446cb..c72650a7136d1 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 @@ class NativeInputEventBuilder { #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 0000000000000..16726f5611670 --- /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 a8f08dd91f35e..04a58f51e2948 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 @@ gfx::NativeEvent NativeInputEventBuilder::CreateEvent( (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 35cfbcb51ab2d..0b3a084a95cab 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 @@ namespace content { 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 8e2f26293d22d..c04ec7aded540 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 @@ size_t WebKeyboardEventTextLength(const char16_t* text) { 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 @@ NativeWebKeyboardEvent::NativeWebKeyboardEvent( 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 64e19bc7e49ec..f9ebad40d85f4 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 @@ TEST(NativeWebKeyboardEventMac, CtrlCmdSpaceKeyDownRoundTrip) { 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 a7cbd153f799d..6fe4b7b37e733 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 @@ void RenderWidgetHostViewMac::ForwardKeyboardEventWithCommands( // 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 fbdd7821cb37b..4b0ab7e307ba1 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 @@ TEST_F(RenderViewTest, MacTestCmdUp) { 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 @@ TEST_F(RenderViewTest, MacTestCmdUp) { 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 @@ TEST_F(RenderViewTest, MacTestCmdUp) { 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 @@ TEST_F(RenderViewTest, MacTestCmdUp) { 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 1e96da71a8437..d50bb63ba9a77 100644 --- a/content/shell/BUILD.gn +++ b/content/shell/BUILD.gn @@ -367,7 +367,10 @@ static_library("content_shell_lib") { } 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 df2594685a0d4..29faa9d91180e 100644 --- a/content/shell/browser/shell_platform_delegate_mac.mm +++ b/content/shell/browser/shell_platform_delegate_mac.mm @@ -345,14 +345,15 @@ bool ShellPlatformDelegate::HandleKeyboardEvent( // 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 c304b8d9052fe..eb3e2acaee166 100644 --- a/content/test/BUILD.gn +++ b/content/test/BUILD.gn @@ -2974,6 +2974,7 @@ test("content_unittests") { 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 22bf9d68d38e1..2d9417a8a832f 100644 --- a/gpu/command_buffer/client/BUILD.gn +++ b/gpu/command_buffer/client/BUILD.gn @@ -90,11 +90,14 @@ source_set("client_sources") { 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 ea2f727c28d98..189a876d94d04 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 c7a777df68036..80edfe22902d9 100644 --- a/ui/gfx/BUILD.gn +++ b/ui/gfx/BUILD.gn @@ -293,7 +293,7 @@ component("gfx") { ":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 @@ source_set("resize_image_dimensions") { } # 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 @@ group("selection_bound") { # 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 @@ source_set("memory_buffer_sources") { "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 @@ source_set("memory_buffer_sources") { 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 0000000000000..89bd8a404fc9f --- /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 46a02a3886345..73283ac678103 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 @@ struct IAccessible; #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 @@ class UITextField; #elif BUILDFLAG(IS_MAC) #ifdef __OBJC__ @class NSCursor; -@class NSEvent; @class NSImage; @class NSView; @class NSWindow; @@ -82,7 +91,6 @@ class UITextField; #else struct objc_object; class NSCursor; -class NSEvent; class NSImage; class NSView; class NSWindow; @@ -121,12 +129,12 @@ constexpr NativeWindow kNullNativeWindow = nullptr; 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 UnimplementedNativeViewAccessible = 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 f37a5a881ac6a..7462baf7cfae6 100644 --- a/ui/views/controls/webview/BUILD.gn +++ b/ui/views/controls/webview/BUILD.gn @@ -41,6 +41,7 @@ component("webview") { "//ui/content_accelerators", "//ui/events", "//ui/events:events_base", + "//ui/gfx:native_widget_types", "//ui/web_dialogs", "//url", ] @@ -61,6 +62,10 @@ component("webview") { 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 e520615b0aafc..badf63787d26b 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 @@ bool UnhandledKeyboardEventHandler::HandleKeyboardEvent( 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 66905c26ae9ce..d067fe63b20f8 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 @@ namespace views { 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 af075e83021d4..d7f5236840939 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 @@ void WebDialogView::SetContentsBounds(WebContents* source, // 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());