723894 - Firefox 13.0a1 Crash @ JSAutoEnterCompartment::enter

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Masatoshi Kimura [:emk]]

https://crash-stats.mozilla.com/report/index/bp-6caad3d3-e728-4b93-b549-82fa52120203

Steps to reproduce:
new MouseEvent("click", null);

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: patch

JSVAL_IS_OBJECT(v) is not equivalent to v.isObject(), but is equivalent to v.isObjectOrNull().

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 594131 [details] [diff] [review]
patch

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

This looks pretty good -- can you add a crashtest for this as well as addressing the comment below? r- for now, but I'm sure r+ on the next patch.

::: js/xpconnect/src/dictionary_helper_gen.py
@@ +381,5 @@
>               "  if (!aCx || !aVal) {\n"
>               "    return NS_OK;\n"
>               "  }\n"
> +             "  NS_ENSURE_STATE(aVal->isObject());\n\n"
> +             "  JSObject& obj = aVal->toObject();\n"

I'd rather see this as |JSObject *obj = &aValue->toObject();| that way you can reduce the changes later and to match other toObject() using code in the tree.

Comment 3

[Masatoshi Kimura [:emk]]

Attachment: patch v2

Resolved review comments.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 594432 [details] [diff] [review]
patch v2

Perfect, thanks.

Comment 5

[Mozilla RelEng Bot]

Autoland Patchset:
	Patches: 594432
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/rev/d1e88e9e3e63
Try run started, revision d1e88e9e3e63. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=d1e88e9e3e63

Comment 6

[Mozilla RelEng Bot]

Try run for d1e88e9e3e63 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=d1e88e9e3e63
Results (out of 208 total builds):
    success: 190
    warnings: 18
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-d1e88e9e3e63

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4dcd64ada95c

Comment 8

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/4dcd64ada95c

Comment 9

[Alex Keybl [:akeybl]]

Recent regression that will likely spike as our testing audience grows. If considered low risk, we'd definitely consider uplifting a fix if nominated.

Comment 10

[Masatoshi Kimura [:emk]]

Comment on attachment 594432 [details] [diff] [review]
patch v2

[Approval Request Comment]
Regression caused by (bug #): 709569
User impact if declined: DoS attack can be made because this is a reproducible crash.
Testing completed (on m-c, etc.): Crashtest added on m-c.
Risk to taking this patch (and alternatives if risky): Low, trivial one line fix.
String changes made by this patch: No.

Comment 11

[Alex Keybl [:akeybl]]

Comment on attachment 594432 [details] [diff] [review]
patch v2

[Triage Comment]
Approved for Aurora 12.

Comment 12

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-aurora/rev/4af6218cce89

Comment 13

[Virgil Dicu [:virgil] [QA]]

Mozilla/5.0 (X11; Linux i686; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (Windows NT 5.1; rv:12.0) Gecko/20100101 Firefox/12.0

Verified in Firefox 12 beta3 on Windows XP, Mac OS 10.6, ubuntu 11.10.

No crash when loading new MouseEvent("click", null); in Error Console. Could previously reproduce.