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

Fixes #3620 Improved SessionStore/BrowserStore sync #3660

Closed
wants to merge 1 commit into from

Conversation

keianhzo
Copy link
Contributor

@keianhzo keianhzo commented Jul 9, 2020

Fixes #3620 When switching and suspending before onSessionStateChange has been called, mSession.mSessionState might be still null and we were adding the session to the BrowserStore twice. I have added a parameter to the recreate method to better track the session recreate origin as it can happen in multiple situations and not all the them require adding the session to the BrowserStore.

@keianhzo keianhzo self-assigned this Jul 9, 2020
@keianhzo keianhzo requested a review from bluemarvin July 9, 2020 14:51
Comment on lines +811 to +812
restore(mIsSuspended ? RECREATE_FROM_SUSPEND : CREATE);

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just use the mIsSuspended flag in the restore function and then we wouldn't need the CREATE, RECREATE, RECREATE_FROM_SUSPENDED flags?

In restore() you could do:

if (!mIsSuspended) {
    mSessionChangeListeners.forEach(listener -> listener.onSessionAdded(this));
}

mIsSuspended = false;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

During a recreation the session is already suspended when restored but we could use a flag instead.

@keianhzo keianhzo added the Uplift PR that needs to be uplifted. label Jul 10, 2020
@bluemarvin bluemarvin removed the Uplift PR that needs to be uplifted. label Jul 10, 2020
@bluemarvin
Copy link
Contributor

Closing in favor of #3668

@bluemarvin bluemarvin closed this Jul 10, 2020
@bluemarvin bluemarvin deleted the v11/browser_store_sync_fix branch July 15, 2020 20:26
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.

The app crashes when switching to normal browsing and back to PB while having all windows opened
2 participants