aura: Support releasing the OutputSurface()

aura: Support releasing the OutputSurface()

BUG=513540
Committed: https://crrev.com/13e78285cc3448216fdd401e0bcdf23471e86a95
Cr-Commit-Position: refs/heads/master@{#352511}

[no sievers, 2015-10-02 23:14:38]

sievers@chromium.org changed reviewers:
+ danakj@chromium.org, piman@chromium.org

[no sievers, 2015-10-02 23:15:19]

ptal

[piman, 2015-10-05 22:21:27]

LGTM+nit

https://codereview.chromium.org/1388543004/diff/20001/ui/compositor/composito...
File ui/compositor/compositor_unittest.cc (right):

https://codereview.chromium.org/1388543004/diff/20001/ui/compositor/composito...
ui/compositor/compositor_unittest.cc:72:
compositor()->RemoveObserver(&observer);
I think you can use DrawWaiterForTest::WaitForCommit(compositor()); instead of
all this (in ui/compositor/test/draw_waiter_for_test.h)

[no sievers, 2015-10-05 23:25:39]

On 2015/10/05 22:21:27, piman (slow to review) wrote:
> LGTM+nit
> 
>
https://codereview.chromium.org/1388543004/diff/20001/ui/compositor/composito...
> File ui/compositor/compositor_unittest.cc (right):
> 
>
https://codereview.chromium.org/1388543004/diff/20001/ui/compositor/composito...
> ui/compositor/compositor_unittest.cc:72:
> compositor()->RemoveObserver(&observer);
> I think you can use DrawWaiterForTest::WaitForCommit(compositor()); instead of
> all this (in ui/compositor/test/draw_waiter_for_test.h)

Thanks, done! I've also added a root layer and set the viewport to non-empty, so
that I can wait for 'did draw' rather than just 'did commit'. Seems a bit
presumptuous to expect an OutputSurface to be ever created if we don't really
draw otherwise.

[no sievers, 2015-10-05 23:45:11]

The CQ bit was checked by sievers@chromium.org

[no sievers, 2015-10-05 23:45:11]

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

[commit-bot: I haz the power, 2015-10-05 23:45:51]

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

[no sievers, 2015-10-05 23:46:35]

The CQ bit was unchecked by sievers@chromium.org

[no sievers, 2015-10-05 23:49:31]

The CQ bit was checked by sievers@chromium.org

[no sievers, 2015-10-05 23:49:31]

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

[commit-bot: I haz the power, 2015-10-05 23:49:49]

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

[commit-bot: I haz the power, 2015-10-06 01:54:51]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-10-06 01:55:51]

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

[danakj, 2015-10-06 18:28:13]

LGTM

https://codereview.chromium.org/1388543004/diff/80001/ui/compositor/composito...
File ui/compositor/compositor_unittest.cc (right):

https://codereview.chromium.org/1388543004/diff/80001/ui/compositor/composito...
ui/compositor/compositor_unittest.cc:147: EXPECT_EQ(gfx::kNullAcceleratedWidget,
might be worth a test that verifies it gets back the right widget for non-null
case? maybe not, pretty simple

[no sievers, 2015-10-06 23:46:28]

https://codereview.chromium.org/1388543004/diff/80001/ui/compositor/composito...
File ui/compositor/compositor_unittest.cc (right):

https://codereview.chromium.org/1388543004/diff/80001/ui/compositor/composito...
ui/compositor/compositor_unittest.cc:147: EXPECT_EQ(gfx::kNullAcceleratedWidget,
On 2015/10/06 18:28:12, danakj_OOO_til_10-12 wrote:
> might be worth a test that verifies it gets back the right widget for non-null
> case? maybe not, pretty simple

But it goes into the non-offscreen creation path then, so I'd actually have to
pass a valid view surface handle here.

[danakj, 2015-10-07 20:44:42]

https://codereview.chromium.org/1388543004/diff/80001/ui/compositor/composito...
File ui/compositor/compositor_unittest.cc (right):

https://codereview.chromium.org/1388543004/diff/80001/ui/compositor/composito...
ui/compositor/compositor_unittest.cc:147: EXPECT_EQ(gfx::kNullAcceleratedWidget,
On 2015/10/06 23:46:27, sievers wrote:
> On 2015/10/06 18:28:12, danakj_OOO_til_10-12 wrote:
> > might be worth a test that verifies it gets back the right widget for
non-null
> > case? maybe not, pretty simple
> 
> But it goes into the non-offscreen creation path then, so I'd actually have to
> pass a valid view surface handle here.

I was thinking of
https://code.google.com/p/chromium/codesearch#chromium/src/ui/compositor/laye...
these tests, not sure that would work here, but might. I don't see where we give
a widget to the compositor there, but they do draw to a xwindow..