[webkit-reviews] review granted: [Bug 121467] [Windows] Clean up WinLauncher by using smart pointers : [Attachment 211861] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Sep 16 21:16:09 PDT 2013

Anders Carlsson <andersca at apple.com> has granted Brent Fulgham
<bfulgham at webkit.org>'s request for review:
Bug 121467: [Windows] Clean up WinLauncher by using smart pointers

Attachment 211861: Patch

------- Additional Comments from Anders Carlsson <andersca at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=211861&action=review

> Tools/WinLauncher/WinLauncher.cpp:82
> +IWebInspectorPtr gInspector;
> +IWebViewPtr gWebView;
> +IWebViewPrivatePtr gWebViewPrivate;
> +IWebPreferencesPtr gStandardPreferences;
> +IWebPreferencesPrivatePtr gPrefsPrivate;

I don't think these need to be smart pointers, and I think that making them
smart pointers will cause global constructors.

> Tools/WinLauncher/WinLauncher.cpp:88
> +std::vector<IWebHistoryItemPtr> gHistoryItems;

Same thing here. Maybe you should just create an application class/struct?

> Tools/WinLauncher/WinLauncher.cpp:537
> +	   static _bstr_t defaultHTML(TEXT("<p style=\"background-color:
#00FF00\">Testing</p><img id=\"webkit logo\"
src=\"http://webkit.org/images/icon-gold.png\" alt=\"Face\"><div
style=\"border: solid blue; background: white;\" contenteditable=\"true\">div
with blue border</div><ul><li>foo<li>bar<li>baz</ul>"));

I don't think this needs to be static.

> Tools/WinLauncher/WinLauncher.cpp:915
> +    static _bstr_t methodBStr(TEXT("GET"));

Same thing here.

More information about the webkit-reviews mailing list