Closed
Bug 831367
Opened 12 years ago
Closed 12 years ago
We should simplify SpecialPowersAPI.bindDOMWindowUtils()
Categories
(Testing :: Mochitest, defect)
Testing
Mochitest
Tracking
(firefox22 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 affected)
RESOLVED
FIXED
mozilla22
People
(Reporter: jgriffin, Assigned: jgriffin)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
The current implementation of SpecialPowersAPI.bindDomWindowUtils() was made before 'wrap' was available, and is a little needlessly complicated. It also causes a memory leak when Marionette uses it, since Marionette instantiates a new SpecialPowers object with each execute_script call.
When an attempt was made to replace the current code with:
function bindDOMWindowUtils(aWindow) {
if (!aWindow)
return
var util = aWindow.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
.getInterface(Components.interfaces.nsIDOMWindowUtils);
return wrapPrivileged(util);
}
it broke a bunch of tests, which apparently rely on the current method of binding. See bug 825802. The affected tests are:
2642 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/contacts/tests/test_contacts_blobs.html | Got an array object - got false, expected true
2643 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/contacts/tests/test_contacts_blobs.html | uncaught exception - TypeError: blobs1 is null at http://mochi.test:8888/tests/dom/contacts/tests/test_contacts_blobs.html:147
2947 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/file/test/test_append_read_data.html | Correct blob data
2948 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/file/test/test_append_read_data.html | Correct size - got 118381, expected 218368
3452 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/file/test/test_write_read_data.html | Correct location - got 118381, expected 218368
3453 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/file/test/test_write_read_data.html | Correct location - got 118381, expected 218368
3454 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/file/test/test_write_read_data.html | Correct blob data
3455 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/file/test/test_write_read_data.html | Correct size - got 118381, expected 218368
19166 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_array.html | uncaught exception - DataCloneError: The object could not be cloned. at http://mochi.test:8888/tests/dom/indexedDB/test/test_file_array.html:51
19169 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_cross_database_copying.html | indexedDB error, 'AbortError'
19170 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_cross_database_copying.html | uncaught exception - AbortError at http://mochi.test:8888/tests/dom/indexedDB/test/test_file_array.html:37
19171 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_cross_database_copying.html | [SimpleTest.finish()] this test already called finish!
19172 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_cross_database_copying.html | [SimpleTest.finish()] this test already called finish!
19173 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_cross_database_copying.html | /tests/dom/indexedDB/test/test_file_array.html finished in a non-clean fashion, probably because it didn't call SimpleTest.finish()
19176 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_delete.html | /tests/dom/indexedDB/test/test_file_array.html finished in a non-clean fashion, probably because it didn't call SimpleTest.finish()
19181 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_os_delete.html | uncaught exception - DataCloneError: The object could not be cloned. at http://mochi.test:8888/tests/dom/indexedDB/test/test_file_os_delete.html:40
19184 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_put_get_object.html | indexedDB error, 'AbortError'
19185 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_put_get_object.html | uncaught exception - AbortError at http://mochi.test:8888/tests/dom/indexedDB/test/test_file_os_delete.html:27
19186 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_put_get_object.html | [SimpleTest.finish()] this test already called finish!
19187 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_put_get_object.html | [SimpleTest.finish()] this test already called finish!
19188 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_put_get_object.html | /tests/dom/indexedDB/test/test_file_os_delete.html finished in a non-clean fashion, probably because it didn't call SimpleTest.finish()
19191 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_put_get_values.html | /tests/dom/indexedDB/test/test_file_os_delete.html finished in a non-clean fashion, probably because it didn't call SimpleTest.finish()
19197 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_quota.html | uncaught exception - DataCloneError: The object could not be cloned. at http://mochi.test:8888/tests/dom/indexedDB/test/test_file_quota.html:42
19301 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_replace.html | uncaught exception - DataCloneError: The object could not be cloned. at http://mochi.test:8888/tests/dom/indexedDB/test/test_file_replace.html:39
19304 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_resurrection_delete.html | indexedDB error, 'AbortError'
19305 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_resurrection_delete.html | uncaught exception - AbortError at http://mochi.test:8888/tests/dom/indexedDB/test/test_file_replace.html:25
19306 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_resurrection_delete.html | [SimpleTest.finish()] this test already called finish!
19307 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_resurrection_delete.html | [SimpleTest.finish()] this test already called finish!
19308 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_resurrection_delete.html | /tests/dom/indexedDB/test/test_file_replace.html finished in a non-clean fashion, probably because it didn't call SimpleTest.finish()
19311 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_resurrection_transaction_abort.html | /tests/dom/indexedDB/test/test_file_replace.html finished in a non-clean fashion, probably because it didn't call SimpleTest.finish()
19316 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_sharing.html | uncaught exception - DataCloneError: The object could not be cloned. at http://mochi.test:8888/tests/dom/indexedDB/test/test_file_sharing.html:39
19319 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_transaction_abort.html | indexedDB error, 'AbortError'
19320 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_transaction_abort.html | uncaught exception - AbortError at http://mochi.test:8888/tests/dom/indexedDB/test/test_file_sharing.html:26
19321 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_transaction_abort.html | [SimpleTest.finish()] this test already called finish!
19322 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_file_transaction_abort.html | [SimpleTest.finish()] this test already called finish!
19325 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_filehandle_quota.html | /tests/dom/indexedDB/test/test_file_transaction_abort.html finished in a non-clean fashion, probably because it didn't call SimpleTest.finish()
19352 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_filehandle_store_snapshot.html | Instance of nsIDOMFile - got true, expected false
19353 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_filehandle_store_snapshot.html | Correct size - got 13, expected 100000
19356 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/indexedDB/test/test_filehandle_store_snapshot.html | uncaught exception - NS_ERROR_FAILURE: Failure arg 0 [nsIDOMFileReader.readAsArrayBuffer] at http://mochi.test:8888/tests/dom/indexedDB/test/file.js:111
from https://tbpl.mozilla.org/php/getParsedLog.php?id=18831926&tree=Try&full=1
and
13341 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/general/test_domWindowUtils.html | at index 0 for data [0, 100] - got [object HTMLDivElement], expected [object HTMLDivElement]
13342 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/general/test_domWindowUtils.html | at index 1 for data [9, 109] - got [object HTMLDivElement], expected [object HTMLDivElement]
13345 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/general/test_domWindowUtils.html | at index 4 for data [0, 100, true, false] - got [object HTMLDivElement], expected [object HTMLDivElement]
13346 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/general/test_domWindowUtils.html | at index 5 for data [9, 109, true, false] - got [object HTMLDivElement], expected [object HTMLDivElement]
13347 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/general/test_domWindowUtils.html | at index 6 for data [0, 100000, true, false] - got [object HTMLDivElement], expected [object HTMLDivElement]
13348 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/general/test_domWindowUtils.html | at index 7 for data [9, 100009, true, false] - got [object HTMLDivElement], expected [object HTMLDivElement]
13349 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/general/test_domWindowUtils.html | at index 8 for data [10, 110, false, true] - got [object HTMLDivElement], expected [object HTMLDivElement]
13350 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/general/test_domWindowUtils.html | at index 9 for data [0, 100, false, false] - got [object HTMLHtmlElement], expected [object HTMLHtmlElement]
13351 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/general/test_domWindowUtils.html | at index 10 for data [0, 100, false, true] - got [object HTMLDivElement], expected [object HTMLDivElement]
13352 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/general/test_domWindowUtils.html | at index 11 for data [10, 100010, true, true] - got [object HTMLDivElement], expected [object HTMLDivElement]
13353 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/general/test_domWindowUtils.html | at index 12 for data [0, 100000, true, false] - got [object HTMLHtmlElement], expected [object HTMLHtmlElement]
13354 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/general/test_domWindowUtils.html | at index 13 for data [0, 100000, true, true] - got [object HTMLDivElement], expected [object HTMLDivElement]
from https://tbpl.mozilla.org/php/getParsedLog.php?id=18831873&tree=Try&full=1.
In order to change the way we bind DOMWindowUtils, we'll probably need to look at all of those tests and update them.
Comment 1•12 years ago
|
||
Bobby, I assume this is just wrapper trouble. Do you have any thoughts?
Comment 2•12 years ago
|
||
These look mostly like identity and instanceof issues. They can probably be fixed mechanically by unwrapping before comparing and doing SpecialPowers_instanceOf or whatever it's called.
Assignee | ||
Comment 3•12 years ago
|
||
One of the things that fails with this patch is here:
http://mxr.mozilla.org/mozilla-central/source/dom/indexedDB/test/file.js#52
The Blob objects returned by this, after applying this patch, do not play nicely with indexedDB calls; it results in e.g., "failed | uncaught exception - DataCloneError: The object could not be cloned. at http://mochi.test:8888/tests/dom/indexedDB/test/test_file_array.html:52"
What do I need to unwrap to make this work?
Comment 5•12 years ago
|
||
It looks like |utils| is a special-powers wrapped object, so anything returned by methods invoked on it will also be wrapped via the transitive behavior. If callers expect regular blobs, getBlob should probably SpecialPowers.unwrap before returning. Note however that, depending on the details of the underlying object, this may or may not return an object that's opaque to the associated content.
Flags: needinfo?(bobbyholley+bmo)
Assignee | ||
Comment 6•12 years ago
|
||
Assignee | ||
Comment 7•12 years ago
|
||
This works on my own machine, we'll see what try says: https://tbpl.mozilla.org/?tree=Try&rev=d4bfa1e985f6
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → jgriffin
Assignee | ||
Comment 8•12 years ago
|
||
Try failures:
2689 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/contacts/tests/test_contacts_blobs.html | Got an array object - got false, expected true
2690 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/contacts/tests/test_contacts_blobs.html | uncaught exception - TypeError: blobs1 is null at http://mochi.test:8888/tests/dom/contacts/tests/test_contacts_blobs.html:147
3002 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/file/test/test_append_read_data.html | Correct blob data
3003 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/file/test/test_append_read_data.html | Correct size - got 118381, expected 218368
3507 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/file/test/test_write_read_data.html | Correct location - got 118381, expected 218368
3508 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/file/test/test_write_read_data.html | Correct location - got 118381, expected 218368
3509 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/file/test/test_write_read_data.html | Correct blob data
3510 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/file/test/test_write_read_data.html | Correct size - got 118381, expected 218368
and
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_chrome-debugging.js | application timed out after 330 seconds with no output
Assignee | ||
Comment 9•12 years ago
|
||
Fixed a few more cases
Assignee | ||
Updated•12 years ago
|
Attachment #714017 -
Attachment is obsolete: true
Assignee | ||
Comment 10•12 years ago
|
||
pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=82d453d890d6
Assignee | ||
Comment 11•12 years ago
|
||
The browser-chrome crash/hang has occurred over a couple of different try runs. See e.g.,
https://tbpl.mozilla.org/php/getParsedLog.php?id=19747713&tree=Try&full=1 - PROCESS-CRASH | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_chrome-debugging.js | application crashed [@ libc-2.11.so + 0xd4aa3]
and
https://tbpl.mozilla.org/php/getParsedLog.php?id=19747803&tree=Try&full=1 -
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_chrome-debugging.js | application timed out after 330 seconds with no output
The debugger tests don't use DOMWindowUtils at all, and only reference SpecialPowers in a couple of places in this file: http://mxr.mozilla.org/mozilla-central/source/browser/devtools/debugger/test/browser_dbg_propertyview-09.js
Any ideas what might be going on, ted or bholley?
Comment 12•12 years ago
|
||
Note that we intentionally kill the process when we detect a hang, so the crash is just whatever the browser was doing when we killed it. Here it just looks like it's spinning the event loop. It looks like that test hooks up the Debugger API, creates a new iframe, and expects to get a "newScript" notification for its global, which it never receives, and thus hangs:
http://mxr.mozilla.org/mozilla-central/source/browser/devtools/debugger/test/browser_dbg_chrome-debugging.js
past, jimb: any idea why the patch here (to SpecialPowers) would break this test in this way?
Comment 13•12 years ago
|
||
This test takes a while to run. At the point where it times out, it should be adding all globals in the browser instance as debuggees, which takes a few seconds even in my i7. Perhaps this change makes the test run a little slower, which tips it off the standard threshold in our slow machines on try. Can you see if requesting a longer timeout fixes it?
Comment 14•12 years ago
|
||
(In reply to Panos Astithas [:past] from comment #13)
> which tips it off the standard threshold in our slow machines on try.
s/off/over/
Assignee | ||
Comment 15•12 years ago
|
||
(In reply to Panos Astithas [:past] from comment #14)
> (In reply to Panos Astithas [:past] from comment #13)
> > which tips it off the standard threshold in our slow machines on try.
>
> s/off/over/
The current timeout is ~300s; should it take that long to run?
Comment 16•12 years ago
|
||
(In reply to Jonathan Griffin (:jgriffin) from comment #15)
> (In reply to Panos Astithas [:past] from comment #14)
> > (In reply to Panos Astithas [:past] from comment #13)
> > > which tips it off the standard threshold in our slow machines on try.
> >
> > s/off/over/
>
> The current timeout is ~300s; should it take that long to run?
I don't think so, but I'm not sure. Could you do another try push with the patch from bug 758172 to get some more logging?
Assignee | ||
Comment 17•12 years ago
|
||
I added that to the patch and pushed to try again: https://tbpl.mozilla.org/?tree=Try&rev=c7cc07325fed
Comment 18•12 years ago
|
||
The problem seems to be that SpiderMonkey does not generate a "newScript" event in time, after the "newGlobal" event we see in the log prior to the timeout. Other tests properly receive such events, so it doesn't look like a more general issue.
My best guess is still that wrapPrivileged is a bit slower than the current code and requesting a longer timeout might fix this already slow test. Can we try that?
Comment 19•12 years ago
|
||
Alternatively, here is a change in the test that might make it quicker to finish, by ensuring the expected scripts are generated faster.
Comment 20•12 years ago
|
||
I have an awfully hard time believing that wrapPrivileged makes this test take >5 minutes to complete. I would guess there's a more subtle bug here, and perhaps the existing SpecialPowers code makes things work, but changing it makes them break.
Assignee | ||
Comment 21•12 years ago
|
||
Pushed with past's patch to try: https://tbpl.mozilla.org/?tree=Try&rev=719d99c565cc
Assignee | ||
Comment 22•12 years ago
|
||
(In reply to Jonathan Griffin (:jgriffin) from comment #21)
> Pushed with past's patch to try:
> https://tbpl.mozilla.org/?tree=Try&rev=719d99c565cc
This works...browser-chrome is all green! Thanks past. Is there any reason not to fold your patch into this one?
Updated•12 years ago
|
Flags: needinfo?(past)
Comment 23•12 years ago
|
||
(In reply to Jonathan Griffin (:jgriffin) from comment #22)
> This works...browser-chrome is all green! Thanks past. Is there any reason
> not to fold your patch into this one?
No, it's actually better than before. Feel free to land them both.
Flags: needinfo?(past)
Assignee | ||
Comment 24•12 years ago
|
||
Running this one more on try, against all the things: https://tbpl.mozilla.org/?tree=Try&rev=e825dda465fb
Assignee | ||
Comment 25•12 years ago
|
||
There were some oranges here on debug Windows, I'm doing some retriggers to see if they go away.
Assignee | ||
Updated•12 years ago
|
Attachment #714164 -
Flags: review?(ted)
Assignee | ||
Updated•12 years ago
|
Attachment #715384 -
Flags: review?(ted)
Assignee | ||
Comment 26•12 years ago
|
||
try is green
Updated•12 years ago
|
Attachment #714164 -
Flags: review?(ted) → review+
Comment 27•12 years ago
|
||
Comment on attachment 715384 [details] [diff] [review]
Fix for chrome-debugging test
Review of attachment 715384 [details] [diff] [review]:
-----------------------------------------------------------------
This should probably get a review from a peer of this code, but this is past's patch so if he's ok with me r+ing it then I'm ok.
Attachment #715384 -
Flags: review?(ted) → review+
Assignee | ||
Comment 28•12 years ago
|
||
Given past's comment #23, I think we're good to land.
Assignee | ||
Comment 29•12 years ago
|
||
Comment 30•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a2dbc433130f
https://hg.mozilla.org/mozilla-central/rev/86a5cbda14e9
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Assignee | ||
Comment 31•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g18/rev/a866d4f1be4c
The test that https://hg.mozilla.org/mozilla-central/rev/86a5cbda14e9 patches is not in mozilla-b2g18, so that patch is not being landed there.
status-b2g18:
--- → fixed
status-b2g18-v1.0.0:
--- → wontfix
status-b2g18-v1.0.1:
--- → affected
status-firefox22:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•