Until the real UI for plugin crash reports is done, I want to get some really hacky UI up, so that we can enable them in mozilla-central. This will unlocalized and will pop up a non-modal dialog for the user to hit send/cancel.

This is hacky temporary UI for submitting plugin-process crashes.

Comment on attachment 421102 [details] [diff] [review] Temporary UI for submitting crashes, rev. 1 >Bug 539048 - Hacky temporary UI for sending plugin-process crash reports, r?luser > >diff --git a/dom/plugins/PluginInstanceChild.cpp b/dom/plugins/PluginInstanceChild.cpp >--- a/dom/plugins/PluginInstanceChild.cpp >+++ b/dom/plugins/PluginInstanceChild.cpp >@@ -415,22 +415,16 @@ XVisualIDToInfo(Display* aDisplay, Visua > return false; > } > #endif > > bool > PluginInstanceChild::AnswerNPP_SetWindow(const NPRemoteWindow& aWindow, > NPError* rv) > { >- >- >- typedef void (*death)(); >- (*(death)NULL)(); >- >- This is just leftover cruft from cjones' patch queue, right? >diff --git a/toolkit/crashreporter/content/crashes.js b/toolkit/crashreporter/content/crashes.js >--- a/toolkit/crashreporter/content/crashes.js >+++ b/toolkit/crashreporter/content/crashes.js >@@ -21,16 +21,17 @@ function parseKeyValuePairs(text) { > if (key && value) > data[key] = value.replace("\\n", "\n", "g").replace("\\\\", "\\", "g"); > } > } > return data; > } > > function parseKeyValuePairsFromFile(file) { >+ Components.utils.reportError("parseKeyValuePairsFromFile\n"); You want to make these dump() or something, or remove them. >- iframe.onload = function() { >+ iframe.addEventListener("load", function() { >+ Components.utils.reportError("loadlistener"); > if (iframe.contentWindow.location == "about:blank") > return; >- iframe.onload = null; > submitForm(iframe, dump, extra, link); >- }; >+ }, true); You need to removeEventListener here (you removed the .onload = null), otherwise this handler will fire again after the form submits and the response loads. >diff --git a/toolkit/crashreporter/content/oopcrashdialog.js b/toolkit/crashreporter/content/oopcrashdialog.js >new file mode 100644 >--- /dev/null >+++ b/toolkit/crashreporter/content/oopcrashdialog.js Can we put a TODO comment at the top of this file listing the bug that's going to replace it with a better solution? >+function getExtraData() { This really feels like it ought to be in nsExceptionHandler.cpp where we do this for the chrome process, but I guess we can fix that later... Ideally we'd be able to just share a hashtable of values that are common to both types of processes, and perhaps have a separate one for the chrome-only bits, and then separate per-process info for the child bits. >+ let appData = Cc["@mozilla.org/xre/app-info;1"].getService(Ci.nsIXULRuntime); >+ appData.QueryInterface(Ci.nsICrashReporter); >+ appData.QueryInterface(Ci.nsIXULAppInfo); >+ >+ let r = ""; >+ r += "ServerURL=" + appData.serverURL.spec + "\n"; >+ r += "BuildID=" + appData.appBuildID + "\n"; >+ r += "ProductName=" + appData.name + "\n"; >+ r += "Vendor=" + appData.vendor + "\n"; >+ r += "Version=" + appData.version + "\n"; >+ r += "CrashTime=" + ((new Date()).getTime() / 1000).toFixed(); Data we're not submitting here: 1) the stuff from bug 535829--we should at least hardcode ProcessType=plugin, file a followup for the rest 2) time since last crash--we probably want "time since last crash of this particular plugin", which will require some extra work, can be a followup 3) StartupTime - we want the startup time of the plugin process, lump this in with the followup from #2 3) InstallTime - we can just take the value that the chrome process uses, but it's only available in the C++. Should get filed somewhere We should also file a followup to find out if we can associate a URL with the plugin process for when we crash. I'm not sure how this would work exactly, but we know the document each plugin instance is loaded from, so perhaps if we crash while doing something from that document we can send the URL? Reproducing crashes without any URLs is going to get hard. >+function collectData() { >+ // HACK: crashes.js uses document.body, so we just alias it >+ document.body = document.getElementById('iframe-holder'); Can you file a followup on making crashes.js more readily usable from elsewhere? We're going to want to share that code when we get the right UI hooked up here. >+function onSubmit() >+{ >+ createAndSubmitForm(id, null); >+ document.documentElement.getButton('accept').disabled = true; >+ return false; >+} It would probably be good to have some sort of indicator that the crash submitted successfully or not. crashes.js is not very good about that, I know. >diff --git a/toolkit/crashreporter/content/oopcrashdialog.xul b/toolkit/crashreporter/content/oopcrashdialog.xul It would probably be good to stick a throbber or something in this dialog, even if it is placeholder UI. >diff --git a/toolkit/crashreporter/google-breakpad/src/client/windows/handler/exception_handler.cc b/toolkit/crashreporter/google-breakpad/src/client/windows/handler/exception_handler.cc The changes in this file look unintentional or unnecessary. >diff --git a/toolkit/crashreporter/nsExceptionHandler.cpp b/toolkit/crashreporter/nsExceptionHandler.cpp >--- a/toolkit/crashreporter/nsExceptionHandler.cpp >+++ b/toolkit/crashreporter/nsExceptionHandler.cpp > // Out-of-process crash reporting API wrappers >+class SubmitCrashReport : public nsRunnable >+{ >+public: >+ SubmitCrashReport(nsIFile* f) : mFile(f) { } I'd call this "dumpFile" instead of "f". >+private: >+ nsCOMPtr<nsIFile> mFile; Same here, "mDumpFile". > gExceptionHandler = new google_breakpad:: >- ExceptionHandler(L"", >+ ExceptionHandler(L"c:\builds\minidumps", This should be fixed by cjones' latest patch, I think. >diff --git a/toolkit/xre/nsSigHandlers.cpp b/toolkit/xre/nsSigHandlers.cpp >--- a/toolkit/xre/nsSigHandlers.cpp >+++ b/toolkit/xre/nsSigHandlers.cpp >@@ -402,17 +402,17 @@ LONG __stdcall FpeHandler(PEXCEPTION_POI > #endif > return EXCEPTION_CONTINUE_EXECUTION; > } > return EXCEPTION_CONTINUE_SEARCH; > } > > void InstallSignalHandlers(const char *ProgramName) > { >- SetUnhandledExceptionFilter(FpeHandler); >+ // SetUnhandledExceptionFilter(FpeHandler); > } Presumably this is an unnecessary change (and fixed by the patches in your other bug). r+ with those changes as long as we make it clear that the XUL+JS here is not going to ship in anything but an alpha, and the bug about the final UI is blocking.

http://hg.mozilla.org/mozilla-central/rev/f98a151769d7 Followups are filed for everything except "make crashes.js more usable", since I think that's subsumed into bug 525849.