Bug 121467 – [Windows] Clean up WinLauncher by using smart pointers

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Brent Fulgham]

Get rid of a lot of ugly (and error-prone) boilerplate by switching to smart pointers for the various data types in the program.

[Brent Fulgham]

Created attachment 211861 [details]
Patch

[Anders Carlsson]

Comment on attachment 211861 [details]
Patch

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.

[Brent Fulgham]

Comment on attachment 211861 [details]
Patch

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

Thanks for looking this over. I'll make some more adjustments in a follow-up patch.

>> Tools/WinLauncher/WinLauncher.cpp:82
>> +IWebPreferencesPrivatePtr gPrefsPrivate;
> 
> I don't think these need to be smart pointers, and I think that making them smart pointers will cause global constructors.

Hmm. I have some other cleanup to do; I'll revise this when I make those changes.

>> Tools/WinLauncher/WinLauncher.cpp:88
>> +std::vector<IWebHistoryItemPtr> gHistoryItems;
> 
> Same thing here. Maybe you should just create an application class/struct?

Agreed. I'll do that next.

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

Done.

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

Done.

[Brent Fulgham]

Committed r155929: <http://trac.webkit.org/changeset/155929>