WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
NEW
193161
Remove or deprecate unused C SPI
https://bugs.webkit.org/show_bug.cgi?id=193161
Summary
Remove or deprecate unused C SPI
Alex Christensen
Reported
2019-01-04 15:01:17 PST
Remove or deprecate unused C SPI
Attachments
Patch
(53.91 KB, patch)
2019-01-04 15:03 PST
,
Alex Christensen
no flags
Details
Formatted Diff
Diff
Patch
(50.10 KB, patch)
2019-01-08 20:45 PST
,
Alex Christensen
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Alex Christensen
Comment 1
2019-01-04 15:03:49 PST
Created
attachment 358379
[details]
Patch
Alex Christensen
Comment 2
2019-01-04 17:06:16 PST
Note: IconDB code use was removed in
rdar://problem/47057658
Alex Christensen
Comment 3
2019-01-08 10:14:06 PST
Could someone fix the gtk build with this patch?
Michael Catanzaro
Comment 4
2019-01-08 11:42:18 PST
In WKPage.cpp, change: namespace API { using namespace WebCore; using namespace WebKit; to: using namespace WebCore; using namespace WebKit; namespace API { That fixes it for me.
Alex Christensen
Comment 5
2019-01-08 11:44:12 PST
We don't want to do that. This means we need to add WebKit::, WebCore::, or using namespace ...; inside a scope somewhere. Could you upload a patch that does that?
Michael Catanzaro
Comment 6
2019-01-08 11:45:49 PST
Looks like this will break TestController: [227/431] Building CXX object Tools/WebKitTestRunne...MakeFiles/WebKitTestRunner.dir/TestController.cpp.o ../../Tools/WebKitTestRunner/TestController.cpp: In destructor ‘WTR::TestController::~TestController()’: ../../Tools/WebKitTestRunner/TestController.cpp:161:69: warning: ‘const OpaqueWKIconDatabase* WKContextGetIconDatabase(WKContextRef)’ is deprecated: No longer supported [-Wdeprecated-declarations] WKIconDatabaseClose(WKContextGetIconDatabase(m_context.get())); ^ In file included from DerivedSources/ForwardingHeaders/WebKit/WKContext.h:1, from ../../Source/WebKit/UIProcess/API/C/WebKit2_C.h:36, from DerivedSources/ForwardingHeaders/WebKit/WebKit2_C.h:1, from ../../Tools/WebKitTestRunner/config.h:31, from ../../Tools/WebKitTestRunner/TestController.cpp:26: ../../Source/WebKit/UIProcess/API/C/WKContext.h:141:29: note: declared here WK_EXPORT WKIconDatabaseRef WKContextGetIconDatabase(WKContextRef context) WK_C_API_DEPRECATED; ^~~~~~~~~~~~~~~~~~~~~~~~ ../../Tools/WebKitTestRunner/TestController.cpp:161:69: warning: ‘const OpaqueWKIconDatabase* WKContextGetIconDatabase(WKContextRef)’ is deprecated: No longer supported [-Wdeprecated-declarations] WKIconDatabaseClose(WKContextGetIconDatabase(m_context.get())); ^ In file included from DerivedSources/ForwardingHeaders/WebKit/WKContext.h:1, from ../../Source/WebKit/UIProcess/API/C/WebKit2_C.h:36, from DerivedSources/ForwardingHeaders/WebKit/WebKit2_C.h:1, from ../../Tools/WebKitTestRunner/config.h:31, from ../../Tools/WebKitTestRunner/TestController.cpp:26: ../../Source/WebKit/UIProcess/API/C/WKContext.h:141:29: note: declared here WK_EXPORT WKIconDatabaseRef WKContextGetIconDatabase(WKContextRef context) WK_C_API_DEPRECATED; ^~~~~~~~~~~~~~~~~~~~~~~~ ../../Tools/WebKitTestRunner/TestController.cpp:161:70: warning: ‘void WKIconDatabaseClose(WKIconDatabaseRef)’ is deprecated: No longer supported [-Wdeprecated-declarations] WKIconDatabaseClose(WKContextGetIconDatabase(m_context.get())); ^ In file included from DerivedSources/ForwardingHeaders/WebKit/WKIconDatabase.h:1, from ../../Tools/WebKitTestRunner/TestController.cpp:45: ../../Source/WebKit/UIProcess/API/C/WKIconDatabase.h:81:16: note: declared here WK_EXPORT void WKIconDatabaseClose(WKIconDatabaseRef iconDatabase) WK_C_API_DEPRECATED; ^~~~~~~~~~~~~~~~~~~ ../../Tools/WebKitTestRunner/TestController.cpp:161:70: warning: ‘void WKIconDatabaseClose(WKIconDatabaseRef)’ is deprecated: No longer supported [-Wdeprecated-declarations] WKIconDatabaseClose(WKContextGetIconDatabase(m_context.get())); ^ In file included from DerivedSources/ForwardingHeaders/WebKit/WKIconDatabase.h:1, from ../../Tools/WebKitTestRunner/TestController.cpp:45: ../../Source/WebKit/UIProcess/API/C/WKIconDatabase.h:81:16: note: declared here WK_EXPORT void WKIconDatabaseClose(WKIconDatabaseRef iconDatabase) WK_C_API_DEPRECATED; ^~~~~~~~~~~~~~~~~~~
Michael Catanzaro
Comment 7
2019-01-08 11:46:37 PST
(In reply to Alex Christensen from
comment #5
)
> We don't want to do that. This means we need to add WebKit::, WebCore::, or > using namespace ...; inside a scope somewhere. Could you upload a patch > that does that?
Hm, sorry I got confused. Usual C++ rule is to avoid using statements in global scope of header files, not source files, but of course we don't want to do that for unified builds. I'll try to make it work.
Michael Catanzaro
Comment 8
2019-01-08 11:59:21 PST
I think you should just pull all the C API files out of unified build using @no-unify (like we do for WPE/GTK API files). When they were added to the unified build, the using statements weren't removed from the global namespace like we did for normal sources (probably because this is quite hard to do for a C API file where all the API functions have to be in the global namespace!). It's not just a problem with WKPage.cpp -- it's coincidence that's the one we're fighting now -- but with all the files in this directory. With some quick search/replaces I've added the WebKit:: namespace in several hundred places where it's missing, but that just gets me error messages exceeding my terminal scrollback, so I don't anticipate success this way.
Alex Christensen
Comment 9
2019-01-08 20:45:53 PST
Created
attachment 358666
[details]
Patch
Alex Christensen
Comment 10
2019-01-09 11:43:53 PST
Michael, that didn't fix the Linux build. I could fix the Mac build with that, but it looks like we don't want to do that.
Michael Catanzaro
Comment 11
2019-01-10 20:12:11 PST
The problem is some other file has these using statements in the global scope, and that file is included in the unified source bundle on all platforms except Linux. Just un-unifying the file is not enough. Doing that alone will only surface the problem on all platforms (as we see on EWS). It's still not going to build unless you also move the using statements: (In reply to Michael Catanzaro from
comment #4
)
> In WKPage.cpp, change: > > namespace API { > > using namespace WebCore; > using namespace WebKit; > > to: > > using namespace WebCore; > using namespace WebKit; > > namespace API { > > That fixes it for me.
Removing the file from the unified build just guarantees you can move the using statements without mucking up unrelated files. Alternatively, once the file is removed from unified build, you can try to make it build standalone by adding WebKit:: and WebCore:: qualifications everywhere, then add it back to unified build once it works if you really want to. But I briefly tried this the other day, and I did not enjoy what I was getting myself into. ;)
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug