Bug 169113 – Add symbols required by TAPI in a header file

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Aakash Jain]

Various symbols in WebKit2 (e.g.: DatabaseServiceInitializer, NetworkServiceInitializer etc.) are present in .mm files, but they are not present in header files. In order to match our headers precisely with the symbols in binary, we should move the declaration of these methods to header file.

[Aakash Jain]

Created attachment 303278 [details]
Proposed patch

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/Platform/ExraPrivateSymbolsForTAPI.h:33:  The parameter name "connection" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit2/Platform/ExraPrivateSymbolsForTAPI.h:35:  The parameter name "connection" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit2/Platform/ExraPrivateSymbolsForTAPI.h:37:  The parameter name "connection" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit2/Platform/ExraPrivateSymbolsForTAPI.h:39:  The parameter name "connection" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 4 in 7 files


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

[Build Bot]

Comment on attachment 303278 [details]
Proposed patch

Attachment 303278 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3226654

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 303290 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 303278 [details]
Proposed patch

Attachment 303278 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3226697

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 303291 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Alexey Proskuryakov]

Comment on attachment 303278 [details]
Proposed patch

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

The EWS failures are real.

> Source/WebKit2/ChangeLog:11
> +        * WebKit2.xcodeproj/project.pbxproj: Added ExraPrivateSymbolsForTAPI.h
> +        * DatabaseProcess/EntryPoint/mac/XPCService/DatabaseServiceEntryPoint.mm: Moved declaration of method
> +        to ExraPrivateSymbolsForTAPI.h

Typo: should be "Extra".

> Source/WebKit2/Platform/ExraPrivateSymbolsForTAPI.h:30
> +#include <wtf/spi/darwin/XPCSPI.h>

Won't all the functions here from XPCSPI.h exported too?

I think that it may be better to try a hack. Instead of creating a proper header, you could just make a fake one, just for TAPI. It does not need to have correct prototypes, this would work:

void DatabaseServiceInitializer();
...

> Source/WebKit2/Platform/ExraPrivateSymbolsForTAPI.h:33
> +WK_EXPORT void DatabaseServiceInitializer(xpc_connection_t connection, xpc_object_t initializerMessage, xpc_object_t priorityBoostMessage);

These need extern "C" or equivalent. Looks like WK2 API headers use this:

#ifdef __cplusplus
extern "C" {
#endif

...


#ifdef __cplusplus
}
#endif

[Aakash Jain]

Created attachment 303357 [details]
Updated patch

Updated patch with review comments incorporated.

[Alexey Proskuryakov]

Comment on attachment 303357 [details]
Updated patch

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

> Source/WebKit2/Platform/ExtraPrivateSymbolsForTAPI.h:41
> +WK_EXPORT void DatabaseServiceInitializer();
> +
> +WK_EXPORT void NetworkServiceInitializer();
> +
> +WK_EXPORT void PluginServiceInitializer();
> +
> +WK_EXPORT void WebContentServiceInitializer();

I don't think that we want WK_EXPORT here (and then WKDeclarationSpecifiers.h is not needed).

Personally, I wouldn't add blank lines between these lines.

> Source/WebKit2/WebKit2.xcodeproj/project.pbxproj:7105
> +				ECA680D31E6904B500731D20 /* ExtraPrivateSymbolsForTAPI.h */,

Did this end up being in any target?

[Alexey Proskuryakov]

Comment on attachment 303357 [details]
Updated patch

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

> Source/WebKit2/WebKit2.xcodeproj/project.pbxproj:1961
> +		ECA680D41E6904B500731D20 /* ExtraPrivateSymbolsForTAPI.h in Headers */ = {isa = PBXBuildFile; fileRef = ECA680D31E6904B500731D20 /* ExtraPrivateSymbolsForTAPI.h */; settings = {ATTRIBUTES = (Private, ); }; };

We definitely don't need the header to be private, it is to be added via a flag for TAPI only.

[Aakash Jain]

Created attachment 303364 [details]
Updated patch

[WebKit Commit Bot]

Comment on attachment 303364 [details]
Updated patch

Clearing flags on attachment: 303364

Committed r213427: <http://trac.webkit.org/changeset/213427>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.