[Webkit-unassigned] [Bug 122319] WinLauncher needs crash report

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=122319


Brent Fulgham <bfulgham at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #213336|review?                     |review-
               Flag|                            |




--- Comment #2 from Brent Fulgham <bfulgham at webkit.org>  2013-10-04 11:55:57 PST ---
(From update of attachment 213336)
View in context: https://bugs.webkit.org/attachment.cgi?id=213336&action=review

This looks great -- but I have a few questions/suggestions before we can land this.

> Tools/WinLauncher/WinLauncher.cpp:180
> +#ifdef _DEBUG

I don't understand this change. Why don't we want to see why the load failed in Release mode? If you don't you will be puzzled by the browser window not displaying any content when you enter a new URL.

> Tools/WinLauncher/WinLauncher.cpp:416
> +    HANDLE miniDumpFile = ::CreateFile(L"CrashReport.dmp", GENERIC_WRITE, 0, 0, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, 0);

You should create this crash report in some definite user-writable location.  See the documentation of SHGetFolderPath (http://msdn.microsoft.com/en-us/library/windows/desktop/bb762181(v=vs.85).aspx) -- I think you probably want to put these files in a sub-folder of CSIDL_LOCAL_APPDATA.  Then input THAT string into this ::CreateFile function.

> Tools/WinLauncher/WinLauncher.cpp:433
> +        processCrashReport();

So will this be dealt with in a future bug? Or is it left for WinLauncher "users" to flesh out in their own releases.  If it's the latter, I think you should add a comment about this in the "WinLauncherReplace.h" header.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.