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

[bugzilla-daemon at webkit.org]

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
https://bugs.webkit.org/show_bug.cgi?id=121467

Attachment 211861: Patch
https://bugs.webkit.org/attachment.cgi?id=211861&action=review

------- 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.