Add Clipboard::GetSequenceNumber test and fix Android.

Add Clipboard::GetSequenceNumber test and fix Android.

BUG=none

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

[dcheng, 2014-01-13 15:51:10]

Actually... I need to update the Android implementation first it looks like. Let
me do that before sending this out for review again. Sorry!

[dcheng, 2014-01-13 23:28:05]

So... I added what I thought would be sufficient plumbing on Android, but I
can't get it to work on Android. bulach@, any ideas why the callback doesn't
ever fire there?

[raymes, 2014-01-14 02:55:47]

lgtm for ui/base/clipboard/clipboard_unittest.cc

https://codereview.chromium.org/136193002/diff/260001/ui/base/clipboard/clipb...
File ui/base/clipboard/clipboard_unittest.cc (right):

https://codereview.chromium.org/136193002/diff/260001/ui/base/clipboard/clipb...
ui/base/clipboard/clipboard_unittest.cc:740: EXPECT_NE(first_sequence_number,
second_sequence_number);
It might be worth checking the case that I fixed (where the number of elements
on the clipboard doesn't change resulting in a duplicate sequence number).

[bulach, 2014-01-14 14:45:04]

https://codereview.chromium.org/136193002/diff/260001/ui/android/java/src/org...
File ui/android/java/src/org/chromium/ui/base/Clipboard.java (right):

https://codereview.chromium.org/136193002/diff/260001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/base/Clipboard.java:32: private class
Listener implements ClipboardManager.OnPrimaryClipChangedListener {
wild guess: in the test, it's doing base::RunLoop().RunUntilIdle(), which only
works for C++ based tasks...

the java-side may trigger a callback later on, without ever going through the
PostTask, MessageLoop, or hitting C++ at all.. 

also note in the test, since the call stack is _inside_ C++, the java side will
never be "pumped", so the callback would potentially never be executed...

see the "internals":
https://android.googlesource.com/platform/frameworks/base/+/b267554/core/java...

looks like it has a Handler there, so that it posts a java message to itself to
then reportPrimaryClipChanged?

could that be the case?

unfortunately, there's no easy way to integrate this that I know of.. :(

[dcheng, 2014-01-14 17:57:34]

On 2014/01/14 14:45:04, bulach wrote:
>
https://codereview.chromium.org/136193002/diff/260001/ui/android/java/src/org...
> File ui/android/java/src/org/chromium/ui/base/Clipboard.java (right):
> 
>
https://codereview.chromium.org/136193002/diff/260001/ui/android/java/src/org...
> ui/android/java/src/org/chromium/ui/base/Clipboard.java:32: private class
> Listener implements ClipboardManager.OnPrimaryClipChangedListener {
> wild guess: in the test, it's doing base::RunLoop().RunUntilIdle(), which only
> works for C++ based tasks...
> 
> the java-side may trigger a callback later on, without ever going through the
> PostTask, MessageLoop, or hitting C++ at all.. 
> 
> also note in the test, since the call stack is _inside_ C++, the java side
will
> never be "pumped", so the callback would potentially never be executed...
> 
> see the "internals":
>
https://android.googlesource.com/platform/frameworks/base/+/b267554/core/java...
> 
> looks like it has a Handler there, so that it posts a java message to itself
to
> then reportPrimaryClipChanged?
> 
> could that be the case?
> 
> unfortunately, there's no easy way to integrate this that I know of.. :(

Hmm, are there no test helpers for Android Chrome for the C++ to yield back to
Java to "do its thing" for testing purposes?
I'm fine with making the test more complicated to make sure it works cross
platform. I'm just hoping there's a way to do it at all...

[bulach, 2014-01-15 15:54:08]

not that I'm aware.. :( the browser tests have something similar, but it has to
install its own message loop in both sides, and it's not that stable...

frankf@ probably knows more though..

[dcheng, 2014-01-15 20:45:34]

On 2014/01/15 15:54:08, bulach wrote:
> not that I'm aware.. :( the browser tests have something similar, but it has
to
> install its own message loop in both sides, and it's not that stable...
> 
> frankf@ probably knows more though..

bulch@: Many thanks for the pointers. Given the complexities on Android, I'm
going to ifdef the test out on Android and land this patch as is for now
(reverting the Android specific changes). I'll followup with the
Android-specific work in another patch.

[commit-bot: I haz the power, 2014-01-15 21:29:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcheng@chromium.org/136193002/400001

[commit-bot: I haz the power, 2014-01-16 02:57:05]

Change committed as 245079