[webkit-reviews] review granted: [Bug 186263] [Win][MiniBrowser] Support multiple windows properly : [Attachment 342034] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jun 6 22:53:45 PDT 2018
Ryosuke Niwa <rniwa at webkit.org> has granted Fujii Hironori
<Hironori.Fujii at sony.com>'s request for review:
Bug 186263: [Win][MiniBrowser] Support multiple windows properly
https://bugs.webkit.org/show_bug.cgi?id=186263
Attachment 342034: Patch
https://bugs.webkit.org/attachment.cgi?id=342034&action=review
--- Comment #15 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 342034
--> https://bugs.webkit.org/attachment.cgi?id=342034
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=342034&action=review
> Tools/MiniBrowser/win/MainWindow.cpp:110
> - m_browserWindow = std::make_unique<MiniBrowser>(m_hMainWnd,
m_hURLBarWnd, usesLayeredWebView, pageLoadTesting);
> + m_browserWindow = adoptRef(new MiniBrowser(m_hMainWnd, m_hURLBarWnd,
usesLayeredWebView, pageLoadTesting));
Please add MiniBrowser::create as we do elsewhere in WebKit instead.
> Tools/MiniBrowser/win/MainWindow.cpp:146
> + RefPtr<MainWindow> thiz =
reinterpret_cast<MainWindow*>(GetWindowLongPtr(hWnd, GWLP_USERDATA));
Can we just call this thisWindow instead of thiz?
> Tools/MiniBrowser/win/MiniBrowser.cpp:79
> + auto c = refCount();
Please don't use a single letter variable like c. Call it count instead.
More information about the webkit-reviews
mailing list