include urls of remote pages in content crash reports

VERIFIED FIXED

Status

defect
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: blassey, Assigned: blassey)

Tracking

Trunk
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Posted patch patch (obsolete) — Splinter Review
this is meant as a stop-gap until bug 581341 is fixed
Attachment #529585 - Flags: review?(mark.finkle)
Comment on attachment 529585 [details] [diff] [review]
patch

>+    let spec = "";

let's use an array:

      let URLs = [];

>     Browser.tabs.forEach(function(aTab) {

>+        spec += (aTab.resurrect().data.entries[0].url + " ");

         let session = aTab.resurrect();
         URLs.push(session.data.entries[0].url);

>     let dumpID = aSubject.hasKey("dumpID") ? aSubject.getProperty("dumpID") : null;

dumpID can be empty, so let's wrap it

      if (dumpID) {

>+    let directoryService = Cc["@mozilla.org/file/directory_service;1"].
>+      getService(Ci.nsIProperties);

all on one line in FE

>+    let pendingDir = directoryService.get("UAppData", Ci.nsIFile);

you never use pendingDir for anything but the extra file, let's just call it "extra" fro the get go

>+    var foStream = Cc["@mozilla.org/network/file-output-stream;1"].
>+	    createInstance(Ci.nsIFileOutputStream);

use "let" and one line

>+    try {
>+	    // use 0x02 | 0x10 to open file for appending.
>+	    foStream.init(extra, 0x02 |  0x10, 0666, 0); 
>+	    foStream.init(extra, 0x02 |  0x10, 0666, 0); 

indent issues (got a TAB in there)

>+      var data = "URL=" + spec + "\n";

use "let" and change | spec | to | URLs.join(" ") |

>+      foStream.close();
>+	    

kill the extra blank

>     browser.__SS_restore = true;
>+    return session;

add a blank line between them


This seems like a good approach until we get the other bug fixed.

Throw up another patch with the fixes and I'll r+
Attachment #529585 - Flags: review?(mark.finkle) → review-
Posted patch patchSplinter Review
Attachment #529585 - Attachment is obsolete: true
Attachment #529637 - Flags: review?(mark.finkle)
Comment on attachment 529637 [details] [diff] [review]
patch

>+    if (dumpID) {
>+      let directoryService = Cc["@mozilla.org/file/directory_service;1"].getService(Ci.nsIProperties);
>+      let extra = directoryService.get("UAppData", Ci.nsIFile);
>+      extra.append("Crash Reports");
>+      extra.append("pending");
>+      extra.append(dumpID + ".extra");
>+      let foStream = Cc["@mozilla.org/network/file-output-stream;1"].createInstance(Ci.nsIFileOutputStream);
>+      try {
>+        // use 0x02 | 0x10 to open file for appending.
>+        foStream.init(extra, 0x02 |  0x10, 0666, 0); 
>+        let data = "URL=" + URLs.join(" ") + "\n";
>+        foStream.write(data, data.length);
>+        foStream.close();
>+      } catch (x) {
>+        dump (x);
>+      }
>+    }

I just realized that appending the URLs could delay the display of the crash dialog. Let's move the code in the if block to:
http://mxr.mozilla.org/mozilla-central/source/mobile/chrome/content/browser.js#2462

That block checks for dumpID and whether the user wants to submit a report, which is perfect. We can write out the URLs and then submit the report.

r+ with that change
Attachment #529637 - Flags: review?(mark.finkle) → review+
pushed http://hg.mozilla.org/mozilla-central/rev/6a3209b9d1fe
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Assignee: nobody → blassey.bugs
Can someone verify this please?
I loaded google.com in a desktop Linux Fennec build, and did kill -ABRT on the content process to get this report:
http://crash-stats.mozilla.com/report/index/06f39bdc-53e5-4f39-b658-f7dd72110505 

I logged in to Socorro and I can see that the URL field on Socorro says:
URL	http://www.google.com/ - Super Sensitive! Don't mess around!
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.