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)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 21
People
(Reporter: jmaher, Assigned: kats)
Details
Attachments
(1 file, 1 obsolete file)
7.28 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•11 years ago
|
||
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•11 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.
Comment 3•11 years ago
|
||
It would be nice if the test was reliable on all devices -- not just tegras and pandas. Could we scale based on DisplayMetrics?
Assignee | ||
Comment 4•11 years ago
|
||
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•11 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.
Assignee | ||
Comment 6•11 years ago
|
||
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•11 years ago
|
||
Lets scale it both on tegra and panda platforms. Maybe some rck2 'random' failures will be cleaned up as a result.
Assignee | ||
Comment 8•11 years ago
|
||
Try build at https://tbpl.mozilla.org/?tree=Try&rev=a88accfce49e (I haven't tested it at all locally except to ensure it compiles).
Assignee | ||
Comment 9•11 years ago
|
||
That wasn't the right patch at all.
Attachment #704595 -
Attachment is obsolete: true
Reporter | ||
Comment 10•11 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+
Assignee | ||
Comment 11•11 years ago
|
||
(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!
Assignee | ||
Comment 12•11 years ago
|
||
Also can you test it on a pandaboard to make sure it actually fixes the problem?
Reporter | ||
Comment 13•11 years ago
|
||
this is working on my local panda board!
Assignee | ||
Comment 14•11 years ago
|
||
Awesome, landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/b06f986df020
Assignee: nobody → bugmail.mozilla
Comment 15•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b06f986df020
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•