Re-enable ForceReclaimResources w/ ui impl side painting

Re-enable ForceReclaimResources w/ ui impl side painting

Disabling ForceReclaimResource on BeginCommit when using ui impl side painting
had the unintended consequence of breaking Display::Draw's mechanism for
not drawing while an OutputSurface was between Commit/Swap. Re enabling
this for now. Tracking handling this in a more robust way in
crbug.com/489515.

BUG=481656, 489515
Committed: https://crrev.com/17ec17f1df5f8100af1e3923d40d34e0f931a7a2
Cr-Commit-Position: refs/heads/master@{#330587}

[ericrk, 2015-05-19 01:13:20]

ericrk@chromium.org changed reviewers:
+ danakj@chromium.org

[ericrk, 2015-05-19 01:14:45]

Patchset #1 (id:1) has been deleted

[danakj, 2015-05-19 01:16:50]

LGTM

https://codereview.chromium.org/1139823008/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/1139823008/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:301: // TODO(ericrk): We should not need to
ForceReclaimResources when using
This will change soon though anyways. So I'd just instead talk about the
Display::Draw stuff depending on this and leave impl-side painting out of it
entirely.

[danakj, 2015-05-19 01:17:04]

Should we add a test somewhere for this?

[ericrk, 2015-05-19 18:23:10]

On 2015/05/19 18:19:09, ericrk wrote:

Let me know if this level of testing seems ok... I was thinking that a more
future-proof test would create both a display and a layer tree host impl, and
would ensure that display::draw failed after LayerTreeHostImpl::BeginCommit -
looking into the complexity of adding this test now.

[danakj, 2015-05-19 18:27:59]

On Tue, May 19, 2015 at 11:23 AM, <ericrk@chromium.org> wrote:

> On 2015/05/19 18:19:09, ericrk wrote:
>
> Let me know if this level of testing seems ok... I was thinking that a more
> future-proof test would create both a display and a layer tree host impl,
> and
> would ensure that display::draw failed after
> LayerTreeHostImpl::BeginCommit -
> looking into the complexity of adding this test now.
>

That sounds like a next level LayerTreeTest.. but hard to write. I don't
think I can recommend that. Lemme read your test..


>
> https://codereview.chromium.org/1139823008/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[danakj, 2015-05-19 18:33:55]

LGTM

https://codereview.chromium.org/1139823008/diff/40001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/1139823008/diff/40001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl_unittest.cc:7809: host_impl_->BeginCommit();
can you verifyexpectations before this so we know where the call comes from?

[ericrk, 2015-05-19 18:38:08]

https://codereview.chromium.org/1139823008/diff/40001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/1139823008/diff/40001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl_unittest.cc:7809: host_impl_->BeginCommit();
On 2015/05/19 18:33:54, danakj wrote:
> can you verifyexpectations before this so we know where the call comes from?

Done.

[ericrk, 2015-05-19 18:38:13]

The CQ bit was checked by ericrk@chromium.org

[ericrk, 2015-05-19 18:38:13]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org
Link to the patchset: https://codereview.chromium.org/1139823008/#ps60001
(title: "Make expectations more constrained")

[commit-bot: I haz the power, 2015-05-19 18:39:04]

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

[commit-bot: I haz the power, 2015-05-19 19:51:03]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-05-19 19:51:57]

Patchset 3 (id:??) landed as
https://crrev.com/17ec17f1df5f8100af1e3923d40d34e0f931a7a2
Cr-Commit-Position: refs/heads/master@{#330587}