Closed Bug 1355548 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: DOM: Events, enhancement, P1)

Product:
Core
Component:
DOM: Events
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Project Flags:
Performance Impact high
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: kanru, Assigned: stone)

References

Details

Attachments

(1 file, 1 obsolete file)

Bug 1355548 - Using cached perference values in WheelTransaction (v2).
8.70 KB, patch
Details | Diff | Splinter Review 
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: nobody → sshih
Flags: needinfo?(sshih)
Whiteboard: [qf] → [qf:p1]
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)
(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 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+
Keywords: checkin-needed
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)

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

    
    

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

          status-firefox55:
          affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Performance Impact: --- → P1
Whiteboard: [qf:p1]

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: