Closed Bug 1206457 Opened 9 years ago Closed 8 years ago

Intermittent e10s browser_LoopRooms_channel.js | Test timed out

Categories

(Hello (Loop) :: Client, defect, P2)

Product:
Hello (Loop)
Component:
Client
Platform:
x86_64
Linux
Type:
defect
Points:
2

Tracking

(e10s+, firefox44 fixed)

Status:
RESOLVED INCOMPLETE
Milestone:
mozilla44
Tracking Flags:
Tracking Status
e10s + ---
firefox44 --- fixed

People

(Reporter: philor, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: intermittent-failure, Whiteboard: [todo-akita])

Attachments

(2 files)

Fix intermittent browser_LoopRooms_channel.js time out, set up the back channel earlier to avoid loading time/setup issues.
6.14 KB, patch
mikedeboer
: review+
Details | Diff | Splinter Review
Second attempt to fix intermittent failure in browser_LoopRooms_channel.js - ensure previous states are cleared when they are used.
1.70 KB, patch
mikedeboer
: review+
Details | Diff | Splinter Review 
      No description provided.
Keywords: leave-open
Whiteboard: [test disabled on ASan e10s]
Tentatively taking.
Assignee: nobody → standard8
This should fix it - the idea here is to set up the channel once and use it where necessary so that it always exists before we do the page load. Then we also check if the data has already been sent back either before or after we're ready for it.

Try seems to like this approach:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=81fe3a5c6c21
Attachment #8663732 - Flags: review?(mdeboer)
Comment on attachment 8663732 [details] [diff] [review]
Fix intermittent browser_LoopRooms_channel.js time out, set up the back channel earlier to avoid loading time/setup issues.

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

Thanks for fixing this!

::: browser/components/loop/test/mochitest/browser_LoopRooms_channel.js
@@ +23,5 @@
>  
>  var fakeRoomList = new Map([[ ROOM_TOKEN, { roomToken: ROOM_TOKEN } ]]);
>  
> +function BackChannel(uri) {
> +  this.init(uri);

You might as well put the `init` function here, instead of a function on the prototype.

@@ +44,5 @@
> +      this.receivedData = data;
> +    });
> +  },
> +
> +  teardown: function() {

nit: somehow I'm used to 'tearDown' instead of 'teardown' :)

@@ +49,5 @@
> +    this.channel.stopListening();
> +  }
> +};
> +
> +var goodBackChannel;

nit: please make these `gGoodBackChannel` and `gBadBackChannel`?

@@ +70,5 @@
>      url: uri.spec + "#" + hash
>    }, () => waitForChannelPromise);
>  }
>  
> +add_task(function* setup() {

You *could* make this an `add_test`, but that will require more boilerplate to actually run it, so... it's better this way :) You can add a `yield undefined` at the bottom if needed.
Attachment #8663732 - Flags: review?(mdeboer) → review+
Iteration: --- → 44.1 - Oct 5
Points: --- → 2
Keywords: leave-open
Rank: 25
Flags: qe-verify-
Flags: firefox-backlog+
Priority: -- → P2
https://hg.mozilla.org/mozilla-central/rev/1e82a58c1761
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [test disabled on ASan e10s]
I think this should be better - I realised that we weren't clearing out the old values of resolve/data from the channel after they were used.
Attachment #8667261 - Flags: review?(mdeboer)
Comment on attachment 8667261 [details] [diff] [review]
Second attempt to fix intermittent failure in browser_LoopRooms_channel.js - ensure previous states are cleared when they are used.

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

Aha! ok, worth a shot ;)
Attachment #8667261 - Flags: review?(mdeboer) → review+
https://hg.mozilla.org/mozilla-central/rev/ab2a3c581f58
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
We actually managed to hit it on Linux32 debug yesterday, the first non-ASan instance, so apparently the ASan connection is just that it's a race requiring a slow running build.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Summary: Intermittent browser_LoopRooms_channel.js | Test timed out → Intermittent e10s browser_LoopRooms_channel.js | Test timed out
Ok, I give up. I have no idea why this is still failing. I've tried ensuring the channel is definitely present before the page loads and that we reset things between tests.

Since I copied this from some of the existing web channel tests, which I assume aren't intermittent, I'll needinfo markh and see if he has any ideas.
Assignee: standard8 → nobody
Flags: needinfo?(markh)
Mark, do you want to back these patches out and mark them obsolete? Or do you still want them in m-c?
Flags: needinfo?(standard8)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #25)
> Mark, do you want to back these patches out and mark them obsolete? Or do
> you still want them in m-c?

We still need them on m-c. I believe they are improvements on what was there, they just haven't 100% fixed the issue.
Flags: needinfo?(standard8)
I'm afraid I can't see anything either :( The failure logs do seem to have "Error: operation not possible on dead CPOW" which I suspect is the root, but without a traceback it's difficult to see what might be causing it - a quick scan of the test and test .html implies it might well be in the webchannel itself - so I guess I'd be looking to instrument WebChannel.jsm at a few points to narrow this down - but the fact it only happened ~5 times in the last week makes even that trickier than it should be :(
Flags: needinfo?(markh)
Blocks: e10s-tests
tracking-e10s: --- → +
Iteration: 44.1 - Oct 5 → ---
This hasn't been seen since December, so I guess something fixed it. Marking as fixed, as we did fix a few things here.
Assignee: nobody → standard8
Status: REOPENED → RESOLVED
Closed: 9 years ago → 8 years ago
Resolution: --- → FIXED
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1206457 says three times this week.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Rank: 25 → 26
I'm not working on this at the moment.
Assignee: standard8 → nobody
Whiteboard: [akita-todo]
Whiteboard: [akita-todo] → [todo-akita]
Support for Hello/Loop has been discontinued.

https://support.mozilla.org/kb/hello-status

Hence closing the old bugs. Thank you for your support.
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: