For #6399: Add method to find session by url #6401
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6401 +/- ##
============================================
- Coverage 78.55% 78.19% -0.37%
+ Complexity 4603 4419 -184
============================================
Files 610 589 -21
Lines 22209 21630 -579
Branches 3253 3176 -77
============================================
- Hits 17447 16914 -533
+ Misses 3431 3409 -22
+ Partials 1331 1307 -24Continue to review full report at Codecov.
|
| */ | ||
| fun findSessionByUrl(url: String): Session? = synchronized(values) { | ||
| values.firstOrNull { session -> session.url == url } | ||
| } |
There was a problem hiding this comment.
Would it be possible to implement this as an extension function in browser-state on BrowserState instead? If possible I would like to avoid adding anything new to SessionManager anymore since we are trying to get rid of it.
Even here I think you would want to maybe limit it to sessions instead of values to not search in custom tabs and web apps?
There was a problem hiding this comment.
One question: Presumably I get an existingTab:TabSessionState. How can I use that to navigate to the opened tab?
Without using something like :
sessionManager.findSessionById(existingTab.id)?.let { sessionManager.select(it) }
There was a problem hiding this comment.
Yeah, we'd still need this workaround "translation" code. That will be something we can get rid of once we start migrating Fenix code from SessionManager to BrowserStore. But at least we do not add new functionality that will end up getting called in various places makign things harder to migrate. :)
There was a problem hiding this comment.
I agree, I just wanted to see if there is something already migrated so I could avoid using sessionManager in yet another place in Fenix. Thank you for the details!
Build succeeded |
3 participants
Pull Request checklist
After merge