Closed Bug 811143 Opened 7 years ago Closed 7 years ago

ports used by user-recommend messaging are never closed if provider doesn't support recommending.

Categories

(Firefox Graveyard :: SocialAPI, defect)

17 Branch
defect
Not set

Tracking

(firefox18-)

RESOLVED FIXED
Firefox 19
Tracking Status
firefox18 - ---

People

(Reporter: pbmaxi, Assigned: markh)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:16.0) Gecko/20100101 Firefox/16.0
Build ID: 20121025210744

Steps to reproduce:

My worker sends a social.user-profile message to the apiPort. When the user logs out from the social provider and logs in as another user, the worker sends social.user-profile again.



Actual results:

The first time the worker sends user-profile, a new port gets opened; at this port the worker gets a social.user-recommend-prompt message.
The same happens again for the second user-profile message. If the procedure is repeated often, the list of ports is growing and growing.


Expected results:

At the opened port, there should arrive a social.port-closing message at some time.
Component: Untriaged → SocialAPI
Which of the ports isn't seeing the close message?  The port on which the user-recommend message is sent will be closed after the worker responds with a 'social.user-recommend-prompt-response' message.  The apiPort is not expected to be closed during the process you outline above.

Could you confirm the above is what you see?  The bug I see here is that the port will never be closed if the worker never sends a 'social.user-recommend-prompt-response' message.
Yes, I'm talking about the port that receives user-recommend. The problem is that I don't want to implement the user recommendation feature, and the documentation says that it is optional to send a social.user-recommend-prompt-response message, so I don't send it, and that's why the port is never closed.
thanks for clarifying - updating the title accordingly.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: x86_64 → All
Summary: ports opened by social.user-profile are not closed → ports used by user-recommend messaging are never closed if provider doesn't support recommending.
I suppose we should just time out the request after some delay and close the port? Though really, I don't like having this "core" functionality handled by a custom port in the front-end code - it seems like something e.g. workerAPI should handle.
This patch moves the request for the recommend info to the workerapi, and the validation and "ownership" of the data to the provider object.  The front-end now just uses Social.provider.recommendInfo.  As a result of this, the workerapi port is used so no ports are left dangling if the feature isn't supported by the provider.
Assignee: nobody → mhammond
Attachment #681800 - Flags: feedback?(gavin.sharp)
Comment on attachment 681800 [details] [diff] [review]
Move recommend info request to workerapi

looks great!
Attachment #681800 - Flags: feedback?(gavin.sharp) → feedback+
Attachment #681800 - Flags: review?(jaws)
Attachment #681800 - Flags: review?(jaws) → review+
https://hg.mozilla.org/mozilla-central/rev/44939c596b54
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
Is this Social API fix still desired on FF18? If so, please nominate asap.
Comment on attachment 681800 [details] [diff] [review]
Move recommend info request to workerapi

This patch still applies directly on beta.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: relatively small memory leak when providers don't support the share button - and as Facebook currently has this disabled, the leak will exist there.
Testing completed (on m-c, etc.): Landed on 19 quite some time ago.  Includes tests
Risk to taking this patch (and alternatives if risky): Relatively low risk restricted to social
String or UUID changes made by this patch: None
Attachment #681800 - Flags: approval-mozilla-beta?
Comment on attachment 681800 [details] [diff] [review]
Move recommend info request to workerapi

If this is a minor leak, let's prevent the chance for regression here by letting the fix ride the trains. I'll untrack.
Attachment #681800 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.