1442167 - Intermittent browser/base/content/test/general/browser_newWindowDrop.js | application terminated with exit code 1

Status: RESOLVED FIXED

(Firefox :: General, defect, P5)

Description

[Treeherder Bug Filer]

Filed by: toros [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=164837500&repo=mozilla-central

https://queue.taskcluster.net/v1/task/f6nxou1cRSClXGdT94t5RA/runs/0/artifacts/public/logs/live_backing.log

12:37:56     INFO -  128 INFO TEST-START | browser/base/content/test/general/browser_newWindowDrop.js
12:38:06     INFO -  GECKO(5164) | MEMORY STAT | vsize 1649MB | vsizeMaxContiguous 135MB | residentFast 1184MB | heapAllocated 558MB
12:38:06     INFO -  129 INFO TEST-OK | browser/base/content/test/general/browser_newWindowDrop.js | took 9552ms
12:38:06     INFO -  130 INFO checking window state
12:38:14     INFO -  TEST-INFO | Main app process: exit 1
12:38:14     INFO -  Buffered messages finished
12:38:14    ERROR -  131 ERROR TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_newWindowDrop.js | application terminated with exit code 1
12:38:14     INFO -  runtests.py | Application ran for: 0:01:41.013000
12:38:14     INFO -  zombiecheck | Reading PID log: c:\users\genericworker\appdata\local\temp\tmprmdpylpidlog
12:38:14     INFO -  ==> process 5164 launched child process 4768 ("Z:\task_1519820001\build\application\firefox\firefox.exe" -contentproc --channel="5164.0.1218705890\642589755" -childID 1 -isForBrowser -intPrefs 43:0|45:0|64:0|69:0|75:0|170:1000|179:0|181:0|195:32|196:256|244:2| -boolPrefs 5:1|37:1|60:1|61:0|121:1|130:1|134:1|144:1|149:1|156:1|182:1|194:1|211:0|297:0| -stringPrefs 216:4;0.01|283:38;{6a2b9df4-ca8f-4392-ad37-ef71f711815d}| -schedulerPrefs 0001,2 -greomni "Z:\task_1519820001\build\application\firefox\omni.ja" -appomni "Z:\task_1519820001\build\application\firefox\browser\omni.ja" -appdir "Z:\task_1519820001\build\application\firefox\browser"  5164 "\\.\pipe\gecko-crash-server-pipe.5164" 1832 tab)
12:38:14     INFO -  ==> process 5164 launched child process 1824 ("Z:\task_1519820001\build\application\firefox\firefox.exe" -contentproc --channel="5164.6.2050809873\1274789255" -childID 2 -isForBrowser -intPrefs 43:0|45:0|64:0|69:0|75:0|170:1000|179:0|181:0|195:32|196:256|244:2| -boolPrefs 5:1|37:1|60:1|61:0|121:1|130:1|134:1|144:1|149:1|156:1|182:1|194:1|211:0|297:0| -stringPrefs 216:4;0.01|283:38;{6a2b9df4-ca8f-4392-ad37-ef71f711815d}| -schedulerPrefs 0001,2 -greomni "Z:\task_1519820001\build\application\firefox\omni.ja" -appomni "Z:\task_1519820001\build\application\firefox\browser\omni.ja" -appdir "Z:\task_1519820001\build\application\firefox\browser"  5164 "\\.\pipe\gecko-crash-server-pipe.5164" 2184 tab)
12:38:14     INFO -  ==> process 5164 launched child process 3784 ("Z:\task_1519820001\build\application\firefox\firefox.exe" -contentproc --channel="5164.12.146237035\2000952846" -childID 3 -isForBrowser -intPrefs 43:0|45:0|64:0|69:0|75:0|170:1000|179:0|181:0|195:32|196:256|244:2| -boolPrefs 5:1|37:1|60:1|61:0|121:1|130:1|134:1|144:1|149:1|156:1|182:1|194:1|211:0|297:0| -stringPrefs 216:4;0.01|283:38;{6a2b9df4-ca8f-4392-ad37-ef71f711815d}| -schedulerPrefs 0001,2 -greomni "Z:\task_1519820001\build\application\firefox\omni.ja" -appomni "Z:\task_1519820001\build\application\firefox\browser\omni.ja" -appdir "Z:\task_1519820001\build\application\firefox\browser"  5164 "\\.\pipe\gecko-crash-server-pipe.5164" 2816 tab)
12:38:14     INFO -  ==> process 5164 launched child process 4448 ("Z:\task_1519820001\build\application\firefox\firefox.exe" -contentproc --channel="5164.18.1613176193\453528976" -childID 4 -isForBrowser -intPrefs 43:0|45:0|64:0|69:0|75:0|170:1000|179:0|181:0|195:32|196:256|244:2| -boolPrefs 5:1|37:1|60:1|61:0|121:1|130:1|134:1|144:1|149:1|156:1|182:1|194:1|211:0|297:0| -stringPrefs 216:4;0.01|283:38;{6a2b9df4-ca8f-4392-ad37-ef71f711815d}| -schedulerPrefs 0001,2 -greomni "Z:\task_1519820001\build\application\firefox\omni.ja" -appomni "Z:\task_1519820001\build\application\firefox\browser\omni.ja" -appdir "Z:\task_1519820001\build\application\firefox\browser"  5164 "\\.\pipe\gecko-crash-server-pipe.5164" 2800 tab)
12:38:14     INFO -  zombiecheck | Checking for orphan process with PID: 4768
12:38:14     INFO -  zombiecheck | Checking for orphan process with PID: 1824
12:38:14     INFO -  zombiecheck | Checking for orphan process with PID: 4448
12:38:14     INFO -  zombiecheck | Checking for orphan process with PID: 3784
12:38:14     INFO -  mozcrash Downloading symbols from: https://queue.taskcluster.net/v1/task/JpmUJ4YqQSuWJEEly2rUxg/artifacts/public/build/target.crashreporter-symbols.zip
12:38:20     INFO -  mozcrash Copy/paste: Z:\task_1519820001\build\win32-minidump_stackwalk.exe c:\users\genericworker\appdata\local\temp\tmprqcz_u.mozrunner\minidumps\12be28b0-c06a-466d-a5f3-13176dea56d7.dmp c:\users\genericworker\appdata\local\temp\tmp0rn8kp
12:38:29     INFO -  mozcrash Saved minidump as Z:\task_1519820001\build\blobber_upload_dir\12be28b0-c06a-466d-a5f3-13176dea56d7.dmp
12:38:29     INFO -  mozcrash Saved app info as Z:\task_1519820001\build\blobber_upload_dir\12be28b0-c06a-466d-a5f3-13176dea56d7.extra
12:38:29  WARNING -  PROCESS-CRASH | browser/base/content/test/general/browser_newWindowDrop.js | application crashed [@ PtrToNodeMatchEntry]
12:38:29     INFO -  Crash dump filename: c:\users\genericworker\appdata\local\temp\tmprqcz_u.mozrunner\minidumps\12be28b0-c06a-466d-a5f3-13176dea56d7.dmp

Comment 1

[Intermittent Failures Robot]

4 failures in 831 pushes (0.005 failures/push) were associated with this bug in the last 7 days.    

Repository breakdown:
* mozilla-central: 4

Platform breakdown:
* windows7-32: 4

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1442167&startday=2018-02-26&endday=2018-03-04&tree=all

Comment 2

[Tooru Fujisawa [:arai]]

looks like browser_newTabDrop.js's testcase was wrong and it's opening more than expected tabs
since "non-url" is fixed to URL :P
we should use "non url" instead

Comment 3

[Tooru Fujisawa [:arai]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=492bdc77ddb7e2ae57eefe529f2a58ad01573315

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Use text that will not be fixed to URL when testing non-URL case.

so, the issue is that, browser_newTabDrop.js runs before browser_newWindowDrop.js, and browser_newTabDrop.js opens more tabs than expected, and it finishes before waiting for all of them,
and unexpected request is triggered while running the next test browser_newWindowDrop.js.

here's the try run with this issue in browser_newTabDrop.js fixed
https://treeherder.mozilla.org/#/jobs?repo=try&revision=492bdc77ddb7e2ae57eefe529f2a58ad01573315
and apparently the crash in browser_newWindowDrop.js disappears

Comment 5

[:Gijs (he/him)]

Comment on attachment 8957042 [details] [diff] [review]
Use text that will not be fixed to URL when testing non-URL case.

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

::: browser/base/content/test/general/browser_newTabDrop.js
@@ +109,5 @@
>  add_task(async function multiple_text() {
>    await dropText(`
> +    non url0
> +    non url1
> +    non url2

Seems to me the test should be failing if it's opening more tabs than expected, esp. here.

Can you add code to that effect inside `drop` ?

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: Use text that will not be fixed to URL when testing non-URL case.

Thank you for reviewing :)

To catch extra tabs, I changed BrowserTestUtils.waitForEvent to bare addEventListener, and keep listening after the expected number of tabs are opened.
and then checks the number of tabs at the end of the test, after removing the listener.

the test fails without fixing "non-url".

Comment 7

[:Gijs (he/him)]

Comment on attachment 8957141 [details] [diff] [review]
Use text that will not be fixed to URL when testing non-URL case.

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

Nice, r=me with one more change, see below.

::: browser/base/content/test/general/browser_newTabDrop.js
@@ +162,5 @@
>    await awaitDrop;
>    ok(true, "Got drop event");
> +
> +  if (expectedTabOpenCount) {
> +    gBrowser.tabContainer.removeEventListener("TabOpen", handler);

So even with these modifications we won't detect tabs opened for cases where `expectedTabOpenCount` is 0, because we won't add the listener.

Can we both add and remove the listener even when `expectedTabOpenCount` is 0? We'd need to check the `if (promiseTabsOpen)` check to check for `expectedTabOpenCount` instead, but otherwise that should be fine.

Comment 8

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e7c9b954c90f74553b342351d11eaad7f4faf8dc
Bug 1442167 - Use text that will not be fixed to URL when testing non-URL case. r=Gijs

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/e7c9b954c90f

Comment 10

[Mark Banner (:standard8)]

Sorry for jumping in here, but you pointed me to this from another bug...

(In reply to Tooru Fujisawa [:arai] from comment #4)
> so, the issue is that, browser_newTabDrop.js runs before
> browser_newWindowDrop.js, and browser_newTabDrop.js opens more tabs than
> expected, and it finishes before waiting for all of them,
> and unexpected request is triggered while running the next test
> browser_newWindowDrop.js.

I'm not convinced that this is the actual issue. Looking at orange factor, these are all "test-verify" raised issues. Test-verify runs each test in turn multiple times, shutting down Firefox & restarting in-between each test. It only runs one test per cycle.

You won't have seen any effects on try, as you didn't run the test-verify tests, and the patch here only affects browser_newTabDrop.js so browser_newWindowDrop.js wouldn't have been run again within test-verify.

As I understand the logs, the actual issue here is that browser_newWindowDrop.js caused a crash (or a crash occurred during it).

For some reason, the logs indicate that browser_tabDrop.js fails as well, but there's no actual reported failure, and I think that is a consequence of the browser_newWindowDrop.js failure.

So I'm not sure this patch fixed the issue here - that being that browser_newWindowDrop.js can crash. You could probably do a dummy push to try that trivially changed that test and ran test-verify. Note it looks like bug 1442014 was also filed at roughly the same time as this one, so that one could take on the full fix.

Comment 11

[Tooru Fujisawa [:arai]]

The tests will be fixed in bug 1448531
The crash doesn't happen for me when I run the test with --verify.