From 0a46071cca8fde9097549e9716648090b5133c21 Mon Sep 17 00:00:00 2001 From: George Wright Date: Thu, 27 Oct 2016 11:04:50 -0400 Subject: [PATCH] Bug 1297790 - Add GPU process feature status to the Telemetry environment r=dvander,gfritzsche data-review=bsmedberg --- gfx/ipc/GPUChild.cpp | 6 +++++ .../telemetry/TelemetryEnvironment.jsm | 22 +++++++++++++++++++ .../telemetry/docs/data/environment.rst | 7 ++++-- .../tests/unit/test_TelemetryEnvironment.js | 4 ++++ widget/GfxInfoBase.cpp | 14 ++++++++++++ widget/GfxInfoBase.h | 1 + widget/nsIGfxInfo.idl | 2 ++ 7 files changed, 54 insertions(+), 2 deletions(-) diff --git a/gfx/ipc/GPUChild.cpp b/gfx/ipc/GPUChild.cpp index 0c56009d3d2b..1497051b432f 100644 --- a/gfx/ipc/GPUChild.cpp +++ b/gfx/ipc/GPUChild.cpp @@ -172,6 +172,12 @@ GPUChild::ActorDestroy(ActorDestroyReason aWhy) #endif Telemetry::Accumulate(Telemetry::SUBPROCESS_ABNORMAL_ABORT, nsDependentCString(XRE_ChildProcessTypeToString(GeckoProcessType_GPU), 1)); + + // Notify the Telemetry environment so that we can refresh and do a subsession split + if (nsCOMPtr obsvc = services::GetObserverService()) { + obsvc->NotifyObservers(nullptr, "compositor:process-aborted", nullptr); + } + } gfxVars::RemoveReceiver(this); diff --git a/toolkit/components/telemetry/TelemetryEnvironment.jsm b/toolkit/components/telemetry/TelemetryEnvironment.jsm index 32baa2ecd2cf..618f75a0e262 100644 --- a/toolkit/components/telemetry/TelemetryEnvironment.jsm +++ b/toolkit/components/telemetry/TelemetryEnvironment.jsm @@ -208,6 +208,7 @@ const PREF_SEARCH_COHORT = "browser.search.cohort"; const PREF_E10S_COHORT = "e10s.rollout.cohort"; const COMPOSITOR_CREATED_TOPIC = "compositor:created"; +const COMPOSITOR_PROCESS_ABORTED_TOPIC = "compositor:process-aborted"; const DISTRIBUTION_CUSTOMIZATION_COMPLETE_TOPIC = "distribution-customization-complete"; const EXPERIMENTS_CHANGED_TOPIC = "experiments-changed"; const GFX_FEATURES_READY_TOPIC = "gfx-features-ready"; @@ -942,6 +943,7 @@ EnvironmentCache.prototype = { _addObservers: function() { // Watch the search engine change and service topics. Services.obs.addObserver(this, COMPOSITOR_CREATED_TOPIC, false); + Services.obs.addObserver(this, COMPOSITOR_PROCESS_ABORTED_TOPIC, false); Services.obs.addObserver(this, DISTRIBUTION_CUSTOMIZATION_COMPLETE_TOPIC, false); Services.obs.addObserver(this, GFX_FEATURES_READY_TOPIC, false); Services.obs.addObserver(this, SEARCH_ENGINE_MODIFIED_TOPIC, false); @@ -950,6 +952,7 @@ EnvironmentCache.prototype = { _removeObservers: function() { Services.obs.removeObserver(this, COMPOSITOR_CREATED_TOPIC); + Services.obs.removeObserver(this, COMPOSITOR_PROCESS_ABORTED_TOPIC); try { Services.obs.removeObserver(this, DISTRIBUTION_CUSTOMIZATION_COMPLETE_TOPIC); } catch (ex) {} @@ -982,6 +985,11 @@ EnvironmentCache.prototype = { // first compositor to be created and then query nsIGfxInfo again. this._updateGraphicsFeatures(); break; + case COMPOSITOR_PROCESS_ABORTED_TOPIC: + // Our compositor process has been killed for whatever reason, so refresh + // our reported graphics features and trigger an environment change. + this._onCompositorProcessAborted(); + break; case DISTRIBUTION_CUSTOMIZATION_COMPLETE_TOPIC: // Distribution customizations are applied after final-ui-startup. query // partner prefs again when they are ready. @@ -1054,6 +1062,20 @@ EnvironmentCache.prototype = { this._onEnvironmentChange("search-engine-changed", oldEnvironment); }, + /** + * Refresh the Telemetry environment and trigger an environment change due to + * a change in compositor process (normally this will mean we've fallen back + * from out-of-process to in-process compositing). + */ + _onCompositorProcessAborted: function() { + this._log.trace("_onCompositorProcessAborted"); + + // Trigger the environment change notification. + let oldEnvironment = Cu.cloneInto(this._currentEnvironment, myScope); + this._updateGraphicsFeatures(); + this._onEnvironmentChange("gfx-features-changed", oldEnvironment); + }, + /** * Update the graphics features object. */ diff --git a/toolkit/components/telemetry/docs/data/environment.rst b/toolkit/components/telemetry/docs/data/environment.rst index ff0d204a4883..b5765e018282 100644 --- a/toolkit/components/telemetry/docs/data/environment.rst +++ b/toolkit/components/telemetry/docs/data/environment.rst @@ -178,7 +178,7 @@ Structure: // "disabled" - User explicitly disabled this default feature. // "failed" - This feature was attempted but failed to initialize. // "available" - User has this feature available. - "d3d11" { // This feature is Windows-only. + d3d11: { // This feature is Windows-only. status: , warp: , // Software rendering (WARP) mode was chosen. textureSharing: // Whether or not texture sharing works. @@ -186,10 +186,13 @@ Structure: blacklisted: , // Whether D3D11 is blacklisted; use to see whether WARP // was blacklist induced or driver-failure induced. }, - "d2d" { // This feature is Windows-only. + d2d: { // This feature is Windows-only. status: , version: , // Either "1.0" or "1.1". }, + gpuProcess: { // Out-of-process compositing ("GPU process") feature + status: , // "Available" means currently in use + }, }, }, }, diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js b/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js index 1320a673cd34..8c8b7feddfbc 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js @@ -635,6 +635,9 @@ function checkSystemSection(data) { Assert.equal(typeof gfxData.features, "object"); Assert.equal(typeof gfxData.features.compositor, "string"); + Assert.equal(typeof gfxData.features.gpuProcess, "object"); + Assert.equal(typeof gfxData.features.gpuProcess.status, "string"); + try { // If we've not got nsIGfxInfoDebug, then this will throw and stop us doing // this test. @@ -647,6 +650,7 @@ function checkSystemSection(data) { let features = gfxInfo.getFeatures(); Assert.equal(features.compositor, gfxData.features.compositor); + Assert.equal(features.gpuProcess.status, gfxData.features.gpuProcess.status); Assert.equal(features.opengl, gfxData.features.opengl); Assert.equal(features.webgl, gfxData.features.webgl); } diff --git a/widget/GfxInfoBase.cpp b/widget/GfxInfoBase.cpp index 5e074d998fcd..742f91b027cd 100644 --- a/widget/GfxInfoBase.cpp +++ b/widget/GfxInfoBase.cpp @@ -48,6 +48,7 @@ using mozilla::MutexAutoLock; nsTArray* GfxInfoBase::mDriverInfo; bool GfxInfoBase::mDriverInfoObserverInitialized; +bool GfxInfoBase::mShutdownOccurred; // Observes for shutdown so that the child GfxDriverInfo list is freed. class ShutdownObserver : public nsIObserver @@ -73,6 +74,8 @@ public: for (uint32_t i = 0; i < DeviceVendorMax; i++) delete GfxDriverInfo::mDeviceVendors[i]; + GfxInfoBase::mShutdownOccurred = true; + return NS_OK; } }; @@ -162,6 +165,7 @@ GetPrefNameForFeature(int32_t aFeature) case nsIGfxInfo::FEATURE_VP8_HW_DECODE: case nsIGfxInfo::FEATURE_VP9_HW_DECODE: case nsIGfxInfo::FEATURE_DX_INTEROP2: + case nsIGfxInfo::FEATURE_GPU_PROCESS: // We don't provide prefs for these features. break; default: @@ -860,6 +864,13 @@ GfxInfoBase::GetFeatureStatusImpl(int32_t aFeature, return NS_OK; } + if (mShutdownOccurred) { + // This is futile; we've already commenced shutdown and our blocklists have + // been deleted. We may want to look into resurrecting the blocklist instead + // but for now, just don't even go there. + return NS_OK; + } + // If an operating system was provided by the derived GetFeatureStatusImpl, // grab it here. Otherwise, the OS is unknown. OperatingSystem os = (aOS ? *aOS : OperatingSystem::Unknown); @@ -1332,6 +1343,9 @@ GfxInfoBase::BuildFeatureStateLog(JSContext* aCx, const FeatureState& aFeature, void GfxInfoBase::DescribeFeatures(JSContext* aCx, JS::Handle aObj) { + JS::Rooted obj(aCx); + gfx::FeatureStatus gpuProcess = gfxConfig::GetValue(Feature::GPU_PROCESS); + InitFeatureObject(aCx, aObj, "gpuProcess", FEATURE_GPU_PROCESS, Some(gpuProcess), &obj); } bool diff --git a/widget/GfxInfoBase.h b/widget/GfxInfoBase.h index ac4cc5940e1c..55c44260df86 100644 --- a/widget/GfxInfoBase.h +++ b/widget/GfxInfoBase.h @@ -78,6 +78,7 @@ public: static nsTArray* mDriverInfo; static bool mDriverInfoObserverInitialized; + static bool mShutdownOccurred; virtual nsString Model() { return EmptyString(); } virtual nsString Hardware() { return EmptyString(); } diff --git a/widget/nsIGfxInfo.idl b/widget/nsIGfxInfo.idl index c9e6e89801a7..3571baedd1b2 100644 --- a/widget/nsIGfxInfo.idl +++ b/widget/nsIGfxInfo.idl @@ -119,6 +119,8 @@ interface nsIGfxInfo : nsISupports const long FEATURE_VP9_HW_DECODE = 18; /* Whether NV_dx_interop2 is supported, starting in 50. */ const long FEATURE_DX_INTEROP2 = 19; + /* Whether the GPU process is supported, starting in 52. */ + const long FEATURE_GPU_PROCESS = 20; /* * A set of return values from GetFeatureStatus