Bug 154317 - Modern IDB: More WK2 IPC Scaffolding
Summary: Modern IDB: More WK2 IPC Scaffolding
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Safari 9
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks: 149117 153808
  Show dependency treegraph
 
Reported: 2016-02-16 16:22 PST by Brady Eidson
Modified: 2016-02-17 12:22 PST (History)
5 users (show)

See Also:


Attachments
Patch v1 (32.98 KB, patch)
2016-02-16 16:25 PST, Brady Eidson
achristensen: review+
Details | Formatted Diff | Diff
Patch for landing (33.56 KB, patch)
2016-02-17 08:36 PST, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch for landing v2 (33.68 KB, patch)
2016-02-17 08:56 PST, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch for landing v3 (33.76 KB, patch)
2016-02-17 09:07 PST, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch for landing v4 (33.82 KB, patch)
2016-02-17 09:18 PST, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch for landing v5 (44.88 KB, patch)
2016-02-17 10:06 PST, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 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.
Comment 1 Brady Eidson 2016-02-16 16:25:01 PST
Created attachment 271506 [details]
Patch v1
Comment 2 Brady Eidson 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.
Comment 3 Michael Catanzaro 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.
Comment 4 Michael Catanzaro 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....
Comment 5 Martin Robinson 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.
Comment 6 Carlos Garcia Campos 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.
Comment 7 Brady Eidson 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!
Comment 8 Brady Eidson 2016-02-17 08:36:25 PST
Created attachment 271554 [details]
Patch for landing
Comment 9 Brady Eidson 2016-02-17 08:56:26 PST
Created attachment 271557 [details]
Patch for landing v2
Comment 10 Brady Eidson 2016-02-17 09:07:30 PST
Created attachment 271560 [details]
Patch for landing v3
Comment 11 Brady Eidson 2016-02-17 09:18:22 PST
Created attachment 271561 [details]
Patch for landing v4
Comment 12 Brady Eidson 2016-02-17 09:36:44 PST
This is never ending...
Comment 13 Brady Eidson 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...
Comment 14 Brady Eidson 2016-02-17 10:06:41 PST
Created attachment 271564 [details]
Patch for landing v5
Comment 15 WebKit Commit Bot 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>