Create a LayerDelegate for recreated layers to draw when their content is invalidated.

Create a LayerDelegate for recreated layers to draw when their content is invalidated.

This is to mitigate the issue where the content of the layer has to be redrawn while dragging a window in another display. This can draw the display items recorded for different device scale factor which can have artifacts, but it's better than empty layer.

The same issue can happen during hiding/showing animation in theory, but I'm leaving them as is because that's very rare and most likely not noticeable.

BUG=604868
TEST=WindowUtilTest.RecreateLayersWithDelegate
TBR=sadrul@chromium.org

Committed: https://crrev.com/e1dbe8e0ce560e5e2979e44b39268ac24bc3fc25
Cr-Commit-Position: refs/heads/master@{#394993}

[oshima, 2016-05-18 16:48:20]

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

[commit-bot: I haz the power, 2016-05-18 16:48:28]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1992853002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1992853002/1

[commit-bot: I haz the power, 2016-05-18 16:53:12]

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

[commit-bot: I haz the power, 2016-05-18 16:53:12]

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

[oshima, 2016-05-18 17:11:21]

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

[commit-bot: I haz the power, 2016-05-18 17:11:30]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1992853002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1992853002/20001

[commit-bot: I haz the power, 2016-05-18 17:18:21]

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

[commit-bot: I haz the power, 2016-05-18 17:18:22]

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

[oshima, 2016-05-18 17:43:15]

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

[commit-bot: I haz the power, 2016-05-18 17:43:25]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1992853002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1992853002/60001

[oshima, 2016-05-18 23:53:48]

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

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

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1992853002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1992853002/120001

[commit-bot: I haz the power, 2016-05-19 01:28:44]

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

[commit-bot: I haz the power, 2016-05-19 01:28:45]

Dry run: This issue passed the CQ dry run.

[oshima, 2016-05-19 15:13:55]

Patchset #1 (id:1) has been deleted

[oshima, 2016-05-19 15:14:00]

Patchset #1 (id:20001) has been deleted

[oshima, 2016-05-19 15:14:04]

Patchset #1 (id:40001) has been deleted

[oshima, 2016-05-19 15:14:14]

Patchset #1 (id:60001) has been deleted

[oshima, 2016-05-19 15:14:17]

Patchset #1 (id:80001) has been deleted

[oshima, 2016-05-19 15:14:21]

Patchset #1 (id:100001) has been deleted

[oshima, 2016-05-19 18:31:03]

Description was changed from

==========
LayerDelegate for recreated layers

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

to

==========
Create a LayerDelegate for recreated layers to draw when their content is
invalidated.

This is to mitigate the issue where the content of the layer has to be redrawn
while dragging a window in another display.

BUG=604868
==========

[oshima, 2016-05-19 18:32:42]

Description was changed from

==========
Create a LayerDelegate for recreated layers to draw when their content is
invalidated.

This is to mitigate the issue where the content of the layer has to be redrawn
while dragging a window in another display.

BUG=604868
==========

to

==========
Create a LayerDelegate for recreated layers to draw when their content is
invalidated.

This is to mitigate the issue where the content of the layer has to be redrawn
while dragging a window in another display.

The same issue can happen during hiding/showing animation in theory, but I'm
leaving them as is because that's very rare and most likely not noticeable. 

BUG=604868
==========

[oshima, 2016-05-19 18:40:25]

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

[oshima, 2016-05-19 18:40:27]

Patchset #1 (id:120001) has been deleted

[commit-bot: I haz the power, 2016-05-19 18:40:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1992853002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1992853002/140001

[oshima, 2016-05-19 18:41:56]

Description was changed from

==========
Create a LayerDelegate for recreated layers to draw when their content is
invalidated.

This is to mitigate the issue where the content of the layer has to be redrawn
while dragging a window in another display.

The same issue can happen during hiding/showing animation in theory, but I'm
leaving them as is because that's very rare and most likely not noticeable. 

BUG=604868
==========

to

==========
Create a LayerDelegate for recreated layers to draw when their content is
invalidated.

This is to mitigate the issue where the content of the layer has to be redrawn
while dragging a window in another display. This can draw the display items
recorded for different device scale factor, but sufficient as this is a
temporary image. 

The same issue can happen during hiding/showing animation in theory, but I'm
leaving them as is because that's very rare and most likely not noticeable. 

BUG=604868
==========

[oshima, 2016-05-19 18:44:43]

Description was changed from

==========
Create a LayerDelegate for recreated layers to draw when their content is
invalidated.

This is to mitigate the issue where the content of the layer has to be redrawn
while dragging a window in another display. This can draw the display items
recorded for different device scale factor, but sufficient as this is a
temporary image. 

The same issue can happen during hiding/showing animation in theory, but I'm
leaving them as is because that's very rare and most likely not noticeable. 

BUG=604868
==========

to

==========
Create a LayerDelegate for recreated layers to draw when their content is
invalidated.

This is to mitigate the issue where the content of the layer has to be redrawn
while dragging a window in another display. This can draw the display items
recorded for different device scale factor which can have artifacts, but it's
better than empty layer.

The same issue can happen during hiding/showing animation in theory, but I'm
leaving them as is because that's very rare and most likely not noticeable. 

BUG=604868
TEST=
==========

[oshima, 2016-05-19 18:44:43]

oshima@chromium.org changed reviewers:
+ sky@chromium.org

[oshima, 2016-05-19 18:45:14]

Description was changed from

==========
Create a LayerDelegate for recreated layers to draw when their content is
invalidated.

This is to mitigate the issue where the content of the layer has to be redrawn
while dragging a window in another display. This can draw the display items
recorded for different device scale factor which can have artifacts, but it's
better than empty layer.

The same issue can happen during hiding/showing animation in theory, but I'm
leaving them as is because that's very rare and most likely not noticeable. 

BUG=604868
TEST=
==========

to

==========
Create a LayerDelegate for recreated layers to draw when their content is
invalidated.

This is to mitigate the issue where the content of the layer has to be redrawn
while dragging a window in another display. This can draw the display items
recorded for different device scale factor which can have artifacts, but it's
better than empty layer.

The same issue can happen during hiding/showing animation in theory, but I'm
leaving them as is because that's very rare and most likely not noticeable. 

BUG=604868
TEST=WindowUtilTest.RecreateLayersWithDelegate
==========

[sky, 2016-05-19 20:06:49]

https://codereview.chromium.org/1992853002/diff/140001/ash/wm/drag_window_con...
File ash/wm/drag_window_controller.cc (right):

https://codereview.chromium.org/1992853002/diff/140001/ash/wm/drag_window_con...
ash/wm/drag_window_controller.cc:32: class DragWindowLayerDelegate : public
ui::LayerDelegate {
Description

https://codereview.chromium.org/1992853002/diff/140001/ash/wm/drag_window_con...
ash/wm/drag_window_controller.cc:34: DragWindowLayerDelegate(ui::LayerDelegate*
delegate)
explicit

https://codereview.chromium.org/1992853002/diff/140001/ash/wm/drag_window_con...
ash/wm/drag_window_controller.cc:34: DragWindowLayerDelegate(ui::LayerDelegate*
delegate)
How do you know the original delegate remains valid for the life of the drag?
Certainly the delegate for the window itself being dragged is valid, but for any
child layers the delegate may be destroyed at any time during the drag, right?

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util.h
File ui/wm/core/window_util.h (right):

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
ui/wm/core/window_util.h:46: ui::LayerDelegate* original_delegate) = 0;
Document ownership and expected lifetime.

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
ui/wm/core/window_util.h:57: // create an delegate for an old layer which had
its own delegate.
nit: 'create an' -> 'create a'

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
File ui/wm/core/window_util_unittest.cc (right):

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
ui/wm/core/window_util_unittest.cc:18: class TestLayerDelegate : public
ui::LayerDelegate {
nit: newline between 17/18, and description.

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
ui/wm/core/window_util_unittest.cc:20: TestLayerDelegate(ui::LayerDelegate*
original_delegate)
explicit

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
ui/wm/core/window_util_unittest.cc:24: void OnPaintLayer(const ui::PaintContext&
context) override {}
nit: // ui::LayerDelegate: (or some other prefix)

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
ui/wm/core/window_util_unittest.cc:39: class TestLayerDelegateFactory : public
::wm::LayerDelegateFactory {
Description.

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
ui/wm/core/window_util_unittest.cc:115: DCHECK_EQ(3u, factory.delegate_count());
ASSERT_EQ

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
ui/wm/core/window_util_unittest.cc:121: DCHECK_EQ(window1->layer()->delegate(),
new_delegate_1->original_delegate());
EXPECT_EQ for all of these

[commit-bot: I haz the power, 2016-05-19 20:56:47]

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

[commit-bot: I haz the power, 2016-05-19 20:56:48]

Dry run: This issue passed the CQ dry run.

[oshima, 2016-05-19 23:27:21]

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

[oshima, 2016-05-19 23:27:26]

Patchset #2 (id:160001) has been deleted

[commit-bot: I haz the power, 2016-05-19 23:28:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1992853002/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1992853002/180001

[oshima, 2016-05-19 23:32:22]

https://codereview.chromium.org/1992853002/diff/140001/ash/wm/drag_window_con...
File ash/wm/drag_window_controller.cc (right):

https://codereview.chromium.org/1992853002/diff/140001/ash/wm/drag_window_con...
ash/wm/drag_window_controller.cc:32: class DragWindowLayerDelegate : public
ui::LayerDelegate {
On 2016/05/19 20:06:49, sky wrote:
> Description

Done.

https://codereview.chromium.org/1992853002/diff/140001/ash/wm/drag_window_con...
ash/wm/drag_window_controller.cc:34: DragWindowLayerDelegate(ui::LayerDelegate*
delegate)
On 2016/05/19 20:06:49, sky wrote:
> How do you know the original delegate remains valid for the life of the drag?
> Certainly the delegate for the window itself being dragged is valid, but for
any
> child layers the delegate may be destroyed at any time during the drag, right?

You're right. The only way to check, without introducing another
delegate/observer, is to traverse the layer tree unfortunately. I feel
introducing such interface just for this use case is overkill, and this does not
happen very often, so I took that approach for now. I'm happy to add one if you
prefer that way so let me know.

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util.h
File ui/wm/core/window_util.h (right):

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
ui/wm/core/window_util.h:46: ui::LayerDelegate* original_delegate) = 0;
On 2016/05/19 20:06:49, sky wrote:
> Document ownership and expected lifetime.

Done.

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
ui/wm/core/window_util.h:57: // create an delegate for an old layer which had
its own delegate.
On 2016/05/19 20:06:49, sky wrote:
> nit: 'create an' -> 'create a'

Done.

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
File ui/wm/core/window_util_unittest.cc (right):

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
ui/wm/core/window_util_unittest.cc:18: class TestLayerDelegate : public
ui::LayerDelegate {
On 2016/05/19 20:06:49, sky wrote:
> nit: newline between 17/18, and description.

Done.

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
ui/wm/core/window_util_unittest.cc:20: TestLayerDelegate(ui::LayerDelegate*
original_delegate)
On 2016/05/19 20:06:49, sky wrote:
> explicit

Done.

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
ui/wm/core/window_util_unittest.cc:24: void OnPaintLayer(const ui::PaintContext&
context) override {}
On 2016/05/19 20:06:49, sky wrote:
> nit: // ui::LayerDelegate: (or some other prefix)

Done.

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
ui/wm/core/window_util_unittest.cc:39: class TestLayerDelegateFactory : public
::wm::LayerDelegateFactory {
On 2016/05/19 20:06:49, sky wrote:
> Description.

Done.

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
ui/wm/core/window_util_unittest.cc:115: DCHECK_EQ(3u, factory.delegate_count());
On 2016/05/19 20:06:49, sky wrote:
> ASSERT_EQ

Done.

https://codereview.chromium.org/1992853002/diff/140001/ui/wm/core/window_util...
ui/wm/core/window_util_unittest.cc:121: DCHECK_EQ(window1->layer()->delegate(),
new_delegate_1->original_delegate());
On 2016/05/19 20:06:49, sky wrote:
> EXPECT_EQ for all of these

Done.

[sky, 2016-05-20 00:05:24]

LGTM

https://codereview.chromium.org/1992853002/diff/200001/ash/wm/drag_window_con...
File ash/wm/drag_window_controller.cc (right):

https://codereview.chromium.org/1992853002/diff/200001/ash/wm/drag_window_con...
ash/wm/drag_window_controller.cc:49: if (IsDelegateValid(drag_window_->layer()))
This is subtle and worth a comment.

[oshima, 2016-05-20 03:16:08]

https://codereview.chromium.org/1992853002/diff/200001/ash/wm/drag_window_con...
File ash/wm/drag_window_controller.cc (right):

https://codereview.chromium.org/1992853002/diff/200001/ash/wm/drag_window_con...
ash/wm/drag_window_controller.cc:49: if (IsDelegateValid(drag_window_->layer()))
On 2016/05/20 00:05:24, sky wrote:
> This is subtle and worth a comment.

Done.

[oshima, 2016-05-20 03:17:18]

Description was changed from

==========
Create a LayerDelegate for recreated layers to draw when their content is
invalidated.

This is to mitigate the issue where the content of the layer has to be redrawn
while dragging a window in another display. This can draw the display items
recorded for different device scale factor which can have artifacts, but it's
better than empty layer.

The same issue can happen during hiding/showing animation in theory, but I'm
leaving them as is because that's very rare and most likely not noticeable. 

BUG=604868
TEST=WindowUtilTest.RecreateLayersWithDelegate
==========

to

==========
Create a LayerDelegate for recreated layers to draw when their content is
invalidated.

This is to mitigate the issue where the content of the layer has to be redrawn
while dragging a window in another display. This can draw the display items
recorded for different device scale factor which can have artifacts, but it's
better than empty layer.

The same issue can happen during hiding/showing animation in theory, but I'm
leaving them as is because that's very rare and most likely not noticeable. 

BUG=604868
TEST=WindowUtilTest.RecreateLayersWithDelegate
TBR=sadrul@chromium.org
==========

[oshima, 2016-05-20 03:17:18]

oshima@chromium.org changed reviewers:
+ sadrul@chromium.org

[oshima, 2016-05-20 03:17:44]

tbr'ing sadrul for trivial change in 
content/browser/renderer_host/render_widget_host_view_aura_unittest.cc

[oshima, 2016-05-20 03:17:49]

The CQ bit was checked by oshima@chromium.org

[oshima, 2016-05-20 03:17:50]

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

[commit-bot: I haz the power, 2016-05-20 03:18:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1992853002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1992853002/220001

[sadrul, 2016-05-20 03:27:19]

lgtm (sky@ should maybe be back in c/b/OWNERS :) )

[commit-bot: I haz the power, 2016-05-20 04:08:28]

Description was changed from

==========
Create a LayerDelegate for recreated layers to draw when their content is
invalidated.

This is to mitigate the issue where the content of the layer has to be redrawn
while dragging a window in another display. This can draw the display items
recorded for different device scale factor which can have artifacts, but it's
better than empty layer.

The same issue can happen during hiding/showing animation in theory, but I'm
leaving them as is because that's very rare and most likely not noticeable. 

BUG=604868
TEST=WindowUtilTest.RecreateLayersWithDelegate
TBR=sadrul@chromium.org
==========

to

==========
Create a LayerDelegate for recreated layers to draw when their content is
invalidated.

This is to mitigate the issue where the content of the layer has to be redrawn
while dragging a window in another display. This can draw the display items
recorded for different device scale factor which can have artifacts, but it's
better than empty layer.

The same issue can happen during hiding/showing animation in theory, but I'm
leaving them as is because that's very rare and most likely not noticeable. 

BUG=604868
TEST=WindowUtilTest.RecreateLayersWithDelegate
TBR=sadrul@chromium.org
==========

[commit-bot: I haz the power, 2016-05-20 04:08:29]

Committed patchset #4 (id:220001)

[commit-bot: I haz the power, 2016-05-20 04:10:22]

Description was changed from

==========
Create a LayerDelegate for recreated layers to draw when their content is
invalidated.

This is to mitigate the issue where the content of the layer has to be redrawn
while dragging a window in another display. This can draw the display items
recorded for different device scale factor which can have artifacts, but it's
better than empty layer.

The same issue can happen during hiding/showing animation in theory, but I'm
leaving them as is because that's very rare and most likely not noticeable. 

BUG=604868
TEST=WindowUtilTest.RecreateLayersWithDelegate
TBR=sadrul@chromium.org
==========

to

==========
Create a LayerDelegate for recreated layers to draw when their content is
invalidated.

This is to mitigate the issue where the content of the layer has to be redrawn
while dragging a window in another display. This can draw the display items
recorded for different device scale factor which can have artifacts, but it's
better than empty layer.

The same issue can happen during hiding/showing animation in theory, but I'm
leaving them as is because that's very rare and most likely not noticeable.

BUG=604868
TEST=WindowUtilTest.RecreateLayersWithDelegate
TBR=sadrul@chromium.org

Committed: https://crrev.com/e1dbe8e0ce560e5e2979e44b39268ac24bc3fc25
Cr-Commit-Position: refs/heads/master@{#394993}
==========

[commit-bot: I haz the power, 2016-05-20 04:10:24]

Patchset 4 (id:??) landed as
https://crrev.com/e1dbe8e0ce560e5e2979e44b39268ac24bc3fc25
Cr-Commit-Position: refs/heads/master@{#394993}