[webkit-reviews] review granted: [Bug 106163] Remove WebProcessInitialization and NetworkProcessInitialization by putting the rest of initialization in ChildProcess derived classes : [Attachment 181423] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jan 4 20:19:20 PST 2013


Darin Adler <darin at apple.com> has granted Sam Weinig <sam at webkit.org>'s request
for review:
Bug 106163: Remove WebProcessInitialization and NetworkProcessInitialization by
putting the rest of initialization in ChildProcess derived classes
https://bugs.webkit.org/show_bug.cgi?id=106163

Attachment 181423: Patch
https://bugs.webkit.org/attachment.cgi?id=181423&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=181423&action=review


> Source/WebKit2/ChangeLog:16
> +	   (NetworkProcess):

You should remove bogus lines like this, or fix the script so it doesn’t add
them. I’m also partial to per-function comments.

> Source/WebKit2/ChangeLog:26
> +	   (WebKit):

Another.

> Source/WebKit2/ChangeLog:28
> +	   (PluginProcess):

Another.

> Source/WebKit2/ChangeLog:45
> +	   (ChildProcess):

Another.

> Source/WebKit2/ChangeLog:51
> +	   (WebKit):

Another.

> Source/WebKit2/NetworkProcess/mac/NetworkProcessMainMac.mm:84
> +    @autoreleasepool {

Why not put the [NSApplication sharedApplication] call inside this pool?

> Source/WebKit2/PluginProcess/mac/PluginProcessMac.mm:281
> +    // FIXME: It would be better to proxy set cursor calls over to the UI
process instead of

Typo: "set cursor calls" should probably be something else, like maybe
"SetCursor calls". I know you just moved this, though.


More information about the webkit-reviews mailing list