| Summary: | [Win][MinBrowser][WK2] Implement createNewPage of WKPageUIClient to open a new window | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Fujii Hironori <Hironori.Fujii> | ||||||
| Component: | Tools / Tests | Assignee: | Fujii Hironori <Hironori.Fujii> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | don.olmstead, lforschler, ross.kirsling, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Fujii Hironori
2019-03-14 03:17:25 PDT
Created attachment 364652 [details]
Patch
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. 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. Created attachment 364750 [details]
Patch for landing
Committed r242984: <https://trac.webkit.org/changeset/242984> |