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

Status: RESOLVED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[pbmaxi]

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.

Comment 1

[Mark Hammond [:markh] [:mhammond]]

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.

Comment 2

[pbmaxi]

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.

Comment 3

[Mark Hammond [:markh] [:mhammond]]

thanks for clarifying - updating the title accordingly.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

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.

Comment 5

[Mark Hammond [:markh] [:mhammond]]

Attachment: Move recommend info request to workerapi

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.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 681800 [details] [diff] [review]
Move recommend info request to workerapi

looks great!

Comment 7

[Mark Hammond [:markh] [:mhammond]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/44939c596b54

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/44939c596b54

Comment 9

[Alex Keybl [:akeybl]]

Is this Social API fix still desired on FF18? If so, please nominate asap.

Comment 10

[Mark Hammond [:markh] [:mhammond]]

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

Comment 11

[Alex Keybl [:akeybl]]

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.