Add Intel macOS workaround for WebGL2 canvas_sub_rectangle tests

third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp

Index: third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp b/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp
index 99c10e624e1c9ab499ccfcb1f60068fb29f6e3ca..5808df2f379e0f832e63aa6ef98ffe62cf9cddec 100644
--- a/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp
+++ b/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp
@@ -189,15 +189,17 @@ void Canvas2DLayerBridge::setLoggerForTesting(std::unique_ptr<Logger> logger) {
 
 bool Canvas2DLayerBridge::shouldAccelerate(AccelerationHint hint) const {
   bool accelerate;
-  if (m_softwareRenderingWhileHidden)
+  if (m_softwareRenderingWhileHidden) {
     accelerate = false;
-  else if (m_accelerationMode == ForceAccelerationForTesting)
+  } else if (m_accelerationMode == ForceAccelerationForTesting) {
     accelerate = true;
-  else if (m_accelerationMode == DisableAcceleration)
+  } else if (m_accelerationMode == DisableAcceleration) {
     accelerate = false;
-  else
+  } else {
     accelerate = hint == PreferAcceleration ||
-                 hint == PreferAccelerationAfterVisibilityChange;
+                 hint == PreferAccelerationAfterVisibilityChange ||
+                 hint == PreferAccelerationWithExtraSurfaceCopy;
+  }
 
   if (accelerate &&
       (!m_contextProvider ||
@@ -540,7 +542,9 @@ SkSurface* Canvas2DLayerBridge::getOrCreateSurface(AccelerationHint hint) {
   if (m_surface)
     return m_surface.get();
 
-  if (m_layer && !isHibernating() && hint == PreferAcceleration &&
+  if (m_layer && !isHibernating() &&
+      (hint == PreferAcceleration ||
+       hint == PreferAccelerationWithExtraSurfaceCopy) &&
       m_accelerationMode != DisableAcceleration) {
     return nullptr;  // re-creation will happen through restore()
   }
@@ -1113,6 +1117,14 @@ sk_sp<SkImage> Canvas2DLayerBridge::newImageSnapshot(AccelerationHint hint,
   // parameters.
   getOrCreateSurface()->notifyContentWillChange(
       SkSurface::kRetain_ContentChangeMode);
+
+  // Create a new SkSurface compatible with m_surface, and discard it
+  // immediately. This nonsense operation can suppress the flaky error, which
+  // will likely happen when later reading or peeking pixels back from the image
+  // snapshot, on Intel MacOS platform(crbug.com/665656).

[qiankun, 2016/12/05 08:25:31]

Brian & Eric, we found this workaround but we didn't root cause why result of
read back from canvas is wrong. Do you have any ideas?

[bsalomon, 2016/12/05 14:12:41]

Sorry, but I don't think we've encountered this particular quirk in Skia before.

+  if (hint == PreferAccelerationWithExtraSurfaceCopy)

[Justin Novosad, 2016/12/05 20:54:01]

This is a misuse of "hint". As it's name suggests, it is just a hint. The
purpose of the hint parameter is to indicate an opportunistic preference that
may or may not be ignored.  Using it for other purposes is confusing and bad for
readability.  Proof of confusingness: this code will be making an unnecessary
copy in cases where the canvas is not gpu accelerated (because the hint does not
force the canvas to be accelerated).  I think it would be cleaner to use the
SnapshotReason instead.  I have an idea how to do this cleanly.  Let me land a
small patch first.

+    m_surface->makeSurface(m_surface->getCanvas()->imageInfo()).reset();

[Zhenyao Mo, 2016/12/05 18:13:36]

This seems a big perf regression.  If we only encounter this bug when uploading
2d canvas to WebGL, should we only trigger this on-demand when we use this 2d
canvas as texture source in WebGL? (I haven't looked into how feasible it is
though, but doing it all the time on Intel Mac seems an overkill).

+
   return m_surface->makeImageSnapshot();
 }