1250866 - [Synced Tabs] Not all tabs are synced if they are opened before user signs in

Status: RESOLVED WORKSFORME

(Firefox :: Sync, defect, P1)

Description

[Liviu Cirdei]

User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID 	20160223030304


Steps to reproduce:

1. On a device (PC / smartphone) open Firefox and open some distinct tabs.
2. Sign in with a Firefox account and perform a sync.
3. On a second device sign in with the same account.
4. From Menu [≡] open Synced Tabs Panel or open the synced tabs sidebar.
5. Inspect the synced tabs.

Expected Result: All tabs opened at step 1 are synced and visible on Synced Tabs panel or Synced tabs sidebar.

Actual Result: Not all tabs are synced. Sometimes few tabs are synced and sometimes no tab is synced (only the device name and the "No open tabs" message).

Notes: 
1. Reproducible: intermittent (70% of the time).
2. Tested this with same PC and two different profiles and also with first device being a mobile device and second device a PC. It reproduces more often when sync is made from a mobile device.

Comment 1

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

This is a side-effect of how the "tabs" engine works (and should be fixed) - it's roughly the tabs version of bug 1235269. Other times tabs might be missed:
* Tabs opened before Sync has initialized.
* Tabs opened during a Sync.

Comment 2

[Lina Butler (ex-Mozilla)]

For the first case, I'm guessing we'll want to manually add all open tabs to the tracker once Sync is initialized. For the second...could we listen for tabs opened during a sync, set a "resync" flag, and sync again once the current one finishes?

Comment 3

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

(In reply to Mark Hammond [:markh] from comment #1)
> This is a side-effect of how the "tabs" engine works (and should be fixed) -
> it's roughly the tabs version of bug 1235269.

Actually, I'm not so sure anymore. The tabs engine only syncs when it notices a tab has changed, but then it just re-fetches every open tab and syncs the lot (which is quite different to how bookmarks work). So assuming some tab change after Sync is initialized, we should find the entire set of tabs, including those opened before Sync was initialized.

These STR sound like they meet that criteria, so I currently can't explain this.

Comment 4

[Lina Butler (ex-Mozilla)]

I'm having a hard time reproducing this on Desktop, Liviu. Here's what I've tried...

* Opening 5 tabs in a window on the second device, waiting for them to finish loading, then signing in to Sync.
* Opening 5 tabs, then navigating between different pages (going back and forward) in two of them.
* 2 windows, with 6-8 tabs loaded in each window.
* Opening 14 different tabs in quick succession, then signing in to Sync before all tabs finished loading. Some of the titles didn't make it across (the panel showed the URL instead), but I was able to see all the tabs.

In all cases, I see the tabs show up in the other profiles. I ended up linking 11 different profiles to my account, and tried disconnecting and reconnecting, too. (In that case, I had to close and relaunch the other profiles, to work around 1250531).

I didn't test with the Android emulator yet, but I'll do that next. I can also upload a build for you that has additional logging, so that we can see why some tabs aren't syncing...

Comment 5

[Liviu Cirdei]

Hi Kit, 

Sure, I can try with a build.

I also tried again several times with latest Nightly and I could reproduce the issue, but I could not figure out what are the exact conditions needed to reproduce it. This time happened when on a first device I signed in an opened the sync Tabs sidebar, and on second device I opened some tabs, then signed in, a sync was made, after that on the second device I disconnected, closed the tabs, reopen other tabs, sign in again...and the issue appeared.

However I think the build's logs can help much more than this.

Comment 6

[Liviu Cirdei]

Attachment: video showing the issue.mp4

Comment 7

[Liviu Cirdei]

Some updates on this. I found a scenario when this is 100% reproducible.
Steps:
1. Open first device, sign in, open synced tabs sidebar.
2. On second device/profile open some tabs (e.g. Facebook, Twitter, mozilla.org, etc).
3. From the second device sign in -  a sync is performed.
4. On first device you can see all the opened tabs from step 2  - OK
5. On second device disconnect your account, close the tabs, and reopen some other tabs (e.g Amazon) + some tabs opened at step 2 (e.g Twitter, Facebook), then reconnect with your account to perform a sync.
6. On first device, inspect the synced tabs.

Result: A new device is listed which contains only the tabs that were not previously synced at step 2. So, in our case only the "Amazon" tab is synced at the step 5 even if there were opened two more tabs. 

I think this is somehow related to 1250531.

Attached is a video showing the entire flow.

Comment 8

[Lina Butler (ex-Mozilla)]

Oh, I see. Thanks for that video, Liviu; very helpful! That is, indeed, related to bug 1250531. We're not cleaning up stale devices, and, because we only show unique tabs (if you have the same tab opened on multiple devices, we'll only show the first one)...we pick the stale device.

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

If we want to enable this feature for 47, SV team would like to see this fixed. Tracked as blocking for Fx47.

Comment 10

[Lina Butler (ex-Mozilla)]

Thanks so much for the steps in comment 7, Liviu; those were very helpful for fixing bug 1250531. Disconnecting and reconnecting the second device without closing Firefox on the first device would cause it to hold on to the cached device record.

The case where "No open tabs" shows up is concerning. You mentioned in the initial STR that all tabs are distinct, so they shouldn't be affected by de-duping. But, in that case, I'd have expected them all to show up under the old device. Are you still able to reproduce that?

Comment 11

[Liviu Cirdei]

I will check to see if that was fixed by bug 1250531 or it is still reproducible. Right now I can't do that because of https://bugzilla.mozilla.org/show_bug.cgi?id=1261753#c11

Comment 12

[Lina Butler (ex-Mozilla)]

Sounds good, Liviu. No particular hurry. I think we'll want to uplift bug 1250531 (and bug 1262021) if we're going to ship Synced Tabs in 47, anyway, but it'd be great to confirm if there's anything else we need to fix for this one.

Thank you!

Comment 13

[Camelia Badau [:cbadau], Desktop Test Engineering]

I've tested on Windows 7 64bit and Mac OS X 10.9.5 using latest Nightly 48.0a1 (buildID: 20160413030239) and I have the following results: 
- Desktop to Desktop - the problem isn't reproducible anymore. Also, I didn't see anymore the case where "No open tabs" shows up. 
- Mobile to Desktop (meaning that you disconnect and reconnect on Mobile and then, on Desktop, you see a new device listed which contains only the tabs that were not previously synced) - the problem is still reproducible. There are the following bugs for mobile side related to this issue: bug 670474, bug 1224278, bug 1224278.

Comment 14

[Lina Butler (ex-Mozilla)]

Thanks for verifying this!

I filed bug 1264498 to port Android's de-duping behavior to Desktop. Mobile clients don't remove themselves when you disconnect Sync, so there isn't a good way to distinguish between dupes and clients with the default name that haven't synced in a while.

Bug 1180287 hides dupes that haven't synced in a week on mobile. For now, I think that's the best we can do on Desktop.

Comment 15

[Camelia Badau [:cbadau], Desktop Test Engineering]

I've tested on latest Aurora 47.0a2 (buildID: 20160418004027) and the issue on "Desktop to Desktop" side is still reproducible here. 

I consider it would be a good idea to uplift this to 47. What do you think?

Comment 16

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

Thanks for herding up all those device bugs Kit, and for testing them Camelia! All the actual work landed in other bugs, so I'm going to close this as WFM and remove the needinfo about uplifts - there's nothing in this bug to uplift!

Comment 17

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Updating status Fx47 to reflect that it's fixed.