aw: Fix invalidate getting lost

aw: Fix invalidate getting lost

An invalidate could be lost in this case:
* SetNeedsContinuousInvalidate starts out as true
* Functor happens on render thread, and SetNeedsContinuousInvalidate
goes false, but because we are on render thread, this is posted back
to UI thread
* Before the task runs on UI, compositor SetNeedsContinuousInvalidate
to true again, since the value is already true, this is a no-op, but
we should really be invalidating now
* The posted task runs and SetNeedsContinuousInvalidate to false,
and we just lost an invalidate

Fix by updating the value with a lock and only post for the invalidate.
This avoids the race causing early out in SetNeedsContinuousInvalidate.

For internal b/13979752
BUG=

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

[boliu, 2014-05-04 19:55:19]

pls cq after review

[boliu, 2014-05-05 16:08:13]

-> sgurun for pst time zone :)

[sgurun-gerrit only, 2014-05-05 17:11:00]

https://codereview.chromium.org/269913002/diff/1/android_webview/browser/brow...
File android_webview/browser/browser_view_renderer.cc (right):

https://codereview.chromium.org/269913002/diff/1/android_webview/browser/brow...
android_webview/browser/browser_view_renderer.cc:420: 
Please add explanation here why you need to set the invalidate under lock. You
don't want somebody else breaking this logic, especially since no test coverage
for it.

[boliu, 2014-05-05 17:13:41]

https://codereview.chromium.org/269913002/diff/1/android_webview/browser/brow...
File android_webview/browser/browser_view_renderer.cc (right):

https://codereview.chromium.org/269913002/diff/1/android_webview/browser/brow...
android_webview/browser/browser_view_renderer.cc:420: 
On 2014/05/05 17:11:01, sgurun wrote:
> Please add explanation here why you need to set the invalidate under lock. You
> don't want somebody else breaking this logic, especially since no test
coverage
> for it.

There is already a comment in the header saying the lock is used for "data that
must be set immediately". Is that good enough for you?

I'm actively working on ubercomp after which we don't need all this thread
hopping.

[sgurun-gerrit only, 2014-05-05 17:24:52]

lgtm

https://codereview.chromium.org/269913002/diff/1/android_webview/browser/brow...
File android_webview/browser/browser_view_renderer.cc (right):

https://codereview.chromium.org/269913002/diff/1/android_webview/browser/brow...
android_webview/browser/browser_view_renderer.cc:420: 
On 2014/05/05 17:13:41, boliu wrote:
> On 2014/05/05 17:11:01, sgurun wrote:
> > Please add explanation here why you need to set the invalidate under lock.
You
> > don't want somebody else breaking this logic, especially since no test
> coverage
> > for it.
> 
> There is already a comment in the header saying the lock is used for "data
that
> must be set immediately". Is that good enough for you?
> 
> I'm actively working on ubercomp after which we don't need all this thread
> hopping.
If it is temporary it is ok, otherwise the explanation on the lock is not that
helpful.

[boliu, 2014-05-05 17:26:06]

The CQ bit was checked by boliu@chromium.org

[commit-bot: I haz the power, 2014-05-05 17:27:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/boliu@chromium.org/269913002/1

[commit-bot: I haz the power, 2014-05-05 20:27:52]

Change committed as 268258