STR:

1. open a few (say, 5 or 7) windows on device A; let's assume the windows are labeled: p, q, ... v
2. spend all your time in 1 or 2 of the windows (let's say windows p and q), opening tabs, loading sites, and ending up with (say) 5 tabs in each window, so p1-p5 and q1-q5
3. open device B on the same sync account, and check synced tabs from device A

ER:

Get the last N tabs from windows p and q, with tabs from windows r/s/t/u/v distantly behind. So the order would be something like p1, p2, p3, q1, p4, q2, ... followed by tabs from the other windows which have been in the background and/or occluded the whole time

AR:
The first N tabs are from N windows - one from each of p/q/r/s/t/u/v. Then those are followed by the background tabs from p and q, and then come all the background tabs from windows r-v.

The cause of this is that we set the last accessed data to Infinity for selected tabs (cf. https://searchfox.org/mozilla-central/rev/9b0bdcc37419e6765223358a31a4a54d62e1cd97/browser/base/content/tabbrowser-tab.js#266 ), and normalize that to Date.now() at the point of syncing. We don't do any window-level processing of background windows.

Instead, we should take whether the window is minimized, occluded or unfocused into account when determining lastAccessed (and updating it in the tabbrowser when the window activates/deactivates / is minimized or restored).

Thanks :gijs for the report! I can reproduce this as well. Will have the team look into this!

(In reply to :Gijs (he/him) from comment #0)

The cause of this is that we set the last accessed data to Infinity for selected tabs (cf. https://searchfox.org/mozilla-central/rev/9b0bdcc37419e6765223358a31a4a54d62e1cd97/browser/base/content/tabbrowser-tab.js#266 ), and normalize that to Date.now() at the point of syncing. We don't do any window-level processing of background windows.

Interesting, thanks. I'd like to avoid Sync trying to work around this by capturing all the events etc to handle window operations or otherwise reimplement lastAccessed. My best thought is that updateLastAccessed(aDate) sets two different properties - _lastAccessed as now, plus, say, _lastSetActive which is always set to Date.now() - and Sync could use this new property for ordering?

WDYT?

(I guess that's assuming the current behaviour of tabbrowser is correct, but I don't see how TBH - lastAccessed for a tab being "now" when it was clearly last used days ago seems wrong for any use-case that for which the term "last accessed" means what I think it means :) But I assume you consider it is correct given you opened the bug in the Sync component?)

(In reply to Mark Hammond [:markh] [:mhammond] from comment #2)

(In reply to :Gijs (he/him) from comment #0)

The cause of this is that we set the last accessed data to Infinity for selected tabs (cf. https://searchfox.org/mozilla-central/rev/9b0bdcc37419e6765223358a31a4a54d62e1cd97/browser/base/content/tabbrowser-tab.js#266 ), and normalize that to Date.now() at the point of syncing. We don't do any window-level processing of background windows.

Interesting, thanks. I'd like to avoid Sync trying to work around this by capturing all the events etc to handle window operations or otherwise reimplement lastAccessed. My best thought is that updateLastAccessed(aDate) sets two different properties - _lastAccessed as now, plus, say, _lastSetActive which is always set to Date.now() - and Sync could use this new property for ordering?

WDYT?

I think just changing lastAccessed to not be Infinity except if the window is/becomes the active window seems like it'd be OK? There are a few other consumers of this information (notably session restore and the ctrl-tab switcher that's used if you have "Ctrl+Tab cycles through tabs in recently used order" turned on) but I think if we basically flip the tabs between Infinity and Date.now() based on whether the window is active they should be OK (but I haven't dug into the details there).

Does that sound reasonable?

(In reply to :Gijs (he/him) from comment #4)

I think just changing lastAccessed to not be Infinity except if the window is/becomes the active window seems like it'd be OK? There are a few other consumers of this information (notably session restore and the ctrl-tab switcher that's used if you have "Ctrl+Tab cycles through tabs in recently used order" turned on) but I think if we basically flip the tabs between Infinity and Date.now() based on whether the window is active they should be OK (but I haven't dug into the details there).

Without taking some action once a window becomes inactive, I suspect that will still have the same problem - ie, "active tab in active window" gets Infinity - then the Window becomes inactive - if the tab keeps infinity, that tab will remain as one of the most-recent - so the active tab probably needs to get normalized back to Date.now() as its window becomes inactive?

I assume it was originally implemented that way for a reason though - any thoughts on who else might want to weigh-in here? A couple of minutes looking at "blame" wasn't that helpful...

(In reply to Mark Hammond [:markh] [:mhammond] from comment #5)

(In reply to :Gijs (he/him) from comment #4)

I think just changing lastAccessed to not be Infinity except if the window is/becomes the active window seems like it'd be OK? There are a few other consumers of this information (notably session restore and the ctrl-tab switcher that's used if you have "Ctrl+Tab cycles through tabs in recently used order" turned on) but I think if we basically flip the tabs between Infinity and Date.now() based on whether the window is active they should be OK (but I haven't dug into the details there).

Without taking some action once a window becomes inactive, I suspect that will still have the same problem - ie, "active tab in active window" gets Infinity - then the Window becomes inactive - if the tab keeps infinity, that tab will remain as one of the most-recent - so the active tab probably needs to get normalized back to Date.now() as its window becomes inactive?

Yes, I assume we'd want activate and deactivate event handlers, or similar but for the occlusion state.

I assume it was originally implemented that way for a reason though - any thoughts on who else might want to weigh-in here? A couple of minutes looking at "blame" wasn't that helpful...

Probably Dão.

(In reply to :Gijs (he/him) from comment #6)

(In reply to Mark Hammond [:markh] [:mhammond] from comment #5)

(In reply to :Gijs (he/him) from comment #4)

I think just changing lastAccessed to not be Infinity except if the window is/becomes the active window seems like it'd be OK? There are a few other consumers of this information (notably session restore and the ctrl-tab switcher that's used if you have "Ctrl+Tab cycles through tabs in recently used order" turned on) but I think if we basically flip the tabs between Infinity and Date.now() based on whether the window is active they should be OK (but I haven't dug into the details there).

Without taking some action once a window becomes inactive, I suspect that will still have the same problem - ie, "active tab in active window" gets Infinity - then the Window becomes inactive - if the tab keeps infinity, that tab will remain as one of the most-recent - so the active tab probably needs to get normalized back to Date.now() as its window becomes inactive?

Yes, I assume we'd want activate and deactivate event handlers, or similar but for the occlusion state.

Sounds good to me.

The severity field is not set for this bug.
:markh, could you have a look please?

For more information, please visit auto_nag documentation.