Bug 169113 - Add symbols required by TAPI in a header file
Summary: Add symbols required by TAPI in a header file
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-02 18:03 PST by Aakash Jain
Modified: 2017-03-04 13:27 PST (History)
7 users (show)

See Also:


Attachments
Proposed patch (11.09 KB, patch)
2017-03-02 18:09 PST, Aakash Jain
ap: review-
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (45.22 KB, application/zip)
2017-03-02 19:00 PST, Build Bot
no flags Details
Archive of layout-test-results from ews124 for ios-simulator-wk2 (198.62 KB, application/zip)
2017-03-02 19:23 PST, Build Bot
no flags Details
Updated patch (6.67 KB, patch)
2017-03-03 16:53 PST, Aakash Jain
ap: review+
ap: commit-queue-
Details | Formatted Diff | Diff
Updated patch (4.84 KB, patch)
2017-03-03 17:13 PST, Aakash Jain
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 2017-03-02 18:03:31 PST
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.
Comment 1 Aakash Jain 2017-03-02 18:09:11 PST
Created attachment 303278 [details]
Proposed patch
Comment 2 WebKit Commit Bot 2017-03-02 18:11:44 PST
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.
Comment 3 Build Bot 2017-03-02 18:59:57 PST
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.
Comment 4 Build Bot 2017-03-02 19:00:01 PST
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
Comment 5 Build Bot 2017-03-02 19:23:24 PST
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.
Comment 6 Build Bot 2017-03-02 19:23:28 PST
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
Comment 7 Alexey Proskuryakov 2017-03-03 15:06:46 PST
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
Comment 8 Aakash Jain 2017-03-03 16:53:19 PST
Created attachment 303357 [details]
Updated patch

Updated patch with review comments incorporated.
Comment 9 Alexey Proskuryakov 2017-03-03 16:57:32 PST
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?
Comment 10 Alexey Proskuryakov 2017-03-03 16:58:13 PST
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.
Comment 11 Aakash Jain 2017-03-03 17:13:19 PST
Created attachment 303364 [details]
Updated patch
Comment 12 WebKit Commit Bot 2017-03-04 11:53:04 PST
Comment on attachment 303364 [details]
Updated patch

Clearing flags on attachment: 303364

Committed r213427: <http://trac.webkit.org/changeset/213427>
Comment 13 WebKit Commit Bot 2017-03-04 11:53:10 PST
All reviewed patches have been landed.  Closing bug.