Replace VSyncMonitor with compositor's VSyncPulseMonitor.

Replace VSyncMonitor with compositor's VSyncPulseMonitor.

Differences:
- Constructor doesn't require View.
- The listener callback is called at most MAX_VSYNC_COUNT times.
- VSYNC_TIMEOUT is not used and Terminator is not created.
- Minimal timer-based ICS support is added, but see http://crbug.com/156397.

BUG=160017
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=167095

[aruslan, 2012-11-09 01:21:10]

Gentlemen, please review.
Once it's done, I'll call in OWNERs.

[Jerome, 2012-11-09 01:59:00]

lgtm

A few nits. then lgtm

https://codereview.chromium.org/11362177/diff/1/content/public/android/java/s...
File
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java
(right):

https://codereview.chromium.org/11362177/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java:29:
* @param vsyncTimeMicro Absolute frame time in microseconds.
vsyncTimeMicroS (typo)

https://codereview.chromium.org/11362177/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java:62:
Context context, VSyncMonitor.Listener listener) {
This should fit on one line

https://codereview.chromium.org/11362177/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java:67:
Context context, VSyncMonitor.Listener listener, boolean enableJBVSync) {
This should fit on one line

https://codereview.chromium.org/11362177/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java:192:
if (delay == 0) mHandler.post(mVSyncRunnableCallback);
you need { } because of the "else"

[aruslan, 2012-11-09 02:10:35]

Thanks!
Sami -- please review and let me know if anything doesn't make sense.

https://codereview.chromium.org/11362177/diff/1/content/public/android/java/s...
File
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java
(right):

https://codereview.chromium.org/11362177/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java:29:
* @param vsyncTimeMicro Absolute frame time in microseconds.
On 2012/11/09 01:59:00, Jerome wrote:
> vsyncTimeMicroS (typo)

Done.

https://codereview.chromium.org/11362177/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java:62:
Context context, VSyncMonitor.Listener listener) {
On 2012/11/09 01:59:00, Jerome wrote:
> This should fit on one line

Done.

https://codereview.chromium.org/11362177/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java:67:
Context context, VSyncMonitor.Listener listener, boolean enableJBVSync) {
On 2012/11/09 01:59:00, Jerome wrote:
> This should fit on one line

Done.

[Sami, 2012-11-09 18:21:26]

https://chromiumcodereview.appspot.com/11362177/diff/4001/content/public/andr...
File
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java
(right):

https://chromiumcodereview.appspot.com/11362177/diff/4001/content/public/andr...
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java:18:
* called MAX_VSYNC_COUNT times (on the closest vertical sync pulse and on the
next one),
The comment in parenthesis isn't quite accurate any since we pulse more than
twice. Also the choreographer always queues the callback for the next vsync
instead of the closest one, so we might want to point out the difference to the
timer emulation here.

https://chromiumcodereview.appspot.com/11362177/diff/4001/content/public/andr...
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java:20:
* {@link VSyncMonitor} should be used.
The last sentence seems unnecessary now :)

https://chromiumcodereview.appspot.com/11362177/diff/4001/content/public/andr...
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java:48:
private int mTriggerNextVsyncCount;
s/Vsync/VSync/g

https://chromiumcodereview.appspot.com/11362177/diff/4001/content/public/andr...
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java:92:
public void run() {
Please call TraceEvent here too, maybe as TraceEvent.instant("VSyncTimer") so we
can tell the difference between the two.

[aruslan, 2012-11-09 19:15:44]

Thanks for the reviews!
Appraising to Jay for the OWNER's LGTM seal.

https://codereview.chromium.org/11362177/diff/4001/content/public/android/jav...
File
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java
(right):

https://codereview.chromium.org/11362177/diff/4001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java:18:
* called MAX_VSYNC_COUNT times (on the closest vertical sync pulse and on the
next one),
On 2012/11/09 18:21:26, Sami wrote:
> The comment in parenthesis isn't quite accurate any since we pulse more than
> twice. Also the choreographer always queues the callback for the next vsync
> instead of the closest one, so we might want to point out the difference to
the
> timer emulation here.

Done.

https://codereview.chromium.org/11362177/diff/4001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java:20:
* {@link VSyncMonitor} should be used.
On 2012/11/09 18:21:26, Sami wrote:
> The last sentence seems unnecessary now :)

Done.

https://codereview.chromium.org/11362177/diff/4001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java:48:
private int mTriggerNextVsyncCount;
On 2012/11/09 18:21:26, Sami wrote:
> s/Vsync/VSync/g

Done.

https://codereview.chromium.org/11362177/diff/4001/content/public/android/jav...
content/public/android/java/src/org/chromium/content/browser/VSyncMonitor.java:92:
public void run() {
On 2012/11/09 18:21:26, Sami wrote:
> Please call TraceEvent here too, maybe as TraceEvent.instant("VSyncTimer") so
we
> can tell the difference between the two.

Done.

[Jay Civelli, 2012-11-09 19:49:50]

lgtm

[commit-bot: I haz the power, 2012-11-10 02:47:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aruslan@chromium.org/11362177/4005

[commit-bot: I haz the power, 2012-11-10 19:07:12]

Change committed as 167095