WKBundlePageLoaderClient's WKBundlePageDidClearWindowObjectForFrameCallback should be a WKBundlePageDidClearWindowObjectForFrameInScriptWorldCallback, since there may be multiple ScriptWorlds for one Frame.
Created attachment 66723 [details] didClearWindowObjectForFrame -> didClearWindowObjectForFrameInScriptWorld
Attachment 66723 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.h:81: Extra space between WKBundlePageDidClearWindowObjectForFrameInScriptWorldCallback and didClearWindowObjectForFrameInScriptWorld [whitespace/declaration] [3] WebKit2/WebProcess/InjectedBundle/InjectedBundlePageLoaderClient.h:34: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 2 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 66726 [details] didClearWindowObjectForFrame -> didClearWindowObjectForFrameInScriptWorld (Take 2 - Style Errors Fixed) Fixed style errors.
Comment on attachment 66726 [details] didClearWindowObjectForFrame -> didClearWindowObjectForFrameInScriptWorld (Take 2 - Style Errors Fixed) I don’t think you need to add “InScriptWord” to the name of the function. That’s the normal naming for Objective-C, but here in C++ I think it is not much clearer and makes the function name longer than necessary. I suppose it’s nice to clarify that this has to be called separately for each world, but I also think the arguments makes that clear. It does seem unfortunate that we will have to create a script world just to pass to the caller. > - WKBundlePageDidClearWindowObjectForFrameCallback didClearWindowObjectForFrame; > + WKBundlePageDidClearWindowObjectForFrameInScriptWorldCallback didClearWindowObjectForFrameInScriptWorld; It looks like this is no longer aligned. This is why I normally request that we not line things up like this. Worth talking to Sam about eliminating the vertical alignment, but for now maybe you should add spaces to keep things lined up. r=me
(In reply to comment #4) > (From update of attachment 66726 [details]) > I don’t think you need to add “InScriptWord” to the name of the function. That’s the normal naming for Objective-C, but here in C++ I think it is not much clearer and makes the function name longer than necessary. I suppose it’s nice to clarify that this has to be called separately for each world, but I also think the arguments makes that clear. > I originally added "InScriptWorld" to match IWebFrameLoadDelegatePrivate2's didClearWindowObjectForFrameInScriptWorld. However, your argument makes more sense, so I just removed the extra "InScriptWorld"s. > It does seem unfortunate that we will have to create a script world just to pass to the caller. > I am not sure how else we would go about getting the script world. > > - WKBundlePageDidClearWindowObjectForFrameCallback didClearWindowObjectForFrame; > > + WKBundlePageDidClearWindowObjectForFrameInScriptWorldCallback didClearWindowObjectForFrameInScriptWorld; > > It looks like this is no longer aligned. This is why I normally request that we not line things up like this. Worth talking to Sam about eliminating the vertical alignment, but for now maybe you should add spaces to keep things lined up. > In my original patch it was aligned, but then the check-webkit-style script issued a warning about too many spaces. I will align it again. > r=me Thanks!
Comment on attachment 66726 [details] didClearWindowObjectForFrame -> didClearWindowObjectForFrameInScriptWorld (Take 2 - Style Errors Fixed) Committed in r66894: http://trac.webkit.org/changeset/66894