[WebKit2] Implement ScriptMessageHandlers
Created attachment 231659 [details] Patch
Attachment 231659 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/UserContent/WebScriptMessageHandler.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/page/WebKitNamespace.h:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/page/UserContentController.h:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/page/UserMessageHandlersNamespace.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/page/UserMessageHandler.cpp:60: Could not find a newline character at the end of the file. [whitespace/ending_newline] [5] ERROR: Source/WebKit2/UIProcess/UserContent/WebUserContentControllerProxy.cpp:32: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebKit2/WebProcess/UserContent/WebUserContentController.h:34: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/bindings/js/JSDOMWindowBase.cpp:42: Bad include order. Mixing system and custom headers. [build/include_order] [4] Total errors found: 8 in 40 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 231663 [details] Patch
Comment on attachment 231663 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231663&action=review > Source/WebCore/WebCore.exp.in:2210 > __ZN7WebCore20applicationIsHRBlockEv > __ZN7WebCore20builtInPDFPluginNameEv > __ZN7WebCore21DeviceOrientationData6createEbdbdbdbb > +__ZN7WebCore21UserContentController13addUserScriptERNS_15DOMWrapperWorldENSt3__110unique_ptrINS_10UserScriptENS3_14default_deleteIS5_EEEE > +__ZN7WebCore21UserContentController17removeUserScriptsERNS_15DOMWrapperWorldE > +__ZN7WebCore21UserContentController31addUserMessageHandlerDescriptorEPNS_28UserMessageHandlerDescriptorE > +__ZN7WebCore21UserContentController34removeUserMessageHandlerDescriptorEPNS_28UserMessageHandlerDescriptorE > +__ZN7WebCore21UserContentController6createEv > +__ZN7WebCore21UserContentControllerD1Ev > __ZN7WebCore21applicationIsApertureEv > __ZN7WebCore21applicationIsVersionsEv > __ZN7WebCore21reportThreadViolationEPKcNS_20ThreadViolationRoundE Please remove this. > Source/WebCore/page/DOMWindow.cpp:750 > + return 0; nullptr. > Source/WebCore/page/UserContentController.h:74 > + void addUserMessageHandlerDescriptor(UserMessageHandlerDescriptor*); > + void removeUserMessageHandlerDescriptor(UserMessageHandlerDescriptor*); These should be references. > Source/WebCore/page/UserMessageHandler.cpp:49 > + m_descriptor->client().didPostMessage(*this, value.get()); You can just pass prpValue.get() directly here and get rid of the RefPtr variable. (And rename prpValue to value). > Source/WebCore/page/UserMessageHandler.h:39 > + static PassRef<UserMessageHandler> create(Frame* frame, UserMessageHandlerDescriptor& descriptor) Frame&. > Source/WebCore/page/UserMessageHandler.h:51 > + explicit UserMessageHandler(Frame*, UserMessageHandlerDescriptor&); This doesn't have to be explicit. > Source/WebCore/page/UserMessageHandlersNamespace.h:47 > + static PassRef<UserMessageHandlersNamespace> create(Frame* frame) Frame&. > Source/WebCore/page/UserMessageHandlersNamespace.h:57 > + explicit UserMessageHandlersNamespace(Frame*); Frame&. > Source/WebCore/page/WebKitNamespace.h:43 > + static PassRefPtr<WebKitNamespace> create(Frame* frame) Frame&. > Source/WebCore/page/WebKitNamespace.h:53 > + explicit WebKitNamespace(Frame*); Frame&. > Source/WebKit2/UIProcess/API/Cocoa/WKScriptMessage.mm:46 > + _body = body; I think you want to copy the body as well. > Source/WebKit2/UIProcess/API/Cocoa/WKUserContentController.mm:127 > + [NSException raise:NSInvalidArgumentException format:@"Attempt to add ScriptMessageHandler with name '%@' when one already exists.", name]; No need to capitalize ScriptMessageHandler, can just be "script message handler". > Source/WebKit2/UIProcess/UserContent/WebUserContentControllerProxy.cpp:133 > + WebProcessProxy* webProcess = WebProcessProxy::fromConnection(connection); Wonky indentation. > Source/WebKit2/WebKit2.xcodeproj/project.pbxproj:2985 > + 7C361D76192803BD0036A59D /* WebUserContentControllerProxyMessageReceiver.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = WebUserContentControllerProxyMessageReceiver.cpp; path = ../../../../../../../../../../../Builds/Debug/DerivedSources/WebKit2/WebUserContentControllerProxyMessageReceiver.cpp; sourceTree = "<group>"; }; > + 7C361D77192803BD0036A59D /* WebUserContentControllerProxyMessages.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = WebUserContentControllerProxyMessages.h; path = ../../../../../../../../../../../Builds/Debug/DerivedSources/WebKit2/WebUserContentControllerProxyMessages.h; sourceTree = "<group>"; }; These have the wrong path. > Source/WebKit2/WebProcess/UserContent/WebUserContentController.cpp:127 > + webPage->pageID(), > + webFrame->frameID(), > + m_identifier, > + IPC::DataReference(value->data()) > + ), m_controller->identifier()); This is not how we indent things in WebKit!.
Comment on attachment 231663 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231663&action=review > Source/WebKit2/WebProcess/UserContent/WebUserContentController.h:62 > + HashMap<uint64_t, RefPtr<WebUserMessageHandlerDescriptorProxy>> m_userMessageHandlerDescriptors; It will break the !ENABLE(USER_MESSAGE_HANDLERS) build, because this class is inside this guard, we can't use it outside.
I didn't want to remove the r+ intentionally, but my comment conflicted with the r+.
Created attachment 231669 [details] Patch
Comment on attachment 231669 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231669&action=review > Source/WebKit2/UIProcess/API/Cocoa/WKUserContentController.mm:110 > + id body = [value toDictionary]; This doesn't handle every object. At the very least we should handle arrays.
Committed r169023: <http://trac.webkit.org/changeset/169023>
(In reply to comment #9) > Committed r169023: <http://trac.webkit.org/changeset/169023> Mac rebaselines landed in https://trac.webkit.org/changeset/169044 to try to unbreak red buildbots and dead EWS bots. Could you possibly verify if the new results are correct or not? (if you don't have time to wait for EWS ...)