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@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(); }