Bug 1589022 - Make the assertion about a compositor options mismatch in RecvAdoptChild more nuanced. r=tnikkel
authorBotond Ballo <botond@mozilla.com>
Mon, 11 Nov 2019 09:23:45 +0000
changeset 502509 e6305a1457f025d598507456b87945036b49bbab
parent 502508 3779e7b766fd91688914046f1efee1a3812f69b0
child 502510 408a199d1d47d782a0a6f43991d72a00a7a7da49
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1589022
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1589022 - Make the assertion about a compositor options mismatch in RecvAdoptChild more nuanced. r=tnikkel If only the APZ enablement changed, produce a warning rather than an assertion. Differential Revision: https://phabricator.services.mozilla.com/D51467
gfx/layers/ipc/CompositorBridgeParent.cpp
--- a/gfx/layers/ipc/CompositorBridgeParent.cpp
+++ b/gfx/layers/ipc/CompositorBridgeParent.cpp
@@ -1730,16 +1730,36 @@ mozilla::ipc::IPCResult CompositorBridge
   LayerTreeOwnerTracker::Get()->Map(aChild, aOwnerPid);
 
   MonitorAutoLock lock(*sIndirectLayerTreesLock);
   NotifyChildCreated(aChild);
   *aOptions = mOptions;
   return IPC_OK();
 }
 
+enum class CompositorOptionsChangeKind {
+  eSupported,
+  eBestEffort,
+  eUnsupported
+};
+
+static CompositorOptionsChangeKind ClassifyCompositorOptionsChange(
+    const CompositorOptions& aOld, const CompositorOptions& aNew) {
+  if (aOld == aNew) {
+    return CompositorOptionsChangeKind::eSupported;
+  }
+  if (aOld.UseAdvancedLayers() == aNew.UseAdvancedLayers() &&
+      aOld.UseWebRender() == aNew.UseWebRender() &&
+      aOld.InitiallyPaused() == aNew.InitiallyPaused()) {
+    // Only APZ enablement changed.
+    return CompositorOptionsChangeKind::eBestEffort;
+  }
+  return CompositorOptionsChangeKind::eUnsupported;
+}
+
 mozilla::ipc::IPCResult CompositorBridgeParent::RecvAdoptChild(
     const LayersId& child) {
   RefPtr<APZUpdater> oldApzUpdater;
   APZCTreeManagerParent* parent;
   bool scheduleComposition = false;
   RefPtr<ContentCompositorBridgeParent> cpcp;
   RefPtr<WebRenderBridgeParent> childWrBridge;
 
@@ -1755,19 +1775,37 @@ mozilla::ipc::IPCResult CompositorBridge
     MonitorAutoLock lock(*sIndirectLayerTreesLock);
     // If child is already belong to this CompositorBridgeParent,
     // no need to handle adopting child.
     if (sIndirectLayerTrees[child].mParent == this) {
       return IPC_OK();
     }
 
     if (sIndirectLayerTrees[child].mParent) {
-      // We currently don't support adopting children from one compositor to
-      // another if the two compositors don't have the same options.
-      MOZ_ASSERT(sIndirectLayerTrees[child].mParent->mOptions == mOptions);
+      switch (ClassifyCompositorOptionsChange(
+          sIndirectLayerTrees[child].mParent->mOptions, mOptions)) {
+        case CompositorOptionsChangeKind::eUnsupported: {
+          MOZ_ASSERT(false,
+                     "Moving tab between windows whose compositor options"
+                     "differ in unsupported ways. Things may break in "
+                     "unexpected ways");
+          break;
+        }
+        case CompositorOptionsChangeKind::eBestEffort: {
+          NS_WARNING(
+              "Moving tab between windows with different APZ enablement. "
+              "This is supported on a best-effort basis, but some things may "
+              "break.");
+          break;
+        }
+        case CompositorOptionsChangeKind::eSupported: {
+          // The common case, no action required.
+          break;
+        }
+      }
       oldApzUpdater = sIndirectLayerTrees[child].mParent->mApzUpdater;
     }
     NotifyChildCreated(child);
     if (sIndirectLayerTrees[child].mLayerTree) {
       sIndirectLayerTrees[child].mLayerTree->SetLayerManager(
           mLayerManager, GetAnimationStorage());
       // Trigger composition to handle a case that mLayerTree was not composited
       // yet by previous CompositorBridgeParent, since nsRefreshDriver might
@@ -1796,28 +1834,21 @@ mozilla::ipc::IPCResult CompositorBridge
         mWrBridge->GetTextureFactoryIdentifier());
     // Pretend we composited, since parent CompositorBridgeParent was replaced.
     TimeStamp now = TimeStamp::Now();
     NotifyPipelineRendered(childWrBridge->PipelineId(), newEpoch, VsyncId(),
                            now, now, now);
   }
 
   if (oldApzUpdater) {
-    // We don't fully support moving a child from an APZ-enabled compositor to a
-    // APZ-disabled compositor. The mOptions assertion above should already
-    // ensure this, since APZ-ness is one of the things in mOptions. Note
-    // however it is possible for mApzUpdater to be non-null here with
-    // oldApzUpdater null, because the child may not have been previously
-    // composited.
-    MOZ_ASSERT(mApzUpdater);
-
-    // As this nonetheless can happen (if e.g. a WebExtension moves a tab
-    // into a popup window), try to handle it gracefully by clearing the
-    // old layer transforms associated with the child. (Since the new compositor
-    // is APZ-disabled, there will be nothing to update the transforms going
+    // If we are moving a child from an APZ-enabled window to an APZ-disabled
+    // window (which can happen if e.g. a WebExtension moves a tab into a
+    // popup window), try to handle it gracefully by clearing the old layer
+    // transforms associated with the child. (Since the new compositor is
+    // APZ-disabled, there will be nothing to update the transforms going
     // forward.)
     if (!mApzUpdater && oldRootController) {
       // Tell the old APZCTreeManager not to send any more layer transforms
       // for this layers ids.
       oldApzUpdater->MarkAsDetached(child);
 
       // Clear the current transforms.
       nsTArray<MatrixMessage> clear;