Fix Surface leaks when closing windows or removing widgets (#1716)

* Fix leak when closing a window (surface.release() not called) * Ensure that surface is released when a widget is removed (before waiting for releaseWidget() call) * Do not recreate the tooltip if already created * Improve WindowWidget.releaseWidget * Hide/show Top and TitleBar widgets instead of recreating them each time
MozillaReality · Aug 30, 2019 · ca2c30b · ca2c30b
1 parent 105bd17
commit ca2c30b
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 17 deletions.
diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/views/UIButton.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/views/UIButton.java
@@ -233,7 +233,9 @@ public void run() {
                 return;
             }
 
-            mTooltipView = new TooltipWidget(getContext());
+            if (mTooltipView == null) {
+                mTooltipView = new TooltipWidget(getContext());
+            }
             mTooltipView.setCurvedMode(mCurvedTooltip);
             mTooltipView.setText(getTooltip());
 

diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/TitleBarWidget.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/TitleBarWidget.java
@@ -37,8 +37,9 @@ public interface Delegate {
 
     private TitleBarBinding mBinding;
     private WindowWidget mAttachedWindow;
-    private boolean mVisible;
+    private boolean mVisible = false;
     private Media mMedia;
+    private boolean mWidgetAdded = false;
 
     public TitleBarWidget(Context aContext) {
         super(aContext);
@@ -123,11 +124,11 @@ public void setVisible(boolean aIsVisible) {
         }
         mVisible = aIsVisible;
         getPlacement().visible = aIsVisible;
-
-        if (aIsVisible) {
+        if (!mWidgetAdded) {
             mWidgetManager.addWidget(this);
+            mWidgetAdded = true;
         } else {
-            mWidgetManager.removeWidget(this);
+            mWidgetManager.updateWidget(this);
         }
     }
 

diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/TopBarWidget.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/TopBarWidget.java
@@ -29,7 +29,8 @@ public class TopBarWidget extends UIWidget implements SessionChangeListener, Wid
     private WindowWidget mAttachedWindow;
     private TopBarWidget.Delegate mDelegate;
     private LinearLayout mMultiWindowControlsContainer;
-    private boolean mVisible;
+    private boolean mVisible = false;
+    private boolean mWidgetAdded = false;
 
     public TopBarWidget(Context aContext) {
         super(aContext);
@@ -171,11 +172,11 @@ public void setVisible(boolean aIsVisible) {
         }
         mVisible = aIsVisible;
         getPlacement().visible = aIsVisible;
-
-        if (aIsVisible) {
+        if (!mWidgetAdded) {
             mWidgetManager.addWidget(this);
+            mWidgetAdded = true;
         } else {
-            mWidgetManager.removeWidget(this);
+            mWidgetManager.updateWidget(this);
         }
     }
 

diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/UIWidget.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/UIWidget.java
@@ -191,13 +191,17 @@ public void handleMoveEvent(float aDeltaX, float aDeltaY, float aDeltaZ, float a
         mWidgetPlacement.rotation = aRotation;
     }
 
-    @Override
-    public void releaseWidget() {
+    private void releaseRenderer() {
         if (mRenderer != null) {
             mRenderer.release();
             mRenderer = null;
         }
         mTexture = null;
+    }
+
+    @Override
+    public void releaseWidget() {
+        releaseRenderer();
         mWidgetManager = null;
     }
 
@@ -327,7 +331,7 @@ public void hide(@HideFlags int aHideFlags) {
             mWidgetPlacement.visible = false;
             if (aHideFlags == REMOVE_WIDGET) {
                 mWidgetManager.removeWidget(this);
-
+                releaseRenderer();
             } else {
                 mWidgetManager.updateWidget(this);
             }

diff --git a/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/WindowWidget.java b/app/src/common/shared/org/mozilla/vrbrowser/ui/widgets/WindowWidget.java
@@ -246,6 +246,12 @@ public void close() {
         mBookmarksView.onDestroy();
         mHistoryView.onDestroy();
         SessionStore.get().destroySessionStack(mWindowId);
+        if (mTopBar != null) {
+            mWidgetManager.removeWidget(mTopBar);
+        }
+        if (mTitleBar != null) {
+            mWidgetManager.removeWidget(mTitleBar);
+        }
     }
 
     public void loadHomeIfNotRestored() {
@@ -783,15 +789,26 @@ public void releaseWidget() {
         mSessionStack.removeProgressListener(this);
         mSessionStack.setHistoryDelegate(null);
         GeckoSession session = mSessionStack.getSession(mSessionId);
-        if (session == null) {
-            return;
-        }
         if (mDisplay != null) {
             mDisplay.surfaceDestroyed();
-            session.releaseDisplay(mDisplay);
+            if (session != null) {
+                session.releaseDisplay(mDisplay);
+            }
             mDisplay = null;
         }
-        session.getTextInput().setView(null);
+        if (session != null) {
+            session.getTextInput().setView(null);
+        }
+        if (mSurface != null) {
+            mSurface.release();
+            mSurface = null;
+        }
+        if (mTexture != null && mRenderer == null) {
+            // Custom SurfaceTexture used for GeckoView
+            mTexture.release();
+            mTexture = null;
+        }
+        super.releaseWidget();
     }