cc: Change texture locking check point

cc: Change texture locking check point

Checking whether texture is acquired by main thread or not in
PendingActivationsShouldBeForced() has no meaning because texture is
always acquired by impl thread in pending state. Instead, this check
should be tested in PendingDrawShouldBeAborted().
Also, additional unittest is added for checking pending activation is
forced when context is lost.

R=brianderson@chromium.org
BUG=347797
TEST=cc_unittests

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255556

[simonhong, 2014-02-28 00:32:55]

PTAL!

[simonhong, 2014-03-05 21:02:24]

On 2014/02/28 00:32:55, simonhong wrote:
> PTAL!

Kindly ping..
(I just worried this review request was cracked in your a bunch of emails.
I know you're busy, so please check this in your free time. :) )

[brianderson, 2014-03-06 01:58:47]

I had to think about this a bit, but you are correct. The main thread should
never have the texture locked when there is a pending tree.

Can you add a DCHECK somewhere to make sure that is true? Perhaps in
SchedulerStateMachine::CheckInvariants.

Also, please consider keeping your commit line-lengths shorter. See the
following email thread:
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/sRRWJwvy2C8

[simonhong, 2014-03-06 13:42:04]

On 2014/03/06 01:58:47, brianderson wrote:
> I had to think about this a bit, but you are correct. The main thread should
> never have the texture locked when there is a pending tree.
> 
> Can you add a DCHECK somewhere to make sure that is true? Perhaps in
> SchedulerStateMachine::CheckInvariants.
> 
> Also, please consider keeping your commit line-lengths shorter. See the
> following email thread:
>
https://groups.google.com/a/chromium.org/forum/#%21topic/chromium-dev/sRRWJwv...

New invariant condition is added and description is re-arranged.
Please check again.
Many thanks for review!

[brianderson, 2014-03-06 17:56:25]

lgtm, pending nits.

https://codereview.chromium.org/184063002/diff/70001/cc/scheduler/scheduler_s...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/184063002/diff/70001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:564: // Main thread should never have
the texture locked when there is a pending
nit: line break above comment.

https://codereview.chromium.org/184063002/diff/70001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:566: if (has_pending_tree_)
Can you change this to:
if (has_pending_tree_ || active_tree_needs_first_draw_)

The impl thread shouldn't give up it's lock until after the commit has been
drawn for the first time.

[simonhong, 2014-03-06 21:04:48]

I addressed your comments.
Thanks!

https://codereview.chromium.org/184063002/diff/70001/cc/scheduler/scheduler_s...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/184063002/diff/70001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:564: // Main thread should never have
the texture locked when there is a pending
On 2014/03/06 17:56:25, brianderson wrote:
> nit: line break above comment.

Done.

https://codereview.chromium.org/184063002/diff/70001/cc/scheduler/scheduler_s...
cc/scheduler/scheduler_state_machine.cc:566: if (has_pending_tree_)
On 2014/03/06 17:56:25, brianderson wrote:
> Can you change this to:
> if (has_pending_tree_ || active_tree_needs_first_draw_)
> 
> The impl thread shouldn't give up it's lock until after the commit has been
> drawn for the first time.

Done.

[simonhong, 2014-03-06 21:04:53]

The CQ bit was checked by simonhong@chromium.org

[commit-bot: I haz the power, 2014-03-06 21:19:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonhong@chromium.org/184063002/90001

[commit-bot: I haz the power, 2014-03-06 22:07:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonhong@chromium.org/184063002/90001

[commit-bot: I haz the power, 2014-03-06 23:43:50]

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

[commit-bot: I haz the power, 2014-03-06 23:43:50]

Try jobs failed on following builders: mac_chromium_compile_dbg,
mac_chromium_rel

[brianderson, 2014-03-06 23:46:45]

The CQ bit was checked by brianderson@chromium.org

[commit-bot: I haz the power, 2014-03-06 23:56:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonhong@chromium.org/184063002/90001

[commit-bot: I haz the power, 2014-03-07 02:18:52]

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

[commit-bot: I haz the power, 2014-03-07 02:18:52]

Try jobs failed on following builders: mac_chromium_rel

[simonhong, 2014-03-07 03:12:34]

The CQ bit was checked by simonhong@chromium.org

[commit-bot: I haz the power, 2014-03-07 03:15:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonhong@chromium.org/184063002/90001

[commit-bot: I haz the power, 2014-03-07 08:56:44]

Change committed as 255556