[webkit-reviews] review granted: [Bug 45217] Indicate which one of the ScriptWorlds for a Frame the Window Object has been cleared for. : [Attachment 66726] didClearWindowObjectForFrame -> didClearWindowObjectForFrameInScriptWorld (Take 2 - Style Errors Fixed)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Sep 7 09:08:40 PDT 2010


Darin Adler <darin at apple.com> has granted Jessie Berlin <jberlin at webkit.org>'s
request for review:
Bug 45217: Indicate which one of the ScriptWorlds for a Frame the Window Object
has been cleared for.
https://bugs.webkit.org/show_bug.cgi?id=45217

Attachment 66726: didClearWindowObjectForFrame ->
didClearWindowObjectForFrameInScriptWorld (Take 2 - Style Errors Fixed)
https://bugs.webkit.org/attachment.cgi?id=66726&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
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


More information about the webkit-reviews mailing list