From 0b8d765dc5178f46b9edc2b85f400897ac2be0f3 Mon Sep 17 00:00:00 2001
From: Manuel Martin <manmartin@mozilla.com>
Date: Tue, 14 Jul 2020 11:27:16 +0200
Subject: [PATCH] Avoid using a boolean flag and move add add/remove code to
 Session

---
 .../vrbrowser/browser/engine/Session.java     | 46 ++++++++-----------
 .../browser/engine/SessionStore.java          | 24 ++++++++--
 2 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/app/src/common/shared/org/mozilla/vrbrowser/browser/engine/Session.java b/app/src/common/shared/org/mozilla/vrbrowser/browser/engine/Session.java
index 079f1c79b6..7e0e57a89b 100644
--- a/app/src/common/shared/org/mozilla/vrbrowser/browser/engine/Session.java
+++ b/app/src/common/shared/org/mozilla/vrbrowser/browser/engine/Session.java
@@ -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 mSessionAdded;
 
     public interface BitmapChangedListener {
         void onBitmapChanged(Session aSession, Bitmap aBitmap);
@@ -114,13 +113,10 @@ public interface DrmStateChangedListener {
     public static final int SESSION_OPEN = 0;
     public static final int SESSION_DO_NOT_OPEN = 1;
 
-    protected Session(Context aContext, GeckoRuntime aRuntime,
-                      @NonNull SessionSettings aSettings) {
+    protected Session(Context aContext, GeckoRuntime aRuntime) {
         mContext = aContext;
         mRuntime = aRuntime;
         initialize();
-        mState = createSession(aSettings);
-        mSessionAdded = true;
     }
 
     protected Session(Context aContext, GeckoRuntime aRuntime, @NonNull SessionState aRestoreState) {
@@ -128,7 +124,11 @@ protected Session(Context aContext, GeckoRuntime aRuntime, @NonNull SessionState
         mRuntime = aRuntime;
         initialize();
         mState = aRestoreState;
-        mSessionAdded = false;
+    }
+
+    public void createSession(@NonNull SessionSettings aSettings) {
+        mState = createSessionState(aSettings);
+        mSessionChangeListeners.forEach(listener -> listener.onSessionAdded(this));
     }
 
     private void initialize() {
@@ -161,8 +161,8 @@ private void initialize() {
 
     protected void shutdown() {
         if (mState.mSession != null) {
-            closeSession(mState);
-            mState.mSession = null;
+            setActive(false);
+            suspend();
         }
 
         if (mState.mParentId != null) {
@@ -172,11 +172,6 @@ protected void shutdown() {
             }
         }
 
-        mSessionChangeListeners.forEach(listener -> {
-            listener.onSessionRemoved(mState.mId);
-            mSessionAdded = false;
-        });
-
         mQueuedCalls.clear();
         mNavigationListeners.clear();
         mProgressListeners.clear();
@@ -430,6 +425,8 @@ public void suspend() {
             return;
         }
 
+        mSessionChangeListeners.forEach(listener -> listener.onSessionRemoved(mState.mId));
+
         Log.d(LOGTAG, "Suspending Session: " + mState.mId);
         closeSession(mState);
         mState.mSession = null;
@@ -458,7 +455,7 @@ private void loadDefaultPage() {
         }
     }
 
-    private void restore(boolean addSession) {
+    private void restore() {
         SessionSettings settings = mState.mSettings;
         if (settings == null) {
             settings = new SessionSettings.Builder()
@@ -470,12 +467,7 @@ private void restore(boolean addSession) {
 
         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 (addSession) {
-            mSessionChangeListeners.forEach(listener -> listener.onSessionAdded(this));
-            mSessionAdded = true;
-        }
+        mSessionChangeListeners.forEach(listener -> listener.onSessionAdded(this));
 
         openSession();
 
@@ -498,7 +490,7 @@ private void restore(boolean addSession) {
     }
 
 
-    private SessionState createSession(@NonNull SessionSettings aSettings) {
+    private SessionState createSessionState(@NonNull SessionSettings aSettings) {
         SessionState state = new SessionState();
         state.mSettings = aSettings;
         state.mSession = createGeckoSession(aSettings);
@@ -541,7 +533,9 @@ void recreateSession() {
 
         mState = mState.recreate();
 
-        restore(false);
+        mSessionChangeListeners.forEach(listener -> listener.onSessionRemoved(mState.mId));
+
+        restore();
 
         for (SessionChangeListener listener: mSessionChangeListeners) {
             listener.onSessionStateChanged(this, true);
@@ -563,9 +557,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) {
@@ -803,7 +795,7 @@ public void setActive(boolean aActive) {
             mState.setActive(aActive);
 
         } else if (aActive) {
-            restore(!mSessionAdded);
+            restore();
 
         } else {
             Log.e(LOGTAG, "ERROR: Setting null GeckoView to inactive!");
@@ -868,7 +860,7 @@ public void toggleServo() {
                 .withServo(!isInstanceOfServoSession(mState.mSession))
                 .build();
 
-        mState = createSession(settings);
+        mState = createSessionState(settings);
         openSession();
         closeSession(previous);
 
diff --git a/app/src/common/shared/org/mozilla/vrbrowser/browser/engine/SessionStore.java b/app/src/common/shared/org/mozilla/vrbrowser/browser/engine/SessionStore.java
index b58233fff4..a10851be3d 100644
--- a/app/src/common/shared/org/mozilla/vrbrowser/browser/engine/SessionStore.java
+++ b/app/src/common/shared/org/mozilla/vrbrowser/browser/engine/SessionStore.java
@@ -148,13 +148,19 @@ public void onTrackingProtectionLevelUpdated(int level) {
         if (BuildConfig.DEBUG) {
             mStoreSubscription = ComponentsAdapter.get().getStore().observeManually(browserState -> {
                 Log.d(LOGTAG, "Session status BEGIN");
-                Log.d(LOGTAG, "BrowserStore: " + browserState.getTabs().size() + ", SessionStore: " + mSessions.size());
+                Log.d(LOGTAG, "[Total] BrowserStore: " + browserState.getTabs().size() + ", SessionStore: " + mSessions.size());
                 for (int i=0; i<browserState.getTabs().size(); i++) {
                     Log.d(LOGTAG, "BrowserStore Session: " + browserState.getTabs().get(i).getId());
                 }
+                int suspendedCount = 0;
                 for (int i=0; i<mSessions.size(); i++) {
-                    Log.d(LOGTAG, "SessionStore Session: " + mSessions.get(i).getId());
+                    boolean suspended = mSessions.get(i).getSessionState().mSession == null && !mSessions.get(i).isActive();
+                    Log.d(LOGTAG, "SessionStore Session: " + mSessions.get(i).getId() + (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;
             });
@@ -169,6 +175,10 @@ private Session addSession(@NonNull Session aSession) {
         mSessions.add(aSession);
         sessionActiveStateChanged();
 
+        if (BuildConfig.DEBUG) {
+            mStoreSubscription.resume();
+        }
+
         return aSession;
     }
 
@@ -186,9 +196,9 @@ 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 session = new Session(mContext, mRuntime);
         session.addSessionChangeListener(this);
-        onSessionAdded(session);
+        session.createSession(aSettings);
         if (aOpenMode == Session.SESSION_OPEN) {
             session.openSession();
             session.setActive(true);
@@ -216,6 +226,9 @@ public Session createSuspendedSession(final String aUri, final boolean aPrivateM
     private void shutdownSession(@NonNull Session aSession) {
         aSession.setPermissionDelegate(null);
         aSession.shutdown();
+        if (BuildConfig.DEBUG) {
+            mStoreSubscription.resume();
+        }
     }
 
     public void destroySession(Session aSession) {
@@ -241,6 +254,9 @@ public void suspendAllInactiveSessions() {
                 session.suspend();
             }
         }
+        if (BuildConfig.DEBUG) {
+            mStoreSubscription.resume();
+        }
     }
 
     public @Nullable Session getSession(String aId) {