Intermittent browser_permissionsPromptAllow.js | Test timed out, Found a tab after previous test timed out: browser_permissionsPrompt.html -

RESOLVED FIXED in Firefox 56

Status

()

Core
DOM: IndexedDB
P5
normal
RESOLVED FIXED
2 years ago
2 months ago

People

(Reporter: KWierso, Assigned: shawnjohnjr)

Tracking

(Blocks: 1 bug, {intermittent-failure})

45 Branch
mozilla56
intermittent-failure
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+, firefox45 affected, firefox56 fixed)

Details

Attachments

(3 attachments, 18 obsolete attachments)

3.27 KB, patch
Details | Diff | Splinter Review
21.99 KB, patch
Details | Diff | Splinter Review
700 bytes, patch
Details | Diff | Splinter Review
(Reporter)

Description

2 years ago
https://treeherder.mozilla.org/logviewer.html#?job_id=16868313&repo=mozilla-inbound

Comment 1

2 years ago
7 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* fx-team: 4
* mozilla-inbound: 2
* try: 1

Platform breakdown:
* linux64: 7

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2015-11-09&endday=2015-11-15&tree=all

Comment 2

2 years ago
6 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 2
* fx-team: 2
* try: 1
* mozilla-aurora: 1

Platform breakdown:
* linux64: 3
* linux32: 3

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-02-22&endday=2016-02-28&tree=all
Blocks: 984139
tracking-e10s: --- → ?

Updated

2 years ago
tracking-e10s: ? → +

Comment 3

2 years ago
11 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 10
* fx-team: 1

Platform breakdown:
* linux32: 6
* linux64: 5

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-02-29&endday=2016-03-06&tree=all

Comment 4

2 years ago
6 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 2
* fx-team: 2
* mozilla-central: 1
* mozilla-aurora: 1

Platform breakdown:
* linux64: 5
* linux32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-04-18&endday=2016-04-24&tree=all

Comment 5

2 years ago
5 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 4
* mozilla-aurora: 1

Platform breakdown:
* linux64: 5

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-04-25&endday=2016-05-01&tree=all

Comment 6

2 years ago
7 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 5
* mozilla-central: 1
* fx-team: 1

Platform breakdown:
* linux64: 6
* linux32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-05-10&endday=2016-05-16&tree=all

Comment 7

2 years ago
5 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 2
* mozilla-central: 2
* mozilla-beta: 1

Platform breakdown:
* linux64: 4
* linux32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-05-16&endday=2016-05-22&tree=all

Comment 8

2 years ago
8 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 4
* mozilla-central: 2
* fx-team: 2

Platform breakdown:
* linux64: 5
* linux32: 3

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-05-23&endday=2016-05-29&tree=all

Comment 9

2 years ago
6 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 4
* mozilla-central: 1
* fx-team: 1

Platform breakdown:
* linux64: 4
* linux32: 2

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-05-30&endday=2016-06-05&tree=all

Comment 10

2 years ago
6 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 4
* mozilla-central: 1
* fx-team: 1

Platform breakdown:
* linux64: 4
* windows7-32: 1
* linux32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-06-06&endday=2016-06-12&tree=all
8 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 2
* mozilla-aurora: 2
* fx-team: 2
* try: 1
* mozilla-central: 1

Platform breakdown:
* linux32: 3
* windows7-32: 2
* windows8-64: 1
* windows7-32-vm: 1
* linux64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-06-20&endday=2016-06-26&tree=all
13 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 5
* autoland: 4
* fx-team: 2
* mozilla-beta: 1
* mozilla-aurora: 1

Platform breakdown:
* windows7-32: 10
* linux64: 2
* windows8-64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-06-27&endday=2016-07-03&tree=all
Intermittent e10s test failure
Priority: -- → P5
5 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 2
* autoland: 2
* mozilla-central: 1

Platform breakdown:
* windows7-32: 2
* linux64: 2
* linux32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-07-04&endday=2016-07-10&tree=all
9 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* autoland: 3
* mozilla-central: 2
* fx-team: 2
* mozilla-inbound: 1
* mozilla-aurora: 1

Platform breakdown:
* linux64: 6
* windows7-32: 2
* linux32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-07-11&endday=2016-07-17&tree=all
7 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 4
* mozilla-central: 2
* autoland: 1

Platform breakdown:
* linux32: 4
* linux64: 2
* windows7-32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-07-18&endday=2016-07-24&tree=all
9 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 7
* mozilla-central: 2

Platform breakdown:
* linux64: 5
* windows7-32: 3
* linux32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-08-01&endday=2016-08-07&tree=all
14 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 9
* autoland: 3
* fx-team: 2

Platform breakdown:
* linux64: 6
* linux32: 5
* windows7-32-vm: 3

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-08-22&endday=2016-08-28&tree=all
5 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 4
* mozilla-beta: 1

Platform breakdown:
* linux64: 3
* windows7-32-vm: 1
* linux32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-08-29&endday=2016-09-04&tree=all
9 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 4
* autoland: 3
* try: 1
* mozilla-central: 1

Platform breakdown:
* linux64: 5
* windows7-32-vm: 4

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-09-05&endday=2016-09-11&tree=all
8 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 4
* mozilla-central: 1
* mozilla-aurora: 1
* fx-team: 1
* autoland: 1

Platform breakdown:
* linux64: 4
* windows7-32-vm: 3
* linux32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-09-12&endday=2016-09-18&tree=all
Summary: Intermittent browser_permissionsPromptAllow.js | Found a tab after previous test timed out: browser_permissionsPrompt.html - → Intermittent browser_permissionsPromptAllow.js | Test timed out, Found a tab after previous test timed out: browser_permissionsPrompt.html -
5 automation job failures were associated with this bug in the last 7 days.

Repository breakdown:
* mozilla-inbound: 2
* autoland: 2
* try: 1

Platform breakdown:
* linux64: 4
* windows7-32-vm: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2016-10-03&endday=2016-10-09&tree=all

Comment 23

6 months ago
15 failures in 891 pushes (0.017 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* autoland: 8
* mozilla-inbound: 4
* mozilla-central: 3

Platform breakdown:
* osx-10-10: 15

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2017-05-22&endday=2017-05-28&tree=all

Comment 24

6 months ago
12 failures in 820 pushes (0.015 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* mozilla-inbound: 7
* autoland: 3
* mozilla-central: 1
* cedar: 1

Platform breakdown:
* osx-10-10: 12

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2017-05-29&endday=2017-06-04&tree=all

Comment 25

5 months ago
6 failures in 814 pushes (0.007 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* autoland: 3
* mozilla-inbound: 1
* mozilla-central: 1
* cedar: 1

Platform breakdown:
* osx-10-10: 5
* linux64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2017-06-12&endday=2017-06-18&tree=all

Comment 26

5 months ago
8 failures in 892 pushes (0.009 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* autoland: 4
* mozilla-inbound: 3
* mozilla-central: 1

Platform breakdown:
* osx-10-10: 8

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2017-06-19&endday=2017-06-25&tree=all

Updated

5 months ago
See Also: → bug 1324163
(Assignee)

Updated

5 months ago
Assignee: nobody → shuang

Comment 27

5 months ago
10 failures in 718 pushes (0.014 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* mozilla-inbound: 6
* autoland: 3
* mozilla-central: 1

Platform breakdown:
* osx-10-10: 9
* linux64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2017-06-26&endday=2017-07-02&tree=all
(Assignee)

Comment 28

5 months ago
try to add more logs:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=94c4046fa2bee46f30d49e6816454f2160767ab7
(Assignee)

Comment 29

5 months ago
log:
https://public-artifacts.taskcluster.net/QBsCeMQKQtGjwl27n3GEWQ/0/public/logs/live_backing.log
(Assignee)

Comment 30

5 months ago
(In reply to Shawn Huang [:shawnjohnjr] from comment #28)
> try to add more logs:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=94c4046fa2bee46f30d49e6816454f2160767ab7

https://treeherder.mozilla.org/logviewer.html#?job_id=111828186&repo=try&lineNumber=3101
(Assignee)

Comment 31

5 months ago
19:40:55     INFO - TEST-START | dom/indexedDB/test/browser_permissionsPromptAllow.js
19:40:55     INFO - GECKO(33583) | !! test1
19:40:55     INFO - GECKO(33583) | !! test 1 creating tab
19:40:55     INFO - GECKO(33583) | !! test 1 addeventlistener
19:40:55     INFO - GECKO(33583) | loading test page: http://mochi.test:8888/browser/dom/indexedDB/test/browser_permissionsPrompt.html
19:40:55     INFO - GECKO(33583) | !! test 1 load
19:40:55     INFO - GECKO(33583) | !! wait for event
19:40:55     INFO - GECKO(33583) | !! in spawn
19:40:56     INFO - GECKO(33583) | !! got event
19:40:56     INFO - GECKO(33583) | !! event.type == success
19:40:56     INFO - GECKO(33583) | !! event.preventDefault();
19:40:56     INFO - GECKO(33583) | !! finishTest
19:40:56     INFO - GECKO(33583) | xxxxxx done !! got finished callback
19:40:56     INFO - GECKO(33583) | !! resolve
19:40:56     INFO - GECKO(33583) | !! callback result, exception
19:40:56     INFO - GECKO(33583) | !! test 1 setFinishedCallback
19:40:56     INFO - GECKO(33583) | !! test 1 execute test2
19:40:56     INFO - GECKO(33583) | !! test2
19:40:56     INFO - GECKO(33583) | !! test 2 creating tab
19:40:56     INFO - GECKO(33583) | loading test page: http://mochi.test:8888/browser/dom/indexedDB/test/browser_permissionsPrompt.html
19:40:56     INFO - GECKO(33583) | !! test 2 load
19:40:56     INFO - GECKO(33583) | !! in spawn
19:40:56     INFO - GECKO(33583) | !! wait for event
19:40:56     INFO - GECKO(33583) | !! got event
19:40:56     INFO - GECKO(33583) | !! event.type == success
19:40:56     INFO - GECKO(33583) | !! event.preventDefault();
19:40:56     INFO - GECKO(33583) | !! finishTest
19:41:40     INFO - GECKO(33583) | xxxxxx done
19:41:40     INFO - TEST-INFO | started process screencapture
19:41:41     INFO - TEST-INFO | screencapture: exit 0
19:41:41     INFO - Buffered messages logged at 19:40:55
19:41:41     INFO - creating tab
19:41:41     INFO - loading test page: http://mochi.test:8888/browser/dom/indexedDB/test/browser_permissionsPrompt.html
19:41:41     INFO - TEST-PASS | dom/indexedDB/test/browser_permissionsPromptAllow.js | prompt showing - 
19:41:41     INFO - Buffered messages logged at 19:40:56
19:41:41     INFO - TEST-PASS | dom/indexedDB/test/browser_permissionsPromptAllow.js | prompt shown - 
19:41:41     INFO - triggering main command
19:41:41     INFO - TEST-PASS | dom/indexedDB/test/browser_permissionsPromptAllow.js | at least one notification displayed - 
19:41:41     INFO - triggering command: Allow Storing Data
19:41:41     INFO - TEST-PASS | dom/indexedDB/test/browser_permissionsPromptAllow.js | prompt hidden - 
19:41:41     INFO - got finished callback
19:41:41     INFO - TEST-PASS | dom/indexedDB/test/browser_permissionsPromptAllow.js | First database creation was successful - 
19:41:41     INFO - TEST-PASS | dom/indexedDB/test/browser_permissionsPromptAllow.js | No exception - 
19:41:41     INFO - TEST-PASS | dom/indexedDB/test/browser_permissionsPromptAllow.js | Correct permission set - 
19:41:41     INFO - creating tab
19:41:41     INFO - loading test page: http://mochi.test:8888/browser/dom/indexedDB/test/browser_permissionsPrompt.html
19:41:41     INFO - Buffered messages finished
19:41:41     INFO - TEST-UNEXPECTED-FAIL | dom/indexedDB/test/browser_permissionsPromptAllow.js | Test timed out -
(Assignee)

Comment 32

5 months ago
How come "window.testFinishedCallback" is null in this case?

good case
00:09:16     INFO - GECKO(19156) | !! test2
00:09:16     INFO - GECKO(19156) | !! test 2 creating tab
00:09:16     INFO - GECKO(19156) | loading test page: http://mochi.test:8888/browser/dom/indexedDB/test/browser_permissionsPrompt.html
00:09:16     INFO - GECKO(19156) | !! test 2 load
00:09:16     INFO - GECKO(19156) | !! wait for event
00:09:16     INFO - GECKO(19156) | !! in spawn
00:09:16     INFO - GECKO(19156) | !! got event
00:09:16     INFO - GECKO(19156) | !! event.type == success
00:09:16     INFO - GECKO(19156) | !! event.preventDefault();
00:09:16     INFO - GECKO(19156) | !! finishTest
00:09:16     INFO - GECKO(19156) | !! entering finishTest()
00:09:16     INFO - GECKO(19156) | !! calling finishTestNow()
00:09:16     INFO - GECKO(19156) | xxxxxx done entering finishTestNow
00:09:16     INFO - GECKO(19156) | testGenerator return
00:09:16     INFO - GECKO(19156) | testGenerator = undefined
00:09:16     INFO - GECKO(19156) | !! entering setTimeout
00:09:16     INFO - GECKO(19156) | !! calling window.testFinishedCallback()
00:09:16     INFO - GECKO(19156) | !! got finished callback
00:09:16     INFO - GECKO(19156) | !! resolve
00:09:16     INFO - GECKO(19156) | !! callback result, exception
00:09:16     INFO - GECKO(19156) | !! test 2 setFinishedCallback
00:09:16     INFO - GECKO(19156) | !! test 2 execute finish
00:09:16     INFO - GECKO(19156) | entering finishTestNow



bad case

00:10:06     INFO - GECKO(34055) | !! test2
00:10:06     INFO - GECKO(34055) | !! test 2 creating tab
00:10:07     INFO - GECKO(34055) | loading test page: http://mochi.test:8888/browser/dom/indexedDB/test/browser_permissionsPrompt.html
00:10:07     INFO - GECKO(34055) | !! test 2 load
00:10:07     INFO - GECKO(34055) | !! in spawn
00:10:07     INFO - GECKO(34055) | !! wait for event
00:10:07     INFO - GECKO(34055) | !! got event
00:10:07     INFO - GECKO(34055) | !! event.type == success
00:10:07     INFO - GECKO(34055) | !! event.preventDefault();
00:10:07     INFO - GECKO(34055) | !! finishTest
00:10:07     INFO - GECKO(34055) | !! entering finishTest()
00:10:07     INFO - GECKO(34055) | !! calling finishTestNow()
00:10:07     INFO - GECKO(34055) | xxxxxx done entering finishTestNow
00:10:07     INFO - GECKO(34055) | testGenerator return
00:10:07     INFO - GECKO(34055) | testGenerator = undefined
00:10:07     INFO - GECKO(34055) | !! entering setTimeout
00:10:07     INFO - GECKO(34055) | !! calling window.testFinishedCallback is null
00:10:51     INFO - TEST-INFO | started process screencapture
00:10:51     INFO - TEST-INFO | screencapture: exit 0
00:10:51     INFO - Buffered messages logged at 00:10:06
(Assignee)

Comment 33

5 months ago
Okay, so in the previous experience, log can't tell the execution sequence between chrome and content window. It's possible that testFinishedCallback() is executed after finishTest() from content is called. So window.testFinishedCallback is null.
(Assignee)

Comment 34

5 months ago
Currently setFinishedCallback() problem impacts:
Bug 1222282: dom/indexedDB/test/browser_forgetThisSite.js
Bug 1222284: dom/indexedDB/test/browser_permissionsPromptAllow.js
Bug 1334950: dom/indexedDB/test/browser_permissionsPromptWorker.js
Bug 1224872: dom/indexedDB/test/browser_perwindow_privateBrowsing.js

So I think it will be better to fix these tests since there are so many test cases failure on muli-e10s.
(Assignee)

Updated

5 months ago
Blocks: 1222282
(Assignee)

Updated

5 months ago
Blocks: 1334950
(Assignee)

Updated

5 months ago
Blocks: 1224872
(Assignee)

Comment 35

5 months ago
(In reply to Shawn Huang [:shawnjohnjr] from comment #33)
> Okay, so in the previous experience, log can't tell the execution sequence
> between chrome and content window. It's possible that testFinishedCallback()
> is executed after finishTest() from content is called. So
> window.testFinishedCallback is null.

http://searchfox.org/mozilla-central/rev/e8f4f51cd543f203e9cb861cecb7545ac43c836c/dom/indexedDB/test/browserHelpers.js#31
(Assignee)

Comment 36

5 months ago
I discussed with Jan, it should be okay to keep postMessage only instead of doing setFinishedCallback to avoid such race condition.

Comment 37

5 months ago
13 failures in 656 pushes (0.02 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* mozilla-inbound: 5
* autoland: 5
* oak: 2
* try: 1

Platform breakdown:
* osx-cross: 10
* osx-10-10: 3

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2017-07-03&endday=2017-07-09&tree=all
(Assignee)

Comment 38

5 months ago
Created attachment 8885562 [details] [diff] [review]
Bug 1222284 - Part 1: Replace setFinishedCallback with waitForMessage in browser_permissionsPromptAllow.js
(Assignee)

Comment 39

5 months ago
Created attachment 8886057 [details] [diff] [review]
Bug 1222284 - Part 2: Replace setFinishedCallback with waitForMessage in browser_permissionsPromptWorker.js
(Assignee)

Comment 40

5 months ago
Created attachment 8886092 [details] [diff] [review]
Bug 1222284 - Part 3: Replace setFinishedCallback with waitForMessage in browser_perwindow_privateBrowsing.js
(Assignee)

Comment 41

5 months ago
Created attachment 8886093 [details] [diff] [review]
Bug 1222284 - Part 4: Replace setFinishedCallback with waitForMessage in browser_forgetThisSite.js
(Assignee)

Updated

5 months ago
Attachment #8886093 - Attachment is obsolete: true
(Assignee)

Comment 42

5 months ago
Created attachment 8886097 [details] [diff] [review]
Bug 1222284 - Part 4: Replace setFinishedCallback with waitForMessage in browser_forgetThisSite.js
(Assignee)

Updated

5 months ago
Attachment #8886057 - Attachment is obsolete: true
(Assignee)

Comment 43

5 months ago
Created attachment 8886099 [details] [diff] [review]
Bug 1222284 - Part 2: Replace setFinishedCallback with waitForMessage in browser_permissionsPromptWorker.js
(Assignee)

Updated

5 months ago
Duplicate of this bug: 1334950
(Assignee)

Updated

5 months ago
Duplicate of this bug: 1224872
(Assignee)

Updated

5 months ago
Duplicate of this bug: 1222282
(Assignee)

Comment 47

5 months ago
Created attachment 8886101 [details] [diff] [review]
Bug 1222284 - Part 5: Remove setFinishedCallback
(Assignee)

Updated

5 months ago
Attachment #8885562 - Flags: review?(btseng)
(Assignee)

Updated

5 months ago
Attachment #8886092 - Flags: review?(btseng)
(Assignee)

Updated

5 months ago
Attachment #8886097 - Flags: review?(btseng)
(Assignee)

Updated

5 months ago
Attachment #8886099 - Flags: review?(btseng)
(Assignee)

Updated

5 months ago
Attachment #8886101 - Flags: review?(btseng)
(Assignee)

Comment 48

5 months ago
This is just syncing solution from bug 1324163 and remove setFinishedCallback. I guess it will make sense to ask Bevis to review.

Comment 49

5 months ago
Ok, I would probably consider to replace:
content.location = testPageURL;
with:
gBrowser.selectedBrowser.loadURI(testPageURL);

to be in sync what you've done recently.

Comment 50

5 months ago
Comment on attachment 8885562 [details] [diff] [review]
Bug 1222284 - Part 1: Replace setFinishedCallback with waitForMessage in browser_permissionsPromptAllow.js

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

You still keep keep testResult and testException, but after setFinishedCallback removal, these two are not handled separately it seems.
I think it should be somehow fixed/unified.

::: dom/indexedDB/test/head.js
@@ +98,5 @@
> +    /* eslint-disable no-undef */
> +    function contentScript() {
> +      addEventListener("message", function(event) {
> +        sendAsyncMessage("testLocal:exception",
> +          {exception: event.data});

"exception" here can contain either result or exception, right ?

Comment 51

5 months ago
(In reply to Jan Varga [:janv] from comment #50)
> You still keep keep testResult and testException, but after
> setFinishedCallback removal, these two are not handled separately it seems.
> I think it should be somehow fixed/unified.

I think either waitForMessage() should get another argument for the exception or exception should be removed everywhere. I don't know what is better/doable since I don't have that much time.
I'll let Bevis to decide ...

Thanks guys.
(Assignee)

Comment 52

5 months ago
(In reply to Jan Varga [:janv] from comment #49)
> Ok, I would probably consider to replace:
> content.location = testPageURL;
> with:
> gBrowser.selectedBrowser.loadURI(testPageURL);
> 
> to be in sync what you've done recently.

Sure. It makes sense.
(Assignee)

Comment 53

5 months ago
(In reply to Jan Varga [:janv] from comment #51)
> (In reply to Jan Varga [:janv] from comment #50)
> > You still keep keep testResult and testException, but after
> > setFinishedCallback removal, these two are not handled separately it seems.
> > I think it should be somehow fixed/unified.
> 
> I think either waitForMessage() should get another argument for the
> exception or exception should be removed everywhere. I don't know what is
> better/doable since I don't have that much time.
> I'll let Bevis to decide ...
> 
> Thanks guys.

I will try to remove exception and keep only one variable. Thanks.
(Assignee)

Comment 54

4 months ago
Comment on attachment 8885562 [details] [diff] [review]
Bug 1222284 - Part 1: Replace setFinishedCallback with waitForMessage in browser_permissionsPromptAllow.js

I will fix issues commented by Jan. Cancel review.
Attachment #8885562 - Attachment is obsolete: true
Attachment #8885562 - Flags: review?(btseng)

Comment 55

4 months ago
3 failures in 720 pushes (0.004 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* mozilla-inbound: 2
* try: 1

Platform breakdown:
* osx-cross: 2
* osx-10-10: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2017-07-10&endday=2017-07-16&tree=all
(Assignee)

Updated

4 months ago
Attachment #8886092 - Attachment is obsolete: true
Attachment #8886092 - Flags: review?(btseng)
(Assignee)

Updated

4 months ago
Attachment #8886097 - Flags: review?(btseng)
(Assignee)

Updated

4 months ago
Attachment #8886097 - Attachment is obsolete: true
(Assignee)

Updated

4 months ago
Attachment #8886099 - Attachment is obsolete: true
Attachment #8886099 - Flags: review?(btseng)
(Assignee)

Updated

4 months ago
Attachment #8886101 - Attachment is obsolete: true
Attachment #8886101 - Flags: review?(btseng)
(Assignee)

Comment 56

4 months ago
Created attachment 8886964 [details] [diff] [review]
Bug 1222284 - Part 1: Replace setFinishedCallback with waitForMessage in browser_permissionsPromptAllow.js
(Assignee)

Comment 57

4 months ago
Created attachment 8886965 [details] [diff] [review]
Bug 1222284 - Part 2: Replace setFinishedCallback with waitForMessage in browser_permissionsPromptWorker.js
(Assignee)

Comment 58

4 months ago
Created attachment 8886966 [details] [diff] [review]
Bug 1222284 - Part 3: Replace setFinishedCallback with waitForMessage in browser_perwindow_privateBrowsing.js
(Assignee)

Comment 59

4 months ago
Created attachment 8886967 [details] [diff] [review]
Bug 1222284 - Part 4: Replace setFinishedCallback with waitForMessage in browser_forgetThisSite.js
(Assignee)

Comment 60

4 months ago
Created attachment 8886968 [details] [diff] [review]
Bug 1222284 - Part 5: Remove setFinishedCallback and testException
(Assignee)

Updated

4 months ago
Attachment #8886968 - Attachment is obsolete: true
(Assignee)

Updated

4 months ago
Attachment #8886965 - Attachment is obsolete: true
(Assignee)

Comment 61

4 months ago
Created attachment 8886971 [details] [diff] [review]
Bug 1222284 - Part 2: Replace setFinishedCallback with waitForMessage in browser_permissionsPromptWorker.js
(Assignee)

Comment 62

4 months ago
Created attachment 8886972 [details] [diff] [review]
Bug 1222284 - Part 5: Remove setFinishedCallback and testException
(Assignee)

Updated

4 months ago
Attachment #8886964 - Flags: review?(btseng)
(Assignee)

Updated

4 months ago
Attachment #8886971 - Flags: review?(btseng)
(Assignee)

Updated

4 months ago
Attachment #8886966 - Flags: review?(btseng)
(Assignee)

Updated

4 months ago
Attachment #8886967 - Flags: review?(btseng)
(Assignee)

Updated

4 months ago
Attachment #8886972 - Flags: review?(btseng)
Comment on attachment 8886964 [details] [diff] [review]
Bug 1222284 - Part 1: Replace setFinishedCallback with waitForMessage in browser_permissionsPromptAllow.js

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

I'd like to see the patches to be arranged as followed and revisit them again:
1. Part 1 only includes the change in browserHelpers.js and head.js of all these parts at once to tell the reader what utility functions to be changed.
2. and Part 2 only includes the changes of these browser_*.(js|html) according to the new utility functions.
You can have browser.ini in a standalone part 3.

In addition, you can explain more about how the new layout of each browser test looks like in the commit message of part 1.
For example, in patch:
- waitForExplicitFinish() will be replaced with add_task().
- gBrowser.selectedBrowser.addEventListener() will be replaced with waitForMessage().
- testFinishedCallback, testResult, and testException will be unified to testResult.

Then, we can focus on the review of patch part 2 after change of part 1 is reviewed.
Attachment #8886964 - Flags: review?(btseng)
(Assignee)

Comment 64

4 months ago
Sure. I will re-arrange the patch set :)
(Assignee)

Updated

4 months ago
Attachment #8886966 - Attachment is obsolete: true
Attachment #8886966 - Flags: review?(btseng)
(Assignee)

Updated

4 months ago
Attachment #8886967 - Attachment is obsolete: true
Attachment #8886967 - Flags: review?(btseng)
(Assignee)

Updated

4 months ago
Attachment #8886971 - Attachment is obsolete: true
Attachment #8886971 - Flags: review?(btseng)
(Assignee)

Updated

4 months ago
Attachment #8886972 - Attachment is obsolete: true
Attachment #8886972 - Flags: review?(btseng)
(Assignee)

Updated

4 months ago
Attachment #8886964 - Attachment is obsolete: true
(Assignee)

Comment 65

4 months ago
Created attachment 8887380 [details] [diff] [review]
Bug 1222284 - Part 1: Replace setFinishedCallback with waitForMessage in browser_permissionsPromptAllow.js
(Assignee)

Comment 66

4 months ago
Created attachment 8887381 [details] [diff] [review]
Bug 1222284 - Part 2: Replace setFinishedCallback with waitForMessage for browser tests
(Assignee)

Comment 67

4 months ago
Created attachment 8887382 [details] [diff] [review]
Bug 1222284 - Part 3: Enable test browser_permissionsPromptWorker.js again on mac
(Assignee)

Updated

4 months ago
Attachment #8887381 - Attachment is obsolete: true
(Assignee)

Comment 68

4 months ago
Created attachment 8887384 [details] [diff] [review]
Bug 1222284 - Part 2: Replace setFinishedCallback with waitForMessage for browser tests
(Assignee)

Updated

4 months ago
Attachment #8887380 - Flags: review?(btseng)
(Assignee)

Updated

4 months ago
Attachment #8887384 - Flags: review?(btseng)
(Assignee)

Updated

4 months ago
Attachment #8887382 - Flags: review?(btseng)
(Assignee)

Comment 69

4 months ago
> In addition, you can explain more about how the new layout of each browser test looks like in the commit message of part 1.
> For example, in patch:
> - waitForExplicitFinish() will be replaced with add_task().
> - gBrowser.selectedBrowser.addEventListener() will be replaced with waitForMessage().
> - testFinishedCallback, testResult, and testException will be unified to testResult.

I prefer to put summary into Part 2.
Attachment #8887380 - Flags: review?(btseng) → review+
Attachment #8887382 - Flags: review?(btseng) → review+
Comment on attachment 8887384 [details] [diff] [review]
Bug 1222284 - Part 2: Replace setFinishedCallback with waitForMessage for browser tests

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

r=me, after the following nits are addressed, thanks!

::: dom/indexedDB/test/browser_permissionsSharedWorker.html
@@ +10,1 @@
>      let testIsIDBDatabase;

nit: we never use this variable. Please remove it as well.

::: dom/indexedDB/test/browser_permissionsWorker.html
@@ +10,1 @@
>      let testIsIDBDatabase;

ditto.
Attachment #8887384 - Flags: review?(btseng) → review+
(Assignee)

Comment 71

4 months ago
(In reply to Bevis Tseng [:bevis][:btseng] from comment #70)
> Comment on attachment 8887384 [details] [diff] [review]
> Bug 1222284 - Part 2: Replace setFinishedCallback with waitForMessage for
> browser tests
> 
> Review of attachment 8887384 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me, after the following nits are addressed, thanks!
> 
> ::: dom/indexedDB/test/browser_permissionsSharedWorker.html
> @@ +10,1 @@
> >      let testIsIDBDatabase;
> 
> nit: we never use this variable. Please remove it as well.
> 
> ::: dom/indexedDB/test/browser_permissionsWorker.html
> @@ +10,1 @@
> >      let testIsIDBDatabase;
> 
> ditto.
Sure. I was wondering if I need to do it but considering this is not related to the original issue, I choose to ignore it.
I will remove it.
Thanks!
(Assignee)

Updated

4 months ago
Attachment #8887380 - Attachment is obsolete: true
(Assignee)

Updated

4 months ago
Attachment #8887382 - Attachment is obsolete: true
(Assignee)

Updated

4 months ago
Attachment #8887384 - Attachment is obsolete: true
(Assignee)

Comment 72

4 months ago
Created attachment 8887416 [details] [diff] [review]
Bug 1222284 - Part 1: Replace setFinishedCallback with waitForMessage, r=btseng
(Assignee)

Comment 73

4 months ago
Created attachment 8887417 [details] [diff] [review]
Bug 1222284 - Part 2: Replace setFinishedCallback with waitForMessage for browser tests, r=btseng
(Assignee)

Comment 74

4 months ago
Created attachment 8887419 [details] [diff] [review]
Bug 1222284 - Part 3: Enable test browser_permissionsPromptWorker.js again on mac, r=btseng
(Assignee)

Comment 75

4 months ago
try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1029ee532e3392b4131c19c47784031a90ada091
(Assignee)

Comment 76

4 months ago
I'm not sure why try server is so slow. I will wait for a while anyway.

Comment 77

4 months ago
Pushed by shuang@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7ebcd8b7d58
Part 1: Replace setFinishedCallback with waitForMessage, r=btseng
https://hg.mozilla.org/integration/mozilla-inbound/rev/c515128e7461
Part 2: Replace setFinishedCallback with waitForMessage for browser tests, r=btseng
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c96d3de9c7c
Part 3: Enable test browser_permissionsPromptWorker.js again on mac, r=btseng
(Assignee)

Comment 78

4 months ago
(In reply to Shawn Huang [:shawnjohnjr] from comment #75)
> try:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=1029ee532e3392b4131c19c47784031a90ada091

Running:
OS X 10.10 opt 50+ times
OS X 10.10 dgb 70+ times

I did not see any indexeddb test cases failure. It looks good now.
https://hg.mozilla.org/mozilla-central/rev/e7ebcd8b7d58
https://hg.mozilla.org/mozilla-central/rev/c515128e7461
https://hg.mozilla.org/mozilla-central/rev/3c96d3de9c7c
Status: NEW → RESOLVED
Last Resolved: 4 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56

Comment 80

4 months ago
1 failures in 822 pushes (0.001 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* mozilla-inbound: 1

Platform breakdown:
* osx-10-10: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2017-07-17&endday=2017-07-23&tree=all

Comment 81

4 months ago
1 failures in 888 pushes (0.001 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* mozilla-beta: 1

Platform breakdown:
* linux32-devedition: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2017-07-31&endday=2017-08-06&tree=all

Comment 82

3 months ago
2 failures in 901 pushes (0.002 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* mozilla-beta: 2

Platform breakdown:
* linux64-nightly: 1
* linux64-devedition: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2017-08-07&endday=2017-08-13&tree=all

Comment 83

2 months ago
1 failures in 885 pushes (0.001 failures/push) were associated with this bug in the last 7 days.    

Repository breakdown:
* mozilla-release: 1

Platform breakdown:
* linux64-nightly: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1222284&startday=2017-09-25&endday=2017-10-01&tree=all
You need to log in before you can comment on or make changes to this bug.