182869 – [WinCairo] Add WKView and WKAPI

RESOLVED FIXED 182869

[WinCairo] Add WKView and WKAPI

https://bugs.webkit.org/show_bug.cgi?id=182869

Summary [WinCairo] Add WKView and WKAPI

Yousuke Kimoto

Reported 2018-02-16 04:57:55 PST

WKView.[cpp|h] and WKAPICastWin.h are required to build wincairo webkit. This ticket is to just add those files.

Attachments
bz182869-1.patch (12.50 KB, patch) 2018-02-16 05:16 PST, Yousuke Kimoto	no flags	Details Formatted Diff Diff
Patch (13.46 KB, patch) 2018-03-05 04:40 PST, Yousuke Kimoto	no flags	Details Formatted Diff Diff
Patch (12.97 KB, patch) 2018-03-08 05:46 PST, Yousuke Kimoto	no flags	Details Formatted Diff Diff
Patch (126.98 KB, patch) 2018-04-11 03:54 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews200 for win-future (12.67 MB, application/zip) 2018-04-11 06:18 PDT, EWS Watchlist	no flags	Details
Patch (117.01 KB, patch) 2018-04-16 23:43 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews103 for mac-sierra (2.28 MB, application/zip) 2018-04-17 00:50 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews206 for win-future (12.60 MB, application/zip) 2018-04-17 01:40 PDT, EWS Watchlist	no flags	Details
Patch (107.33 KB, patch) 2018-04-23 20:15 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Patch for landing (107.49 KB, patch) 2018-04-24 20:11 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Show Obsolete (9) View All Add attachment proposed patch, testcase, etc.

Yousuke Kimoto

Comment 1 2018-02-16 05:16:17 PST

Created attachment 334031 [details] bz182869-1.patch

Yousuke Kimoto

Comment 2 2018-03-02 02:29:38 PST

Comment on attachment 334031 [details] bz182869-1.patch For wincairo webkit, WKAPICast.h needs to include "WKAPICastWin.h". I'll fix it.

Yousuke Kimoto

Comment 3 2018-03-05 04:40:33 PST

Created attachment 334997 [details] Patch

Alex Christensen

Comment 4 2018-03-05 13:47:48 PST

Comment on attachment 334997 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334997&action=review > Source/WebKit/UIProcess/API/C/win/WKView.cpp:65 > +void WKViewSetIsInWindow(WKViewRef viewRef, bool isInWindow) > +{ > + notImplemented(); > +} Let's not add unimplemented API. > Source/WebKit/UIProcess/API/C/win/WKView.cpp:112 > +void WKViewSetDrawsTransparentBackground(WKViewRef viewRef, bool drawsTransparentBackground) > +{ > +} ditto. > Source/WebKit/UIProcess/API/C/win/WKView.cpp:117 > +bool WKViewDrawsTransparentBackground(WKViewRef viewRef) > +{ > + return false; > +} This is also basically unimplemented. > Source/WebKit/UIProcess/API/C/win/WKView.h:76 > + WK_EXPORT void WKViewSetInitialFocus(WKViewRef view, bool forward); forward is probably the wrong name here.

Yousuke Kimoto

Comment 5 2018-03-08 05:46:11 PST

Created attachment 335295 [details] Patch

Yousuke Kimoto

Comment 6 2018-03-08 05:56:06 PST

Thank you for your review. (In reply to Alex Christensen from comment #4) > Comment on attachment 334997 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=334997&action=review > > > Source/WebKit/UIProcess/API/C/win/WKView.cpp:65 > > +void WKViewSetIsInWindow(WKViewRef viewRef, bool isInWindow) > > +{ > > + notImplemented(); > > +} > > Let's not add unimplemented API. Fixed. > > Source/WebKit/UIProcess/API/C/win/WKView.cpp:112 > > +void WKViewSetDrawsTransparentBackground(WKViewRef viewRef, bool drawsTransparentBackground) > > +{ > > +} > > ditto. > > Source/WebKit/UIProcess/API/C/win/WKView.cpp:117 > > +bool WKViewDrawsTransparentBackground(WKViewRef viewRef) > > +{ > > + return false; > > +} > > This is also basically unimplemented. Those two APIs are not used for now, so they were just removed. > > Source/WebKit/UIProcess/API/C/win/WKView.h:76 > > + WK_EXPORT void WKViewSetInitialFocus(WKViewRef view, bool forward); > > forward is probably the wrong name here. "isForward" is used instead by mimicking WKViewSetIsInWindow()'s boolean variable name.

Alex Christensen

Comment 7 2018-03-28 15:55:44 PDT

Comment on attachment 335295 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=335295&action=review This is adding a file that doesn't compile. Let's not. > Source/WebKit/UIProcess/API/C/win/WKAPICastWin.h:2 > + * Copyright (C) 2010 Apple Inc. All rights reserved. If this is resurrecting old code, please indicate the initial commit this came from. > Source/WebKit/UIProcess/API/C/win/WKAPICastWin.h:38 > +WK_ADD_API_MAPPING(WKViewRef, WebView) Is WebView correct? I think we might want something more like WebPageProxy here. > Source/WebKit/UIProcess/API/C/win/WKView.cpp:32 > +#include "WebView.h" WebView.h doesn't exist in modern WebKit. Does this compile?

Fujii Hironori

Comment 8 2018-04-11 03:54:08 PDT

Created attachment 337690 [details] Patch

EWS Watchlist

Comment 9 2018-04-11 06:17:55 PDT

Comment on attachment 337690 [details] Patch Attachment 337690 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/7282666 New failing tests: http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-audio.html

EWS Watchlist

Comment 10 2018-04-11 06:18:06 PDT

Created attachment 337694 [details] Archive of layout-test-results from ews200 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews200 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

Fujii Hironori

Comment 11 2018-04-13 21:16:58 PDT

Could anyone review?

Alex Christensen

Comment 12 2018-04-16 08:50:30 PDT

Comment on attachment 337690 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337690&action=review > Source/WebKit/UIProcess/Launcher/win/ProcessLauncherWin.cpp:35 > +const LPCWSTR webKitDLLName = L"WebKit2.dll"; Do we want this name? > Source/WebKit/UIProcess/win/WebContextMenuProxyWin.h:51 > + WebPageProxy* m_page; It looks like this could be a WebPageProxy&. > Source/WebKit/UIProcess/win/WebView.cpp:72 > +SOFT_LINK_LIBRARY(IMM32) Why are we soft linking instead of directly linking? > Source/WebKit/UIProcess/win/WebView.cpp:245 > + , m_toolTipWindow(0) These should be { nullptr } in the header. > Source/WebKit/UIProcess/win/WebView.h:57 > + static WebView* create(RECT rect, const API::PageConfiguration& configuration, HWND parentWindow) This should return a std::unique_ptr or UniqueRef.

Fujii Hironori

Comment 13 2018-04-16 21:51:51 PDT

Comment on attachment 337690 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337690&action=review Thank you for the review. >> Source/WebKit/UIProcess/Launcher/win/ProcessLauncherWin.cpp:35 >> +const LPCWSTR webKitDLLName = L"WebKit2.dll"; > > Do we want this name? I will remove the variable. >> Source/WebKit/UIProcess/win/WebView.cpp:72 >> +SOFT_LINK_LIBRARY(IMM32) > > Why are we soft linking instead of directly linking? I don't know the reason. SOFT_LINK was used since Bug 51049. I will link it directly.

Fujii Hironori

Comment 14 2018-04-16 22:52:52 PDT

Comment on attachment 337690 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337690&action=review >> Source/WebKit/UIProcess/win/WebView.h:57 >> + static WebView* create(RECT rect, const API::PageConfiguration& configuration, HWND parentWindow) > > This should return a std::unique_ptr or UniqueRef. WebView is an APIObject. It needs to be leaked to convert C object. WebProcessPool::create return Ref<WebProcessPool> and using leakRef() to convert C object. > static Ref<WebProcessPool> create(API::ProcessPoolConfiguration&); > WKContextRef WKContextCreate() > { > auto configuration = API::ProcessPoolConfiguration::createWithLegacyOptions(); > return toAPI(&WebProcessPool::create(configuration).leakRef()); > } I will use this style for WebView.

Fujii Hironori

Comment 15 2018-04-16 23:43:21 PDT

Created attachment 338081 [details] Patch

EWS Watchlist

Comment 16 2018-04-17 00:50:22 PDT

Comment on attachment 338081 [details] Patch Attachment 338081 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/7340707 New failing tests: animations/needs-layout.html

EWS Watchlist

Comment 17 2018-04-17 00:50:23 PDT

Created attachment 338086 [details] Archive of layout-test-results from ews103 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-sierra Platform: Mac OS X 10.12.6

EWS Watchlist

Comment 18 2018-04-17 01:40:04 PDT

Comment on attachment 338081 [details] Patch Attachment 338081 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/7341027 New failing tests: http/tests/security/video-poster-cross-origin-crash2.html

EWS Watchlist

Comment 19 2018-04-17 01:40:16 PDT

Created attachment 338091 [details] Archive of layout-test-results from ews206 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

Fujii Hironori

Comment 20 2018-04-17 22:58:24 PDT

Could you review again, Alex?

Fujii Hironori

Comment 21 2018-04-23 03:17:29 PDT

Could anyone review this?

Fujii Hironori

Comment 22 2018-04-23 20:15:48 PDT

Created attachment 338632 [details] Patch * Removed half-baked IME support to simplify this patch.

Alex Christensen

Comment 23 2018-04-24 11:00:18 PDT

Comment on attachment 338632 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=338632&action=review > Source/WebKit/UIProcess/Launcher/ProcessLauncher.h:96 > + HANDLE m_hProcess; You might consider using a Win32Handle here.

Fujii Hironori

Comment 24 2018-04-24 20:11:06 PDT

Created attachment 338701 [details] Patch for landing

Fujii Hironori

Comment 25 2018-04-24 20:36:34 PDT

Committed r230982: <https://trac.webkit.org/changeset/230982>

Radar WebKit Bug Importer

Comment 26 2018-04-24 20:38:56 PDT

<rdar://problem/39709271>

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware Unspecified

OS Unspecified

Product WebKit

Component WebKit2

Assignee

Fujii Hironori

Reported

2018-02-16 04:57 PST

Modified

2018-04-24 20:38 PDT History

CC List

9 users Show

URL

Keywords InRadar

Depends on

Blocks

174003

Dependencies

tree graph