659777 - Cannot access remote/desktop tabs: "Svc.Private is undefined"

Status: VERIFIED FIXED

(Firefox :: Sync, defect)

Description

[Matt Brubeck (:mbrubeck)]

Steps to reproduce:
1. Connect Fennec to a sync account.
2. Open the "Desktop" tab on the awesome screen.

Expected results: List of remote tabs is displayed.
Actual results: No remote tabs are displayed.

Console error: "Svc.Private is undefined" resource://services-sync/engines/tab.js Line 219

This is reproducible in both desktop and Android, in both Nightly (7.0a1) and Aurora (6.0a2).

Comment 1

[Matt Brubeck (:mbrubeck)]

Regression from bug 648364 - there is no private browsing service in Fennec, and Svc.Private no longer falls back on a dummy implementation.

Comment 2

[Matt Brubeck (:mbrubeck)]

Attachment: patch

The problem is that nsIPrivateBrowsingService is not implemented in Fennec, and Sync no longer falls back on a dummy implementation.  This patch modifies the Svc.Private getter to return undefined always (instead of throwing an exception the first time and returning undefined later), and checks the existence of Svc.Private before trying to use it.

Alternately, you could go back to using a dummy service for Svc.Private, or we could implement a dummy service in Fennec.

Comment 3

[Philipp von Weitershausen [:philikon]]

Comment on attachment 535220 [details] [diff] [review]
patch

>+XPCOMUtils.defineLazyGetter(Svc, "Private", function() {
>+  try {
>+    return Cc["@mozilla.org/privatebrowsing;1"].getService(Ci["nsIPrivateBrowsingService"]);
>+  } catch (e) {
>+    return undefined;
>+  }
>+});

Please add a comment above this stanza to explain why we're doing this lazy getter the long way round.

Looks good otherwise. Please commit on services-central, not m-c. I can also land it for you if you prefer.

Comment 4

[Philipp von Weitershausen [:philikon]]

Matt, btw, do you think there's a way to unit test this? I want to make sure we only make mistakes once ;)

(We really need TPS (nee Crossweave) tests to run against an actual Fennec instance for the 'mobile' test run, then we would actually have caught this.)

Comment 5

[Matt Brubeck (:mbrubeck)]

http://hg.mozilla.org/services/services-central/rev/054adb76530d

We can try to add some Fennec browser-chrome tests to catch sync bugs, until we are able to get the xpcshell tests running for mobile.

Comment 6

[Matt Brubeck (:mbrubeck)]

Comment on attachment 535220 [details] [diff] [review]
patch

Requesting approval-mozilla-aurora.  This is a regression compared to Firefox 5 that affects Firefox 6 and breaks major functionality in Fennec.  The fix is low risk and localized (adding a single try-catch block and null guards).

The alternate to this fix is to back out bug 648364 which was a large refactoring (over 1000 lines changed).  Since other changes to the code have landed on top of that refactoring, it probably cannot be backed out without manual conflict resolution.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 535220 [details] [diff] [review]
patch

Looks safe

Comment 8

[Matt Brubeck (:mbrubeck)]

Pushed to Aurora for Firefox 6: http://hg.mozilla.org/releases/mozilla-aurora/rev/c2a6c74bcfa4

This is not yet fixed on trunk.  It will land in the next merge from services-central to mozilla-central.

Comment 9

[Cristian Nicolae (:xti)]

Attachment: screenshot

Comment 11

[Philipp von Weitershausen [:philikon]]

STRs for QA: Tabs from Other Computers should work on Fennec again now (was briefly broken on Aurora and Nightlies).

Comment 12

[Aaron Train [:aaronmt]]

WFM: Verified Fixed on Aurora

Mozilla/5.0 (Android; Linux armv7l; rv:6.0a2) Gecko/20110531 Firefox/6.0a2 Fennec/6.0a2

Comment 13

[Philipp von Weitershausen [:philikon]]

http://hg.mozilla.org/mozilla-central/rev/054adb76530d

Comment 15

[Andreea Pod]

Verified fixed on Firefox 6 Beta 2: Mozilla /5.0 (Android;Linux armv7l;rv:6.0) Gecko/20110713 Firefox/6.0 Fennec/6.0

Device: LG Optimus 2X (Android 2.2)