Bug 185011 – Use unified build for NetworkProcess

• Component: WebKit2
• Status: RESOLVED FIXED

[Michael Catanzaro]

Let's take this piece-by-piece.

Keith, I'm about to attach a patch that switches CMake to use unified build, and moves around using statements in the associated source files, and #undefs macros where needed.

Would you be interested in making the corresponding changes to the XCode build?

[Michael Catanzaro]

(Note I'm only touching cross-platform files here.)

[Michael Catanzaro]

Created attachment 338830 [details]
Patch

[Michael Catanzaro]

Anyone interested in helping with the XCode portion of this?

[Michael Catanzaro]

Ping reviewers. Let's land CMake first for now. Apple can follow up with XCode later.

[Michael Catanzaro]

Ping reviewers

[Michael Catanzaro]

Comment on attachment 338830 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=338830&action=review

> Source/WebKit/NetworkProcess/Cookies/WebCookieManager.cpp:43
> -using namespace WebCore;
> -
>  namespace WebKit {
> +using namespace WebCore;

There is a question of code style: should we leave a blank line after namespace WebKit { and before using namespace WebCore;?

My inclination would have been to do so, to match the previous code style. But Keith consistently avoided the blank line when moving WebCore and the other projects to unified sources, and that is a much larger amount of code, so that's why I chose to avoid it here. There will be inconsistency either way.

[Alex Christensen]

Comment on attachment 338830 [details]
Patch

I think this is fine.  If someone is bothered by the inconsistent spacing they can establish a style guideline and fix it.

[Tim Horton]

*** Bug 188719 has been marked as a duplicate of this bug. ***

[Michael Catanzaro]

This will surely break Cocoa ports if committed now; it needs help from Tim or someone with XCode. The EWS are green because Cocoa ports didn't use Sources.txt at the time I uploaded the patch.

Basically I think each of these unified source patches is going to require an Apple developer, since the required XCode changes are too hard to do blindly. But CMake ports should no longer require any changes after bug #188732 lands.

[Tim Horton]

Yeah, I’m planning on taking a stab at this shortly.

[Michael Catanzaro]

Please try bug #188732 first, because that bug will get harder to handle otherwise.

[Tim Horton]

(In reply to Michael Catanzaro from comment #11)
> Please try bug #188732 first, because that bug will get harder to handle
> otherwise.

Good point. OK, will go over there.

[Tim Horton]

Created attachment 347560 [details]
Patch

[Michael Catanzaro]

Comment on attachment 347560 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=347560&action=review

> Source/WebKit/ChangeLog:1
> [phone]  Michael Catanzaro  <[e-mail]>

If you like, you can add yourself here when we have shared patches: 2018-08-20  Michael Catanzaro  <[e-mail]> and Timothy Horton <[e-mail]>

It doesn't show up perfectly in trac, but it works.

Anyway, doesn't matter.

[Tim Horton]

(In reply to Michael Catanzaro from comment #14)
> Comment on attachment 347560 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=347560&action=review
> 
> > Source/WebKit/ChangeLog:1
> > [phone]  Michael Catanzaro  <[e-mail]>
> 
> If you like, you can add yourself here when we have shared patches:
> 2018-08-20  Michael Catanzaro  <[e-mail]> and Timothy Horton
> <[e-mail]>
> 
> It doesn't show up perfectly in trac, but it works.
> 
> Anyway, doesn't matter.

You're doing the hard work, and I don't need to boost my patch count :P

[WebKit Commit Bot]

Comment on attachment 347560 [details]
Patch

Clearing flags on attachment: 347560

Committed r235101: <https://trac.webkit.org/changeset/235101>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/43537028>