Closed Bug 743669 Opened 8 years ago Closed 8 years ago

fix testcheckerboard2 to work on android 2.2

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 14
Tracking Status
firefox14 --- fixed

People

(Reporter: jmaher, Assigned: kats)

References

Details

Attachments

(1 file, 1 obsolete file)

last week blassey looked at testcheckerboard2 and has a solution in place for it to work on the tegras.
Assignee: nobody → jmaher
Status: NEW → ASSIGNED
Attachment #613288 - Flags: review?(bugmail.mozilla)
Comment on attachment 613288 [details] [diff] [review]
fix tcheckerboard2 to work on android 2.2 (1.0)

Review of attachment 613288 [details] [diff] [review]:
-----------------------------------------------------------------

This flattens events from multiple touch points (like a pinch-zoom) into a single touch point. From the javadoc for addBatch: "The current values in the event are added to a list of historical values." - this is not what we want.
Attachment #613288 - Flags: review?(bugmail.mozilla) → review-
we can't realistically support multi-touch
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
FWIW, I tried applying this patch locally and ran it on Galaxy Tab. It didn't work, in that the pinch zooms in testCheckerboard2 didn't get replayed properly. Looking at https://github.com/android/platform_frameworks_base/blob/9066cfe9886ac131c34d59ed0e2d287b0e3c0087/core/java/android/view/MotionEvent.java (a really old version of MotionEvent.java) still shows that addBatch adds the data to the history list rather than adding it as a new touch point so I don't think it's something specific to froyo+.
blassey and I were looking at this and we found a hidden API that should work. I wrote up a patch to use it on pre-gingerbread and pushed it to try here: https://tbpl.mozilla.org/?tree=Try&rev=2a632a37fa77 (I also copied testCheck2 to testCheck so that the tcheckerboard numbers from that try run should be from testCheck2). If that works I'll upload the patch for review.
Assignee: jmaher → bugmail.mozilla
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Above try run failed with build errors (partly stupidity on my part, partly it revealed that hidden APIs are also not accessible at compile-time). I modified the patch to use reflection instead and pushed to https://tbpl.mozilla.org/?tree=Try&rev=be9dee04ccbf but it also failed, for reason(s) unknown. Trying again with theoretically more logging at https://tbpl.mozilla.org/?tree=Try&rev=4b2ff5524af0
Finally got this to work..
Attachment #613288 - Attachment is obsolete: true
Attachment #614279 - Flags: review?(blassey.bugs)
(The successful try run is at https://tbpl.mozilla.org/?tree=Try&rev=5799dfd93680 which shows tcheckerboard returning 0.87).
Comment on attachment 614279 [details] [diff] [review]
fix tcheckerboard2 to work on android 2.2 (v2)

Review of attachment 614279 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/tests/MotionEventReplayer.java.in
@@ +66,5 @@
>          }
>          return Integer.parseInt(value);
>      }
>  
> +    public void replayEvents(InputStream eventDescriptions) throws Exception {

In general I'd prefer to declare the exceptions this could throw, but invoke throws so many. How about we declare it as "throws IOExeption, ReflectionException" where ReflectionException is an Exception you define here to rethrow anything that getMethod or invoke throws.

::: mobile/android/base/tests/testCheck2.java.in
@@ +33,5 @@
>          // replay the events
>          try {
>              mer.replayEvents(getAsset("testcheck2-motionevents"));
> +            // give it some time to draw any final frames
> +            Thread.sleep(1000);

hrm... sleeps in tests always make me nervous. But I see this already existed prior to this change.
Attachment #614279 - Flags: review?(blassey.bugs) → review+
(In reply to Brad Lassey [:blassey] from comment #9)
> In general I'd prefer to declare the exceptions this could throw, but invoke
> throws so many. How about we declare it as "throws IOExeption,
> ReflectionException" where ReflectionException is an Exception you define
> here to rethrow anything that getMethod or invoke throws.

I just listed them all, I prefer that to creating a wrapper exception. I left the catch block as catch(Exception).

https://hg.mozilla.org/integration/mozilla-inbound/rev/428c6bf76857
Target Milestone: --- → Firefox 14
https://hg.mozilla.org/mozilla-central/rev/428c6bf76857
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.