Bug 186421

Summary: [Win][MiniBrowser] Add a new BrowserWindow interface to abstract WK1 and WK2 BrowserWindow
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: Tools / TestsAssignee: Fujii Hironori <Hironori.Fujii>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, darin, lforschler, pvollan, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 184770    
Attachments:
Description Flags
Patch
none
Patch none

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.