859715 - SocialServiceInternal.providers is undefined

Status: VERIFIED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Panos Astithas (he/him) [:past] (please ni?)]

I noticed that my facebook toolbar was gone in the latest Nightly and while trying to re-enable it by visiting Facebook, I got this:

Timestamp: 4/9/13 11:44:31 AM
Error: TypeError: SocialServiceInternal.providers is undefined
Source File: resource://gre/modules/SocialService.jsm
Line: 252

I'm pretty sure it was there yesterday.

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

Aaaand the toolbar is back again today!
I think it updated in between reboots and there are no errors in the error console.

Comment 2

[Shane Caraveo (:mixedpuppy)]

The only way I've been able to cause this to happen is by directly messing with preferences.  I could see it happen with interim builds of aurora and nightly.   Nonetheless, we should protect against the exception that causes this, since if it does happen it social is broken.

Comment 3

[Shane Caraveo (:mixedpuppy)]

Attachment: migration startup fix

Includes a test, but here is manual repro as well.

Reproduction:

1. start fx22, enable a provider
2. enter about:config and reset the pref for that provider
3. start fx23

- expected result:
  no provider is enabled.  you can go to the activation page and renable
- what does happen without this patch:
  no provider is enabled.  you cannot activate any provider without first resetting social.activeProviders in about:config 

This situation really only occurred as an interim problem between some patch landings, but it is best to protect the startup of social.

Comment 4

[Shane Caraveo (:mixedpuppy)]

https://tbpl.mozilla.org/?tree=Try&rev=3cabd4ea757e

Comment 5

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

Comment on attachment 736016 [details] [diff] [review]
migration startup fix

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

::: toolkit/components/social/SocialService.jsm
@@ +165,5 @@
>          string.data = JSON.stringify(manifest);
>          Services.prefs.setComplexValue(prefname, Ci.nsISupportsString, string);
>        }
>      }
> +    ActiveProviders.flush();

It looks like this is going to be called on every startup - how about a boolean to indicate if the flush() is needed just to help keep startup snappy?

@@ +224,5 @@
> +  } catch(e) {
> +    // no matter what, if migration fails we do not want to render social
> +    // unusable. Worst case scenario is that, when upgrading Firefox, previously
> +    // enabled providers are not migrated.
> +    Cu.reportError(e);

I think it might be worth something like Cu.reportError("Error migrating social settings: " + e);

Comment 6

[Shane Caraveo (:mixedpuppy)]

Attachment: migration startup fix

addressing comments, but only doing flush in the error case, carrying forward r+

Comment 7

[Shane Caraveo (:mixedpuppy)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/aad035133e33

Comment 8

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 736537 [details] [diff] [review]
migration startup fix

[Approval Request Comment]
User impact if declined: potential failure to start social if user messed with prefs or used an addon to install social provider prefs
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): low, in rare cases social will fail to work
String or IDL/UUID changes made by this patch: none

Comment 9

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

(In reply to Shane Caraveo (:mixedpuppy) from comment #8)

Where's the "Bug caused by (feature/regressing bug #)" line? The bug that caused this issue should be listed in the "blocks" field.

Comment 10

[Shane Caraveo (:mixedpuppy)]

migration startup fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 851936
User impact if declined: potential failure to start social if user messed with prefs or used an addon to install social provider prefs
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): low, in rare cases social will fail to work
String or IDL/UUID changes made by this patch: none

Comment 11

[Shane Caraveo (:mixedpuppy)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8a7f1e3e3e40

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/aad035133e33

Comment 13

[Manuela Muntean [Away]]

I wasn't able to reproduce this issue using the suggestions from the description and comment 3, with the Nightly build from 2013-04-09 on Mac OSX 10.8.3 in 32bit mode.

Since there is already an automated test for this issue, is necessary a manual verification also? If it is, could please someone provide more detailed steps to reproduce? Thanks!

Comment 14

[Shane Caraveo (:mixedpuppy)]

IMO this doesn't need further manual testing.

Comment 15

[Manuela Muntean [Away]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #14)
> IMO this doesn't need further manual testing.

Thanks for the reply!

Marking this verified fixed for Fx 22, based on comment 14.