Enable GPU acceleration in 2D OffscreenCanvases on main thread

Enable GPU acceleration in 2D OffscreenCanvases on main thread

This change allows OffscreenCanvasRenderingContext2D to create
an AcceleratedImageBufferSurface for its backing on platforms
that support GPU-accelerated canvas.

The plumbing of the runtime option for controlling the
availability of accelerated canvases was moved from WebSettings
to RuntimeEnableFeatures so that we no longer need to access
the DOM in order to access the feature flag.

BUG=593514
Committed: https://crrev.com/524668f2a8ed8d87a049a2dc1e74ff0134c8f82f
Cr-Commit-Position: refs/heads/master@{#410423}

[Justin Novosad, 2016-07-27 19:11:43]

The CQ bit was checked by junov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-27 19:11:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Justin Novosad, 2016-07-27 19:12:21]

junov@chromium.org changed reviewers:
+ xidachen@chromium.org, xlai@chromium.org

[Justin Novosad, 2016-07-27 19:12:22]

PTAL

[commit-bot: I haz the power, 2016-07-27 19:14:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-27 19:14:18]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)

[Justin Novosad, 2016-07-27 19:47:57]

The CQ bit was checked by junov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-27 19:48:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-27 19:54:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-27 19:54:13]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Justin Novosad, 2016-07-27 20:04:40]

junov@chromium.org changed reviewers:
+ esprehn@chromium.org

[Justin Novosad, 2016-07-27 20:04:41]

+esprehn for blink API review

[xlai (Olivia), 2016-07-28 15:10:22]

Hmmm...Why don't you re-use the accelerated2dCanvas flag? Reasons:

1. The flag is already used in multiple tests. You got to rebase each and every
of these tests in this CL, which might be a bit irrelevant to the main theme of
this CL.
2. If OffscreenCanvas is on the main thread, we can easily access the flag when
context is created; if OffscreenCanvas is on the worker thread, now we already
have WorkerSettings available. Just like the way "disableReadingFromCanvas"
does. Unless this flag value keeps changing when OffscreenCanvas.2dcontext is
alive; but letting that happen sounds not a good idea either.

[Justin Novosad, 2016-08-02 17:59:06]

On 2016/07/28 15:10:22, xlai (Olivia) wrote:
> Hmmm...Why don't you re-use the accelerated2dCanvas flag? Reasons:
> 
> 1. The flag is already used in multiple tests. You got to rebase each and
every
> of these tests in this CL, which might be a bit irrelevant to the main theme
of
> this CL.
> 2. If OffscreenCanvas is on the main thread, we can easily access the flag
when
> context is created; if OffscreenCanvas is on the worker thread, now we already
> have WorkerSettings available. Just like the way "disableReadingFromCanvas"
> does. Unless this flag value keeps changing when OffscreenCanvas.2dcontext is
> alive; but letting that happen sounds not a good idea either.

RuntimeEnabledFeatures was simply a better place for the flag and will be
cleaner going forward. WebSettings require going through the frame/document,
which is not logical for global settings that are not frame-specific.

[xlai (Olivia), 2016-08-02 20:50:27]

On 2016/08/02 17:59:06, junov (slow until August 8) wrote:
> On 2016/07/28 15:10:22, xlai (Olivia) wrote:
> > Hmmm...Why don't you re-use the accelerated2dCanvas flag? Reasons:
> > 
> > 1. The flag is already used in multiple tests. You got to rebase each and
> every
> > of these tests in this CL, which might be a bit irrelevant to the main theme
> of
> > this CL.
> > 2. If OffscreenCanvas is on the main thread, we can easily access the flag
> when
> > context is created; if OffscreenCanvas is on the worker thread, now we
already
> > have WorkerSettings available. Just like the way "disableReadingFromCanvas"
> > does. Unless this flag value keeps changing when OffscreenCanvas.2dcontext
is
> > alive; but letting that happen sounds not a good idea either.
> 
> RuntimeEnabledFeatures was simply a better place for the flag and will be
> cleaner going forward. WebSettings require going through the frame/document,
> which is not logical for global settings that are not frame-specific.

This makes sense to me. Please move forward in rebasing the failing tests.

[Justin Novosad, 2016-08-02 22:14:41]

The CQ bit was checked by junov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 22:15:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Justin Novosad, 2016-08-02 22:15:51]

> This makes sense to me. Please move forward in rebasing the failing tests.

Done. I actually had to change the tests to make them access the setting in its
new place.

[commit-bot: I haz the power, 2016-08-02 23:29:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-02 23:29:18]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Justin Novosad, 2016-08-03 05:00:12]

The CQ bit was checked by junov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-03 05:00:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-03 06:13:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-03 06:13:52]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[xlai (Olivia), 2016-08-03 20:05:49]

On 2016/08/02 22:15:51, junov (slow until August 8) wrote:
> > This makes sense to me. Please move forward in rebasing the failing tests.
> 
> Done. I actually had to change the tests to make them access the setting in
its
> new place.

lgtm

[esprehn, 2016-08-05 21:28:38]

please fix the static or namespace issue before landing. :)

lgtm

https://codereview.chromium.org/2192493003/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/offscreencanvas2d/OffscreenCanvasRenderingContext2D.cpp
(right):

https://codereview.chromium.org/2192493003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/offscreencanvas2d/OffscreenCanvasRenderingContext2D.cpp:85:
bool shouldAccelerate(IntSize surfaceSize)
needs static or anonymous namespace

https://codereview.chromium.org/2192493003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in (right):

https://codereview.chromium.org/2192493003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in:30:
Accelerated2dCanvas status=stable, settable_from_internals=True
direct to stable? What's the purpose of the flag? :)

[Justin Novosad, 2016-08-05 21:34:44]

>
https://codereview.chromium.org/2192493003/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in:30:
> Accelerated2dCanvas status=stable, settable_from_internals=True
> direct to stable? What's the purpose of the flag? :)

Yes, this feature has been stable for years (M21, I believe?).  It is an old
flag, I am just moving the flag from WebSettings to RuntimeEnabledFeatures in
this patch, the current purpose of the flag is for disabling accelerated 2d
canvas via command-line or via gpu blacklisting.

[Justin Novosad, 2016-08-05 21:52:23]

The CQ bit was checked by junov@chromium.org

[Justin Novosad, 2016-08-05 21:52:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
esprehn@chromium.org, xlai@chromium.org
Link to the patchset: https://codereview.chromium.org/2192493003/#ps60001
(title: "static")

[commit-bot: I haz the power, 2016-08-05 21:53:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-05 23:08:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-05 23:08:55]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Justin Novosad, 2016-08-08 18:08:17]

The CQ bit was checked by junov@chromium.org

[Justin Novosad, 2016-08-08 18:08:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
esprehn@chromium.org, xlai@chromium.org
Link to the patchset: https://codereview.chromium.org/2192493003/#ps80001
(title: "fix dev tools failure")

[commit-bot: I haz the power, 2016-08-08 18:08:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-08 20:17:10]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-08 20:18:57]

Description was changed from

==========
Enable GPU acceleration in 2D OffscreenCanvases on main thread

This change allows OffscreenCanvasRenderingContext2D to create
an AcceleratedImageBufferSurface for its backing on platforms
that support GPU-accelerated canvas.

The plumbing of the runtime option for controlling the
availability of accelerated canvases was moved from WebSettings
to RuntimeEnableFeatures so that we no longer need to access
the DOM in order to access the feature flag.

BUG=593514
==========

to

==========
Enable GPU acceleration in 2D OffscreenCanvases on main thread

This change allows OffscreenCanvasRenderingContext2D to create
an AcceleratedImageBufferSurface for its backing on platforms
that support GPU-accelerated canvas.

The plumbing of the runtime option for controlling the
availability of accelerated canvases was moved from WebSettings
to RuntimeEnableFeatures so that we no longer need to access
the DOM in order to access the feature flag.

BUG=593514

Committed: https://crrev.com/524668f2a8ed8d87a049a2dc1e74ff0134c8f82f
Cr-Commit-Position: refs/heads/master@{#410423}
==========

[commit-bot: I haz the power, 2016-08-08 20:18:59]

Patchset 5 (id:??) landed as
https://crrev.com/524668f2a8ed8d87a049a2dc1e74ff0134c8f82f
Cr-Commit-Position: refs/heads/master@{#410423}