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

Commit

Permalink
Fixes #3671 BrowserState-SessionStore sync (#3669)
Browse files Browse the repository at this point in the history 
* Fixes BrowserState-SessionStore sync

* Avoid using a boolean flag and move add add/remove code to Session
  • Loading branch information
@keianhzo @bluemarvin
keianhzo authored and bluemarvin committed Jul 14, 2020
1 parent 30c828a commit fa3c3ef
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 42 deletions.
66 changes: 38 additions & 28 deletions app/src/common/shared/org/mozilla/vrbrowser/browser/engine/Session.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ public class Session implements ContentBlocking.Delegate, GeckoSession.Navigatio
private transient byte[] mPrivatePage;
private transient boolean mFirstContentfulPaint;
private transient long mKeepAlive;
private transient boolean mSessionRemoved;

public interface BitmapChangedListener {
void onBitmapChanged(Session aSession, Bitmap aBitmap);
Expand All @@ -110,19 +109,39 @@ public interface DrmStateChangedListener {
}

@IntDef(value = { SESSION_OPEN, SESSION_DO_NOT_OPEN})
public @interface SessionOpenModeFlags {}
public static final int SESSION_OPEN = 0;
public static final int SESSION_DO_NOT_OPEN = 1;
@interface SessionOpenModeFlags {}
static final int SESSION_OPEN = 0;
static final int SESSION_DO_NOT_OPEN = 1;

protected Session(Context aContext, GeckoRuntime aRuntime,
@NonNull SessionSettings aSettings) {
@NonNull
static Session createSession(Context aContext, GeckoRuntime aRuntime, @NonNull SessionSettings aSettings, @Session.SessionOpenModeFlags int aOpenMode, @NonNull SessionChangeListener listener) {
Session session = new Session(aContext, aRuntime, aSettings);
session.addSessionChangeListener(listener);
listener.onSessionAdded(session);
if (aOpenMode == Session.SESSION_OPEN) {
session.openSession();
session.setActive(true);
}

return session;
}

@NonNull
static Session createSuspendedSession(Context aContext, GeckoRuntime aRuntime, @NonNull SessionState aRestoreState, @NonNull SessionChangeListener listener) {
Session session = new Session(aContext, aRuntime, aRestoreState);
session.addSessionChangeListener(listener);

return session;
}

private Session(Context aContext, GeckoRuntime aRuntime, @NonNull SessionSettings aSettings) {
mContext = aContext;
mRuntime = aRuntime;
initialize();
mState = createSession(aSettings);
mState = createSessionState(aSettings);
}

protected Session(Context aContext, GeckoRuntime aRuntime, @NonNull SessionState aRestoreState) {
private Session(Context aContext, GeckoRuntime aRuntime, @NonNull SessionState aRestoreState) {
mContext = aContext;
mRuntime = aRuntime;
initialize();
Expand Down Expand Up @@ -159,8 +178,8 @@ private void initialize() {

protected void shutdown() {
if (mState.mSession != null) {
closeSession(mState);
mState.mSession = null;
setActive(false);
suspend();
}

if (mState.mParentId != null) {
Expand All @@ -170,13 +189,6 @@ protected void shutdown() {
}
}

if (!mSessionRemoved) {
mSessionChangeListeners.forEach(listener -> {
listener.onSessionRemoved(mState.mId);
});
mSessionRemoved = true;
}

mQueuedCalls.clear();
mNavigationListeners.clear();
mProgressListeners.clear();
Expand Down Expand Up @@ -430,6 +442,8 @@ public void suspend() {
return;
}

mSessionChangeListeners.forEach(listener -> listener.onSessionRemoved(mState.mId));

Log.d(LOGTAG, "Suspending Session: " + mState.mId);
closeSession(mState);
mState.mSession = null;
Expand Down Expand Up @@ -470,12 +484,7 @@ private void restore() {

mState.mSession = createGeckoSession(settings);

// We call restore when a session is first activated and when it's recreated.
// We only need to notify of the session creation if it's not a recreation.
if (mSessionRemoved) {
mSessionChangeListeners.forEach(listener -> listener.onSessionAdded(this));
mSessionRemoved = false;
}
mSessionChangeListeners.forEach(listener -> listener.onSessionAdded(this));

openSession();

Expand All @@ -498,7 +507,7 @@ private void restore() {
}


private SessionState createSession(@NonNull SessionSettings aSettings) {
private SessionState createSessionState(@NonNull SessionSettings aSettings) {
SessionState state = new SessionState();
state.mSettings = aSettings;
state.mSession = createGeckoSession(aSettings);
Expand Down Expand Up @@ -541,6 +550,8 @@ void recreateSession() {

mState = mState.recreate();

mSessionChangeListeners.forEach(listener -> listener.onSessionRemoved(mState.mId));

restore();

for (SessionChangeListener listener: mSessionChangeListeners) {
Expand All @@ -563,9 +574,7 @@ void openSession() {
mState.mSession.open(mRuntime);
}

mSessionChangeListeners.forEach(listener -> {
listener.onSessionOpened(this);
});
mSessionChangeListeners.forEach(listener -> listener.onSessionOpened(this));
}

private void closeSession(@NonNull SessionState aState) {
Expand Down Expand Up @@ -804,6 +813,7 @@ public void setActive(boolean aActive) {

} else if (aActive) {
restore();

} else {
Log.e(LOGTAG, "ERROR: Setting null GeckoView to inactive!");
}
Expand Down Expand Up @@ -867,7 +877,7 @@ public void toggleServo() {
.withServo(!isInstanceOfServoSession(mState.mSession))
.build();

mState = createSession(settings);
mState = createSessionState(settings);
openSession();
closeSession(previous);

Expand Down
42 changes: 28 additions & 14 deletions app/src/common/shared/org/mozilla/vrbrowser/browser/engine/SessionStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import android.content.Context;
import android.content.res.Configuration;
import android.os.Bundle;
import android.util.Log;
import android.util.Pair;

Expand Down Expand Up @@ -149,8 +148,21 @@ public void onTrackingProtectionLevelUpdated(int level) {
if (BuildConfig.DEBUG) {
mStoreSubscription = ComponentsAdapter.get().getStore().observeManually(browserState -> {
Log.d(LOGTAG, "Session status BEGIN");
browserState.getTabs().forEach(tabSessionState -> Log.d(LOGTAG, "BrowserStore Session: " + tabSessionState.getId()));
mSessions.forEach(session -> Log.d(LOGTAG, "SessionStore Session: " + session.getId()));
Log.d(LOGTAG, "[Total] BrowserStore: " + browserState.getTabs().size() + ", SessionStore: " + mSessions.size());
for (int i=0; i<browserState.getTabs().size(); i++) {
boolean isPrivate = browserState.getTabs().get(i).getContent().getPrivate();
Log.d(LOGTAG, "BrowserStore Session: " + browserState.getTabs().get(i).getId() + (isPrivate ? " (PB)" : ""));
}
int suspendedCount = 0;
for (int i=0; i<mSessions.size(); i++) {
boolean suspended = mSessions.get(i).getSessionState().mSession == null && !mSessions.get(i).isActive();
boolean isPrivate = mSessions.get(i).isPrivateMode();
Log.d(LOGTAG, "SessionStore Session: " + mSessions.get(i).getId() + (isPrivate ? " (PB)" : "") + (suspended ? " (suspended)" : ""));
if (suspended) {
suspendedCount++;
}
}
Log.d(LOGTAG, "[Alive] BrowserStore: " + browserState.getTabs().size() + ", SessionStore: " + (mSessions.size() - suspendedCount));
Log.d(LOGTAG, "Session status END");
return null;
});
Expand All @@ -165,6 +177,10 @@ private Session addSession(@NonNull Session aSession) {
mSessions.add(aSession);
sessionActiveStateChanged();

if (BuildConfig.DEBUG) {
mStoreSubscription.resume();
}

return aSession;
}

Expand All @@ -182,20 +198,13 @@ public Session createSession(boolean openSession, boolean aPrivateMode) {

@NonNull
Session createSession(@NonNull SessionSettings aSettings, @Session.SessionOpenModeFlags int aOpenMode) {
Session session = new Session(mContext, mRuntime, aSettings);
session.addSessionChangeListener(this);
if (aOpenMode == Session.SESSION_OPEN) {
onSessionAdded(session);
session.openSession();
session.setActive(true);
}
Session session = Session.createSession(mContext, mRuntime, aSettings, aOpenMode, this);
return addSession(session);
}

@NonNull
public Session createSuspendedSession(SessionState aRestoreState) {
Session session = new Session(mContext, mRuntime, aRestoreState);
session.addSessionChangeListener(this);
Session session = Session.createSuspendedSession(mContext, mRuntime, aRestoreState, this);
return addSession(session);
}

Expand All @@ -204,14 +213,16 @@ public Session createSuspendedSession(final String aUri, final boolean aPrivateM
SessionState state = new SessionState();
state.mUri = aUri;
state.mSettings = new SessionSettings(new SessionSettings.Builder().withDefaultSettings(mContext).withPrivateBrowsing(aPrivateMode));
Session session = new Session(mContext, mRuntime, state);
session.addSessionChangeListener(this);
Session session = Session.createSuspendedSession(mContext, mRuntime, state, this);
return addSession(session);
}

private void shutdownSession(@NonNull Session aSession) {
aSession.setPermissionDelegate(null);
aSession.shutdown();
if (BuildConfig.DEBUG) {
mStoreSubscription.resume();
}
}

public void destroySession(Session aSession) {
Expand All @@ -237,6 +248,9 @@ public void suspendAllInactiveSessions() {
session.suspend();
}
}
if (BuildConfig.DEBUG) {
mStoreSubscription.resume();
}
}

public @Nullable Session getSession(String aId) {
Expand Down

0 comments on commit fa3c3ef

Please sign in to comment.