[tbb-commits] [tor-browser] 15/311: Bug 1747116 - Guard against null native window in AndroidCompositorWidget r=gfx-reviewers, geckoview-reviewers, aosmond, m_kato, a=dsmith

[gitolite role]

This is an automated email from the git hooks/post-receive script.

pierov pushed a commit to branch geckoview-99.0.1-11.0-1
in repository tor-browser.

commit 7ef1f0e80025adf8b0b825c4698a3847adb1966e
Author: Jamie Nicol <jnicol at mozilla.com>
AuthorDate: Fri Jan 7 10:25:53 2022 +0000

    Bug 1747116 - Guard against null native window in AndroidCompositorWidget r=gfx-reviewers,geckoview-reviewers,aosmond,m_kato, a=dsmith
    
    If AndroidCompositorWidget's surface reference points to a surface
    that has been destroyed, we can end up with a null ANativeWindow
    pointer. This can result in crashes when using it to query the window
    size.
    
    This patch makes it so that we use the native window to query the size
    only when the surface has changed rather than for every call to
    GetClientSize(). This allows us to guard against a null pointer in a
    single place. If we have a null pointer then return false from
    OnCompositorSurfaceChanged(). CompositorBridgeParent::ResumeComposition()
    will handle that by not resuming the compositor, like it already does
    if WebRenderBridgeParent::Resume() fails.
    
    Additonally, when we receive a pause event from GeckoView ensure that
    we always set the mCompositorPaused flag to true, even if the
    UiCompositorController is null. This avoids a possible cause of the
    situation described above - if we receive a pause event (eg the app is
    minimised) during compositor reinitialization (while the
    UiCompositorController is temporarily null) we would not set that flag
    to true, and would therefore resume compositing in to an invalid
    surface.
    
    Depends on D135117
    
    Differential Revision: https://phabricator.services.mozilla.com/D135118
---
 gfx/layers/ipc/CompositorBridgeParent.cpp  |  4 ++--
 widget/CompositorWidget.h                  |  4 +++-
 widget/android/AndroidCompositorWidget.cpp | 31 ++++++++++++++++++++++--------
 widget/android/AndroidCompositorWidget.h   |  3 ++-
 widget/android/nsWindow.cpp                |  3 ++-
 5 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/gfx/layers/ipc/CompositorBridgeParent.cpp b/gfx/layers/ipc/CompositorBridgeParent.cpp
index dfc601143f379..f4769df26f86b 100644
--- a/gfx/layers/ipc/CompositorBridgeParent.cpp
+++ b/gfx/layers/ipc/CompositorBridgeParent.cpp
@@ -619,9 +619,9 @@ void CompositorBridgeParent::ResumeComposition() {
 
   MonitorAutoLock lock(mResumeCompositionMonitor);
 
-  mWidget->OnResumeComposition();
+  bool resumed = mWidget->OnResumeComposition();
+  resumed = resumed && mWrBridge->Resume();
 
-  bool resumed = mWrBridge->Resume();
   if (!resumed) {
 #ifdef MOZ_WIDGET_ANDROID
     // We can't get a surface. This could be because the activity changed
diff --git a/widget/CompositorWidget.h b/widget/CompositorWidget.h
index 0c3a693f47875..4d38bc6220358 100644
--- a/widget/CompositorWidget.h
+++ b/widget/CompositorWidget.h
@@ -195,8 +195,10 @@ class CompositorWidget {
    *
    * This is called from CompositorBridgeParent::ResumeComposition,
    * immediately prior to webrender being resumed.
+   *
+   * Returns true if composition can be successfully resumed, else false.
    */
-  virtual void OnResumeComposition() {}
+  virtual bool OnResumeComposition() { return true; }
 
   /**
    * Return the size of the drawable area of the widget.
diff --git a/widget/android/AndroidCompositorWidget.cpp b/widget/android/AndroidCompositorWidget.cpp
index d4d9ee88e84bc..217063637f0d9 100644
--- a/widget/android/AndroidCompositorWidget.cpp
+++ b/widget/android/AndroidCompositorWidget.cpp
@@ -6,6 +6,7 @@
 
 #include "AndroidCompositorWidget.h"
 
+#include "mozilla/gfx/Logging.h"
 #include "mozilla/widget/PlatformWidgetTypes.h"
 #include "nsWindow.h"
 
@@ -18,7 +19,8 @@ AndroidCompositorWidget::AndroidCompositorWidget(
     : CompositorWidget(aOptions),
       mWidgetId(aInitData.widgetId()),
       mNativeWindow(nullptr),
-      mFormat(WINDOW_FORMAT_RGBA_8888) {}
+      mFormat(WINDOW_FORMAT_RGBA_8888),
+      mClientSize(0, 0) {}
 
 AndroidCompositorWidget::~AndroidCompositorWidget() {
   if (mNativeWindow) {
@@ -73,24 +75,37 @@ void AndroidCompositorWidget::EndRemoteDrawingInRegion(
   ANativeWindow_unlockAndPost(mNativeWindow);
 }
 
-void AndroidCompositorWidget::OnResumeComposition() {
+bool AndroidCompositorWidget::OnResumeComposition() {
   OnCompositorSurfaceChanged();
-}
 
-EGLNativeWindowType AndroidCompositorWidget::GetEGLNativeWindow() {
-  return (EGLNativeWindowType)mSurface.Get();
-}
+  if (!mSurface) {
+    gfxCriticalError() << "OnResumeComposition called with null Surface";
+    return false;
+  }
 
-LayoutDeviceIntSize AndroidCompositorWidget::GetClientSize() {
   JNIEnv* const env = jni::GetEnvForThread();
   ANativeWindow* const nativeWindow =
       ANativeWindow_fromSurface(env, reinterpret_cast<jobject>(mSurface.Get()));
+  if (!nativeWindow) {
+    gfxCriticalError() << "OnResumeComposition called with invalid Surface";
+    return false;
+  }
+
   const int32_t width = ANativeWindow_getWidth(nativeWindow);
   const int32_t height = ANativeWindow_getHeight(nativeWindow);
+  mClientSize = LayoutDeviceIntSize(width, height);
 
   ANativeWindow_release(nativeWindow);
 
-  return LayoutDeviceIntSize(width, height);
+  return true;
+}
+
+EGLNativeWindowType AndroidCompositorWidget::GetEGLNativeWindow() {
+  return (EGLNativeWindowType)mSurface.Get();
+}
+
+LayoutDeviceIntSize AndroidCompositorWidget::GetClientSize() {
+  return mClientSize;
 }
 
 }  // namespace widget
diff --git a/widget/android/AndroidCompositorWidget.h b/widget/android/AndroidCompositorWidget.h
index f1d2e7774ccab..42779b2f7a46c 100644
--- a/widget/android/AndroidCompositorWidget.h
+++ b/widget/android/AndroidCompositorWidget.h
@@ -44,7 +44,7 @@ class AndroidCompositorWidget : public CompositorWidget {
       gfx::DrawTarget* aDrawTarget,
       const LayoutDeviceIntRegion& aInvalidRegion) override;
 
-  void OnResumeComposition() override;
+  bool OnResumeComposition() override;
 
   AndroidCompositorWidget* AsAndroid() override { return this; }
 
@@ -56,6 +56,7 @@ class AndroidCompositorWidget : public CompositorWidget {
   ANativeWindow* mNativeWindow;
   ANativeWindow_Buffer mBuffer;
   int32_t mFormat;
+  LayoutDeviceIntSize mClientSize;
 };
 
 }  // namespace widget
diff --git a/widget/android/nsWindow.cpp b/widget/android/nsWindow.cpp
index e811212f2d1ab..42b30a62e9cf3 100644
--- a/widget/android/nsWindow.cpp
+++ b/widget/android/nsWindow.cpp
@@ -1114,9 +1114,10 @@ class LayerViewSupport final
   void SyncPauseCompositor() {
     MOZ_ASSERT(AndroidBridge::IsJavaUiThread());
 
+    mCompositorPaused = true;
+
     if (RefPtr<UiCompositorControllerChild> child =
             GetUiCompositorControllerChild()) {
-      mCompositorPaused = true;
       child->Pause();
     }
 

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.