fix(background): update icon change logic; using TabState for sessions
#420
Conversation
github-actions
bot
added
the
area: background
Extension builds preview
|
){kind=icon}
){kind=icon}
src/background/services/tabState.ts
Outdated
|
|
||
| getSessions(tab: Tabs.Tab) { | ||
| if (!this.sessions.has(tab)) { | ||
| this.sessions.set(tab, new Map()) |
There was a problem hiding this comment.
We're setting in a "get" as it helps to keep code clean. Initializing empty Map shouldn't be expensive (in terms of memory or CPU).
There was a problem hiding this comment.
This is exactly about what I was thinking right now. It looks like whenever we will change the focus from one tab to another we will perform the set (one for resume and one for pause). The reason I believe we should avoid this, is that people (me included), have a lot of tabs open and we will rely on the service worker to be suspended to actually free up the memory.
There was a problem hiding this comment.
As the key is Tab, memory freeing isn't an issue though.
Empty Map doesn't take much memory.
|
There's some issue in this approach. It keeps showing "this site is not monetized". Will investigate. |
const t1 = await this.getTabById(tabId)
const t2 = await this.getTabById(tabId)
console.log(t1 === t2) // falseMeaning overpaying isn't really fixed as of now. |
|
Blocked by #286 |
github-actions
bot
added
the
area: popup
sidvishnoi
requested review from
dianafulga and
raducristianpopa
and removed request for
raducristianpopa
sidvishnoi
changed the title
sessions to TabStatesessions
sidvishnoi
changed the title
sessionssessions
Not yet, as we need to handle on-page link changes too. Will do as follow-up. |
Context
Closes #381
This PR got a bit bigger, with related/unrelated changes. Can split if wanted.
sessionstoTabStateservice, adding some utilsSET_IS_SITE_MONETIZEDmessage event (moved into SendToPopup)