cc: Don't leak the texture returned in CopyOutputRequest ever.

cc: Don't leak the texture returned in CopyOutputRequest ever.

Currently we use a WeakPtr on the GLRenderer to prevent crashing
if the GLRenderer goes away before the embedder is done with the
texture returned in the CopyOutputRequest.

But if the GLRenderer goes away, we just leak the texture. :(

So, now that we have ContextProvider available, we hold a ref
on the provider inside the Callback, and this way we can always
destroy the texture when it is released. :)

Also, we can remove the WeakPtrFactory from GLRenderer.

R=piman
BUG=255634
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=218148

[danakj, 2013-08-16 02:37:03]

Phew, this test was a bit tricky to write, I hope it looks reasonable.

[piman, 2013-08-16 03:32:26]

The prod code changes LG, but not sure about threading in the test

https://codereview.chromium.org/22852006/diff/8001/cc/trees/layer_tree_host_u...
File cc/trees/layer_tree_host_unittest.cc (right):

https://codereview.chromium.org/22852006/diff/8001/cc/trees/layer_tree_host_u...
cc/trees/layer_tree_host_unittest.cc:3116: EXPECT_TRUE(result_);
result_ is set on the main thread, don't we have a potential data race here?

https://codereview.chromium.org/22852006/diff/8001/cc/trees/layer_tree_host_u...
cc/trees/layer_tree_host_unittest.cc:3136: result_.reset();
Is it ok to run the mailbox callback on the compositor thread?

[danakj, 2013-08-16 18:18:38]

https://codereview.chromium.org/22852006/diff/8001/cc/trees/layer_tree_host_u...
File cc/trees/layer_tree_host_unittest.cc (right):

https://codereview.chromium.org/22852006/diff/8001/cc/trees/layer_tree_host_u...
cc/trees/layer_tree_host_unittest.cc:3116: EXPECT_TRUE(result_);
On 2013/08/16 03:32:27, piman wrote:
> result_ is set on the main thread, don't we have a potential data race here?

I think it's fine, because we get here because of the SNC() in the result
callback.

https://codereview.chromium.org/22852006/diff/8001/cc/trees/layer_tree_host_u...
cc/trees/layer_tree_host_unittest.cc:3136: result_.reset();
On 2013/08/16 03:32:27, piman wrote:
> Is it ok to run the mailbox callback on the compositor thread?

oh, darn. I had done this better earlier. it can actually be called from any
thread and will post task to the impl. but we should verify it works on a
non-impl thread.

[danakj, 2013-08-16 18:28:28]

PTAL

[piman, 2013-08-16 18:46:07]

OK, LGTM.

[commit-bot: I haz the power, 2013-08-16 18:48:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/22852006/25001

[commit-bot: I haz the power, 2013-08-16 20:07:14]

Retried try job too often on linux_rel for step(s) cc_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[danakj, 2013-08-16 21:23:46]

Oh, nice, the added DCHECKs in TestContextProvider found some weirdness in
tests.

[danakj, 2013-08-16 21:47:50]

Ah, a couple output surface tests want to muck with state on the test content
before it's bound. I added an Unbound version of the TestContext getter that
doesn't check bound with a big comment about how to use it.

[commit-bot: I haz the power, 2013-08-16 21:54:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/22852006/42002

[piman, 2013-08-17 00:09:51]

lgtm

[commit-bot: I haz the power, 2013-08-17 02:29:12]

Change committed as 218148