From eca52a3f8f8ee514aaa91abb9d6266a0724f6f5c Mon Sep 17 00:00:00 2001 From: Roy Tam Date: Fri, 8 Nov 2019 11:34:50 +0800 Subject: [PATCH] import changes from rmottola/Arctic-Fox: - Revert Bug 1125848 - Consolidate PCompositor's creation-destruction logic because of 10.7 MacOS bustage (ceda5a133) - adapt assertion to be as introduced in Bug 1125848 (877071282) - Bug 1160190 followup. Make ServiceWorker actually disabled on mulet so we can reopen the CLOSED TREE. r=bkelly (1a03ee7c2) - Bug 1123846 - Restrict some activities to be provided by the system app r=ferjm (c7ca76805) - fix header include order (01b1289df) - Bug 1151644 - Don't disallow the basic compositor backend. r=jrmuizel (b70633afd) - Bug 1155823 - Properly shutdown the CompositorVsyncDispatcher. r=kats (a3dee13e8) - add gfxCrashReporterUtils as of 1180688 2015-07-13 (433fa6bdb) - Bug 1029673 - Correctly report OMTC compositing in crash reports - r=Bas (87fc22936) - Bug 1180688 - Detect whether the widget will be able to present frames with BasicCompositor on Mac. r=mstange (842ed309f) --- b2g/app/b2g.js | 8 + dom/activities/ActivitiesService.jsm | 17 + dom/activities/ActivityProxy.js | 16 + dom/activities/moz.build | 2 + dom/activities/tests/mochi/manifest.webapp | 6 + .../tests/mochi/manifest.webapp^headers^ | 1 + dom/activities/tests/mochi/mochitest.ini | 9 + dom/activities/tests/mochi/system.webapp | 6 + .../tests/mochi/system.webapp^headers^ | 1 + .../tests/mochi/test_dev_mode_activity.html | 290 ++++++++++++++++++ gfx/layers/basic/BasicCompositor.cpp | 6 + gfx/layers/basic/BasicCompositor.h | 2 +- gfx/layers/d3d11/CompositorD3D11.cpp | 8 + gfx/layers/d3d9/CompositorD3D9.cpp | 8 + gfx/layers/ipc/CompositorChild.cpp | 79 +---- gfx/layers/ipc/CompositorChild.h | 9 - gfx/layers/opengl/CompositorOGL.cpp | 3 + gfx/src/gfxCrashReporterUtils.cpp | 117 +++++++ gfx/src/gfxCrashReporterUtils.h | 49 +++ gfx/src/moz.build | 2 + layout/build/nsLayoutStatics.cpp | 2 +- widget/cocoa/nsChildView.h | 1 + widget/cocoa/nsChildView.mm | 15 + widget/nsBaseWidget.cpp | 108 ++++--- widget/nsBaseWidget.h | 1 - widget/nsIWidget.h | 11 + widget/windows/nsWindow.cpp | 16 +- 27 files changed, 664 insertions(+), 129 deletions(-) create mode 100644 dom/activities/tests/mochi/manifest.webapp create mode 100644 dom/activities/tests/mochi/manifest.webapp^headers^ create mode 100644 dom/activities/tests/mochi/mochitest.ini create mode 100644 dom/activities/tests/mochi/system.webapp create mode 100644 dom/activities/tests/mochi/system.webapp^headers^ create mode 100644 dom/activities/tests/mochi/test_dev_mode_activity.html create mode 100644 gfx/src/gfxCrashReporterUtils.cpp create mode 100644 gfx/src/gfxCrashReporterUtils.h diff --git a/b2g/app/b2g.js b/b2g/app/b2g.js index c079f974cd..284cd5f292 100644 --- a/b2g/app/b2g.js +++ b/b2g/app/b2g.js @@ -1062,3 +1062,11 @@ pref("gfx.vsync.hw-vsync.enabled", false); pref("gfx.vsync.compositor", false); pref("gfx.touch.resample", false); #endif + +// Comma separated list of activity names that can only be provided by +// the system app in dev mode. +pref("dom.activities.developer_mode_only", "import-app"); + +// mulet apparently loads firefox.js as well as b2g.js, so we have to explicitly +// disable serviceworkers here to get them disabled in mulet. +pref("dom.serviceWorkers.enabled", false); diff --git a/dom/activities/ActivitiesService.jsm b/dom/activities/ActivitiesService.jsm index f442e57fdf..eaecb5db6e 100644 --- a/dom/activities/ActivitiesService.jsm +++ b/dom/activities/ActivitiesService.jsm @@ -355,6 +355,23 @@ let Activities = { calleeApp.appStatus !== Ci.nsIPrincipal.APP_STATUS_CERTIFIED) { return false; } + + // If the activity is in the developer mode activity list, only let the + // system app be a provider. + let isSystemApp = false; + let isDevModeActivity = false; + try { + isSystemApp = + aResult.manifest == Services.prefs.getCharPref("b2g.system_manifest_url"); + isDevModeActivity = + Services.prefs.getCharPref("dom.activities.developer_mode_only") + .split(",").indexOf(aMsg.options.name) !== -1; + } catch(e) {} + + if (isDevModeActivity && !isSystemApp) { + return false; + } + return ActivitiesServiceFilter.match(aMsg.options.data, aResult.description.filters); }; diff --git a/dom/activities/ActivityProxy.js b/dom/activities/ActivityProxy.js index 560ee798bc..4d65fc2ba7 100644 --- a/dom/activities/ActivityProxy.js +++ b/dom/activities/ActivityProxy.js @@ -69,6 +69,22 @@ ActivityProxy.prototype = { return; } + // Check the activities that are restricted to be used in dev mode. + let devMode = false; + let isDevModeActivity = false; + try { + devMode = Services.prefs.getBoolPref("dom.apps.developer_mode"); + isDevModeActivity = + Services.prefs.getCharPref("dom.activities.developer_mode_only") + .split(",").indexOf(aOptions.name) !== -1; + + } catch(e) {} + if (isDevModeActivity && !devMode) { + Services.DOMRequest.fireErrorAsync(this.activity, "SecurityError"); + Services.obs.notifyObservers(null, "Activity:Error", null); + return; + } + cpmm.addMessageListener("Activity:FireSuccess", this); cpmm.addMessageListener("Activity:FireError", this); diff --git a/dom/activities/moz.build b/dom/activities/moz.build index 98a5601e34..4a4a455430 100644 --- a/dom/activities/moz.build +++ b/dom/activities/moz.build @@ -8,6 +8,8 @@ DIRS += ['interfaces'] XPCSHELL_TESTS_MANIFESTS += ['tests/unit/xpcshell.ini'] +MOCHITEST_CHROME_MANIFESTS += ['tests/mochi/mochitest.ini'] + EXPORTS.mozilla.dom += [ 'Activity.h', ] diff --git a/dom/activities/tests/mochi/manifest.webapp b/dom/activities/tests/mochi/manifest.webapp new file mode 100644 index 0000000000..dfdee325c4 --- /dev/null +++ b/dom/activities/tests/mochi/manifest.webapp @@ -0,0 +1,6 @@ +{ + "name": "Random app", + "activities": { + "import-app": { "blob": { "required": true } } + } +} diff --git a/dom/activities/tests/mochi/manifest.webapp^headers^ b/dom/activities/tests/mochi/manifest.webapp^headers^ new file mode 100644 index 0000000000..3cea33fec8 --- /dev/null +++ b/dom/activities/tests/mochi/manifest.webapp^headers^ @@ -0,0 +1 @@ +Content-Type: application/manifest+json \ No newline at end of file diff --git a/dom/activities/tests/mochi/mochitest.ini b/dom/activities/tests/mochi/mochitest.ini new file mode 100644 index 0000000000..23925f10b9 --- /dev/null +++ b/dom/activities/tests/mochi/mochitest.ini @@ -0,0 +1,9 @@ +[DEFAULT] +skip-if = e10s +support-files = + system.webapp + system.webapp^headers^ + manifest.webapp + manifest.webapp^headers^ + +[test_dev_mode_activity.html] diff --git a/dom/activities/tests/mochi/system.webapp b/dom/activities/tests/mochi/system.webapp new file mode 100644 index 0000000000..cc109330de --- /dev/null +++ b/dom/activities/tests/mochi/system.webapp @@ -0,0 +1,6 @@ +{ + "name": "System app", + "activities": { + "import-app": { "blob": { "required": true } } + } +} diff --git a/dom/activities/tests/mochi/system.webapp^headers^ b/dom/activities/tests/mochi/system.webapp^headers^ new file mode 100644 index 0000000000..3cea33fec8 --- /dev/null +++ b/dom/activities/tests/mochi/system.webapp^headers^ @@ -0,0 +1 @@ +Content-Type: application/manifest+json \ No newline at end of file diff --git a/dom/activities/tests/mochi/test_dev_mode_activity.html b/dom/activities/tests/mochi/test_dev_mode_activity.html new file mode 100644 index 0000000000..c13f39bec5 --- /dev/null +++ b/dom/activities/tests/mochi/test_dev_mode_activity.html @@ -0,0 +1,290 @@ + + + + + Test for Bug {1123846} + + + + + + +Mozilla Bug {1123846} +

+ +
+
+
+ + diff --git a/gfx/layers/basic/BasicCompositor.cpp b/gfx/layers/basic/BasicCompositor.cpp index e8af9e0de2..7b419a79a0 100644 --- a/gfx/layers/basic/BasicCompositor.cpp +++ b/gfx/layers/basic/BasicCompositor.cpp @@ -82,6 +82,12 @@ BasicCompositor::~BasicCompositor() MOZ_COUNT_DTOR(BasicCompositor); } +bool +BasicCompositor::Initialize() +{ + return mWidget ? mWidget->InitCompositor(this) : false; +}; + int32_t BasicCompositor::GetMaxTextureSize() const { diff --git a/gfx/layers/basic/BasicCompositor.h b/gfx/layers/basic/BasicCompositor.h index 71c4271bf7..b534adb0bf 100644 --- a/gfx/layers/basic/BasicCompositor.h +++ b/gfx/layers/basic/BasicCompositor.h @@ -48,7 +48,7 @@ protected: virtual ~BasicCompositor(); public: - virtual bool Initialize() override { return true; }; + virtual bool Initialize() override; virtual void Destroy() override; diff --git a/gfx/layers/d3d11/CompositorD3D11.cpp b/gfx/layers/d3d11/CompositorD3D11.cpp index c8f0065279..e3a4c1fb76 100644 --- a/gfx/layers/d3d11/CompositorD3D11.cpp +++ b/gfx/layers/d3d11/CompositorD3D11.cpp @@ -16,6 +16,7 @@ #include "mozilla/layers/Effects.h" #include "nsWindowsHelpers.h" #include "gfxPrefs.h" +#include "gfxCrashReporterUtils.h" #include "gfxVR.h" #include "mozilla/EnumeratedArray.h" @@ -130,6 +131,8 @@ CompositorD3D11::Initialize() { bool force = gfxPrefs::LayersAccelerationForceEnabled(); + ScopedGfxFeatureReporter reporter("D3D11 Layers", force); + if (!gfxPlatform::CanUseDirect3D11()) { NS_WARNING("Direct3D 11-accelerated layers are not supported on this system."); return false; @@ -381,6 +384,11 @@ CompositorD3D11::Initialize() DXGI_MWA_NO_WINDOW_CHANGES); } + if (!mWidget->InitCompositor(this)) { + return false; + } + + reporter.SetSuccessful(); return true; } diff --git a/gfx/layers/d3d9/CompositorD3D9.cpp b/gfx/layers/d3d9/CompositorD3D9.cpp index f20448c4cb..a242c43bac 100644 --- a/gfx/layers/d3d9/CompositorD3D9.cpp +++ b/gfx/layers/d3d9/CompositorD3D9.cpp @@ -16,6 +16,7 @@ #include "mozilla/layers/PCompositorParent.h" #include "mozilla/layers/LayerManagerComposite.h" #include "gfxPrefs.h" +#include "gfxCrashReporterUtils.h" namespace mozilla { namespace layers { @@ -41,6 +42,8 @@ CompositorD3D9::Initialize() { bool force = gfxPrefs::LayersAccelerationForceEnabled(); + ScopedGfxFeatureReporter reporter("D3D9 Layers", force); + if (!gfxPlatform::CanUseDirect3D9()) { NS_WARNING("Direct3D 9-accelerated layers are not supported on this system."); return false; @@ -58,6 +61,11 @@ CompositorD3D9::Initialize() return false; } + if (!mWidget->InitCompositor(this)) { + return false; + } + + reporter.SetSuccessful(); return true; } diff --git a/gfx/layers/ipc/CompositorChild.cpp b/gfx/layers/ipc/CompositorChild.cpp index 7c975d3e92..c469a49389 100644 --- a/gfx/layers/ipc/CompositorChild.cpp +++ b/gfx/layers/ipc/CompositorChild.cpp @@ -5,7 +5,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "mozilla/layers/CompositorChild.h" -#include "mozilla/layers/CompositorParent.h" #include // for size_t #include "ClientLayerManager.h" // for ClientLayerManager #include "base/message_loop.h" // for MessageLoop @@ -40,58 +39,19 @@ Atomic CompositableForwarder::sSerialCounter(0); CompositorChild::CompositorChild(ClientLayerManager *aLayerManager) : mLayerManager(aLayerManager) - , mCanSend(false) + , mCanSend(true) { } CompositorChild::~CompositorChild() { - if (mCanSend) { - gfxCriticalError() << "CompositorChild was not deinitialized"; - } -} - -static void DeferredDestroyCompositor(nsRefPtr aCompositorParent, - nsRefPtr aCompositorChild) -{ - // Bug 848949 needs to be fixed before - // we can close the channel properly - //aCompositorChild->Close(); } void CompositorChild::Destroy() { - // This must not be called from the destructor! - MOZ_ASSERT(mRefCnt != 0); - - if (!mCanSend) { - return; - } - - mCanSend = false; - - // Destroying the layer manager may cause all sorts of things to happen, so - // let's make sure there is still a reference to keep this alive whatever - // happens. - nsRefPtr selfRef = this; - - SendWillStop(); - // The call just made to SendWillStop can result in IPC from the - // CompositorParent to the CompositorChild (e.g. caused by the destruction - // of shared memory). We need to ensure this gets processed by the - // CompositorChild before it gets destroyed. It suffices to ensure that - // events already in the MessageLoop get processed before the - // CompositorChild is destroyed, so we add a task to the MessageLoop to - // handle compositor desctruction. - - // From now on the only message we can send is Stop. - - if (mLayerManager) { - mLayerManager->Destroy(); - mLayerManager = nullptr; - } - + mLayerManager->Destroy(); + mLayerManager = nullptr; // start from the end of the array because Destroy() can cause the // LayerTransactionChild to be removed from the array. for (int i = ManagedPLayerTransactionChild().Length() - 1; i >= 0; --i) { @@ -99,13 +59,8 @@ CompositorChild::Destroy() static_cast(ManagedPLayerTransactionChild()[i]); layers->Destroy(); } - + MOZ_ASSERT(!mCanSend); SendStop(); - - // The DeferredDestroyCompositor task takes ownership of compositorParent and - // will release them when it runs. - MessageLoop::current()->PostTask(FROM_HERE, - NewRunnableFunction(DeferredDestroyCompositor, mCompositorParent, selfRef)); } bool @@ -132,8 +87,6 @@ CompositorChild::Create(Transport* aTransport, ProcessId aOtherPid) return nullptr; } - child->mCanSend = true; - // We release this ref in ActorDestroy(). sCompositor = child.forget().take(); @@ -146,18 +99,6 @@ CompositorChild::Create(Transport* aTransport, ProcessId aOtherPid) return sCompositor; } -bool -CompositorChild::OpenSameProcess(CompositorParent* aParent) -{ - MOZ_ASSERT(aParent); - - mCompositorParent = aParent; - mCanSend = Open(mCompositorParent->GetIPCChannel(), - CompositorParent::CompositorLoop(), - ipc::ChildSide); - return mCanSend; -} - /*static*/ CompositorChild* CompositorChild::Get() { @@ -388,7 +329,14 @@ CompositorChild::ActorDestroy(ActorDestroyReason aWhy) NS_RUNTIMEABORT("ActorDestroy by IPC channel failure at CompositorChild"); } #endif - + if (sCompositor) { + sCompositor->Release(); + sCompositor = nullptr; + } + // We don't want to release the ref to sCompositor here, during + // cleanup, because that will cause it to be deleted while it's + // still being used. So defer the deletion to after it's not in + // use. MessageLoop::current()->PostTask( FROM_HERE, NewRunnableMethod(this, &CompositorChild::Release)); @@ -527,6 +475,9 @@ CompositorChild::CancelNotifyAfterRemotePaint(TabChild* aTabChild) bool CompositorChild::SendWillStop() { + MOZ_ASSERT(mCanSend); + // From now on the only two messages we can send are WillStop and Stop. + mCanSend = false; return PCompositorChild::SendWillStop(); } diff --git a/gfx/layers/ipc/CompositorChild.h b/gfx/layers/ipc/CompositorChild.h index 09a9249ec5..0f2afd75be 100644 --- a/gfx/layers/ipc/CompositorChild.h +++ b/gfx/layers/ipc/CompositorChild.h @@ -60,12 +60,6 @@ public: static PCompositorChild* Create(Transport* aTransport, ProcessId aOtherProcess); - /** - * Initialize the CompositorChild and open the connection in the non-multi-process - * case. - */ - bool OpenSameProcess(CompositorParent* aParent); - static CompositorChild* Get(); static bool ChildProcessHasCompositor() { return sCompositor != nullptr; } @@ -174,9 +168,6 @@ private: void* aLayerTransactionChild); nsRefPtr mLayerManager; - // When not multi-process, hold a reference to the CompositorParent to keep it - // alive. This reference should be null in multi-process. - nsRefPtr mCompositorParent; // The ViewID of the FrameMetrics is used as the key for this hash table. // While this should be safe to use since the ViewID is unique diff --git a/gfx/layers/opengl/CompositorOGL.cpp b/gfx/layers/opengl/CompositorOGL.cpp index 525794a1fa..b55b7cf007 100644 --- a/gfx/layers/opengl/CompositorOGL.cpp +++ b/gfx/layers/opengl/CompositorOGL.cpp @@ -13,6 +13,7 @@ #include "Layers.h" // for WriteSnapshotToDumpFile #include "LayerScope.h" // for LayerScope #include "gfx2DGlue.h" // for ThebesFilter +#include "gfxCrashReporterUtils.h" // for ScopedGfxFeatureReporter #include "GraphicsFilter.h" // for GraphicsFilter #include "gfxPlatform.h" // for gfxPlatform #include "gfxPrefs.h" // for gfxPrefs @@ -208,6 +209,8 @@ CompositorOGL::Initialize() { bool force = gfxPrefs::LayersAccelerationForceEnabled(); + ScopedGfxFeatureReporter reporter("GL Layers", force); + // Do not allow double initialization MOZ_ASSERT(mGLContext == nullptr, "Don't reinitialize CompositorOGL"); diff --git a/gfx/src/gfxCrashReporterUtils.cpp b/gfx/src/gfxCrashReporterUtils.cpp new file mode 100644 index 0000000000..b7321e95e7 --- /dev/null +++ b/gfx/src/gfxCrashReporterUtils.cpp @@ -0,0 +1,117 @@ +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "gfxCrashReporterUtils.h" + +#if defined(MOZ_CRASHREPORTER) +#define MOZ_GFXFEATUREREPORTER 1 +#endif + +#ifdef MOZ_GFXFEATUREREPORTER +#include "gfxCrashReporterUtils.h" +#include // for strcmp +#include "mozilla/Assertions.h" // for MOZ_ASSERT_HELPER2 +#include "mozilla/Services.h" // for GetObserverService +#include "mozilla/StaticMutex.h" +#include "mozilla/mozalloc.h" // for operator new, etc +#include "nsAutoPtr.h" // for nsRefPtr +#include "nsCOMPtr.h" // for nsCOMPtr +#include "nsError.h" // for NS_OK, NS_FAILED, nsresult +#include "nsExceptionHandler.h" // for AppendAppNotesToCrashReport +#include "nsID.h" +#include "nsIEventTarget.h" // for NS_DISPATCH_NORMAL +#include "nsIObserver.h" // for nsIObserver, etc +#include "nsIObserverService.h" // for nsIObserverService +#include "nsIRunnable.h" // for nsIRunnable +#include "nsISupports.h" +#include "nsString.h" // for nsAutoCString, nsCString, etc +#include "nsTArray.h" // for nsTArray +#include "nsThreadUtils.h" // for NS_DispatchToMainThread, etc +#include "nscore.h" // for NS_IMETHOD, NS_IMETHODIMP, etc + +namespace mozilla { + +static nsTArray *gFeaturesAlreadyReported = nullptr; +static StaticMutex gFeaturesAlreadyReportedMutex; + +class ObserverToDestroyFeaturesAlreadyReported final : public nsIObserver +{ + +public: + NS_DECL_ISUPPORTS + NS_DECL_NSIOBSERVER + + ObserverToDestroyFeaturesAlreadyReported() {} +private: + virtual ~ObserverToDestroyFeaturesAlreadyReported() {} +}; + +NS_IMPL_ISUPPORTS(ObserverToDestroyFeaturesAlreadyReported, + nsIObserver) + +NS_IMETHODIMP +ObserverToDestroyFeaturesAlreadyReported::Observe(nsISupports* aSubject, + const char* aTopic, + const char16_t* aData) +{ + if (!strcmp(aTopic, "xpcom-shutdown")) { + StaticMutexAutoLock al(gFeaturesAlreadyReportedMutex); + if (gFeaturesAlreadyReported) { + delete gFeaturesAlreadyReported; + gFeaturesAlreadyReported = nullptr; + } + } + return NS_OK; +} + +class RegisterObserverRunnable : public nsRunnable { +public: + NS_IMETHOD Run() override { + // LeakLog made me do this. Basically, I just wanted gFeaturesAlreadyReported to be a static nsTArray, + // and LeakLog was complaining about leaks like this: + // leaked 1 instance of nsTArray_base with size 8 bytes + // leaked 7 instances of nsStringBuffer with size 8 bytes each (56 bytes total) + // So this is a work-around using a pointer, and using a nsIObserver to deallocate on xpcom shutdown. + // Yay for fighting bloat. + nsCOMPtr observerService = mozilla::services::GetObserverService(); + if (!observerService) + return NS_OK; + nsRefPtr observer = new ObserverToDestroyFeaturesAlreadyReported; + observerService->AddObserver(observer, "xpcom-shutdown", false); + return NS_OK; + } +}; + +void +ScopedGfxFeatureReporter::WriteAppNote(char statusChar) +{ + StaticMutexAutoLock al(gFeaturesAlreadyReportedMutex); + + if (!gFeaturesAlreadyReported) { + gFeaturesAlreadyReported = new nsTArray; + nsCOMPtr r = new RegisterObserverRunnable(); + NS_DispatchToMainThread(r); + } + + nsAutoCString featureString; + featureString.AppendPrintf("%s%c ", + mFeature, + mStatusChar); + + if (!gFeaturesAlreadyReported->Contains(featureString)) { + gFeaturesAlreadyReported->AppendElement(featureString); + CrashReporter::AppendAppNotesToCrashReport(featureString); + } +} + +} // end namespace mozilla + +#else + +namespace mozilla { +void ScopedGfxFeatureReporter::WriteAppNote(char) {} +} + +#endif diff --git a/gfx/src/gfxCrashReporterUtils.h b/gfx/src/gfxCrashReporterUtils.h new file mode 100644 index 0000000000..f67139a79a --- /dev/null +++ b/gfx/src/gfxCrashReporterUtils.h @@ -0,0 +1,49 @@ +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef gfxCrashReporterUtils_h__ +#define gfxCrashReporterUtils_h__ + +#include "gfxCore.h" + +namespace mozilla { + +/** \class ScopedGfxFeatureReporter + * + * On creation, adds "FeatureName?" to AppNotes + * On destruction, adds "FeatureName-", or "FeatureName+" if you called SetSuccessful(). + * + * Any such string is added at most once to AppNotes, and is subsequently skipped. + * + * This ScopedGfxFeatureReporter class is designed to be fool-proof to use in functions that + * have many exit points. We don't want to encourage having function with many exit points. + * It just happens that our graphics features initialization functions are like that. + */ +class NS_GFX ScopedGfxFeatureReporter +{ +public: + explicit ScopedGfxFeatureReporter(const char *aFeature, bool force = false) + : mFeature(aFeature), mStatusChar('-') + { + WriteAppNote(force ? '!' : '?'); + } + ~ScopedGfxFeatureReporter() { + WriteAppNote(mStatusChar); + } + void SetSuccessful() { mStatusChar = '+'; } + + class AppNoteWritingRunnable; + +protected: + const char *mFeature; + char mStatusChar; + +private: + void WriteAppNote(char statusChar); +}; + +} // end namespace mozilla + +#endif // gfxCrashReporterUtils_h__ diff --git a/gfx/src/moz.build b/gfx/src/moz.build index 84c84a3279..a5cf9ecb75 100644 --- a/gfx/src/moz.build +++ b/gfx/src/moz.build @@ -14,6 +14,7 @@ XPIDL_MODULE = 'gfx' EXPORTS += [ 'FilterSupport.h', 'gfxCore.h', + 'gfxCrashReporterUtils.h', 'nsBoundingMetrics.h', 'nsColor.h', 'nsColorNameList.h', @@ -46,6 +47,7 @@ if CONFIG['MOZ_X11']: UNIFIED_SOURCES += [ 'FilterSupport.cpp', + 'gfxCrashReporterUtils.cpp', 'nsColor.cpp', 'nsFont.cpp', 'nsFontMetrics.cpp', diff --git a/layout/build/nsLayoutStatics.cpp b/layout/build/nsLayoutStatics.cpp index 06a8f04cec..c00eb1c504 100644 --- a/layout/build/nsLayoutStatics.cpp +++ b/layout/build/nsLayoutStatics.cpp @@ -131,8 +131,8 @@ using namespace mozilla::system; #include "nsDocument.h" #include "mozilla/dom/HTMLVideoElement.h" #include "CameraPreferences.h" -#include "MediaDecoder.h" #include "TouchManager.h" +#include "MediaDecoder.h" using namespace mozilla; using namespace mozilla::net; diff --git a/widget/cocoa/nsChildView.h b/widget/cocoa/nsChildView.h index 54e643a7b2..3d88077a8f 100644 --- a/widget/cocoa/nsChildView.h +++ b/widget/cocoa/nsChildView.h @@ -529,6 +529,7 @@ public: already_AddRefed StartRemoteDrawing() override; void EndRemoteDrawing() override; void CleanupRemoteDrawing() override; + bool InitCompositor(mozilla::layers::Compositor* aCompositor) override; APZCTreeManager* APZCTM() { return mAPZC ; } diff --git a/widget/cocoa/nsChildView.mm b/widget/cocoa/nsChildView.mm index e990325d68..8f8233f766 100644 --- a/widget/cocoa/nsChildView.mm +++ b/widget/cocoa/nsChildView.mm @@ -2493,6 +2493,8 @@ nsChildView::EnsureVibrancyManager() already_AddRefed nsChildView::StartRemoteDrawing() { + // should have created the GLPresenter in InitCompositor. + MOZ_ASSERT(mGLPresenter); if (!mGLPresenter) { mGLPresenter = GLPresenter::CreateForWindow(this); @@ -2537,6 +2539,19 @@ nsChildView::CleanupRemoteDrawing() mGLPresenter = nullptr; } +bool +nsChildView::InitCompositor(Compositor* aCompositor) +{ + if (aCompositor->GetBackendType() == LayersBackend::LAYERS_BASIC) { + if (!mGLPresenter) { + mGLPresenter = GLPresenter::CreateForWindow(this); + } + + return !!mGLPresenter; + } + return true; +} + void nsChildView::DoRemoteComposition(const nsIntRect& aRenderRect) { diff --git a/widget/nsBaseWidget.cpp b/widget/nsBaseWidget.cpp index cd90165e60..e4b9376caa 100644 --- a/widget/nsBaseWidget.cpp +++ b/widget/nsBaseWidget.cpp @@ -181,24 +181,45 @@ nsBaseWidget::Shutdown() mShutdownObserver = nullptr; } +static void DeferredDestroyCompositor(nsRefPtr aCompositorParent, + nsRefPtr aCompositorChild) +{ + // Bug 848949 needs to be fixed before + // we can close the channel properly + //aCompositorChild->Close(); +} + void nsBaseWidget::DestroyCompositor() { if (mCompositorChild) { - // XXX CompositorChild and CompositorParent might be re-created in - // ClientLayerManager destructor. See bug 1133426. - nsRefPtr compositorChild = mCompositorChild; - nsRefPtr compositorParent = mCompositorParent; - mCompositorChild->Destroy(); - } -} + nsRefPtr compositorChild = mCompositorChild.forget(); + nsRefPtr compositorParent = mCompositorParent.forget(); -void nsBaseWidget::DestroyLayerManager() -{ - if (mLayerManager) { - mLayerManager->Destroy(); - mLayerManager = nullptr; + compositorChild->SendWillStop(); + // New LayerManager, CompositorParent and CompositorChild might be created + // as a result of internal GetLayerManager() call. + compositorChild->Destroy(); + + // The call just made to SendWillStop can result in IPC from the + // CompositorParent to the CompositorChild (e.g. caused by the destruction + // of shared memory). We need to ensure this gets processed by the + // CompositorChild before it gets destroyed. It suffices to ensure that + // events already in the MessageLoop get processed before the + // CompositorChild is destroyed, so we add a task to the MessageLoop to + // handle compositor desctruction. + + // The DefferedDestroyCompositor task takes ownership of compositorParent and + // will release them when it runs. + MessageLoop::current()->PostTask(FROM_HERE, + NewRunnableFunction(DeferredDestroyCompositor, compositorParent, + compositorChild)); + } + + // Can have base widgets that are things like tooltips + // which don't have CompositorVsyncDispatchers + if (mCompositorVsyncDispatcher) { + mCompositorVsyncDispatcher->Shutdown(); } - DestroyCompositor(); } //------------------------------------------------------------------------- @@ -213,6 +234,11 @@ nsBaseWidget::~nsBaseWidget() static_cast(mLayerManager.get())->ClearRetainerWidget(); } + if (mLayerManager) { + mLayerManager->Destroy(); + mLayerManager = nullptr; + } + if (mShutdownObserver) { // If the shutdown observer is currently processing observers, // then UnregisterShutdownObserver won't stop our Observer @@ -222,7 +248,7 @@ nsBaseWidget::~nsBaseWidget() nsContentUtils::UnregisterShutdownObserver(mShutdownObserver); } - DestroyLayerManager(); + DestroyCompositor(); #ifdef NOISY_WIDGET_LEAKS gNumWidgets--; @@ -230,11 +256,6 @@ nsBaseWidget::~nsBaseWidget() #endif delete mOriginalBounds; - - // Can have base widgets that are things like tooltips which don't have CompositorVsyncDispatchers - if (mCompositorVsyncDispatcher) { - mCompositorVsyncDispatcher->Shutdown(); - } } //------------------------------------------------------------------------- @@ -1061,12 +1082,8 @@ void nsBaseWidget::CreateCompositor(int aWidth, int aHeight) MOZ_ASSERT(gfxPlatform::UsesOffMainThreadCompositing(), "This function assumes OMTC"); - MOZ_ASSERT(!mCompositorParent && !mCompositorChild, - "Should have properly cleaned up the previous PCompositor pair beforehand"); - - if (mCompositorChild) { - mCompositorChild->Destroy(); - } + MOZ_ASSERT(!mCompositorParent, + "Should have properly cleaned up the previous CompositorParent beforehand"); // Recreating this is tricky, as we may still have an old and we need // to make sure it's properly destroyed by calling DestroyCompositor! @@ -1079,9 +1096,11 @@ void nsBaseWidget::CreateCompositor(int aWidth, int aHeight) CreateCompositorVsyncDispatcher(); mCompositorParent = NewCompositorParent(aWidth, aHeight); + MessageChannel *parentChannel = mCompositorParent->GetIPCChannel(); nsRefPtr lm = new ClientLayerManager(this); + MessageLoop *childMessageLoop = CompositorParent::CompositorLoop(); mCompositorChild = new CompositorChild(lm); - mCompositorChild->OpenSameProcess(mCompositorParent); + mCompositorChild->Open(parentChannel, childMessageLoop, ipc::ChildSide); // Make sure the parent knows it is same process. mCompositorParent->SetOtherProcessId(kCurrentProcessId); @@ -1096,37 +1115,32 @@ void nsBaseWidget::CreateCompositor(int aWidth, int aHeight) nsTArray backendHints; GetPreferredCompositorBackends(backendHints); -#if !defined(MOZ_X11) && !defined(XP_WIN) - if (!mRequireOffMainThreadCompositing && - !Preferences::GetBool("layers.offmainthreadcomposition.force-basic", false)) { - for (size_t i = 0; i < backendHints.Length(); ++i) { - if (backendHints[i] == LayersBackend::LAYERS_BASIC) { - backendHints[i] = LayersBackend::LAYERS_NONE; - } - } - } -#endif - bool success = false; if (!backendHints.IsEmpty()) { shadowManager = mCompositorChild->SendPLayerTransactionConstructor( backendHints, 0, &textureFactoryIdentifier, &success); } - ShadowLayerForwarder* lf = lm->AsShadowForwarder(); + if (success) { + ShadowLayerForwarder* lf = lm->AsShadowForwarder(); + if (!lf) { + lm = nullptr; + mCompositorChild = nullptr; + return; + } + lf->SetShadowManager(shadowManager); + lf->IdentifyTextureHost(textureFactoryIdentifier); + ImageBridgeChild::IdentifyCompositorTextureHost(textureFactoryIdentifier); + WindowUsesOMTC(); - if (!success || !lf) { - NS_WARNING("Failed to create an OMT compositor."); - DestroyCompositor(); + mLayerManager = lm.forget(); return; } - lf->SetShadowManager(shadowManager); - lf->IdentifyTextureHost(textureFactoryIdentifier); - ImageBridgeChild::IdentifyCompositorTextureHost(textureFactoryIdentifier); - WindowUsesOMTC(); - - mLayerManager = lm.forget(); + NS_WARNING("Failed to create an OMT compositor."); + DestroyCompositor(); + // Compositor child had the only reference to LayerManager and will have + // deallocated it when being freed. } bool nsBaseWidget::ShouldUseOffMainThreadCompositing() diff --git a/widget/nsBaseWidget.h b/widget/nsBaseWidget.h index 6e9e8f9570..f13e6c738b 100644 --- a/widget/nsBaseWidget.h +++ b/widget/nsBaseWidget.h @@ -441,7 +441,6 @@ protected: * reached (This is the case with gtk2 for instance). */ void DestroyCompositor(); - void DestroyLayerManager(); nsIWidgetListener* mWidgetListener; nsIWidgetListener* mAttachedWidgetListener; diff --git a/widget/nsIWidget.h b/widget/nsIWidget.h index be737c7073..cdf3d7aec5 100644 --- a/widget/nsIWidget.h +++ b/widget/nsIWidget.h @@ -46,6 +46,7 @@ class PluginWidgetChild; } namespace layers { class Composer2D; +class Compositor; class CompositorChild; class LayerManager; class LayerManagerComposite; @@ -1861,6 +1862,16 @@ class nsIWidget : public nsISupports { uint32_t aNativeMessage, uint32_t aModifierFlags) = 0; + /** + * A hook for the widget to prepare a Compositor, during the latter's initialization. + * + * If this method returns true, it means that the widget will be able to + * present frames from the compoositor. + * Returning false will cause the compositor's initialization to fail, and + * a different compositor backend will be used (if any). + */ + virtual bool InitCompositor(mozilla::layers::Compositor*) { return true; } + /** * A shortcut to SynthesizeNativeMouseEvent, abstracting away the native message. * aPoint is location in device pixels to which the mouse pointer moves to. diff --git a/widget/windows/nsWindow.cpp b/widget/windows/nsWindow.cpp index 1893af45aa..08b0171f39 100644 --- a/widget/windows/nsWindow.cpp +++ b/widget/windows/nsWindow.cpp @@ -674,7 +674,11 @@ NS_METHOD nsWindow::Destroy() * On windows the LayerManagerOGL destructor wants the widget to be around for * cleanup. It also would like to have the HWND intact, so we nullptr it here. */ - DestroyLayerManager(); + if (mLayerManager) { + mLayerManager->Destroy(); + } + mLayerManager = nullptr; + DestroyCompositor(); /* We should clear our cached resources now and not wait for the GC to * delete the nsWindow. */ @@ -3325,13 +3329,10 @@ nsWindow::GetLayerManager(PLayerTransactionChild* aShadowManager, } if (!mLayerManager) { + MOZ_ASSERT(!mCompositorParent && !mCompositorChild); mLayerManager = CreateBasicLayerManager(); } - // If we don't have a layer manager at this point we shouldn't have a - // PCompositor actor pair either. - MOZ_ASSERT(mLayerManager || (!mCompositorParent && !mCompositorChild)); - NS_ASSERTION(mLayerManager, "Couldn't provide a valid layer manager."); return mLayerManager; @@ -6597,7 +6598,10 @@ nsWindow::IsPopup() void nsWindow::ClearCompositor(nsWindow* aWindow) { - aWindow->DestroyLayerManager(); + if (aWindow->mLayerManager) { + aWindow->mLayerManager = nullptr; + aWindow->DestroyCompositor(); + } } bool