Intermittent browser_mozLoop_chat.js | Condition didn't pass. -

RESOLVED FIXED in Firefox 46

Status

Hello (Loop)
Client
P2
normal
Rank:
20
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: KWierso, Assigned: mikedeboer)

Tracking

({intermittent-failure})

unspecified
mozilla48
Unspecified
Mac OS X
intermittent-failure
Points:
8
Dependency tree / graph

Firefox Tracking Flags

(firefox45 wontfix, firefox46 fixed, firefox47 fixed, firefox48 fixed, firefox-esr45 wontfix)

Details

(Whiteboard: [btpp-fix-now][btpp-follow-up-2016-03-10])

Attachments

(6 attachments)

(Reporter)

Description

3 years ago
https://treeherder.mozilla.org/logviewer.html#?job_id=3473802&repo=b2g-inbound


The first instances of this I'm seeing are on this merge from m-c to b2g-inbound:
https://hg.mozilla.org/integration/b2g-inbound/pushloghtml?changeset=21f92638bb14

I'm seeing a bunch of Loop changes in there. Any ideas, Mike/Mark?
Flags: needinfo?(standard8)
Flags: needinfo?(mdeboer)
(Assignee)

Comment 1

3 years ago
This looks like the big move to an add-on, although I can't tell you what might be the exact cause of this issue.
Except that - looking at the logs - the `promiseWaitForCondition` in the 'test_mozLoop_hangupAllChatWindows' test doesn't seem to resolve.

The only way I *think* we can attempt to fix this is to print all the open chatboxes and see why the 'LoopHangupNow' event doesn't close the chat window.
Blocks: 1223573
Flags: needinfo?(mdeboer)
Mike's right - I also can't tell think what the issue would be. I'll tag myself for doing some try pushes.
Assignee: nobody → standard8
Flags: needinfo?(standard8)
(Assignee)

Comment 3

3 years ago
Mark, please also see the change I propose to the test in bug 1227539. We're passing the wrong argument to LoopRooms.open, for sure.

Comment 4

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

Repository breakdown:
* mozilla-inbound: 8
* fx-team: 6
* b2g-inbound: 6
* mozilla-central: 3
* try: 1

Platform breakdown:
* osx-10-6: 23
* windows8-64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1229195&startday=2015-11-30&endday=2015-12-06&tree=all

Comment 5

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

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

Platform breakdown:
* osx-10-6: 12
* windows7-32: 2
* windows8-64: 1

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

Comment 6

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

Repository breakdown:
* mozilla-inbound: 12
* fx-team: 11
* b2g-inbound: 5
* try: 2
* mozilla-central: 1

Platform breakdown:
* osx-10-6: 18
* windows7-32: 10
* linux32: 2
* windows8-64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1229195&startday=2015-12-14&endday=2015-12-20&tree=all
(Assignee)

Updated

3 years ago
Depends on: 1227539

Comment 7

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

Repository breakdown:
* mozilla-inbound: 6
* mozilla-aurora: 4
* try: 3
* mozilla-central: 2
* fx-team: 2
* b2g-inbound: 1

Platform breakdown:
* osx-10-6: 10
* windows7-32: 6
* windowsxp: 1
* linux32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1229195&startday=2015-12-21&endday=2015-12-27&tree=all
This is either fixed or a lot rarer now. Its a bit hard to say, but I'm going to mark it as fixed. If it continues being an issue then we can get a fresh bug filed.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Mike, this seems to have been started again, or something very similar, since we landed the e10s stuff:

https://treeherder.mozilla.org/#/jobs?repo=fx-team&fromchange=f09ea2712445&selectedJob=7064227

 14:35:38     INFO -  222 INFO Entering test test_mozLoop_hangupAllChatWindows
 14:35:38     INFO -  223 INFO Console message: [JavaScript Warning: "Unknown pseudo-class or pseudo-element '-webkit-input-placeholder'.  Ruleset ignored due to bad selector." {file: "chrome://loop/content/shared/css/conversation.css" line: 1035 column: 31 source: ".text-chat-box > form > input::-webkit-input-placeholder {"}]
 14:35:38     INFO -  224 INFO Console message: [JavaScript Warning: "Unknown pseudo-class or pseudo-element '-ms-input-placeholder'.  Ruleset ignored due to bad selector." {file: "chrome://loop/content/shared/css/conversation.css" line: 1050 column: 30 source: ".text-chat-box > form > input:-ms-input-placeholder {"}]
 14:35:38     INFO -  225 INFO Console message: [JavaScript Warning: "Unknown pseudo-class or pseudo-element 'input-placeholder'.  Ruleset ignored due to bad selector." {file: "chrome://loop/content/shared/css/conversation.css" line: 1055 column: 30 source: ".text-chat-box > form > input:input-placeholder {"}]
 14:35:38     INFO -  226 INFO Console message: [JavaScript Warning: "Unknown pseudo-class or pseudo-element '-webkit-input-placeholder'.  Ruleset ignored due to bad selector." {file: "chrome://loop/content/shared/css/conversation.css" line: 1035 column: 31 source: ".text-chat-box > form > input::-webkit-input-placeholder {"}]
 14:35:38     INFO -  227 INFO Console message: [JavaScript Warning: "Unknown pseudo-class or pseudo-element '-ms-input-placeholder'.  Ruleset ignored due to bad selector." {file: "chrome://loop/content/shared/css/conversation.css" line: 1050 column: 30 source: ".text-chat-box > form > input:-ms-input-placeholder {"}]
 14:35:38     INFO -  228 INFO Console message: [JavaScript Warning: "Unknown pseudo-class or pseudo-element 'input-placeholder'.  Ruleset ignored due to bad selector." {file: "chrome://loop/content/shared/css/conversation.css" line: 1055 column: 30 source: ".text-chat-box > form > input:input-placeholder {"}]
 14:35:38     INFO -  229 INFO Console message: [JavaScript Warning: "unreachable code after return statement" {file: "chrome://loop/content/shared/vendor/sdk.js" line: 11464 column: 4 source: "    eval(obj);
 14:35:38     INFO -  "}]
 14:35:38     INFO -  230 INFO Console message: [JavaScript Warning: "unreachable code after return statement" {file: "chrome://loop/content/shared/vendor/sdk.js" line: 11464 column: 4 source: "    eval(obj);
 14:35:38     INFO -  "}]
 14:35:38     INFO -  231 INFO Console message: [JavaScript Warning: "mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create" {file: "chrome://loop/content/shared/vendor/sdk.js" line: 12164}]
 14:35:38     INFO -  232 INFO Console message: [JavaScript Warning: "unreachable code after return statement" {file: "chrome://loop/content/shared/vendor/sdk.js" line: 11464 column: 4 source: "    eval(obj);
 14:35:38     INFO -  "}]
 14:35:38     INFO -  233 INFO Console message: [JavaScript Warning: "unreachable code after return statement" {file: "chrome://loop/content/shared/vendor/sdk.js" line: 11464 column: 4 source: "    eval(obj);
 14:35:38     INFO -  "}]
 14:35:38     INFO -  234 INFO Console message: [JavaScript Warning: "mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create" {file: "chrome://loop/content/shared/vendor/sdk.js" line: 12164}]
 14:35:38     INFO -  235 INFO Console message: [JavaScript Error: "The Components object is deprecated. It will soon be removed." {file: "chrome://loop/content/shared/js/utils.js" line: 9}]
 14:35:38     INFO -  236 INFO Console message: [JavaScript Error: "The Components object is deprecated. It will soon be removed." {file: "chrome://loop/content/shared/js/utils.js" line: 9}]
 14:35:38     INFO -  237 INFO Console message: [JavaScript Warning: "Empty string passed to getElementById()." {file: "chrome://global/content/bindings/remote-browser.xml" line: 222}]
 14:35:38     INFO -  238 INFO Console message: [JavaScript Warning: "Empty string passed to getElementById()." {file: "chrome://global/content/bindings/remote-browser.xml" line: 222}]
 14:35:38     INFO -  239 INFO Console message: [JavaScript Error: "TypeError: content is null" {file: "chrome://browser/content/social-content.js" line: 62}]
 14:35:38     INFO -  240 INFO TEST-UNEXPECTED-FAIL | browser/extensions/loop/chrome/test/mochitest/browser_mozLoop_chat.js | Condition didn't pass. -
 14:35:38     INFO -  Stack trace:
 14:35:38     INFO -  chrome://mochitests/content/browser/browser/extensions/loop/chrome/test/mochitest/head.js:waitForCondition/interval<:78
 14:35:38     INFO -  Not taking screenshot here: see the one that was previously logged
 14:35:38     INFO -  241 INFO TEST-UNEXPECTED-FAIL | browser/extensions/loop/chrome/test/mochitest/browser_mozLoop_chat.js | chat window should have been closed - false == true - JS frame :: chrome://mochitests/content/browser/browser/extensions/loop/chrome/test/mochitest/browser_mozLoop_chat.js :: test_mozLoop_hangupAllChatWindows :: line 38
 14:35:38     INFO -  Stack trace:
 14:35:38     INFO -      chrome://mochitests/content/browser/browser/extensions/loop/chrome/test/mochitest/browser_mozLoop_chat.js:test_mozLoop_hangupAllChatWindows:38
 14:35:38     INFO -      Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937:23
 14:35:38     INFO -      this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816:7
 14:35:38     INFO -      Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
 14:35:38     INFO -      this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
 14:35:38     INFO -      Promise.prototype.then@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:454:5
 14:35:38     INFO -      this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:705:7
 14:35:38     INFO -      TaskImpl_run@resource://gre/modules/Task.jsm:324:13
 14:35:38     INFO -      TaskImpl@resource://gre/modules/Task.jsm:280:3
 14:35:38     INFO -      createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:254:14
 14:35:38     INFO -      Tester_execTest/<@chrome://mochikit/content/browser-test.js:808:21
 14:35:38     INFO -      TaskImpl_run@resource://gre/modules/Task.jsm:319:40
 14:35:38     INFO -      TaskImpl@resource://gre/modules/Task.jsm:280:3
 14:35:38     INFO -      createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:254:14
 14:35:38     INFO -      Task_spawn@resource://gre/modules/Task.jsm:168:12
 14:35:38     INFO -      Tester_execTest@chrome://mochikit/content/browser-test.js:803:9
 14:35:38     INFO -      Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:723:7
 14:35:38     INFO -      SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:741:59
 14:35:38     INFO -  242 INFO Leaving test test_mozLoop_hangupAllChatWindows
Status: RESOLVED → REOPENED
Flags: needinfo?(mdeboer)
Resolution: FIXED → ---
(Assignee)

Updated

2 years ago
Assignee: standard8 → mdeboer
Status: REOPENED → ASSIGNED
Flags: needinfo?(mdeboer)

Comment 12

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

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

Platform breakdown:
* linux64: 13
* linux32: 4
* windows8-64: 3
* osx-10-6: 2
* osx-10-10: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1229195&startday=2016-02-01&endday=2016-02-07&tree=all
Created attachment 8717038 [details] [review]
[loop] mikedeboer:bug-1229195-dayum > mozilla:master
(Assignee)

Comment 16

2 years ago
Created attachment 8717039 [details] [diff] [review]
Patch 1: explicitly link 'content' to the frame script global scope
Attachment #8717039 - Flags: review?(standard8)
(Assignee)

Updated

2 years ago
Attachment #8717038 - Flags: review?(standard8)

Comment 17

2 years ago
17 automation job failures were associated with this bug yesterday.

Repository breakdown:
* mozilla-inbound: 11
* fx-team: 5
* try: 1

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

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1229195&startday=2016-02-08&endday=2016-02-08&tree=all
Attachment #8717039 - Flags: review?(standard8) → review+
Comment on attachment 8717038 [details] [review]
[loop] mikedeboer:bug-1229195-dayum > mozilla:master

Looks good. r=Standard8
Attachment #8717038 - Flags: review?(standard8) → review+
(Assignee)

Updated

2 years ago
Iteration: --- → 47.2 - Feb 22
Points: --- → 8
(Assignee)

Comment 19

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/0c51994fbafe65be49c253ed58390f1ef4de691c
Bug 1229195: explicitly link 'content' to the frame script global scope to prevent out-of-scope errors. r=Standard8
(Assignee)

Updated

2 years ago
Blocks: 1245937

Comment 21

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0c51994fbafe
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47

Comment 22

2 years ago
19 automation job failures were associated with this bug yesterday.

Repository breakdown:
* mozilla-inbound: 14
* fx-team: 3
* try: 1
* mozilla-central: 1

Platform breakdown:
* linux64: 11
* linux32: 6
* windowsxp: 1
* windows7-32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1229195&startday=2016-02-10&endday=2016-02-10&tree=all
(Reporter)

Comment 23

2 years ago
This is still happening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 24

2 years ago
17 automation job failures were associated with this bug yesterday.

Repository breakdown:
* mozilla-inbound: 11
* mozilla-central: 3
* fx-team: 3

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

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

Comment 25

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

Repository breakdown:
* mozilla-inbound: 48
* fx-team: 17
* mozilla-central: 7
* try: 4
* mozilla-aurora: 2

Platform breakdown:
* linux64: 54
* linux32: 12
* windows7-32: 6
* windows8-64: 3
* windowsxp: 2
* osx-10-10: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1229195&startday=2016-02-08&endday=2016-02-14&tree=all
(Assignee)

Comment 26

2 years ago
Comment on attachment 8717039 [details] [diff] [review]
Patch 1: explicitly link 'content' to the frame script global scope

Approval Request Comment
[Feature/regressing bug #]: Bug 1154277
[User impact if declined]: Patches in bug 1154277 increased the orange factor of this intermittent. This patch reduces that number, albeit not eliminating entirely.
[Describe test coverage new/current, TreeHerder]: Landed on m-c.
[Risks and why]: minor.
[String/UUID change made/needed]: n/a.
Attachment #8717039 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 27

2 years ago
(In reply to Wes Kocher (:KWierso) from comment #23)
> This is still happening.

This probably _does_ need the Loop/ Hello add-on update to improve the situation.
status-firefox45: --- → affected
status-firefox46: --- → affected
Comment on attachment 8717039 [details] [diff] [review]
Patch 1: explicitly link 'content' to the frame script global scope

Improve tests, taking it.
Attachment #8717039 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8717039 [details] [diff] [review]
Patch 1: explicitly link 'content' to the frame script global scope

[Triage Comment]
Needed for beta too. Should be in 45 beta 6.
Attachment #8717039 - Flags: approval-mozilla-beta+

Comment 30

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/2a42ee967471
status-firefox46: affected → fixed
(Assignee)

Comment 31

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/27634a68a77f
status-firefox45: affected → fixed
backed this out in https://hg.mozilla.org/releases/mozilla-beta/rev/b454ec296bf6 on request from sylvestre for bustage like :

https://treeherder.mozilla.org/logviewer.html#?job_id=810287&repo=mozilla-beta
status-firefox45: fixed → affected
Flags: needinfo?(mdeboer)

Comment 33

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/a4b809b4d3b1
status-firefox45: affected → fixed
Flags: needinfo?(mdeboer)

Comment 34

2 years ago
15 automation job failures were associated with this bug yesterday.

Repository breakdown:
* mozilla-inbound: 7
* fx-team: 6
* mozilla-central: 2

Platform breakdown:
* linux64: 7
* windows7-32: 4
* osx-10-10: 2
* osx-10-6: 1
* linux32: 1

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

Comment 35

2 years ago
19 automation job failures were associated with this bug yesterday.

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

Platform breakdown:
* linux64: 12
* linux32: 3
* windows8-64: 2
* windowsxp: 1
* osx-10-10: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1229195&startday=2016-02-17&endday=2016-02-17&tree=all
Created attachment 8720760 [details] [review]
[loop] mikedeboer:bug-1229195-waitLonger > mozilla:master
(Assignee)

Updated

2 years ago
Status: REOPENED → ASSIGNED
(Assignee)

Comment 38

2 years ago
Comment on attachment 8720760 [details] [review]
[loop] mikedeboer:bug-1229195-waitLonger > mozilla:master

What I've been able to gather from the logs so far is:

1) `content is null` errors in social-content.js is still the most common error, especially on Linux X64. This was not occurring in the patch I pushed to try before last, which returned early with a guard. I pushed a similar patch to try just moments ago, so I can reconfirm.
2) Beta does not have the Map.prototype#clear removal, so there are JS errors occurring in MozLoopService.jsm. This likely doesn't cause the test to fail, but it's certainly not helping things.
3) I other logs I just see the regular flow of messages, but the `promiseWaitForCondition` just times out. So I'd like to up the amount of time we wait in a test to at least 20 seconds. Right now it's 10 seconds. I could live with a minute too.
Attachment #8720760 - Flags: review?(mdeboer)
(Assignee)

Updated

2 years ago
status-firefox45: fixed → affected
status-firefox46: fixed → affected
status-firefox47: fixed → affected
Target Milestone: mozilla47 → mozilla45
(Assignee)

Updated

2 years ago
Attachment #8720760 - Flags: review?(mdeboer) → review?(standard8)
Comment on attachment 8720760 [details] [review]
[loop] mikedeboer:bug-1229195-waitLonger > mozilla:master

r=Standard8 with the comment addition requested in the bug.
Attachment #8720760 - Flags: review?(standard8) → review+
(Assignee)

Comment 41

2 years ago
Created attachment 8720840 [details] [diff] [review]
Patch 2: ignore messages when content is null
Attachment #8720840 - Flags: review?(standard8)
Blocks: 1248604
Rank: 15
Priority: -- → P1
Whiteboard: [btpp-fix-now]

Comment 42

2 years ago
18 automation job failures were associated with this bug yesterday.

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

Platform breakdown:
* linux64: 8
* linux32: 5
* windows7-32: 2
* osx-10-6: 2
* osx-10-10: 1

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

Comment 43

2 years ago
(In reply to Mike de Boer [:mikedeboer] from comment #41)
> Created attachment 8720840 [details] [diff] [review]
> Patch 2: ignore messages when content is null

Why is content even null here? In what circumstances? And isn't this then going to catch out any other message listener in the same script?
Flags: needinfo?(mdeboer)
(Assignee)

Comment 45

2 years ago
(In reply to :Gijs Kruitbosch from comment #43)
> Why is content even null here?

If I knew, I wouldn't have put the guard here but attempted a proper fix. ;-)

> In what circumstances?

This is happening when a message arrives in the script when the browser element is being destroyed by a) the chatbox is removed from the DOM or b) the detached dialog window is closed.
What I think is happening is that the listeners have a longer lifetime than the `content` object has, which is evidently already GC'd whilst the listeners are alive and kicking.
In my try pushes, I've only seen messages being skipped over of the type 'Social:CustomEvent' and event type 'Loop:HangupNow'. This doesn't affect the unit tests, as the windows are closed properly after all.

> And isn't this then
> going to catch out any other message listener in the same script?

Well in this case no, because there's no point for _any_ of the listeners to be invoked when `content` is null.
Flags: needinfo?(mdeboer)

Comment 46

2 years ago
(In reply to Mike de Boer [:mikedeboer] from comment #45)
> (In reply to :Gijs Kruitbosch from comment #43)
> > And isn't this then
> > going to catch out any other message listener in the same script?
> 
> Well in this case no, because there's no point for _any_ of the listeners to
> be invoked when `content` is null.

Yes, I guess what I meant was: whenever we set content to null, it feels like we should remove all the listeners that were attached from within that scope and/or stop sending messages to them.
Comment on attachment 8720840 [details] [diff] [review]
Patch 2: ignore messages when content is null

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

You might want to consider removing the listeners as well as per Gijs' comment.
Attachment #8720840 - Flags: review?(standard8) → review+

Comment 48

2 years ago
(In reply to Mark Banner (:standard8) from comment #47)
> Comment on attachment 8720840 [details] [diff] [review]
> Patch 2: ignore messages when content is null
> 
> Review of attachment 8720840 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> You might want to consider removing the listeners as well as per Gijs'
> comment.

To try to be even more explicit: I think whatever is GC'ing content / clearing things up here should be responsible for killing the listeners / messages. So we should find out what it is and fix it "properly" or we'll just run into this next time someone does whatever Hello is doing here.
(Assignee)

Comment 49

2 years ago
(In reply to :Gijs Kruitbosch from comment #48)
> To try to be even more explicit: I think whatever is GC'ing content /
> clearing things up here should be responsible for killing the listeners /
> messages. So we should find out what it is and fix it "properly" or we'll
> just run into this next time someone does whatever Hello is doing here.

I agree. I'll file another bug to investigate this further. Landing this fix for a frequently failing test is good for tree health.
(Assignee)

Updated

2 years ago
Blocks: 1250871

Comment 51

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d785fddce4c1
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago2 years ago
status-firefox47: affected → fixed
Resolution: --- → FIXED

Comment 52

2 years ago
16 automation job failures were associated with this bug yesterday.

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

Platform breakdown:
* linux64: 8
* windows7-32: 3
* linux32: 3
* windowsxp: 1
* windows8-64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1229195&startday=2016-02-26&endday=2016-02-26&tree=all
OrangeFactor confirms that this is still hitting regularly since comment 51.
Status: RESOLVED → REOPENED
Flags: needinfo?(mdeboer)
Resolution: FIXED → ---
status-firefox47: fixed → affected
Target Milestone: mozilla45 → ---

Comment 54

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

Repository breakdown:
* mozilla-inbound: 31
* try: 24
* fx-team: 19
* mozilla-aurora: 5
* mozilla-central: 4

Platform breakdown:
* linux64: 39
* osx-10-6: 19
* linux32: 9
* windows8-64: 6
* windows7-32: 6
* windowsxp: 4

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1229195&startday=2016-02-22&endday=2016-02-28&tree=all
(Assignee)

Comment 55

2 years ago
Next Fx Hello update will have a `waitForCondition` update to allow waiting a longer amount of time.
Flags: needinfo?(mdeboer)
FTR, the additional fix landed in fx-team yesterday, and mozilla-central today.

Comment 57

2 years ago
22 automation job failures were associated with this bug yesterday.

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

Platform breakdown:
* linux64: 11
* linux32: 6
* windowsxp: 2
* windows8-64: 1
* osx-10-6: 1
* osx-10-10: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1229195&startday=2016-03-01&endday=2016-03-01&tree=all
Whiteboard: [btpp-fix-now] → [btpp-fix-now][btpp-follow-up-2016-03-10]

Comment 58

2 years ago
23 automation job failures were associated with this bug yesterday.

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

Platform breakdown:
* linux64: 13
* linux32: 4
* osx-10-6: 3
* windows7-32: 2
* windowsxp: 1

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

Comment 59

2 years ago
30 automation job failures were associated with this bug yesterday.

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

Platform breakdown:
* linux64: 14
* linux32: 8
* osx-10-10: 4
* windows8-64: 2
* windowsxp: 1
* osx-10-6: 1

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

Comment 60

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

Repository breakdown:
* mozilla-inbound: 66
* fx-team: 36
* try: 12
* mozilla-aurora: 9
* mozilla-central: 4

Platform breakdown:
* linux64: 65
* linux32: 30
* osx-10-6: 9
* osx-10-10: 8
* windows8-64: 6
* windowsxp: 5
* windows7-32: 4

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

Comment 61

2 years ago
17 automation job failures were associated with this bug yesterday.

Repository breakdown:
* mozilla-inbound: 5
* try: 4
* fx-team: 4
* mozilla-beta: 3
* mozilla-central: 1

Platform breakdown:
* linux64: 11
* linux32: 4
* windows8-64: 1
* windows7-32: 1

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

Comment 62

2 years ago
19 automation job failures were associated with this bug yesterday.

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

Platform breakdown:
* linux64: 11
* linux32: 5
* windowsxp: 1
* windows8-64: 1
* osx-10-6: 1

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

Comment 63

2 years ago
25 automation job failures were associated with this bug yesterday.

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

Platform breakdown:
* linux64: 13
* linux32: 6
* windows7-32: 3
* windowsxp: 1
* windows8-64: 1
* osx-10-6: 1

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

Updated

2 years ago
No longer blocks: 1248604
Rank: 15 → 20
Priority: P1 → P2
Given the ongoing high failure rate, I think it's disabling time (especially since it appears the the priority on fixing it was just decreased).
Flags: needinfo?(mdeboer)

Comment 65

2 years ago
15 automation job failures were associated with this bug yesterday.

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

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

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

Comment 66

2 years ago
15 automation job failures were associated with this bug yesterday.

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

Platform breakdown:
* linux64: 11
* linux32: 2
* windows8-64: 1
* osx-10-6: 1

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

Comment 67

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

Repository breakdown:
* mozilla-inbound: 50
* fx-team: 29
* try: 12
* mozilla-central: 6
* mozilla-aurora: 6
* mozilla-beta: 4

Platform breakdown:
* linux64: 65
* linux32: 19
* windows8-64: 7
* windows7-32: 7
* windowsxp: 3
* osx-10-6: 3
* osx-10-10: 3

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

Comment 68

2 years ago
Nah, I'm going to try and fix it this week.
Flags: needinfo?(mdeboer)
(Assignee)

Comment 70

2 years ago
Created attachment 8730648 [details] [diff] [review]
Patch 3: resolve the promise when the chat window has actually finished loading, not right away

Well, if this isn't worth a facepalm, I don't know what is. Oboy.
Attachment #8730648 - Flags: review?(standard8)
(Assignee)

Comment 71

2 years ago
Try is looking _very_ green.
Comment on attachment 8730648 [details] [diff] [review]
Patch 3: resolve the promise when the chat window has actually finished loading, not right away

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

r+ once you've improved the comments.

::: browser/extensions/loop/chrome/content/modules/MozLoopService.jsm
@@ +1117,5 @@
>          // away to circumvent glitches.
>          chatboxInstance.setAttribute("customSize", "loopDefault");
>          chatboxInstance.parentNode.setAttribute("customSize", "loopDefault");
>          Chat.loadButtonSet(chatboxInstance, "minimize,swap," + kChatboxHangupButton.id);
> +      } else {

As discussed, it'd be good to move the unit test comment to this section, and make it a bit clearer as to what it is relating to.
Attachment #8730648 - Flags: review?(standard8) → review+
(Assignee)

Comment 73

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/c1516ee3efd8a0329fccf93ba0398c69ed296ea4
Bug 1229195: resolve the promise when the chat window has actually finished loading, not right away. r=Standard8
(Assignee)

Updated

2 years ago
Status: REOPENED → ASSIGNED
Created attachment 8730791 [details] [review]
[loop] mikedeboer:bug-1229195-resolveSomethingSomething > mozilla:master
status-firefox45: affected → wontfix
status-firefox48: --- → affected
status-firefox-esr45: --- → affected
When you ready, please request uplift of this to whatever branches won't pick up the fix automatically via an upstream update (I'm looking at you, esr45).
Flags: needinfo?(mdeboer)
(Reporter)

Comment 77

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c1516ee3efd8
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago2 years ago
status-firefox48: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48

Comment 78

2 years ago
17 automation job failures were associated with this bug yesterday.

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

Platform breakdown:
* linux64: 7
* windows7-32: 3
* linux32: 3
* osx-10-6: 2
* windowsxp: 1
* windows8-64: 1

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

Comment 79

2 years ago
Comment on attachment 8730648 [details] [diff] [review]
Patch 3: resolve the promise when the chat window has actually finished loading, not right away

[Approval Request Comment]
User impact if declined: This patch is required to land to fix a frequent intermittent failure in the browser_mozLoop_chat.js browser-chrome mochitest.
Fix Landed on Version: 48 (current m-c)
Risk to taking this patch (and alternatives if risky): minor, please approve m-a and m-b first if that's more convenient.
String or UUID changes made by this patch: n/a.
Flags: needinfo?(mdeboer)
Attachment #8730648 - Flags: approval-mozilla-esr45?
Attachment #8730648 - Flags: approval-mozilla-beta?
Attachment #8730648 - Flags: approval-mozilla-aurora?
Comment on attachment 8730648 [details] [diff] [review]
Patch 3: resolve the promise when the chat window has actually finished loading, not right away

One-line fix for an intermittent failure, Aurora47+, Beta46+
Attachment #8730648 - Flags: approval-mozilla-beta?
Attachment #8730648 - Flags: approval-mozilla-beta+
Attachment #8730648 - Flags: approval-mozilla-aurora?
Attachment #8730648 - Flags: approval-mozilla-aurora+

Comment 81

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/f94242ec4259
status-firefox47: affected → fixed

Comment 82

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/d7acfd3f4683
status-firefox46: affected → fixed

Comment 83

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

Repository breakdown:
* mozilla-inbound: 19
* mozilla-beta: 9
* mozilla-aurora: 5
* try: 2
* mozilla-central: 1
* fx-team: 1

Platform breakdown:
* linux64: 18
* linux32: 6
* windows7-32: 4
* windows8-64: 3
* osx-10-6: 3
* windowsxp: 2
* osx-10-10: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1229195&startday=2016-03-14&endday=2016-03-20&tree=all
Comment on attachment 8730648 [details] [diff] [review]
Patch 3: resolve the promise when the chat window has actually finished loading, not right away


Simplify the life of sheriff, taking it.
Should be in 45.1.0
Attachment #8730648 - Flags: approval-mozilla-esr45? → approval-mozilla-esr45+
(Reporter)

Comment 85

2 years ago
This doesn't apply cleanly to esr45, can we see what's missing there?
Flags: needinfo?(mdeboer)
(Reporter)

Updated

2 years ago
Flags: needinfo?(sledru)
This can wait (and hello is disabled in hello)
Flags: needinfo?(sledru)
(Assignee)

Comment 87

2 years ago
Since Hello is disabled on ESR, no need to push it up there.
Flags: needinfo?(mdeboer)
so mike can we won't fix this then for esr45 ?
Flags: needinfo?(mdeboer)
(Assignee)

Comment 89

2 years ago
Aye!
status-firefox-esr45: affected → wontfix
Flags: needinfo?(mdeboer)
You need to log in before you can comment on or make changes to this bug.