922770 - Increase default threshold for "stable frame" in eideticker

Status: RESOLVED FIXED

(Testing Graveyard :: Eideticker, defect)

Description

[William Lachance (:wlach)]

In this capture: http://eideticker.wrla.ch/framediff-view.html?title=Frame%20Difference%20Startup%20to%20about:home%20%28fresh%20profile%29%20%282013-09-30%29&video=videos/video-1380613046.49.webm&framediff=framediffs/framediff-1380613056.1.json&actionlog=

It's pretty clear that it's done loading at around 2.7s, but there's a transition where the scrollbar disappears, which momentarily causes a 2580 pixel change, which exceeds the threshold, so we say it ends at around 3.25 seconds.

Let's increase the default threshold to 4096, as actual major changes in loading are likely to exceed that greatly (as the above capture shows).

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

I looked at some of the pageload tests, to see if increasing the threshold would effect their "end times", but I saw pixel differences in the 7K range at the very end of pageload. Bumping the threshold to 4096 won't suddenly make pageload tests seem to run faster, which is a good thing.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

William - Any ETA for this change?

Comment 3

[William Lachance (:wlach)]

Attachment: 0001-Bug-922770-Set-stable-frame-threshold-to-be-less-tha.patch

Adjust framediff threshold default (implicitly this is an Android-only change).

Comment 4

[William Lachance (:wlach)]

(In reply to Mark Finkle (:mfinkle) from comment #2)
> William - Any ETA for this change?

Sorry for the delay on such a simple change, was on PTO after the summit and things have generally been hectic.

I have applied to patch manually pending review, so new builds should be using the setting.

Comment 5

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 817232 [details] [diff] [review]
0001-Bug-922770-Set-stable-frame-threshold-to-be-less-tha.patch

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

Are the submodule updated intentional?

Comment 6

[William Lachance (:wlach)]

(In reply to Dave Hunt (:davehunt) from comment #5)
> Comment on attachment 817232 [details] [diff] [review]
> 0001-Bug-922770-Set-stable-frame-threshold-to-be-less-tha.patch
> 
> Review of attachment 817232 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Are the submodule updated intentional?

No. I'll upload a new patch without this problem.

Comment 7

[William Lachance (:wlach)]

Attachment: Remove accidental submodule updates

Comment 8

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 817468 [details] [diff] [review]
Remove accidental submodule updates

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

Looks good.

Comment 9

[William Lachance (:wlach)]

Pushed: https://github.com/mozilla/eideticker/commit/27c59525e585d4b33bfbb73fb521dca455e8e197