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

Commit fa3c3ef

Browse files
keianhzobluemarvin
keianhzo
authored and
bluemarvin
committed
Fixes #3671 BrowserState-SessionStore sync (#3669)
* Fixes BrowserState-SessionStore sync * Avoid using a boolean flag and move add add/remove code to Session
1 parent 30c828a commit fa3c3ef

2 files changed

Lines changed: 66 additions & 42 deletions

File tree

‎app/src/common/shared/org/mozilla/vrbrowser/browser/engine/Session.java‎

Lines changed: 38 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ public class Session implements ContentBlocking.Delegate, GeckoSession.Navigatio
9191
private transient byte[] mPrivatePage;
9292
private transient boolean mFirstContentfulPaint;
9393
private transient long mKeepAlive;
94-
private transient boolean mSessionRemoved;
9594

9695
public interface BitmapChangedListener {
9796
void onBitmapChanged(Session aSession, Bitmap aBitmap);
@@ -110,19 +109,39 @@ public interface DrmStateChangedListener {
110109
}
111110

112111
@IntDef(value = { SESSION_OPEN, SESSION_DO_NOT_OPEN})
113-
public @interface SessionOpenModeFlags {}
114-
public static final int SESSION_OPEN = 0;
115-
public static final int SESSION_DO_NOT_OPEN = 1;
112+
@interface SessionOpenModeFlags {}
113+
static final int SESSION_OPEN = 0;
114+
static final int SESSION_DO_NOT_OPEN = 1;
116115

117-
protected Session(Context aContext, GeckoRuntime aRuntime,
118-
@NonNull SessionSettings aSettings) {
116+
@NonNull
117+
static Session createSession(Context aContext, GeckoRuntime aRuntime, @NonNull SessionSettings aSettings, @Session.SessionOpenModeFlags int aOpenMode, @NonNull SessionChangeListener listener) {
118+
Session session = new Session(aContext, aRuntime, aSettings);
119+
session.addSessionChangeListener(listener);
120+
listener.onSessionAdded(session);
121+
if (aOpenMode == Session.SESSION_OPEN) {
122+
session.openSession();
123+
session.setActive(true);
124+
}
125+
126+
return session;
127+
}
128+
129+
@NonNull
130+
static Session createSuspendedSession(Context aContext, GeckoRuntime aRuntime, @NonNull SessionState aRestoreState, @NonNull SessionChangeListener listener) {
131+
Session session = new Session(aContext, aRuntime, aRestoreState);
132+
session.addSessionChangeListener(listener);
133+
134+
return session;
135+
}
136+
137+
private Session(Context aContext, GeckoRuntime aRuntime, @NonNull SessionSettings aSettings) {
119138
mContext = aContext;
120139
mRuntime = aRuntime;
121140
initialize();
122-
mState = createSession(aSettings);
141+
mState = createSessionState(aSettings);
123142
}
124143

125-
protected Session(Context aContext, GeckoRuntime aRuntime, @NonNull SessionState aRestoreState) {
144+
private Session(Context aContext, GeckoRuntime aRuntime, @NonNull SessionState aRestoreState) {
126145
mContext = aContext;
127146
mRuntime = aRuntime;
128147
initialize();
@@ -159,8 +178,8 @@ private void initialize() {
159178

160179
protected void shutdown() {
161180
if (mState.mSession != null) {
162-
closeSession(mState);
163-
mState.mSession = null;
181+
setActive(false);
182+
suspend();
164183
}
165184

166185
if (mState.mParentId != null) {
@@ -170,13 +189,6 @@ protected void shutdown() {
170189
}
171190
}
172191

173-
if (!mSessionRemoved) {
174-
mSessionChangeListeners.forEach(listener -> {
175-
listener.onSessionRemoved(mState.mId);
176-
});
177-
mSessionRemoved = true;
178-
}
179-
180192
mQueuedCalls.clear();
181193
mNavigationListeners.clear();
182194
mProgressListeners.clear();
@@ -430,6 +442,8 @@ public void suspend() {
430442
return;
431443
}
432444

445+
mSessionChangeListeners.forEach(listener -> listener.onSessionRemoved(mState.mId));
446+
433447
Log.d(LOGTAG, "Suspending Session: " + mState.mId);
434448
closeSession(mState);
435449
mState.mSession = null;
@@ -470,12 +484,7 @@ private void restore() {
470484

471485
mState.mSession = createGeckoSession(settings);
472486

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

480489
openSession();
481490

@@ -498,7 +507,7 @@ private void restore() {
498507
}
499508

500509

501-
private SessionState createSession(@NonNull SessionSettings aSettings) {
510+
private SessionState createSessionState(@NonNull SessionSettings aSettings) {
502511
SessionState state = new SessionState();
503512
state.mSettings = aSettings;
504513
state.mSession = createGeckoSession(aSettings);
@@ -541,6 +550,8 @@ void recreateSession() {
541550

542551
mState = mState.recreate();
543552

553+
mSessionChangeListeners.forEach(listener -> listener.onSessionRemoved(mState.mId));
554+
544555
restore();
545556

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

566-
mSessionChangeListeners.forEach(listener -> {
567-
listener.onSessionOpened(this);
568-
});
577+
mSessionChangeListeners.forEach(listener -> listener.onSessionOpened(this));
569578
}
570579

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

805814
} else if (aActive) {
806815
restore();
816+
807817
} else {
808818
Log.e(LOGTAG, "ERROR: Setting null GeckoView to inactive!");
809819
}
@@ -867,7 +877,7 @@ public void toggleServo() {
867877
.withServo(!isInstanceOfServoSession(mState.mSession))
868878
.build();
869879

870-
mState = createSession(settings);
880+
mState = createSessionState(settings);
871881
openSession();
872882
closeSession(previous);
873883

‎app/src/common/shared/org/mozilla/vrbrowser/browser/engine/SessionStore.java‎

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import android.content.Context;
44
import android.content.res.Configuration;
5-
import android.os.Bundle;
65
import android.util.Log;
76
import android.util.Pair;
87

@@ -149,8 +148,21 @@ public void onTrackingProtectionLevelUpdated(int level) {
149148
if (BuildConfig.DEBUG) {
150149
mStoreSubscription = ComponentsAdapter.get().getStore().observeManually(browserState -> {
151150
Log.d(LOGTAG, "Session status BEGIN");
152-
browserState.getTabs().forEach(tabSessionState -> Log.d(LOGTAG, "BrowserStore Session: " + tabSessionState.getId()));
153-
mSessions.forEach(session -> Log.d(LOGTAG, "SessionStore Session: " + session.getId()));
151+
Log.d(LOGTAG, "[Total] BrowserStore: " + browserState.getTabs().size() + ", SessionStore: " + mSessions.size());
152+
for (int i=0; i<browserState.getTabs().size(); i++) {
153+
boolean isPrivate = browserState.getTabs().get(i).getContent().getPrivate();
154+
Log.d(LOGTAG, "BrowserStore Session: " + browserState.getTabs().get(i).getId() + (isPrivate ? " (PB)" : ""));
155+
}
156+
int suspendedCount = 0;
157+
for (int i=0; i<mSessions.size(); i++) {
158+
boolean suspended = mSessions.get(i).getSessionState().mSession == null && !mSessions.get(i).isActive();
159+
boolean isPrivate = mSessions.get(i).isPrivateMode();
160+
Log.d(LOGTAG, "SessionStore Session: " + mSessions.get(i).getId() + (isPrivate ? " (PB)" : "") + (suspended ? " (suspended)" : ""));
161+
if (suspended) {
162+
suspendedCount++;
163+
}
164+
}
165+
Log.d(LOGTAG, "[Alive] BrowserStore: " + browserState.getTabs().size() + ", SessionStore: " + (mSessions.size() - suspendedCount));
154166
Log.d(LOGTAG, "Session status END");
155167
return null;
156168
});
@@ -165,6 +177,10 @@ private Session addSession(@NonNull Session aSession) {
165177
mSessions.add(aSession);
166178
sessionActiveStateChanged();
167179

180+
if (BuildConfig.DEBUG) {
181+
mStoreSubscription.resume();
182+
}
183+
168184
return aSession;
169185
}
170186

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

183199
@NonNull
184200
Session createSession(@NonNull SessionSettings aSettings, @Session.SessionOpenModeFlags int aOpenMode) {
185-
Session session = new Session(mContext, mRuntime, aSettings);
186-
session.addSessionChangeListener(this);
187-
if (aOpenMode == Session.SESSION_OPEN) {
188-
onSessionAdded(session);
189-
session.openSession();
190-
session.setActive(true);
191-
}
201+
Session session = Session.createSession(mContext, mRuntime, aSettings, aOpenMode, this);
192202
return addSession(session);
193203
}
194204

195205
@NonNull
196206
public Session createSuspendedSession(SessionState aRestoreState) {
197-
Session session = new Session(mContext, mRuntime, aRestoreState);
198-
session.addSessionChangeListener(this);
207+
Session session = Session.createSuspendedSession(mContext, mRuntime, aRestoreState, this);
199208
return addSession(session);
200209
}
201210

@@ -204,14 +213,16 @@ public Session createSuspendedSession(final String aUri, final boolean aPrivateM
204213
SessionState state = new SessionState();
205214
state.mUri = aUri;
206215
state.mSettings = new SessionSettings(new SessionSettings.Builder().withDefaultSettings(mContext).withPrivateBrowsing(aPrivateMode));
207-
Session session = new Session(mContext, mRuntime, state);
208-
session.addSessionChangeListener(this);
216+
Session session = Session.createSuspendedSession(mContext, mRuntime, state, this);
209217
return addSession(session);
210218
}
211219

212220
private void shutdownSession(@NonNull Session aSession) {
213221
aSession.setPermissionDelegate(null);
214222
aSession.shutdown();
223+
if (BuildConfig.DEBUG) {
224+
mStoreSubscription.resume();
225+
}
215226
}
216227

217228
public void destroySession(Session aSession) {
@@ -237,6 +248,9 @@ public void suspendAllInactiveSessions() {
237248
session.suspend();
238249
}
239250
}
251+
if (BuildConfig.DEBUG) {
252+
mStoreSubscription.resume();
253+
}
240254
}
241255

242256
public @Nullable Session getSession(String aId) {

0 commit comments

Comments
 (0)