Bug 1567054: Always call EnsureTarget in CanvasRenderingContext2D::GetSurfaceSnapshot. r=jrmuizel
authorBob Owen <bobowencode@gmail.com>
Thu, 08 Aug 2019 17:41:39 +0000
changeset 547523 024f04feae0b73db9946416f7730c90c291308cf
parent 547522 90128608235b621d5ac783f55ae5111110d38398
child 547524 6065f42eab8e68f86b83f693e8b8fc01f9e6b81a
push id11848
push userffxbld-merge
push dateMon, 26 Aug 2019 19:26:25 +0000
treeherdermozilla-beta@9b31bfdfac10 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs1567054
milestone70.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 1567054: Always call EnsureTarget in CanvasRenderingContext2D::GetSurfaceSnapshot. r=jrmuizel This reverts to the previous behavior when the function was defined in the header file, where we always called EnsureTarget first. Not doing this introduces an as yet unexplained performance issue in certain circumstances. Differential Revision: https://phabricator.services.mozilla.com/D41230
dom/canvas/CanvasRenderingContext2D.cpp
--- a/dom/canvas/CanvasRenderingContext2D.cpp
+++ b/dom/canvas/CanvasRenderingContext2D.cpp
@@ -1667,23 +1667,23 @@ CanvasRenderingContext2D::GetInputStream
 }
 
 already_AddRefed<mozilla::gfx::SourceSurface>
 CanvasRenderingContext2D::GetSurfaceSnapshot(gfxAlphaType* aOutAlphaType) {
   if (aOutAlphaType) {
     *aOutAlphaType = (mOpaque ? gfxAlphaType::Opaque : gfxAlphaType::Premult);
   }
 
-  if (!mBufferProvider) {
-    if (!EnsureTarget()) {
-      MOZ_ASSERT(
-          mTarget == sErrorTarget,
-          "On EnsureTarget failure mTarget should be set to sErrorTarget.");
-      return mTarget->Snapshot();
-    }
+  // For GetSurfaceSnapshot we always call EnsureTarget even if mBufferProvider
+  // already exists, otherwise we get performance issues. See bug 1567054.
+  if (!EnsureTarget()) {
+    MOZ_ASSERT(
+        mTarget == sErrorTarget,
+        "On EnsureTarget failure mTarget should be set to sErrorTarget.");
+    return mTarget->Snapshot();
   }
 
   // The concept of BorrowSnapshot seems a bit broken here, but the original
   // code in GetSurfaceSnapshot just returned a snapshot from mTarget, which
   // amounts to breaking the concept implicitly.
   RefPtr<SourceSurface> snapshot = mBufferProvider->BorrowSnapshot();
   RefPtr<SourceSurface> retSurface = snapshot;
   mBufferProvider->ReturnSnapshot(snapshot.forget());