Bug 195740 – [Win][MinBrowser][WK2] Implement createNewPage of WKPageUIClient to open a new window

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Fujii Hironori]

[Win][MinBrowser][WK2] Implement createNewPage of WKPageUIClient to open a new window

[Fujii Hironori]

Created attachment 364652 [details]
Patch

[Ross Kirsling]

Comment on attachment 364652 [details]
Patch

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

Seems fine overall from my perspective.

> Tools/MiniBrowser/win/MainWindow.h:36
> +    using BrowserWindowFactory = std::function<Ref<BrowserWindow>(HWND mainWnd, HWND urlBarWnd, bool usesLayeredWebView, bool pageLoadTesting)>;

Shouldn't this be WTF::Function?

> Tools/MiniBrowser/win/WebKitBrowserWindow.cpp:304
> +    auto& thisWindow = toWebKitBrowserWindow(clientInfo);

Doesn't look like you're using this one.

[Fujii Hironori]

Comment on attachment 364652 [details]
Patch

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

>> Tools/MiniBrowser/win/MainWindow.h:36
>> +    using BrowserWindowFactory = std::function<Ref<BrowserWindow>(HWND mainWnd, HWND urlBarWnd, bool usesLayeredWebView, bool pageLoadTesting)>;
> 
> Shouldn't this be WTF::Function?

I prefer std::function to WTF::Function in MiniBrowser for the same reason I prefer std::vector to WTF::Vector.
Bug 189846 Comment 12
Bug 191101 Comment 3

>> Tools/MiniBrowser/win/WebKitBrowserWindow.cpp:304
>> +    auto& thisWindow = toWebKitBrowserWindow(clientInfo);
> 
> Doesn't look like you're using this one.

Will fix.

[Fujii Hironori]

Created attachment 364750 [details]
Patch for landing

[Fujii Hironori]

Committed r242984: <https://trac.webkit.org/changeset/242984>

[Radar WebKit Bug Importer]

<rdar://problem/48912595>