Closed Bug 1362228 Opened 2 years ago Closed 2 years ago

Intermittent dom/quota/test/browser_permissionsPromptUnknown.js | Test timed out -

Categories

(Core :: DOM: Quota Manager, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 --- fixed
firefox56 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: shawnjohnjr)

Details

(Keywords: intermittent-failure, Whiteboard: [stockwell fixed:timing])

Attachments

(1 file, 6 obsolete files)

10.85 KB, patch
Details | Diff | Splinter Review
Assignee: nobody → shuang
I can't reproduce this bug. And I wonder what's failure rate?
It seems that timeout symptom only happened on osx-10-10.
Somehow, I suspect this happened when doing Shutdown leaks collections.
I have no idea why ShutdownLeaks collections stuck for a while.



16:27:14     INFO - TEST-UNEXPECTED-FAIL | dom/quota/test/browser_permissionsPromptUnknown.js | Test timed out - 
16:27:14     INFO - GECKO(1871) | MEMORY STAT | vsize 4458MB | residentFast 410MB | heapAllocated 101MB
16:27:14     INFO - TEST-OK | dom/quota/test/browser_permissionsPromptUnknown.js | took 45035ms
16:27:14     INFO - checking window state
16:27:14     INFO - Not taking screenshot here: see the one that was previously logged
16:27:14     INFO - TEST-UNEXPECTED-FAIL | dom/quota/test/browser_permissionsPromptUnknown.js | Found a browser window after previous test timed out - 
16:27:14     INFO - GECKO(1871) | must wait for focus
16:27:16     INFO - GECKO(1871) | Completed ShutdownLeaks collections in process 1873
16:27:16     INFO - GECKO(1871) | Completed ShutdownLeaks collections in process 1875
16:27:16     INFO - GECKO(1871) | Completed ShutdownLeaks collections in process 1874
16:27:16     INFO - GECKO(1871) | Completed ShutdownLeaks collections in process 1872
16:27:17     INFO - GECKO(1871) | Completed ShutdownLeaks collections in process 1871
this really picked up on May 23rd, I am doing some retriggers to help figure out where this was introduced:
https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-searchStr=osx%20browser-chrome-2%20debug&tochange=aa48bb3f494410de514041e8c06ca15c38680181&fromchange=74d4005d165ff18bb7fa8948b740450d7c3b414f&selectedJob=101276698

:shawnjohnjr, as this has 55 failures documented last week and the failure rate is continuing this week, would it be possible to get a resolution on this in the next 2 weeks?
Flags: needinfo?(shuang)
Whiteboard: [stockwell needswork]
(In reply to Joel Maher ( :jmaher) from comment #6)
> this really picked up on May 23rd, I am doing some retriggers to help figure
> out where this was introduced:
> https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-
> searchStr=osx%20browser-chrome-
> 2%20debug&tochange=aa48bb3f494410de514041e8c06ca15c38680181&fromchange=74d400
> 5d165ff18bb7fa8948b740450d7c3b414f&selectedJob=101276698
> 
> :shawnjohnjr, as this has 55 failures documented last week and the failure
> rate is continuing this week, would it be possible to get a resolution on
> this in the next 2 weeks?

Yes. I will try to fix it ASAP.
Flags: needinfo?(shuang)
13:46:12     INFO - TEST-START | dom/quota/test/browser_permissionsPromptUnknown.js

....

13:46:57     INFO - TEST-INFO | started process screencapture
13:46:57     INFO - TEST-INFO | screencapture: exit 0
13:46:57     INFO - Buffered messages logged at 13:46:12
13:46:57     INFO - Entering test bound testPermissionUnknownInPrivateWindow
13:46:57     INFO - Creating private window
13:46:57     INFO - Console message: OpenGL compositor Initialized Succesfully.


I need to check why the test got stuck over 40 seconds. And it only happened on macosx.
In another macosx debug build.

15:47:12     INFO - TEST-START | dom/quota/test/browser_permissionsPromptUnknown.js
15:47:12     INFO - GECKO(1928) | ++DOCSHELL 0x11a029000 == 7 [pid = 1928] [id = {c25b2182-0412-5b46-84a3-27bd0c0f941a}]
15:47:12     INFO - GECKO(1928) | ++DOMWINDOW == 16 (0x11a02a000) [pid = 1928] [serial = 16] [outer = 0x0]
15:47:12     INFO - GECKO(1928) | ++DOMWINDOW == 17 (0x11a02e800) [pid = 1928] [serial = 17] [outer = 0x11a02a000]
15:47:12     INFO - GECKO(1928) | ++DOCSHELL 0x11a451000 == 8 [pid = 1928] [id = {0702fd21-7731-2944-bcad-3e1ac60047b6}]
15:47:12     INFO - GECKO(1928) | ++DOMWINDOW == 18 (0x11a454800) [pid = 1928] [serial = 18] [outer = 0x0]
15:47:12     INFO - GECKO(1928) | ++DOCSHELL 0x11a459000 == 9 [pid = 1928] [id = {3770e317-6030-6442-a948-203934983954}]
15:47:12     INFO - GECKO(1928) | ++DOMWINDOW == 19 (0x11a45e000) [pid = 1928] [serial = 19] [outer = 0x0]
15:47:13     INFO - GECKO(1928) | ++DOCSHELL 0x11a81a000 == 10 [pid = 1928] [id = {ca0a4f20-3cee-854d-9c12-a722d1838e04}]
15:47:13     INFO - GECKO(1928) | ++DOMWINDOW == 20 (0x11a81b000) [pid = 1928] [serial = 20] [outer = 0x0]
15:47:13     INFO - GECKO(1928) | ++DOMWINDOW == 21 (0x11add2800) [pid = 1928] [serial = 21] [outer = 0x11a81b000]
15:47:13     INFO - GECKO(1928) | ++DOCSHELL 0x12124e000 == 3 [pid = 1929] [id = {dc858da4-42a0-2146-8d14-471c5ec43603}]
15:47:13     INFO - GECKO(1928) | ++DOMWINDOW == 7 (0x12124e800) [pid = 1929] [serial = 7] [outer = 0x0]
15:47:13     INFO - GECKO(1928) | ++DOMWINDOW == 8 (0x121255000) [pid = 1929] [serial = 8] [outer = 0x12124e800]
15:47:13     INFO - GECKO(1928) | ++DOMWINDOW == 22 (0x11aea2000) [pid = 1928] [serial = 22] [outer = 0x11a454800]
15:47:13     INFO - GECKO(1928) | ++DOMWINDOW == 23 (0x11d19b800) [pid = 1928] [serial = 23] [outer = 0x11a45e000]
15:47:13     INFO - GECKO(1928) | ++DOMWINDOW == 9 (0x121007800) [pid = 1929] [serial = 9] [outer = 0x12124e800]
15:47:13     INFO - GECKO(1928) | ++DOCSHELL 0x11da2c800 == 4 [pid = 1931] [id = {cce3ea84-c85a-044c-9001-ecfa4d2a5457}]
15:47:13     INFO - GECKO(1928) | ++DOMWINDOW == 10 (0x11da2d000) [pid = 1931] [serial = 10] [outer = 0x0]
15:47:13     INFO - GECKO(1928) | ++DOMWINDOW == 11 (0x11da33800) [pid = 1931] [serial = 11] [outer = 0x11da2d000]
15:47:13     INFO - GECKO(1928) | ++DOMWINDOW == 12 (0x11da3a800) [pid = 1931] [serial = 12] [outer = 0x11da2d000]
15:47:13     INFO - GECKO(1928) | ++DOCSHELL 0x1208b5000 == 2 [pid = 1932] [id = {c7282170-53e0-1242-84e5-a4bd221a175a}]
15:47:13     INFO - GECKO(1928) | ++DOMWINDOW == 4 (0x1208b5800) [pid = 1932] [serial = 4] [outer = 0x0]
15:47:14     INFO - GECKO(1928) | ++DOMWINDOW == 5 (0x121471000) [pid = 1932] [serial = 5] [outer = 0x1208b5800]
15:47:14     INFO - GECKO(1928) | [Parent 1928] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80070057: file /home/worker/workspace/build/src/netwerk/base/nsChannelClassifier.cpp, line 219
15:47:14     INFO - GECKO(1928) | [Parent 1928] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80070057: file /home/worker/workspace/build/src/netwerk/base/nsChannelClassifier.cpp, line 219
15:47:14     INFO - GECKO(1928) | ++DOMWINDOW == 6 (0x12146e000) [pid = 1932] [serial = 6] [outer = 0x1208b5800]
15:47:14     INFO - GECKO(1928) | ++DOMWINDOW == 13 (0x11da44000) [pid = 1931] [serial = 13] [outer = 0x11da2d000]
15:47:14     INFO - GECKO(1928) | [Child 1932] WARNING: attempt to modify an immutable nsStandardURL: file /home/worker/workspace/build/src/netwerk/base/nsStandardURL.cpp, line 1736
15:47:14     INFO - GECKO(1928) | [Parent 1928] WARNING: 'NS_FAILED( directory->GetDirectoryEntries(getter_AddRefs(entries)))', file /home/worker/workspace/build/src/dom/quota/ActorsParent.cpp, line 7283
15:47:14     INFO - GECKO(1928) | [Parent 1928] WARNING: 'NS_FAILED( directory->GetDirectoryEntries(getter_AddRefs(entries)))', file /home/worker/workspace/build/src/dom/quota/ActorsParent.cpp, line 7283
15:47:14     INFO - GECKO(1928) | --DOCSHELL 0x120faf800 == 2 [pid = 1929] [id = {aa0f186c-8323-514c-9346-6f6e6337e268}]
15:47:14     INFO - GECKO(1928) | --DOCSHELL 0x11fc35000 == 1 [pid = 1929] [id = {c149354f-8f98-4a4d-8384-fe88593f2988}]
15:47:18     INFO - GECKO(1928) | --DOCSHELL 0x11f16c800 == 2 [pid = 1930] [id = {a0687d46-3d36-534c-9612-d21323355282}]
15:47:18     INFO - GECKO(1928) | --DOCSHELL 0x11d542000 == 1 [pid = 1930] [id = {dd06bd9a-7b80-1247-88a6-a8395f7bd890}]
15:47:19     INFO - GECKO(1928) | --DOCSHELL 0x11d794000 == 3 [pid = 1931] [id = {621e2298-dac6-9f4a-9367-abe62adf2f24}]
15:47:19     INFO - GECKO(1928) | --DOCSHELL 0x11caea000 == 2 [pid = 1931] [id = {c59c58c1-6641-e04e-be2e-139b564fdaf1}]
15:47:19     INFO - GECKO(1928) | --DOCSHELL 0x11aa40800 == 1 [pid = 1931] [id = {d311098b-47c1-2344-a6f7-54daceb03e37}]
15:47:20     INFO - GECKO(1928) | --DOCSHELL 0x11e842000 == 1 [pid = 1932] [id = {bd3d57df-a25e-e947-bb4c-588c2084aac2}]
15:47:24     INFO - GECKO(1928) | --DOCSHELL 0x11a81a000 == 9 [pid = 1928] [id = {ca0a4f20-3cee-854d-9c12-a722d1838e04}]
15:47:24     INFO - GECKO(1928) | --DOMWINDOW == 22 (0x131e4d800) [pid = 1928] [serial = 14] [outer = 0x0] [url = about:blank]
15:47:25     INFO - GECKO(1928) | --DOMWINDOW == 8 (0x121255000) [pid = 1929] [serial = 8] [outer = 0x0] [url = about:blank]
15:47:25     INFO - GECKO(1928) | --DOMWINDOW == 7 (0x11fe53800) [pid = 1929] [serial = 2] [outer = 0x0] [url = about:blank]
15:47:25     INFO - GECKO(1928) | --DOMWINDOW == 6 (0x1207bf000) [pid = 1929] [serial = 3] [outer = 0x0] [url = about:blank]
15:47:27     INFO - GECKO(1928) | --DOMWINDOW == 21 (0x12c007000) [pid = 1928] [serial = 7] [outer = 0x0] [url = about:blank]
15:47:27     INFO - GECKO(1928) | --DOMWINDOW == 20 (0x11a81b000) [pid = 1928] [serial = 20] [outer = 0x0] [url = about:blank]
15:47:28     INFO - GECKO(1928) | --DOMWINDOW == 6 (0x11f173800) [pid = 1930] [serial = 4] [outer = 0x0] [url = about:blank]
15:47:28     INFO - GECKO(1928) | --DOMWINDOW == 5 (0x120fb0000) [pid = 1929] [serial = 5] [outer = 0x0] [url = about:blank]
15:47:28     INFO - GECKO(1928) | --DOMWINDOW == 4 (0x11fc35800) [pid = 1929] [serial = 1] [outer = 0x0] [url = about:blank]
15:47:29     INFO - GECKO(1928) | --DOMWINDOW == 12 (0x11acad000) [pid = 1931] [serial = 2] [outer = 0x0] [url = about:blank]
15:47:29     INFO - GECKO(1928) | --DOMWINDOW == 11 (0x11d8b5000) [pid = 1931] [serial = 8] [outer = 0x0] [url = about:blank]
15:47:29     INFO - GECKO(1928) | --DOMWINDOW == 10 (0x11d77a000) [pid = 1931] [serial = 5] [outer = 0x0] [url = about:blank]
15:47:29     INFO - GECKO(1928) | --DOMWINDOW == 9 (0x11da33800) [pid = 1931] [serial = 11] [outer = 0x0] [url = about:blank]
15:47:29     INFO - GECKO(1928) | --DOMWINDOW == 8 (0x11da3a800) [pid = 1931] [serial = 12] [outer = 0x0] [url = about:blank]
15:47:30     INFO - GECKO(1928) | --DOMWINDOW == 5 (0x121471000) [pid = 1932] [serial = 5] [outer = 0x0] [url = about:blank]
15:47:30     INFO - GECKO(1928) | --DOMWINDOW == 4 (0x11eb97000) [pid = 1932] [serial = 2] [outer = 0x0] [url = about:blank]
15:47:31     INFO - GECKO(1928) | --DOMWINDOW == 7 (0x11caea800) [pid = 1931] [serial = 4] [outer = 0x0] [url = https://example.com/browser/dom/quota/test/browser_permissionsPrompt.html]
15:47:31     INFO - GECKO(1928) | --DOMWINDOW == 6 (0x11aa41000) [pid = 1931] [serial = 1] [outer = 0x0] [url = https://example.com/browser/dom/quota/test/browser_permissionsPrompt.html]
15:47:31     INFO - GECKO(1928) | --DOMWINDOW == 5 (0x11d794800) [pid = 1931] [serial = 7] [outer = 0x0] [url = https://example.com/browser/dom/quota/test/browser_permissionsPrompt.html]
15:47:32     INFO - GECKO(1928) | --DOMWINDOW == 19 (0x11add2800) [pid = 1928] [serial = 21] [outer = 0x0] [url = about:blank]
15:47:32     INFO - GECKO(1928) | --DOMWINDOW == 18 (0x12c01c800) [pid = 1928] [serial = 8] [outer = 0x0] [url = about:blank]
15:47:32     INFO - GECKO(1928) | --DOMWINDOW == 3 (0x12123c000) [pid = 1929] [serial = 6] [outer = 0x0] [url = about:blank]
15:47:32     INFO - GECKO(1928) | --DOMWINDOW == 2 (0x120f9a800) [pid = 1929] [serial = 4] [outer = 0x0] [url = about:blank]
15:47:34     INFO - GECKO(1928) | --DOMWINDOW == 5 (0x11f16d000) [pid = 1930] [serial = 3] [outer = 0x0] [url = https://example.com/browser/dom/quota/test/browser_permissionsPrompt.html]
15:47:34     INFO - GECKO(1928) | --DOMWINDOW == 4 (0x11d542800) [pid = 1930] [serial = 1] [outer = 0x0] [url = about:blank]
15:47:36     INFO - GECKO(1928) | --DOMWINDOW == 4 (0x11b572000) [pid = 1931] [serial = 3] [outer = 0x0] [url = https://example.com/browser/dom/quota/test/browser_permissionsPrompt.html]
15:47:36     INFO - GECKO(1928) | --DOMWINDOW == 3 (0x11d785800) [pid = 1931] [serial = 6] [outer = 0x0] [url = https://example.com/browser/dom/quota/test/browser_permissionsPrompt.html]
15:47:36     INFO - GECKO(1928) | --DOMWINDOW == 2 (0x11d8bd000) [pid = 1931] [serial = 9] [outer = 0x0] [url = https://example.com/browser/dom/quota/test/browser_permissionsPrompt.html]
15:47:36     INFO - GECKO(1928) | --DOMWINDOW == 3 (0x11e842800) [pid = 1932] [serial = 1] [outer = 0x0] [url = https://example.com/browser/dom/quota/test/browser_permissionsPrompt.html]
15:47:38     INFO - GECKO(1928) | --DOMWINDOW == 3 (0x11d74f000) [pid = 1930] [serial = 2] [outer = 0x0] [url = about:blank]
15:47:38     INFO - GECKO(1928) | --DOMWINDOW == 2 (0x11f184000) [pid = 1930] [serial = 5] [outer = 0x0] [url = https://example.com/browser/dom/quota/test/browser_permissionsPrompt.html]
15:47:40     INFO - GECKO(1928) | --DOMWINDOW == 2 (0x11f4a6800) [pid = 1932] [serial = 3] [outer = 0x0] [url = https://example.com/browser/dom/quota/test/browser_permissionsPrompt.html]
15:48:42     INFO - TEST-INFO | started process screencapture
15:48:43     INFO - TEST-INFO | screencapture: exit 0
15:48:43     INFO - Buffered messages logged at 15:47:12
15:48:43     INFO - Entering test bound testPermissionUnknownInPrivateWindow
15:48:43     INFO - Creating private window
15:48:43     INFO - Buffered messages logged at 15:47:13
15:48:43     INFO - Console message: OpenGL compositor Initialized Succesfully.
15:48:43     INFO - Version: 2.1 INTEL-10.6.33
15:48:43     INFO - Vendor: Intel Inc.
15:48:43     INFO - Renderer: Intel Iris OpenGL Engine
15:48:43     INFO - FBO Texture Target: TEXTURE_2D
15:48:43     INFO - Creating private tab
15:48:43     INFO - Loading test page: https://example.com/browser/dom/quota/test/browser_permissionsPrompt.html
15:48:43     INFO - Buffered messages finished
15:48:43     INFO - TEST-UNEXPECTED-FAIL | dom/quota/test/browser_permissionsPromptUnknown.js | Test timed out -
Whiteboard: [stockwell needswork] → [stockwell needswork][6/8]
Whiteboard: [stockwell needswork][6/8] → [stockwell needswork][ETA:6/8]
Test case always got delay after test case got started.
Before test bound "Entering test bound testPermissionUnknownInPrivateWindow" is executed.
Then I saw 'started process screencapture', this is very strange.

22:09:36     INFO - TEST-START | dom/quota/test/browser_permissionsPromptUnknown.js
22:10:21     INFO - TEST-INFO | started process screencapture
22:10:21     INFO - TEST-INFO | screencapture: exit 0
22:10:21     INFO - Buffered messages logged at 22:09:36
22:10:21     INFO - Entering test bound testPermissionUnknownInPrivateWindow
22:10:21     INFO - Creating private window
http://searchfox.org/mozilla-central/source/testing/mochitest/runtests.py#2740
Maybe it's because timeout happened first then, automation test runner tries to do dump screen.
But then we enter "test bound testPermissionUnknownInPrivateWindow". But that's the first line of test.
From the logs, I can only tell it stuck before executing test function, and it doesn't even start the main test case yet.
Between "TEST-START | dom/quota/test/browser_permissionsPromptUnknown.js" and "Entering test bound testPermissionUnknownInPrivateWindow", and it stucks in mochitest.
It's unclear to me why entering test took so much time.
I will add more logs in testing/mochitest/browser-test.js to check.
Whiteboard: [stockwell needswork][ETA:6/8] → [stockwell needswork][ETA:6/13]
So it looks like the timestamp in the logs on try-server misleads me. I guess it's just because timed out and the log doesn't flush out immediately, which makes me think it stuck somewhere when loading test cases.

So after calling persist() from content process browser_permissionsPrompt.html, we don't have further log prints out.
I will add more logs to investigate why promiseMessage() doesn't get called. Since in private browsing mode, permission prompt won't be showed, so we should expect that persist() shall fallback to query persisted status immediately.

During investigation, I found try server sometimes hit similar problems when executing indexeddb permission prompt tests, but fail rate is lower.
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7bbae22a5abfe089d038f2e2341c1fa59a740932&selectedJob=105752300

https://treeherder.mozilla.org/logviewer.html#?job_id=105752300&repo=try&lineNumber=25570

So right now, this issue is much clearer, it's not related to persist(), but chrome process never received message from content process. I'm afraid of finishTest() closed message channel.
thanks for working on this :shawnjohnjr!
I suspected that after BrowserLoaded got called and been resolved, we did not guarantee postMessage() happens after addEventListener. Although logs did not show this fact, since they are on different processes and we can't expect the correctness of timestamp.
Maybe we can loadFrameScript and deal with this problem in chrome privileged script, then we don't have to worry about after BrowserLoaded, promiseMessage() doesn't get executed.
(In reply to Shawn Huang [:shawnjohnjr] from comment #26)
> try:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=48dd1f5e1196b413f7464844cf4949838c6fee2b

Good news is that this bug seems to be resolved, try server looks good.
Hi Jan,
This issue can be found both dom/quota, and dom/indexeddb (bug 1324163).
Fail rate is high. They all fail on macosx, but bug 1324163 also shows other platforms got the same problem.

After BrowserLoaded() gets called and persist() been resolved, there is no guarantee window.postMessage() [1]
is executed after addEventListener(). Once BrowserLoaded() returns, web content script will be executed, and window.postMessage() in the content process may send message first and then promiseMessage() [2] is called. This creates a potential timing issue and it can cause intermittent bugs.

In order to call window.postMessage() after addEventListener(),
make sure messageManager.loadFrameScript() is registered "message" in chrome privileged script, instead of via web content.

We don't call ContentTask.spawn() here since it depends on web content (BrowserLoaded) and we have to register listener before web content is loaded as early as possible.


[1] http://searchfox.org/mozilla-central/rev/d67ef71097da4d1aa344c9d9c672e49a7228e765/dom/quota/test/browserHelpers.js#39

[2] http://searchfox.org/mozilla-central/rev/d67ef71097da4d1aa344c9d9c672e49a7228e765/dom/quota/test/head.js#80
thanks for the patch :shawnjohnjr, looking forward to this getting fixed soon!
I should unregister the call as Jan suggested in the Storage meeting. So let me update the patch.
Attachment #8877444 - Attachment is obsolete: true
Attachment #8877444 - Flags: review?(jvarga)
Comment on attachment 8880261 [details] [diff] [review]
Bug 1362228 - Make sure messageManager.loadFrameScript() is called before window.postMessage()

Call mm.removeMessageListener() and mm.removeDelayedFrameScript() to clean up gracefully.
Attachment #8880261 - Flags: review?(jvarga)
Comment on attachment 8880261 [details] [diff] [review]
Bug 1362228 - Make sure messageManager.loadFrameScript() is called before window.postMessage()

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

::: dom/quota/test/browser_permissionsPromptAllow.js
@@ +16,3 @@
>    info("Loading test page: " + testPageURL);
>    gBrowser.selectedBrowser.loadURI(testPageURL);
>    yield BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser);

It seems to me, that we don't have to call BrowserTestUtils.browserLoaded() now since promiseMessage does all the job.

@@ +25,5 @@
>      triggerMainCommand(this);
>    });
>    registerPopupEventHandler("popuphidden", function () {
>      ok(true, "prompt hidden");
>    });

I also believe these handlers should be registered sooner.

::: dom/quota/test/head.js
@@ +84,5 @@
> +    let frameScript = "data:," +
> +      "addEventListener('message', function(event) {" +
> +      "  sendAsyncMessage('testLocal:persisted'," +
> +      "    {persisted: event.data});" +
> +      "}, true, true);";

So, where's the "{once: true}" ?
I think you need to keep that or call removeEventListener() manually.
This all would be nicer if you defined a function like contentScript() and then call mm.loadFrameScript("data:,(" + contentScript.toString() + ")();", true);
Also, addEventListener is now called with 4 arguments, 3rd and 4th is true, why ?

Btw, I found a very similar trick used in https://dxr.mozilla.org/mozilla-central/source/browser/components/safebrowsing/content/test/browser_bug400731.js#5
Attachment #8880261 - Flags: review?(jvarga)
I also think that promiseMessage isn't a good name anymore, maybe waitForMessage ?
Is this on try now ?
Comment on attachment 8882276 [details] [diff] [review]
Bug 1362228 - Make sure messageManager.loadFrameScript() is called before window.postMessage()

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

Just fix the nits and that's it. You don't have to push to try again.

::: dom/quota/test/browser_permissionsPromptAllow.js
@@ +19,5 @@
>      triggerMainCommand(this);
>    });
>    registerPopupEventHandler("popuphidden", function () {
>      ok(true, "prompt hidden");
>    });

Nit: These 3 register calls should go either before or after addTab call.
I guess, before addTab would be slightly better. I'm saying that because sometimes in your patch the register calls happen before addTab and sometimes after.
Let's be consistent.

@@ +23,5 @@
>    });
>  
> +  info("Loading test page: " + testPageURL);
> +  gBrowser.selectedBrowser.loadURI(testPageURL);
> +  await waitForMessage(true, gBrowser);

Nit: Sometimes there's a blank line between loadURI and waitForMessage, and sometimes not. Let's be consistent here too.
I think no blank line is ok.

::: dom/quota/test/head.js
@@ +86,5 @@
> +          {persisted: event.data});
> +      }, {once: true}, true);
> +    }
> +
> +    let script = "data:,(" + contentScript.toString() + ")();";

Nit: Add a blank line here.
Attachment #8882276 - Flags: feedback+
(In reply to Jan Varga [:janv] from comment #42)
> Comment on attachment 8882276 [details] [diff] [review]
> Bug 1362228 - Make sure messageManager.loadFrameScript() is called before
> window.postMessage()
> 
> Review of attachment 8882276 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Just fix the nits and that's it. You don't have to push to try again.
> 
> ::: dom/quota/test/browser_permissionsPromptAllow.js
> @@ +19,5 @@
> >      triggerMainCommand(this);
> >    });
> >    registerPopupEventHandler("popuphidden", function () {
> >      ok(true, "prompt hidden");
> >    });
> 
> Nit: These 3 register calls should go either before or after addTab call.
> I guess, before addTab would be slightly better. I'm saying that because
> sometimes in your patch the register calls happen before addTab and
> sometimes after.
> Let's be consistent.
Sure.
> 
> @@ +23,5 @@
> >    });
> >  
> > +  info("Loading test page: " + testPageURL);
> > +  gBrowser.selectedBrowser.loadURI(testPageURL);
> > +  await waitForMessage(true, gBrowser);
> 
> Nit: Sometimes there's a blank line between loadURI and waitForMessage, and
> sometimes not. Let's be consistent here too.
> I think no blank line is ok.
> 
Okay
> ::: dom/quota/test/head.js
> @@ +86,5 @@
> > +          {persisted: event.data});
> > +      }, {once: true}, true);
> > +    }
> > +
> > +    let script = "data:,(" + contentScript.toString() + ")();";
> 
> Nit: Add a blank line here.
Okay
Comment on attachment 8882582 [details] [diff] [review]
Bug 1362228 - Make sure messageManager.loadFrameScript() is called before window.postMessage()

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

Looks good!
Attachment #8882582 - Flags: review?(jvarga) → review+
Pushed by shuang@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/394dd98ad362
Make sure messageManager.loadFrameScript() is called before window.postMessage(), r=janv
Whiteboard: [stockwell needswork][ETA:6/13] → [stockwell fixed:timing]
https://hg.mozilla.org/mozilla-central/rev/394dd98ad362
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.