WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 208894
Remove support for WebKitSystemInterface
https://bugs.webkit.org/show_bug.cgi?id=208894
Summary
Remove support for WebKitSystemInterface
Keith Rollin
Reported
2020-03-10 16:38:51 PDT
This project provided support for facilities that are now handled elsewhere and is no longer needed.
Attachments
Patch
(209.30 KB, patch)
2020-03-10 16:40 PDT
,
Keith Rollin
no flags
Details
Formatted Diff
Diff
Patch
(209.49 KB, patch)
2020-03-16 13:05 PDT
,
Keith Rollin
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2020-03-10 16:38:59 PDT
<
rdar://problem/60297590
>
Keith Rollin
Comment 2
2020-03-10 16:40:42 PDT
Created
attachment 393186
[details]
Patch
Keith Rollin
Comment 3
2020-03-10 16:42:05 PDT
To reviewers, I'm mostly worried about this change: - # The static runtime is required for AppleWin due to WebKitSystemInterface.lib - # being compiled with a static runtime. - set(MSVC_STATIC_RUNTIME ON) Is it appropriate to remove this settings?
Brent Fulgham
Comment 4
2020-03-11 10:54:18 PDT
Comment on
attachment 393186
[details]
Patch I love it. Delete delete delete! r=me
Konstantin Tokarev
Comment 5
2020-03-11 11:01:56 PDT
(In reply to Keith Rollin from
comment #3
)
> To reviewers, I'm mostly worried about this change: > > - # The static runtime is required for AppleWin due to > WebKitSystemInterface.lib > - # being compiled with a static runtime. > - set(MSVC_STATIC_RUNTIME ON) > > Is it appropriate to remove this settings?
This depends on how do you build downstream projects and how you distribute their binaries. 1. Runtimes must match, so if you are building WebKit with static runtime (/MT) you should also build application with /MT, if you switch WebKit to shared runtime (/MD) application build settings need to be changed too. 2. If application is built with /MT, it's fully self-contained, while /MD application requires respective msvcrt dlls to be installed.
Keith Rollin
Comment 6
2020-03-11 12:50:35 PDT
On the one hand, it sounds like keeping the setting is good for backward compatibility with WebKit clients. On the other hand, allowing for a /MD style build to a shared runtime configuration would allow for smaller applications. Who makes the decision on which route we want to take? If I don't hear anything, I'll revert this setting in the name of backward compatibility.
Keith Rollin
Comment 7
2020-03-16 13:05:19 PDT
Created
attachment 393674
[details]
Patch
Per Arne Vollan
Comment 8
2020-03-16 13:30:31 PDT
(In reply to Keith Rollin from
comment #6
)
> On the one hand, it sounds like keeping the setting is good for backward > compatibility with WebKit clients. On the other hand, allowing for a /MD > style build to a shared runtime configuration would allow for smaller > applications. > > Who makes the decision on which route we want to take? If I don't hear > anything, I'll revert this setting in the name of backward compatibility.
I think maintaining backward compatibility is a good call.
WebKit Commit Bot
Comment 9
2020-03-16 14:49:45 PDT
The commit-queue encountered the following flaky tests while processing
attachment 393674
[details]
: http/tests/security/contentSecurityPolicy/report-status-code-zero-when-using-https.html
bug 197297
(author:
dbates@webkit.org
) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 10
2020-03-16 14:50:27 PDT
Comment on
attachment 393674
[details]
Patch Clearing flags on attachment: 393674 Committed
r258520
: <
https://trac.webkit.org/changeset/258520
>
WebKit Commit Bot
Comment 11
2020-03-16 14:50:29 PDT
All reviewed patches have been landed. Closing bug.
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