Closed Bug 1842895 Opened 9 months ago Closed 9 months ago

The SessionStoreInternal _closedTabs and _closedWindowTabs are confusingly named

Categories

(Firefox :: Session Restore, task)

task

Tracking

()

RESOLVED FIXED
117 Branch
Tracking Status
firefox117 --- fixed

People

(Reporter: sfoster, Assigned: sfoster)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fidefe-firefox-view] )

Attachments

(1 file)

We have these two weakmaps SessionStoreInternal._closedTabs and SessionStoreInternal._closedWindowTabs that are used to capture finalizing state that happens async as a window or tab is closed (AIUI.) They both use a browser.permanentKey as the key to lookup a tab or closedTabs collection respectively, and this entry is ephemeral - existing only to bridge the moment when the tab or window is closing, and the unload events are received and state is finalized.

Given the other work we have in-flight around closed tabs and closed windows, what used to be conveniently short property names are now a real source of potential confusion. I'd like to rename them to better reflect their purpose. This would be refactoring internal to SessionStore and have no impact on any of the callers.

Maybe something like?

  • _closedTabs => _browserToClosingTabMap
  • _closedWindowTabs => _browserToClosingWindowClosedTabsMap
Assignee: nobody → sfoster
Status: NEW → ASSIGNED

hg blame suggests you might have an informed opinion here :mconley?

Flags: needinfo?(mconley)

Hoooo boy this is a blast from the past.

So to me, some of the subtlety here is the difference between a tab closed on its own or a tab closed because it was in a window that also closed. If we can capture that in the naming, that might be best. I do like using closing here though to properly represent how they're ephemeral.

Maybe:

  • _closedTabs => _browserToClosingTabMap
  • _closedWindowTabs => _browserToTabClosingByWindowMap?

Naming is hard. :/

Flags: needinfo?(mconley)
Attachment #9343582 - Attachment description: Bug 1842895 - Rename SessionStoreInternal._closedTabs and _closedWindowTabs to differentiate from other closedTabs collections. r?mconley! → WIP: Bug 1842895 - Rename SessionStoreInternal._closedTabs and _closedWindowTabs to differentiate from
Attachment #9343582 - Attachment description: WIP: Bug 1842895 - Rename SessionStoreInternal._closedTabs and _closedWindowTabs to differentiate from → Bug 1842895 - Rename SessionStoreInternal._closedTabs and _closedWindowTabs to differentiate from other closedTabs collections. r?sclements!
Pushed by sfoster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/62e9b36475c9
Rename SessionStoreInternal._closedTabs and _closedWindowTabs to differentiate from other closedTabs collections. r=sessionstore-reviewers,sclements
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → 117 Branch
Blocks: 1850630
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: