Closed Bug 807078 Opened 8 years ago Closed 8 years ago

login and logout listeners not getting cleaned up after trusty ui close

Categories

(Core Graveyard :: Identity, enhancement)

enhancement
Not set
normal

Tracking

(blocking-basecamp:+, firefox18 fixed, firefox19 fixed)

RESOLVED FIXED
mozilla19
blocking-basecamp +
Tracking Status
firefox18 --- fixed
firefox19 --- fixed

People

(Reporter: jedp, Assigned: jedp)

References

Details

(Whiteboard: [LOE:M][feature-complete])

Attachments

(1 file, 2 obsolete files)

The event listeners in the b2g/components/SignInToWebsite pipe object are not getting cleaned up properly because the pipe is not receiving the "finished" message from identity.js.  This is causing the event callbacks to fire two, three, four, etc. times per message.

It appears that once gaia shuts down the identity.js window, no callbacks fire, so the close handlers don't get a chance to tell gecko to clean up.
Clean up multiple listeners by sending a finished message to gecko first, and then messaging gaia.
Attachment #676742 - Flags: feedback?(zack.carter)
blocking-basecamp: --- → ?
Comment on attachment 676742 [details] [diff] [review]
clean up message listeners before closing window

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

This fixes a problem that emerged when we started testing against the trusty ui.  There may have been a race condition all along, but it wasn't evident to me with the regular popup
Attachment #676742 - Flags: feedback?(zack.carter) → review?(benadida)
memory leak, marking blocker.
blocking-basecamp: ? → +
Comment on attachment 676742 [details] [diff] [review]
clean up message listeners before closing window

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

r+ with tiny comment nit below.

::: b2g/chrome/content/identity.js
@@ +62,2 @@
>  /*
>   * Notify the UI to close the dialog and return to the caller application

if you're removing the callback, change this comment.
changed comment per Comment #4 r=benadida
Attachment #676742 - Attachment is obsolete: true
Attachment #676742 - Flags: review?(benadida)
Blocks: 804143
This doesn't apply cleanly to m-c. Please rebase.
Keywords: checkin-needed
New patch fixes collision with m-c
Attachment #677157 - Attachment is obsolete: true
(In reply to Ryan VanderMeulen from comment #6)
> This doesn't apply cleanly to m-c. Please rebase.

Thanks, Ryan.  Should be good to go, now.
Keywords: checkin-needed
Whiteboard: [LOE:M] → [LOE:M][feature-complete]
https://hg.mozilla.org/mozilla-central/rev/0e7914709279
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
I take it the right way to verify this is to go through the persona login flows and make sure I'm not seeing multiple unexpected events getting fired, right?
Keywords: verifyme
QA Contact: jsmith
(In reply to Jason Smith [:jsmith] from comment #12)
> I take it the right way to verify this is to go through the persona login
> flows and make sure I'm not seeing multiple unexpected events getting fired,
> right?

Yes.  We're new to marionette tests; once we've got them figured out, there will be a test for this.
Keywords: verifyme
QA Contact: jsmith
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.