Bug 186421 - [Win][MiniBrowser] Add a new BrowserWindow interface to abstract WK1 and WK2 BrowserWindow
Summary: [Win][MiniBrowser] Add a new BrowserWindow interface to abstract WK1 and WK2 ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: InRadar
Depends on:
Blocks: 184770
  Show dependency tree / graph
 
Reported: 2018-06-07 20:09 PDT by Fujii Hironori
Modified: 2018-06-08 15:08 PDT (History)
7 users (show)

See Also:

Attachments
Patch (10.94 KB, patch)
2018-06-07 20:20 PDT, Fujii Hironori 		no flags Details | Formatted Diff | Diff
Patch (10.93 KB, patch)
2018-06-07 22:05 PDT, Fujii Hironori 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2018-06-07 20:09:37 PDT 
[Win][MiniBrowser] Add a new BrowserWindow interface to abstract WK1 and WK2 BrowserWindow

This is the core patch to make MiniBrowser to support WK1 and WK2 windows (Bug 184770).

I will rename MiniBrowser class to WK1BrowserWindow in a follow-up patch (Bug 184770 Comment 12).
Comment 1 Fujii Hironori 2018-06-07 20:20:50 PDT 
Created attachment 342233 [details]
Patch
Comment 2 Ryosuke Niwa 2018-06-07 21:15:18 PDT 
Comment on attachment 342233 [details]
Patch

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

> Tools/MiniBrowser/win/PrintWebUIDelegate.cpp:74
> +    MiniBrowser* newBrowserWindow = static_cast<MiniBrowser*>(newWindow.browserWindow());

Can we use MiniBrowser& or auto& instead?
Comment 3 Fujii Hironori 2018-06-07 21:17:41 PDT 
Comment on attachment 342233 [details]
Patch

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

Thank you for the review.

>> Tools/MiniBrowser/win/PrintWebUIDelegate.cpp:74
>> +    MiniBrowser* newBrowserWindow = static_cast<MiniBrowser*>(newWindow.browserWindow());
> 
> Can we use MiniBrowser& or auto& instead?

Agreed. Will fix with auto&.
Comment 4 Fujii Hironori 2018-06-07 22:05:34 PDT 
Created attachment 342241 [details]
Patch
Comment 5 Fujii Hironori 2018-06-07 22:14:31 PDT 
Committed r232616: <https://trac.webkit.org/changeset/232616>
Comment 6 Radar WebKit Bug Importer 2018-06-07 22:15:21 PDT 
<rdar://problem/40924038>
Comment 7 Darin Adler 2018-06-08 09:32:31 PDT 
(In reply to Fujii Hironori from comment #0)
> I will rename MiniBrowser class to WK1BrowserWindow in a follow-up patch
> (Bug 184770 Comment 12).

I don’t think you should do that. The older WebKit programming model is no longer referred to as "WebKit 1" but rather "legacy WebKit" or "WebKitLegacy".
Comment 8 Fujii Hironori 2018-06-08 15:08:09 PDT 
(In reply to Darin Adler from comment #7)
> (In reply to Fujii Hironori from comment #0)
> > I will rename MiniBrowser class to WK1BrowserWindow in a follow-up patch
> > (Bug 184770 Comment 12).
> 
> I don’t think you should do that. The older WebKit programming model is no
> longer referred to as "WebKit 1" but rather "legacy WebKit" or
> "WebKitLegacy".

Thank you for the feedback.
We are discussing it in Bug 186427.