Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

Fixes #3671 BrowserState-SessionStore sync #3669

Merged
merged 2 commits into from
Jul 14, 2020
Merged

Conversation

keianhzo
Copy link
Contributor

@keianhzo keianhzo commented Jul 13, 2020

Fixes #3671

Make sure to test the following scenarios:

  • New Window creation: from either Tray/Context menu
  • New Tab creation: from either the Tabs panel/Context menu
  • New Background tab creation (Long press link and open tab from the Context menu)
  • Session restore: quit and reopen app with active&suspended sessions (Suspended sessions won't be added to the Browser Store until first activated)
  • Session suspension/restore: open 3+ windows in regular model, switch to private, open another 3+ windows and go back and forth between regular/private
  • Session recreation: Open several active/suspeded sessions, got to Settings->Privacy and clear cache/data

Logcat filtering by SessionStore will show the BrowserStore-SessionStore sync state including the alive vs suspended totals.

Note: Suspended session will be alive at the SessionStore level but not at the BrowserStore level, we remove them from the BrowserStore when suspended, we only want alive ones there.

@keianhzo keianhzo added the Uplift PR that needs to be uplifted. label Jul 13, 2020
@keianhzo keianhzo self-assigned this Jul 13, 2020
@keianhzo keianhzo changed the title Fixes BrowserState-SessionStore sync Fixes #3671 BrowserState-SessionStore sync Jul 13, 2020
Copy link
Contributor

@bluemarvin bluemarvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If mSessionAdded is in the wrong state, we should fix that instead of passing in boolean to override it.

Copy link
Contributor

@MortimerGoro MortimerGoro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not creating the mState in the constructor adds some risks. Parts of the code assume that mState is never null (e..g shutdown, dump listeners, or others)

@keianhzo
Copy link
Contributor Author

@MortimerGoro Shouldn't happen as we are calling createSession right after creating the Session instance with sets the state but I've added an static method to handle the whole session creation and avoid missing the state creation.

@bluemarvin bluemarvin merged commit db4bae0 into main Jul 14, 2020
@bluemarvin bluemarvin deleted the v11/browser_state_sync branch July 14, 2020 18:46
bluemarvin pushed a commit that referenced this pull request Jul 14, 2020
* Fixes BrowserState-SessionStore sync

* Avoid using a boolean flag and move add add/remove code to Session
@bluemarvin bluemarvin removed the Uplift PR that needs to be uplifted. label Jul 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FxA] The Log In page is displayed in a loading state when trying to login
3 participants