Closed Bug 791215 Opened 8 years ago Closed 7 years ago

Test plugin hang submission

Categories

(Core :: Plug-ins, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: benjamin, Assigned: gfritzsche)

References

Details

Attachments

(2 files, 2 obsolete files)

Bug 791009 showed a minor mistake in the plugin hang submissions which should have been caught by an automated test. We have tests for crash submission, I think, but we should test hang submission and in particular the multiple-minidump bits.
Status: NEW → ASSIGNED
Depends on: 791009
OS: Linux → All
Hardware: x86_64 → All
Fix RangeError in fake crashreport server showing up in above try run:
> toolkit/crashreporter/test/browser/crashreport.sjs: RangeError: arguments array passed to Function.prototype.apply is too large on line 60
Attachment #661881 - Flags: review?(ted.mielczarek)
Comment on attachment 661756 [details] [diff] [review]
Add missing hang submission test

Review of attachment 661756 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/plugins/test/mochitest/test_hang_submit.xul
@@ +112,5 @@
> +    return;
> +  }
> +
> +  // Default plugin hang timeout is too high for mochitests
> +  Services.prefs.setIntPref("dom.ipc.plugins.timeoutSecs", 5);

Can you set this even lower without breaking things? 5 seconds is a long time to wait for a test.

@@ +125,5 @@
> +  // Override the crash reporter URL to send to our fake server
> +  Services.prefs.setCharPref("toolkit.crashreporter.pluginHangSubmitURL", SERVER_URL);
> +
> +  // Hook into plugin crash events
> +  Services.obs.addObserver(testObserver, "crash-report-status", true);  

nit: trailing whitespace on this line.
Attachment #661756 - Flags: review?(ted.mielczarek) → review+
Attachment #661881 - Flags: review?(ted.mielczarek) → review+
> ::: dom/plugins/test/mochitest/test_hang_submit.xul
> > +  // Default plugin hang timeout is too high for mochitests
> > +  Services.prefs.setIntPref("dom.ipc.plugins.timeoutSecs", 5);
> 
> Can you set this even lower without breaking things? 5 seconds is a long
> time to wait for a test.

Good point, i just followed what the other hang-tests used so far. 

Chris, can you remember if you chose that value specifically in test_hanging.html? 
Did you run into issues with a too low value causing premature hang-kills on loaded machines?
It was definitely for test_hanging.  I don't recall what factored into its choice though, too long ago :).
Attachment #661881 - Attachment is obsolete: true
Fine on try with 1s hang timeout:
https://tbpl.mozilla.org/?tree=Try&rev=0f074d007cb9
Attachment #661756 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/156f00b9ac92
https://hg.mozilla.org/mozilla-central/rev/bafe7d48e4c5
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.