Skip to content

Commit 42b6e86

Browse files
cdumezcdumez
committed
Address more Safer CPP failures in _WKRemoteWebInspectorViewController.mm / WKWebProcessPlugInBrowserContextController.mm / InjectedBundle.h
https://bugs.webkit.org/show_bug.cgi?id=290273 Reviewed by Geoffrey Garen. * Source/WebKit/SaferCPPExpectations/NoUnretainedMemberCheckerExpectations: * Source/WebKit/UIProcess/API/Cocoa/_WKRemoteWebInspectorViewController.mm: * Source/WebKit/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm: (-[WKWebProcessPlugInBrowserContextController _setEditingDelegate:]): * Source/WebKit/WebProcess/InjectedBundle/InjectedBundle.cpp: (WebKit::InjectedBundle::InjectedBundle): * Source/WebKit/WebProcess/InjectedBundle/InjectedBundle.h: * Source/WebKit/WebProcess/InjectedBundle/mac/InjectedBundleMac.mm: (WebKit::InjectedBundle::initialize): Canonical link: https://commits.webkit.org/292566@main
1 parent 0ecadc1 commit 42b6e86

7 files changed

Lines changed: 23 additions & 19 deletions

File tree

‎Source/WebKit/SaferCPPExpectations/ForwardDeclCheckerExpectations‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ UIProcess/RemoteLayerTree/RemoteLayerTreeHost.mm
2828
UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.cpp
2929
UIProcess/WebProcessPool.h
3030
WebProcess/Extensions/Bindings/JSWebExtensionWrapper.h
31-
WebProcess/InjectedBundle/InjectedBundle.h
3231
WebProcess/Inspector/WebInspectorClient.cpp
3332
WebProcess/Plugins/PDF/UnifiedPDF/PDFDataDetectorOverlayController.mm
3433
WebProcess/Plugins/PDF/UnifiedPDF/PDFPresentationController.mm

‎Source/WebKit/SaferCPPExpectations/NoUnretainedMemberCheckerExpectations‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1 @@
1-
UIProcess/API/Cocoa/_WKRemoteWebInspectorViewController.mm
21
UIProcess/mac/PageClientImplMac.h
3-
WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm
4-
WebProcess/InjectedBundle/InjectedBundle.h

‎Source/WebKit/UIProcess/API/Cocoa/_WKRemoteWebInspectorViewController.mm‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#import "_WKInspectorConfigurationInternal.h"
3737
#import "_WKInspectorDebuggableInfoInternal.h"
3838
#import <wtf/TZoneMallocInlines.h>
39+
#import <wtf/WeakObjCPtr.h>
3940

4041
#if ENABLE(INSPECTOR_EXTENSIONS)
4142
#import "APIInspectorExtension.h"
@@ -78,11 +79,11 @@ void closeFromFrontend() override
7879

7980
Ref<API::InspectorConfiguration> configurationForRemoteInspector(RemoteWebInspectorUIProxy& inspector) override
8081
{
81-
return downcast<API::InspectorConfiguration>([m_controller.configuration _apiObject]);
82+
return downcast<API::InspectorConfiguration>([[m_controller configuration] _apiObject]);
8283
}
8384

8485
private:
85-
_WKRemoteWebInspectorViewController *m_controller;
86+
WeakObjCPtr<_WKRemoteWebInspectorViewController> m_controller;
8687
};
8788

8889
} // namespace WebKit

‎Source/WebKit/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ - (void)_setEditingDelegate:(id <WKWebProcessPlugInEditingDelegate>)editingDeleg
604604
public:
605605
explicit Client(WKWebProcessPlugInBrowserContextController *controller)
606606
: m_controller { controller }
607-
, m_delegateMethods { m_controller->_editingDelegate.get() }
607+
, m_delegateMethods { controller->_editingDelegate.get() }
608608
{
609609
}
610610

@@ -614,7 +614,8 @@ bool shouldInsertText(WebKit::WebPage&, const WTF::String& text, const std::opti
614614
if (!m_delegateMethods.shouldInsertText)
615615
return true;
616616

617-
return [m_controller->_editingDelegate.get() _webProcessPlugInBrowserContextController:m_controller shouldInsertText:text replacingRange:wrapper(*WebKit::createHandle(rangeToReplace)) givenAction:toWK(action)];
617+
RetainPtr controller = m_controller.get();
618+
return [controller->_editingDelegate.get() _webProcessPlugInBrowserContextController:controller.get() shouldInsertText:text replacingRange:wrapper(*WebKit::createHandle(rangeToReplace)) givenAction:toWK(action)];
618619
}
619620

620621
bool shouldChangeSelectedRange(WebKit::WebPage&, const std::optional<WebCore::SimpleRange>& fromRange, const std::optional<WebCore::SimpleRange>& toRange, WebCore::Affinity affinity, bool stillSelecting) final
@@ -636,31 +637,35 @@ bool shouldChangeSelectedRange(WebKit::WebPage&, const std::optional<WebCore::Si
636637
NSSelectionAffinity apiAffinity = affinity == WebCore::Affinity::Upstream ? NSSelectionAffinityUpstream : NSSelectionAffinityDownstream;
637638
#endif
638639

639-
return [m_controller->_editingDelegate.get() _webProcessPlugInBrowserContextController:m_controller shouldChangeSelectedRange:apiFromRange.get() toRange:apiToRange.get() affinity:apiAffinity stillSelecting:stillSelecting];
640+
RetainPtr controller = m_controller.get();
641+
return [controller->_editingDelegate.get() _webProcessPlugInBrowserContextController:controller.get() shouldChangeSelectedRange:apiFromRange.get() toRange:apiToRange.get() affinity:apiAffinity stillSelecting:stillSelecting];
640642
}
641643

642644
void didChange(WebKit::WebPage&, const String&) final
643645
{
644646
if (!m_delegateMethods.didChange)
645647
return;
646648

647-
[m_controller->_editingDelegate.get() _webProcessPlugInBrowserContextControllerDidChangeByEditing:m_controller];
649+
RetainPtr controller = m_controller.get();
650+
[controller->_editingDelegate.get() _webProcessPlugInBrowserContextControllerDidChangeByEditing:controller.get()];
648651
}
649652

650653
void willWriteToPasteboard(WebKit::WebPage&, const std::optional<WebCore::SimpleRange>& range) final
651654
{
652655
if (!m_delegateMethods.willWriteToPasteboard)
653656
return;
654657

655-
[m_controller->_editingDelegate.get() _webProcessPlugInBrowserContextController:m_controller willWriteRangeToPasteboard:wrapper(WebKit::createHandle(range).get())];
658+
RetainPtr controller = m_controller.get();
659+
[controller->_editingDelegate.get() _webProcessPlugInBrowserContextController:controller.get() willWriteRangeToPasteboard:wrapper(WebKit::createHandle(range).get())];
656660
}
657661

658662
void getPasteboardDataForRange(WebKit::WebPage&, const std::optional<WebCore::SimpleRange>& range, Vector<String>& pasteboardTypes, Vector<RefPtr<WebCore::SharedBuffer>>& pasteboardData) final
659663
{
660664
if (!m_delegateMethods.getPasteboardDataForRange)
661665
return;
662666

663-
auto dataByType = [m_controller->_editingDelegate.get() _webProcessPlugInBrowserContextController:m_controller pasteboardDataForRange:wrapper(WebKit::createHandle(range).get())];
667+
RetainPtr controller = m_controller.get();
668+
auto dataByType = [controller->_editingDelegate.get() _webProcessPlugInBrowserContextController:controller.get() pasteboardDataForRange:wrapper(WebKit::createHandle(range).get())];
664669
for (NSString *type in dataByType) {
665670
pasteboardTypes.append(type);
666671
pasteboardData.append(WebCore::SharedBuffer::create(dataByType[type]));
@@ -672,7 +677,8 @@ void didWriteToPasteboard(WebKit::WebPage&) final
672677
if (!m_delegateMethods.didWriteToPasteboard)
673678
return;
674679

675-
[m_controller->_editingDelegate.get() _webProcessPlugInBrowserContextControllerDidWriteToPasteboard:m_controller];
680+
RetainPtr controller = m_controller.get();
681+
[controller->_editingDelegate.get() _webProcessPlugInBrowserContextControllerDidWriteToPasteboard:controller.get()];
676682
}
677683

678684
bool performTwoStepDrop(WebKit::WebPage&, WebCore::DocumentFragment& fragment, const WebCore::SimpleRange& range, bool isMove) final
@@ -682,10 +688,11 @@ bool performTwoStepDrop(WebKit::WebPage&, WebCore::DocumentFragment& fragment, c
682688

683689
auto rangeHandle = WebKit::createHandle(range);
684690
auto nodeHandle = WebKit::InjectedBundleNodeHandle::getOrCreate(&fragment);
685-
return [m_controller->_editingDelegate.get() _webProcessPlugInBrowserContextController:m_controller performTwoStepDrop:wrapper(*nodeHandle) atDestination:wrapper(*rangeHandle) isMove:isMove];
691+
RetainPtr controller = m_controller.get();
692+
return [controller->_editingDelegate.get() _webProcessPlugInBrowserContextController:controller.get() performTwoStepDrop:wrapper(*nodeHandle) atDestination:wrapper(*rangeHandle) isMove:isMove];
686693
}
687694

688-
WKWebProcessPlugInBrowserContextController *m_controller;
695+
WeakObjCPtr<WKWebProcessPlugInBrowserContextController> m_controller;
689696
const struct DelegateMethods {
690697
DelegateMethods(RetainPtr<id <WKWebProcessPlugInEditingDelegate>> delegate)
691698
: shouldInsertText([delegate respondsToSelector:@selector(_webProcessPlugInBrowserContextController:shouldInsertText:replacingRange:givenAction:)])

‎Source/WebKit/WebProcess/InjectedBundle/InjectedBundle.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ RefPtr<InjectedBundle> InjectedBundle::create(WebProcessCreationParameters& para
101101

102102
InjectedBundle::InjectedBundle(const WebProcessCreationParameters& parameters)
103103
: m_path(parameters.injectedBundlePath)
104-
, m_platformBundle(0)
104+
, m_platformBundle(nullptr)
105105
, m_client(makeUnique<API::InjectedBundle::Client>())
106106
{
107107
}

‎Source/WebKit/WebProcess/InjectedBundle/InjectedBundle.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ class Connection;
6060
namespace WebKit {
6161

6262
#if USE(FOUNDATION)
63-
typedef NSBundle *PlatformBundle;
63+
typedef RetainPtr<NSBundle> PlatformBundle;
6464
#elif USE(GLIB)
6565
typedef ::GModule* PlatformBundle;
6666
#else

‎Source/WebKit/WebProcess/InjectedBundle/mac/InjectedBundleMac.mm‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,15 +118,15 @@ static NSEventModifierFlags currentModifierFlags(id self, SEL _cmd)
118118
}
119119
}
120120

121-
m_platformBundle = [[NSBundle alloc] initWithPath:m_path];
121+
m_platformBundle = adoptNS([[NSBundle alloc] initWithPath:m_path]);
122122
if (!m_platformBundle) {
123123
WTFLogAlways("InjectedBundle::load failed - Could not create the bundle.\n");
124124
return false;
125125
}
126126

127127
WKBundleAdditionalClassesForParameterCoderFunctionPtr additionalClassesForParameterCoderFunction = nullptr;
128128
WKBundleInitializeFunctionPtr initializeFunction = nullptr;
129-
if (NSString *executablePath = m_platformBundle.executablePath) {
129+
if (NSString *executablePath = [m_platformBundle executablePath]) {
130130
if (dlopen_preflight(executablePath.fileSystemRepresentation)) {
131131
// We don't hold onto this handle anywhere more permanent since we never dlclose.
132132
if (void* handle = dlopen(executablePath.fileSystemRepresentation, RTLD_LAZY | RTLD_GLOBAL | RTLD_FIRST)) {

0 commit comments

Comments
 (0)