| Summary: | Use unified sources to build all of the NetworkProcess code | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||
| Component: | New Bugs | Assignee: | Tim Horton <thorton> | ||||||
| Status: | RESOLVED DUPLICATE | ||||||||
| Severity: | Normal | CC: | achristensen, cdumez, cgarcia, dbates, eric.carlson, ews-watchlist, ggaren, keith_miller, mcatanzaro, simon.fraser | ||||||
| Priority: | P2 | ||||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Tim Horton
2018-08-18 00:39:03 PDT
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. Created attachment 347438 [details]
Patch
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.
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. BTW: I will be available to help with the CMake ports. (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. Created attachment 347454 [details]
Patch
If Michael's approach is cleaner, great. I like this, so r=me. I uploaded an attempt to fix ios. Good night! (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. Comment on attachment 347454 [details] Patch After seeing https://bugs.webkit.org/show_bug.cgi?id=185011 this looks like a mess. Yeah, the other plan is clearly better. *** This bug has been marked as a duplicate of bug 185011 *** |