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

RESOLVED FIXED in Firefox 21

Status

()

Firefox for Android
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jmaher, Assigned: kats)

Tracking

unspecified
Firefox 21
ARM
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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.
(Reporter)

Comment 2

5 years ago
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?
(Reporter)

Comment 5

5 years ago
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.
(Reporter)

Comment 7

5 years ago
Lets scale it both on tegra and panda platforms.  Maybe some rck2 'random' failures will be cleaned up as a result.
Created attachment 704595 [details] [diff] [review]
Scale events

Try build at https://tbpl.mozilla.org/?tree=Try&rev=a88accfce49e (I haven't tested it at all locally except to ensure it compiles).
Created attachment 704597 [details] [diff] [review]
Scale events

That wasn't the right patch at all.
Attachment #704595 - Attachment is obsolete: true
(Reporter)

Comment 10

5 years ago
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?
(Reporter)

Comment 13

5 years ago
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

Comment 15

5 years ago
https://hg.mozilla.org/mozilla-central/rev/b06f986df020
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
You need to log in before you can comment on or make changes to this bug.