Bug 154317

Summary: Modern IDB: More WK2 IPC Scaffolding
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebCore Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, cgarcia, commit-queue, mcatanzaro, mrobinson
Priority: P2    
Version: Safari 9   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 149117, 153808    
Attachments:
Description Flags
Patch v1
achristensen: review+
Patch for landing
none
Patch for landing v2
none
Patch for landing v3
none
Patch for landing v4
none
Patch for landing v5 none

Brady Eidson
Reported 2016-02-16 16:22:29 PST
Modern IDB: More WK2 IPC Scaffolding This includes new messages.in files, all the messages that will be needed, and declarations for all the encoders/decoders, etc. Very little of it is actually implemented for now.
Attachments
Patch v1 (32.98 KB, patch)
2016-02-16 16:25 PST, Brady Eidson 		achristensen: review+
DetailsFormatted DiffDiff
Patch for landing (33.56 KB, patch)
2016-02-17 08:36 PST, Brady Eidson 		no flags
DetailsFormatted DiffDiff
Patch for landing v2 (33.68 KB, patch)
2016-02-17 08:56 PST, Brady Eidson 		no flags
DetailsFormatted DiffDiff
Patch for landing v3 (33.76 KB, patch)
2016-02-17 09:07 PST, Brady Eidson 		no flags
DetailsFormatted DiffDiff
Patch for landing v4 (33.82 KB, patch)
2016-02-17 09:18 PST, Brady Eidson 		no flags
DetailsFormatted DiffDiff
Patch for landing v5 (44.88 KB, patch)
2016-02-17 10:06 PST, Brady Eidson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Brady Eidson
Comment 1 2016-02-16 16:25:01 PST
Created attachment 271506 [details] Patch v1
Brady Eidson
Comment 2 2016-02-16 17:18:40 PST
Seems like the Linux CMake builds *should* be finding the headers that are being complained about... not sure why they aren't.
Michael Catanzaro
Comment 3 2016-02-16 18:01:25 PST
Hm, if I replace #include <WebCore/IDBCursorInfo.h> with #include "IDBCursorInfo.h" in the DerivedSources file, it makes it to the next #include... investigating.
Michael Catanzaro
Comment 4 2016-02-16 18:27:57 PST
I'm not making any progress on this tonight, sorry; will try again tomorrow if someone doesn't figure it out first. I have not managed to figure out how it is the #include <WebCore/foo.h> syntax works at all for any foo.h that does not live in a directory named WebCore in some include path. I've grown accustomed to this style of #includes, but never before stopped to think about how it works... seems like magic now....
Martin Robinson
Comment 5 2016-02-16 18:54:16 PST
(In reply to comment #4) > I'm not making any progress on this tonight, sorry; will try again tomorrow > if someone doesn't figure it out first. > > I have not managed to figure out how it is the #include <WebCore/foo.h> > syntax works at all for any foo.h that does not live in a directory named > WebCore in some include path. I've grown accustomed to this style of > #includes, but never before stopped to think about how it works... seems > like magic now.... These forwarding headers used to be generated by a CMake macro called WEBKIT_CREATE_FORWARDING_HEADERS into the WebKitBuild/Release/DerivedSources/ForwardingHeaders/ directory. Each forwarding header is composed of a single #include of the path to the real file. Now I don't see that called anywhere for WebKitGTK+, unless I'm missing it.
Carlos Garcia Campos
Comment 6 2016-02-16 23:12:13 PST
Forwarding headers are created by the generate-forwarding-headers.pl that scans the given directory looking for headers referenced that need a forwarding header. The problem here is that the header that requires the forwarding header is only referenced by generated code, and the script only scans Source/WebKit2. In all other cases it works because there's code in Source/WebKit that also has the same header, so when the derived sources are compiled, the forwarding header has already been created. So, to fix this, we should explicitly include <WebCore/IDBCursorInfo.h> in WebIDBConnectionToServer.cpp instead of relying on it being included by the messages header. Another solution would be to add another target in the makefile to run generate-forwarding-headers.pl right after the ipc code is generated by before it's compiled to create forwarding headers for the WebKit2 derived sources dir.
Brady Eidson
Comment 7 2016-02-17 08:34:01 PST
(In reply to comment #6) > Forwarding headers are created by the generate-forwarding-headers.pl that > scans the given directory looking for headers referenced that need a > forwarding header. The problem here is that the header that requires the > forwarding header is only referenced by generated code, and the script only > scans Source/WebKit2. In all other cases it works because there's code in > Source/WebKit that also has the same header, so when the derived sources are > compiled, the forwarding header has already been created. So, to fix this, > we should explicitly include <WebCore/IDBCursorInfo.h> in > WebIDBConnectionToServer.cpp instead of relying on it being included by the > messages header. Another solution would be to add another target in the > makefile to run generate-forwarding-headers.pl right after the ipc code is > generated by before it's compiled to create forwarding headers for the > WebKit2 derived sources dir. I'll include it for now - It *will* have to be included eventually, anyways. Thanks for figuring this out!
Brady Eidson
Comment 8 2016-02-17 08:36:25 PST
Created attachment 271554 [details] Patch for landing
Brady Eidson
Comment 9 2016-02-17 08:56:26 PST
Created attachment 271557 [details] Patch for landing v2
Brady Eidson
Comment 10 2016-02-17 09:07:30 PST
Created attachment 271560 [details] Patch for landing v3
Brady Eidson
Comment 11 2016-02-17 09:18:22 PST
Created attachment 271561 [details] Patch for landing v4
Brady Eidson
Comment 12 2016-02-17 09:36:44 PST
This is never ending...
Brady Eidson
Comment 13 2016-02-17 09:38:17 PST
Well it appears that the optimizations that clang does to realize methods aren't needed don't exist here. More implementation here we come...
Brady Eidson
Comment 14 2016-02-17 10:06:41 PST
Created attachment 271564 [details] Patch for landing v5
WebKit Commit Bot
Comment 15 2016-02-17 11:21:57 PST
Comment on attachment 271564 [details] Patch for landing v5 Clearing flags on attachment: 271564 Committed r196705: <http://trac.webkit.org/changeset/196705>
Note You need to log in before you can comment on or make changes to this bug.