[blink] Request compositing update when canvas hibernates/wakes up.

[blink] Request compositing update when canvas hibernates/wakes up.

Not requesting the update can lead to the canvas not
being shown, as in the bug referenced below.

BUG=708445
Review-Url: https://codereview.chromium.org/2808493002
Cr-Commit-Position: refs/heads/master@{#465206}
Committed: https://chromium.googlesource.com/chromium/src/+/5047940999a9cf13e8635e61112fdaa7db885726

[Eric Seckler, 2017-04-07 16:32:18]

Description was changed from

==========
FREEZE.indexed

BUG=
==========

to

==========
[blink] Request compositing update when canvas visibility changes.

BUG=708445
==========

[Eric Seckler, 2017-04-07 17:01:09]

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

[commit-bot: I haz the power, 2017-04-07 17:01:46]

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

[Eric Seckler, 2017-04-07 17:04:15]

Description was changed from

==========
[blink] Request compositing update when canvas visibility changes.

BUG=708445
==========

to

==========
[blink] Request compositing update when canvas visibility changes.

Not requesting the update can lead to the canvas not
being shown, as in the bug referenced below.

BUG=708445
==========

[Eric Seckler, 2017-04-07 17:05:32]

eseckler@chromium.org changed reviewers:
+ chrishtr@chromium.org

[Eric Seckler, 2017-04-07 17:05:32]

Chris, I believe this is a more appropriate fix, unit test included. Thanks!

[chrishtr, 2017-04-07 17:12:33]

https://codereview.chromium.org/2808493002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right):

https://codereview.chromium.org/2808493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:1220:
setNeedsCompositingUpdate();
Which of the conditions below depend on page visibility?

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/HTML...

[Eric Seckler, 2017-04-07 17:27:11]

https://codereview.chromium.org/2808493002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right):

https://codereview.chromium.org/2808493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:1220:
setNeedsCompositingUpdate();
On 2017/04/07 17:12:32, chrishtr wrote:
> Which of the conditions below depend on page visibility?
> 
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/HTML...

CanvasRenderingContext2D::isComposited() -> isAccelerated() ->
ImageBuffer::isAccelerated() -> Canvas2DImageBufferSurface::isAccelerated() ->
Canvas2DLayerBridge::isAccelerated() -> isHibernating()

And Canvas2DLayerBridge::hibernate() is eventually called by setIsHidden()
above.

[Justin Novosad, 2017-04-07 18:50:21]

I don't think this is the right fix because the switch to hibernation is
asynchronous (gets scheduled as an idle task when visibility changes). 
Therefore, your solution risk being flaky.  The compositing update should be
triggered by Canvas2DLayerBridge::hibernate()
Here is how I suggest you solve it:  Create a new method
ImageBuffer::setNeedsCompositingUpdate(), which you can call from
Canvas2DLayerBridge (m_imageBuffer).  Then ImageBuffer can propagate that signal
to the HTMLCanvasElement by going through the ImageBufferClient interface, which
is inherited by HTMLCanvasElement.

[commit-bot: I haz the power, 2017-04-07 18:55:47]

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

[commit-bot: I haz the power, 2017-04-07 18:55:47]

Dry run: This issue passed the CQ dry run.

[Eric Seckler, 2017-04-10 15:18:14]

On 2017/04/07 18:50:21, Justin Novosad wrote:
> I don't think this is the right fix because the switch to hibernation is
> asynchronous (gets scheduled as an idle task when visibility changes). 
> Therefore, your solution risk being flaky.  The compositing update should be
> triggered by Canvas2DLayerBridge::hibernate()
> Here is how I suggest you solve it:  Create a new method
> ImageBuffer::setNeedsCompositingUpdate(), which you can call from
> Canvas2DLayerBridge (m_imageBuffer).  Then ImageBuffer can propagate that
signal
> to the HTMLCanvasElement by going through the ImageBufferClient interface,
which
> is inherited by HTMLCanvasElement.

Thanks, that makes sense. PTAL :)

[Eric Seckler, 2017-04-10 15:19:10]

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

[commit-bot: I haz the power, 2017-04-10 15:19:32]

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

[Eric Seckler, 2017-04-10 15:19:59]

Description was changed from

==========
[blink] Request compositing update when canvas visibility changes.

Not requesting the update can lead to the canvas not
being shown, as in the bug referenced below.

BUG=708445
==========

to

==========
[blink] Request compositing update when canvas hibernates/wakes up.

Not requesting the update can lead to the canvas not
being shown, as in the bug referenced below.

BUG=708445
==========

[commit-bot: I haz the power, 2017-04-10 17:18:58]

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

[commit-bot: I haz the power, 2017-04-10 17:18:59]

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_...)

[Eric Seckler, 2017-04-11 07:32:59]

https://codereview.chromium.org/2808493002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DTest.cpp
(right):

https://codereview.chromium.org/2808493002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DTest.cpp:1423:
ElementRequestsCompositingUpdateOnHibernateAndWakeUp) {
The test seems to be failing on mac - is mac using a different setup?

[Eric Seckler, 2017-04-11 07:40:32]

https://codereview.chromium.org/2808493002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DTest.cpp
(right):

https://codereview.chromium.org/2808493002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DTest.cpp:1423:
ElementRequestsCompositingUpdateOnHibernateAndWakeUp) {
On 2017/04/11 07:32:59, Eric Seckler wrote:
> The test seems to be failing on mac - is mac using a different setup?

Ah, there's CANVAS2D_HIBERNATION_ENABLED :) Test should be fixed!

[Eric Seckler, 2017-04-11 07:40:38]

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

[commit-bot: I haz the power, 2017-04-11 07:40:49]

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, 2017-04-11 07:54:15]

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

[commit-bot: I haz the power, 2017-04-11 07:54:15]

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_...)

[Eric Seckler, 2017-04-11 08:43:53]

Patchset #4 (id:60001) has been deleted

[Eric Seckler, 2017-04-11 08:44:12]

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

[commit-bot: I haz the power, 2017-04-11 08:44:47]

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, 2017-04-11 08:46:55]

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

[commit-bot: I haz the power, 2017-04-11 08:46:56]

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-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Eric Seckler, 2017-04-11 08:53:00]

Patchset #4 (id:80001) has been deleted

[Eric Seckler, 2017-04-11 08:53:06]

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

[commit-bot: I haz the power, 2017-04-11 08:53:28]

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, 2017-04-11 09:58:24]

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

[commit-bot: I haz the power, 2017-04-11 09:58:28]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Eric Seckler, 2017-04-11 10:53:48]

Patchset #4 (id:100001) has been deleted

[Eric Seckler, 2017-04-11 10:53:55]

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

[commit-bot: I haz the power, 2017-04-11 10:54:13]

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, 2017-04-11 12:18:13]

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

[commit-bot: I haz the power, 2017-04-11 12:18:15]

Dry run: This issue passed the CQ dry run.

[Eric Seckler, 2017-04-13 08:10:36]

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

[Eric Seckler, 2017-04-13 08:10:37]

Chris, Justin, PTAL :)

[chrishtr, 2017-04-13 14:39:53]

I defer to junov@ on this review.

[Justin Novosad, 2017-04-13 16:45:49]

lgtm

[Eric Seckler, 2017-04-13 16:48:29]

The CQ bit was checked by eseckler@chromium.org

[commit-bot: I haz the power, 2017-04-13 16:49:13]

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, 2017-04-13 17:07:13]

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

[commit-bot: I haz the power, 2017-04-13 17:07:15]

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

[Eric Seckler, 2017-04-18 10:22:20]

The CQ bit was checked by eseckler@chromium.org

[Eric Seckler, 2017-04-18 10:22:20]

The patchset sent to the CQ was uploaded after l-g-t-m from junov@chromium.org
Link to the patchset: https://codereview.chromium.org/2808493002/#ps140001
(title: "rebase")

[commit-bot: I haz the power, 2017-04-18 10:22:37]

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, 2017-04-18 12:18:43]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1492510940446590,
"parent_rev": "d64d884420dfcea54bcdcfe5d09386bcb9999fc4", "commit_rev":
"5047940999a9cf13e8635e61112fdaa7db885726"}

[commit-bot: I haz the power, 2017-04-18 12:18:52]

Description was changed from

==========
[blink] Request compositing update when canvas hibernates/wakes up.

Not requesting the update can lead to the canvas not
being shown, as in the bug referenced below.

BUG=708445
==========

to

==========
[blink] Request compositing update when canvas hibernates/wakes up.

Not requesting the update can lead to the canvas not
being shown, as in the bug referenced below.

BUG=708445

Review-Url: https://codereview.chromium.org/2808493002
Cr-Commit-Position: refs/heads/master@{#465206}
Committed:
https://chromium.googlesource.com/chromium/src/+/5047940999a9cf13e8635e61112f...
==========

[commit-bot: I haz the power, 2017-04-18 12:18:54]

Committed patchset #5 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/5047940999a9cf13e8635e61112f...