WheelTransaction::GetTimeoutTime() should probably use cached preference value

RESOLVED FIXED in Firefox 55

Status

()

enhancement
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: kanru, Assigned: stone)

Tracking

Trunk
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [qf:p1])

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

2 years ago
This function is used in EventStateManager so I think we should cache it. It seems gfxPrefs already have cache for it.

Same to the WheelTransaction::GetIgnoreMoveDelayTime() function.
Stone, do you think you could take this?
Flags: needinfo?(sshih)
Priority: -- → P1
Assignee

Updated

2 years ago
Assignee: nobody → sshih
Flags: needinfo?(sshih)
Whiteboard: [qf] → [qf:p1]
Assignee

Comment 2

2 years ago
Assuming we should also handle other similar cases which are WheelTransaction::GetAccelerationStart, WheelTransaction::GetAccelerationFactor, and preference 'test.mousescroll'.

Please let me know if that is not as your expectation. Thanks.
Flags: needinfo?(kchen)
Reporter

Comment 3

2 years ago
(In reply to Ming-Chou Shih [:stone] from comment #2)
> Assuming we should also handle other similar cases which are
> WheelTransaction::GetAccelerationStart,
> WheelTransaction::GetAccelerationFactor, and preference 'test.mousescroll'.
> 
> Please let me know if that is not as your expectation. Thanks.

Yes, it looks like they can also be cached. Some of them are in gfxPrefs too!
Flags: needinfo?(kchen)
Comment hidden (mozreview-request)
Comment on attachment 8860786 [details]
Bug 1355548 - Using cached perference values in WheelTransaction.

https://reviewboard.mozilla.org/r/132738/#review135586

::: dom/events/EventStateManager.cpp:334
(Diff revision 1)
> +  static bool sIsWeelTransactionStaticsInitialized = false;
> +  if (!sIsWeelTransactionStaticsInitialized) {

How about to check this in WheelTransaction::Prefs::InitializeStatics() with local static variables? Then, it's guaranteed that it runs only once even if new callers would come.
Attachment #8860786 - Flags: review?(masayuki) → review+
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Keywords: checkin-needed

Comment 8

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2e0e49cb827f
Using cached perference values in WheelTransaction. r=masayuki
Keywords: checkin-needed
Backed out for failing mochitests widget/tests/test_wheeltransaction.xul and gfx/layers/apz/test/mochitest/test_scroll_inactive_bug1190112.html:

https://hg.mozilla.org/integration/autoland/rev/d22b143974b77c4521dee796aa4eb856b5464bb7

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=2e0e49cb827f65d788d8b86a7239a0ef9814bff8&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable

Failure log gfx/layers/apz/test/mochitest/test_scroll_inactive_bug1190112.html |: https://treeherder.mozilla.org/logviewer.html#?job_id=93745735&repo=autoland

[task 2017-04-24T13:22:11.903352Z] 13:22:11     INFO - TEST-START | gfx/layers/apz/test/mochitest/test_scroll_inactive_bug1190112.html
[task 2017-04-24T13:22:12.446943Z] 13:22:12     INFO - GECKO(1878) | Flushing APZ repaints was a no-op, triggering callback directly...
[task 2017-04-24T13:22:12.575045Z] 13:22:12     INFO - TEST-INFO | started process screentopng
[task 2017-04-24T13:22:14.102359Z] 13:22:14     INFO - TEST-INFO | screentopng: exit 0
[task 2017-04-24T13:22:14.102787Z] 13:22:14     INFO - Buffered messages logged at 13:22:12
[task 2017-04-24T13:22:14.104943Z] 13:22:14     INFO - TEST-PASS | gfx/layers/apz/test/mochitest/test_scroll_inactive_bug1190112.html | viewport should not have scrolled 
[task 2017-04-24T13:22:14.106457Z] 13:22:14     INFO - TEST-PASS | gfx/layers/apz/test/mochitest/test_scroll_inactive_bug1190112.html | subframe should have scrolled 
[task 2017-04-24T13:22:14.106792Z] 13:22:14     INFO - Buffered messages finished
[task 2017-04-24T13:22:14.108514Z] 13:22:14     INFO - TEST-UNEXPECTED-FAIL | gfx/layers/apz/test/mochitest/test_scroll_inactive_bug1190112.html | viewport should have scrolled 
[task 2017-04-24T13:22:14.108884Z] 13:22:14     INFO -     doOuterScroll/<@gfx/layers/apz/test/mochitest/test_scroll_inactive_bug1190112.html:515:5
[task 2017-04-24T13:22:14.109169Z] 13:22:14     INFO -     waitForPaints/<@SimpleTest/EventUtils.js:619:11
[task 2017-04-24T13:22:14.109416Z] 13:22:14     INFO -     waitForPaints@SimpleTest/paint_listener.js:77:5
[task 2017-04-24T13:22:14.111712Z] 13:22:14     INFO -     waitForPaints/<@SimpleTest/paint_listener.js:65:22
[task 2017-04-24T13:22:14.113699Z] 13:22:14     INFO -     setTimeout handler*SimpleTest_setTimeoutShim@SimpleTest/SimpleTest.js:672:12
[task 2017-04-24T13:22:14.117422Z] 13:22:14     INFO -     paintListener@SimpleTest/paint_listener.js:32:7
[task 2017-04-24T13:22:14.119234Z] 13:22:14     INFO -     EventListener.handleEvent*@SimpleTest/paint_listener.js:35:3
[task 2017-04-24T13:22:14.121131Z] 13:22:14     INFO -     @SimpleTest/paint_listener.js:1:2

Failure log widget/tests/test_wheeltransaction.xul: https://treeherder.mozilla.org/logviewer.html#?job_id=93745701&repo=autoland

[task 2017-04-24T13:34:07.284208Z] 13:34:07     INFO - TEST-START | widget/tests/test_wheeltransaction.xul
[task 2017-04-24T13:39:09.581171Z] 13:39:09     INFO - TEST-INFO | started process screentopng
[task 2017-04-24T13:39:09.907797Z] 13:39:09     INFO - TEST-INFO | screentopng: exit 0
[task 2017-04-24T13:39:09.907878Z] 13:39:09     INFO - Buffered messages logged at 13:34:07
[task 2017-04-24T13:39:09.909178Z] 13:39:09     INFO - TEST-PASS | widget/tests/test_wheeltransaction.xul | passed: Continuous scrolling test for root view (vertical/forward) 
[task 2017-04-24T13:39:09.910052Z] 13:39:09     INFO - TEST-PASS | widget/tests/test_wheeltransaction.xul | passed: Continuous scrolling test for root view (vertical/forward) 
[task 2017-04-24T13:39:09.910661Z] 13:39:09     INFO - TEST-PASS | widget/tests/test_wheeltransaction.xul | passed: Continuous scrolling test for root view (vertical/forward) 
[task 2017-04-24T13:39:09.911286Z] 13:39:09     INFO - TEST-PASS | widget/tests/test_wheeltransaction.xul | passed: Continuous scrolling test for root view (vertical/forward) 
[task 2017-04-24T13:39:09.911806Z] 13:39:09     INFO - Buffered messages finished
[task 2017-04-24T13:39:09.912509Z] 13:39:09     INFO - TEST-UNEXPECTED-FAIL | widget/tests/test_wheeltransaction.xul | Test timed out.
Flags: needinfo?(sshih)
Assignee

Comment 10

2 years ago
Attachment #8860786 - Attachment is obsolete: true
Flags: needinfo?(sshih)

Comment 12

2 years ago
Pushed by sshih@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ec32f078201d
Using cached perference values in WheelTransaction. r=masayuki
https://hg.mozilla.org/mozilla-central/rev/ec32f078201d
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.