Skip to content

Commit e6c5f91

Browse files
committed
Address safer CPP failures in WebViewImpl.h
https://bugs.webkit.org/show_bug.cgi?id=290220 Reviewed by Geoffrey Garen. * Source/WebKit/SaferCPPExpectations/NoUnretainedMemberCheckerExpectations: * Source/WebKit/UIProcess/mac/WebViewImpl.h: * Source/WebKit/UIProcess/mac/WebViewImpl.mm: (WebKit::WebViewImpl::collectKeyboardLayoutCommandsForEvent): (WebKit::WebViewImpl::doCommandBySelector): (WebKit::WebViewImpl::insertText): Canonical link: https://commits.webkit.org/292563@main
1 parent d6d2a4c commit e6c5f91

6 files changed

Lines changed: 26 additions & 23 deletions

File tree

Source/WebKit/SaferCPPExpectations/ForwardDeclCheckerExpectations

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ UIProcess/Cocoa/WebPageProxyCocoa.mm
2727
UIProcess/RemoteLayerTree/RemoteLayerTreeHost.mm
2828
UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.cpp
2929
UIProcess/WebProcessPool.h
30-
UIProcess/mac/WebViewImpl.h
3130
WebProcess/Extensions/Bindings/JSWebExtensionWrapper.h
3231
WebProcess/InjectedBundle/InjectedBundle.h
3332
WebProcess/Inspector/WebInspectorClient.cpp
@@ -37,4 +36,4 @@ WebProcess/Plugins/PDF/UnifiedPDF/UnifiedPDFPlugin.mm
3736
WebProcess/WebAuthentication/WebAuthenticatorCoordinator.cpp
3837
WebProcess/WebCoreSupport/WebChromeClient.cpp
3938
WebProcess/WebCoreSupport/WebLocalFrameLoaderClient.cpp
40-
WebProcess/WebPage/WebPage.cpp
39+
WebProcess/WebPage/WebPage.cpp
Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
UIProcess/API/Cocoa/_WKRemoteWebInspectorViewController.mm
22
UIProcess/DisplayLink.h
33
UIProcess/mac/PageClientImplMac.h
4-
UIProcess/mac/WebViewImpl.h
54
WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm
65
WebProcess/InjectedBundle/InjectedBundle.h

Source/WebKit/UIProcess/API/mac/WKWebViewMac.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1705,7 +1705,7 @@ - (_WKThumbnailView *)_thumbnailView
17051705
{
17061706
if (!_impl)
17071707
return nil;
1708-
return _impl->thumbnailView();
1708+
return _impl->thumbnailView().autorelease();
17091709
}
17101710

17111711
- (void)_setIgnoresAllEvents:(BOOL)ignoresAllEvents

Source/WebKit/UIProcess/mac/PageClientImplMac.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ - (BOOL)_hostsLayersInWindowServer;
161161
NSWindow *PageClientImpl::activeWindow() const
162162
{
163163
if (m_impl && m_impl->thumbnailView())
164-
return m_impl->thumbnailView().window;
164+
return [m_impl->thumbnailView() window];
165165
if (m_impl && m_impl->targetWindowForMovePreparation())
166166
return m_impl->targetWindowForMovePreparation();
167167
return m_view.window;

Source/WebKit/UIProcess/mac/WebViewImpl.h

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include "WKTextAnimationType.h"
3737
#include <WebCore/DOMPasteAccess.h>
3838
#include <WebCore/FocusDirection.h>
39+
#include <WebCore/KeypressCommand.h>
3940
#include <WebCore/PlatformPlaybackSessionInterface.h>
4041
#include <WebCore/ScrollTypes.h>
4142
#include <WebCore/ShareableBitmap.h>
@@ -184,7 +185,6 @@ enum class ReplacementBehavior : uint8_t;
184185

185186
namespace WebCore {
186187
struct DragItem;
187-
struct KeypressCommand;
188188
#if ENABLE(DIGITAL_CREDENTIALS_UI)
189189
struct DigitalCredentialsRequestData;
190190
#endif
@@ -527,7 +527,7 @@ class WebViewImpl final : public CanMakeWeakPtr<WebViewImpl>, public CanMakeChec
527527
CALayer *acceleratedCompositingRootLayer() const { return m_rootLayer.get(); }
528528

529529
void setThumbnailView(_WKThumbnailView *);
530-
_WKThumbnailView *thumbnailView() const { return m_thumbnailView; }
530+
RetainPtr<_WKThumbnailView> thumbnailView() const { return m_thumbnailView.get(); }
531531

532532
void setHeaderBannerLayer(CALayer *);
533533
CALayer *headerBannerLayer() const { return m_headerBannerLayer.get(); }
@@ -985,7 +985,7 @@ class WebViewImpl final : public CanMakeWeakPtr<WebViewImpl>, public CanMakeChec
985985
RetainPtr<CALayer> m_headerBannerLayer;
986986
RetainPtr<CALayer> m_footerBannerLayer;
987987

988-
_WKThumbnailView *m_thumbnailView { nullptr };
988+
WeakObjCPtr<_WKThumbnailView> m_thumbnailView;
989989

990990
RetainPtr<_WKRemoteObjectRegistry> m_remoteObjectRegistry;
991991

@@ -1012,7 +1012,14 @@ ALLOW_DEPRECATED_DECLARATIONS_END
10121012
// the application to distinguish the case of a new event from one
10131013
// that has been already sent to WebCore.
10141014
RetainPtr<NSEvent> m_keyDownEventBeingResent;
1015-
Vector<WebCore::KeypressCommand>* m_collectedKeypressCommands { nullptr };
1015+
1016+
struct CheckedCommands : public CanMakeCheckedPtr<CheckedCommands> {
1017+
WTF_MAKE_FAST_ALLOCATED;
1018+
WTF_OVERRIDE_DELETE_FOR_CHECKED_PTR(CheckedCommands);
1019+
public:
1020+
Vector<WebCore::KeypressCommand> commands;
1021+
};
1022+
CheckedPtr<CheckedCommands> m_collectedKeypressCommands;
10161023

10171024
String m_lastStringForCandidateRequest;
10181025
NSInteger m_lastCandidateRequestSequenceNumber;

Source/WebKit/UIProcess/mac/WebViewImpl.mm

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@
105105
#import <WebCore/FixedContainerEdges.h>
106106
#import <WebCore/FontAttributeChanges.h>
107107
#import <WebCore/FontAttributes.h>
108-
#import <WebCore/KeypressCommand.h>
109108
#import <WebCore/LegacyNSPasteboardTypes.h>
110109
#import <WebCore/LoaderNSURLExtras.h>
111110
#import <WebCore/LocalizedStrings.h>
@@ -4063,15 +4062,15 @@ static String commandNameForSelector(SEL selector)
40634062

40644063
void WebViewImpl::reparentLayerTreeInThumbnailView()
40654064
{
4066-
m_thumbnailView._thumbnailLayer = m_rootLayer.get();
4065+
[m_thumbnailView.get() _setThumbnailLayer: m_rootLayer.get()];
40674066
}
40684067

40694068
void WebViewImpl::updateThumbnailViewLayer()
40704069
{
4071-
_WKThumbnailView *thumbnailView = m_thumbnailView;
4070+
RetainPtr thumbnailView = m_thumbnailView.get();
40724071
ASSERT(thumbnailView);
40734072

4074-
if (thumbnailView._waitingForSnapshot && [m_view window])
4073+
if ([thumbnailView _waitingForSnapshot] && [m_view window])
40754074
reparentLayerTreeInThumbnailView();
40764075
}
40774076

@@ -5064,10 +5063,10 @@ static bool eventKeyCodeIsZeroOrNumLockOrFn(NSEvent *event)
50645063

50655064
Vector<WebCore::KeypressCommand> WebViewImpl::collectKeyboardLayoutCommandsForEvent(NSEvent *event)
50665065
{
5067-
Vector<WebCore::KeypressCommand> commands;
5068-
50695066
if ([event type] != NSEventTypeKeyDown)
5070-
return commands;
5067+
return { };
5068+
5069+
CheckedCommands commands;
50715070

50725071
ASSERT(!m_collectedKeypressCommands);
50735072
m_collectedKeypressCommands = &commands;
@@ -5081,12 +5080,12 @@ static bool eventKeyCodeIsZeroOrNumLockOrFn(NSEvent *event)
50815080

50825081
if (auto menu = NSApp.mainMenu; event.modifierFlags & NSEventModifierFlagFunction
50835082
&& [menu respondsToSelector:@selector(_containsItemMatchingEvent:includingDisabledItems:)] && [menu _containsItemMatchingEvent:event includingDisabledItems:YES]) {
5084-
commands.removeAllMatching([](auto& command) {
5083+
commands.commands.removeAllMatching([](auto& command) {
50855084
return command.commandName == "insertText:"_s;
50865085
});
50875086
}
50885087

5089-
return commands;
5088+
return WTFMove(commands.commands);
50905089
}
50915090

50925091
void WebViewImpl::interpretKeyEvent(NSEvent *event, void(^completionHandler)(BOOL handled, const Vector<WebCore::KeypressCommand>& commands))
@@ -5119,9 +5118,9 @@ static bool eventKeyCodeIsZeroOrNumLockOrFn(NSEvent *event)
51195118
{
51205119
LOG(TextInput, "doCommandBySelector:\"%s\"", sel_getName(selector));
51215120

5122-
if (auto* keypressCommands = m_collectedKeypressCommands) {
5121+
if (m_collectedKeypressCommands) {
51235122
WebCore::KeypressCommand command(NSStringFromSelector(selector));
5124-
keypressCommands->append(command);
5123+
m_collectedKeypressCommands->commands.append(command);
51255124
LOG(TextInput, "...stored");
51265125
m_page->registerKeypressCommandName(command.commandName);
51275126
} else {
@@ -5173,11 +5172,10 @@ static bool eventKeyCodeIsZeroOrNumLockOrFn(NSEvent *event)
51735172
// - If it's from an input method, then we should insert the text now.
51745173
// - If it's sent outside of keyboard event processing (e.g. from Character Viewer, or when confirming an inline input area with a mouse),
51755174
// then we also execute it immediately, as there will be no other chance.
5176-
Vector<WebCore::KeypressCommand>* keypressCommands = m_collectedKeypressCommands;
5177-
if (keypressCommands && !m_isTextInsertionReplacingSoftSpace) {
5175+
if (m_collectedKeypressCommands && !m_isTextInsertionReplacingSoftSpace) {
51785176
ASSERT(replacementRange.location == NSNotFound);
51795177
WebCore::KeypressCommand command("insertText:"_s, text);
5180-
keypressCommands->append(command);
5178+
m_collectedKeypressCommands->commands.append(command);
51815179
LOG(TextInput, "...stored");
51825180
m_page->registerKeypressCommandName(command.commandName);
51835181
return;

0 commit comments

Comments
 (0)