Last Comment Bug 659777 - Cannot access remote/desktop tabs: "Svc.Private is undefined"
: Cannot access remote/desktop tabs: "Svc.Private is undefined"
Status: VERIFIED FIXED
[verified in services]
: regression
Product: Cloud Services
Classification: Client Software
Component: Firefox Sync: Backend (show other bugs)
: unspecified
: All All
: -- major (vote)
: mozilla6
Assigned To: Matt Brubeck (:mbrubeck)
:
Mentors:
: 660659 (view as bug list)
Depends on:
Blocks: 659689 639711 648364
  Show dependency treegraph
 
Reported: 2011-05-25 15:09 PDT by Matt Brubeck (:mbrubeck)
Modified: 2012-02-21 13:40 PST (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
unaffected
fixed
6+


Attachments
patch (3.44 KB, patch)
2011-05-25 15:48 PDT, Matt Brubeck (:mbrubeck)
philipp: review+
mark.finkle: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
screenshot (43.57 KB, image/png)
2011-05-27 00:22 PDT, Cristian Nicolae (:xti)
no flags Details

Description Matt Brubeck (:mbrubeck) 2011-05-25 15:09:48 PDT
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) 2011-05-25 15:18:52 PDT
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) 2011-05-25 15:48:33 PDT
Created attachment 535220 [details] [diff] [review]
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] 2011-05-25 15:57:17 PDT
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] 2011-05-25 16:12:41 PDT
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) 2011-05-25 16:49:58 PDT
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) 2011-05-25 16:59:16 PDT
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?) 2011-05-25 18:09:06 PDT
Comment on attachment 535220 [details] [diff] [review]
patch

Looks safe
Comment 8 Matt Brubeck (:mbrubeck) 2011-05-26 13:27:27 PDT
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) 2011-05-27 00:22:30 PDT
Created attachment 535572 [details]
screenshot
Comment 10 Philipp von Weitershausen [:philikon] 2011-05-30 09:21:19 PDT
*** Bug 660659 has been marked as a duplicate of this bug. ***
Comment 11 Philipp von Weitershausen [:philikon] 2011-05-31 14:42:36 PDT
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] 2011-06-01 12:25:19 PDT
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] 2011-06-02 10:33:44 PDT
http://hg.mozilla.org/mozilla-central/rev/054adb76530d
Comment 14 Kevin Brosnan [:kbrosnan] 2011-06-10 20:00:55 PDT
*** Bug 627370 has been marked as a duplicate of this bug. ***
Comment 15 Andreea Pod 2011-07-20 08:34:54 PDT
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)

Note You need to log in before you can comment on or make changes to this bug.