Closed Bug 832635 Opened 11 years ago Closed 11 years ago

talos rck2 fails on panda boards due to a smaller screen resolution

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 21

People

(Reporter: jmaher, Assigned: kats)

Details

Attachments

(1 file, 1 obsolete file)

I looked into why rck2 is failing and it seems that our events that we replay access pixels outside of the application:
http://dxr.mozilla.org/mozilla-central/mobile/android/base/tests/assets/testcheck2-motionevents.html?string=testcheck2-motionevents

The resolution we have to work with is:
683 D/GeckoLayerClient( 6606): Screen-size changed to (1280,672)
01-19 00:58:43.683 D/GeckoLayerClient( 6606): Window-size changed to (1280,616)
01-19 00:58:43.

Many of the replay events indicate y coordinate >700.  I assume we can just recapture this on a smaller screen, or manually edit the motionevents file to have smaller coordinates.

I am not sure how this will affect the tegras.
I would rather modify MotionEventReplayer to scale the replay coordinates to fit the new screen dimension. So something like actual_x = SCREEN_WIDTH * x / TEGRA_WIDTH.

That way the test should be unchanged for the tegras, and will be scaled appropriately for the pandas. Do you know what the equivalent "window size" is on the tegras? It will need to be hard-coded into MotionEventReplayer.
a tegra has these dimensions:
	Screen width/height:1024/768
	Browser inner width/height: 1024/695

in this case we would want do use y and height instead of the x/width values.
It would be nice if the test was reliable on all devices -- not just tegras and pandas. Could we scale based on DisplayMetrics?
Yes, in my comment where I said "SCREEN_WIDTH" I meant the current device screen width, which would be gotten from DisplayMetrics. But I'm confused: if the Tegra has an inner height of 695, and the replay events have a y coordinate > 700, shouldn't it be broken there too?
I am confused by that as well.  It could be that we have 768 pixels and the top 73 pixels are the android bar, so tapping at 733 would still be inside the browser.
I see MotionEvents with y values of >800 in the log too. I created that event trace on a galaxy nexus, so it's pretty tall. Maybe I should just scale it relative to that. It'll probably change the talos numbers on the tegras too but it should just be a one-time thing and then we'll have normalized numbers from here on out.
Lets scale it both on tegra and panda platforms.  Maybe some rck2 'random' failures will be cleaned up as a result.
Attached patch Scale events (obsolete) — Splinter Review
Try build at https://tbpl.mozilla.org/?tree=Try&rev=a88accfce49e (I haven't tested it at all locally except to ensure it compiles).
Attached patch Scale eventsSplinter Review
That wasn't the right patch at all.
Attachment #704595 - Attachment is obsolete: true
Comment on attachment 704597 [details] [diff] [review]
Scale events

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

this patch looks pretty good, also try is green.

::: mobile/android/base/tests/testCheck2.java.in
@@ +30,5 @@
>           * overall performance, but doesn't really allow identifying which part is slow.
>           */
>  
> +        MotionEventReplayer mer = new MotionEventReplayer(getInstrumentation(), mDriver.getGeckoLeft(), mDriver.getGeckoTop(),
> +                                                          mDriver.getGeckoWidth(), mDriver.getGeckoHeight());

nit: fix the spacing here, maybe you have tabs or somethign?
Attachment #704597 - Flags: review+
(In reply to Joel Maher (:jmaher) from comment #10)
> this patch looks pretty good, also try is green.

It looks like the rck2 test is going to go from ~4.1 down to ~3.5. Just a heads-up.

> >  
> > +        MotionEventReplayer mer = new MotionEventReplayer(getInstrumentation(), mDriver.getGeckoLeft(), mDriver.getGeckoTop(),
> > +                                                          mDriver.getGeckoWidth(), mDriver.getGeckoHeight());
> 
> nit: fix the spacing here, maybe you have tabs or somethign?

I aligned the wrapped line to the other arguments, but perhaps this line is too long to do that. I'll fix it on landing when inbound reopens. Thanks for the review!
Also can you test it on a pandaboard to make sure it actually fixes the problem?
this is working on my local panda board!
Awesome, landed on inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/b06f986df020
Assignee: nobody → bugmail.mozilla
https://hg.mozilla.org/mozilla-central/rev/b06f986df020
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: