Bug 188719 – Use unified sources to build all of the NetworkProcess code

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: New Bugs

[Tim Horton]

Use unified sources to build all of the NetworkProcess code

[Tim Horton]

1) I've only done a Mac open source release build, so this is not going to build everywhere yet.
2) The amount of WebCore:: is ... very high. I wonder if we really think this is OK.
3) I feel like we could use a clang plugin to do most of this.

[Tim Horton]

Created attachment 347438 [details]
Patch

[EWS Watchlist]

Attachment 347438 [details] did not pass style-queue:


ERROR: Source/WebKit/NetworkProcess/NetworkProcess.cpp:613:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 1 in 37 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Michael Catanzaro]

I have a smaller patch for this in bug #185011, but didn't try hard enough to find a reviewer.

See also: bug #185010. Let's use that bug to coordinate and avoid duplicate work.

(In reply to Tim Horton from comment #1)
> 2) The amount of WebCore:: is ... very high. I wonder if we really think
> this is OK.

Instead of removing 'using namespace WebCore' entirely, I would move it inside the WebKit namespace instead. That's what I did in bug #185011, and matches the approach Keith used for WebCore. It's a lot simpler, and it works, at least in NetworkProcess. I figure where it doesn't work, we can use @no-unify or find other solutions case-by-case.

If you want to try that approach, then I would start with my patch, add your XCode build system changes, your changes to SourcesCocoa.txt, and redo just the *.mm files.

[Michael Catanzaro]

BTW: I will be available to help with the CMake ports.

[Tim Horton]

(In reply to Michael Catanzaro from comment #4)
> I have a smaller patch for this in bug #185011, but didn't try hard enough
> to find a reviewer.
> 
> See also: bug #185010. Let's use that bug to coordinate and avoid duplicate
> work.
> 
> (In reply to Tim Horton from comment #1)
> > 2) The amount of WebCore:: is ... very high. I wonder if we really think
> > this is OK.
> 
> Instead of removing 'using namespace WebCore' entirely, I would move it
> inside the WebKit namespace instead. That's what I did in bug #185011, and
> matches the approach Keith used for WebCore. It's a lot simpler, and it
> works, at least in NetworkProcess. I figure where it doesn't work, we can
> use @no-unify or find other solutions case-by-case.

INTERESTING, I didn't know that worked.

> If you want to try that approach, then I would start with my patch, add your
> XCode build system changes, your changes to SourcesCocoa.txt, and redo just
> the *.mm files.

Sure, I'll try your way soon.

[Alex Christensen]

Created attachment 347454 [details]
Patch

[Alex Christensen]

If Michael's approach is cleaner, great.  I like this, so r=me.  I uploaded an attempt to fix ios.  Good night!

[Michael Catanzaro]

(In reply to Tim Horton from comment #6)
> INTERESTING, I didn't know that worked.

This is what Keith did for all of WebCore: I was just copying him. It "works" in that any name conflicts will now be confined to uses within the WebKit namespace. So it's not perfect, but it's good enough. Clashes can be dealt with on a case-by-case basis when adding new files.

[Alex Christensen]

Comment on attachment 347454 [details]
Patch

After seeing https://bugs.webkit.org/show_bug.cgi?id=185011 this looks like a mess.

[Tim Horton]

Yeah, the other plan is clearly better.

*** This bug has been marked as a duplicate of bug 185011 ***