Last Comment Bug 657844 - SynthesizeMouseMove calls should be disabled in fennec
: SynthesizeMouseMove calls should be disabled in fennec
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Event Handling (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: mozilla7
Assigned To: Oleg Romashin (:romaxa)
:
Mentors:
Depends on:
Blocks: 606672
  Show dependency treegraph
 
Reported: 2011-05-17 18:10 PDT by Oleg Romashin (:romaxa)
Modified: 2011-05-25 02:53 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Disable synthMouseMove for mobile (750 bytes, patch)
2011-05-18 09:56 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Splinter Review
Pref version of synthMouseMove disable (1.51 KB, patch)
2011-05-18 10:08 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Splinter Review
Pref version of synthMouseMove disable (1.52 KB, patch)
2011-05-18 10:10 PDT, Oleg Romashin (:romaxa)
bzbarsky: review+
Details | Diff | Splinter Review
Version with cacheBool var (1.56 KB, patch)
2011-05-20 13:04 PDT, Oleg Romashin (:romaxa)
bzbarsky: review-
Details | Diff | Splinter Review
Version with cacheBool var v2 (2.33 KB, patch)
2011-05-20 15:34 PDT, Oleg Romashin (:romaxa)
bzbarsky: review+
Details | Diff | Splinter Review
Added default value (2.96 KB, patch)
2011-05-23 07:39 PDT, Oleg Romashin (:romaxa)
bzbarsky: review+
Details | Diff | Splinter Review

Description Oleg Romashin (:romaxa) 2011-05-17 18:10:12 PDT
While scrolling Fennec UI remote-viewport we do call SynthesizeMouseMove from PresShell::DidDoReflow http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#7865

IIRC that is needed in order to update cursor position when FF scrolled with mouse wheel, but cursor stay on the same screen position (content behind cursor changed)....

I think this is just wasting CPU time Event handling, dispatching HitTest calls just for nothing on Mobile...

And I think we should find some conditions which help us to disable this for Mobile fennec UI process...
Comment 1 Oleg Romashin (:romaxa) 2011-05-18 09:56:23 PDT
Created attachment 533311 [details] [diff] [review]
Disable synthMouseMove for mobile
Comment 2 Oleg Romashin (:romaxa) 2011-05-18 09:57:43 PDT
Not sure about other platforms which could use MOBILE_OPTIMIZE_GFX but possible having mouse pointer... like neetbooks et.c...
Probably it is better to make some pref, and control it per XUL application client.
Comment 3 Oleg Romashin (:romaxa) 2011-05-18 10:08:03 PDT
Created attachment 533314 [details] [diff] [review]
Pref version of synthMouseMove disable

should be more flexible for different platforms handling and different xulrunner applications
Comment 4 Oleg Romashin (:romaxa) 2011-05-18 10:10:42 PDT
Created attachment 533317 [details] [diff] [review]
Pref version of synthMouseMove disable

ups, typo bug
Comment 5 Boris Zbarsky [:bz] 2011-05-18 10:32:05 PDT
Comment on attachment 533311 [details] [diff] [review]
Disable synthMouseMove for mobile

Hmm.  Why is a gfx-specific define the right thing to condition this on?
Comment 6 Oleg Romashin (:romaxa) 2011-05-20 11:32:46 PDT
Comment on attachment 533311 [details] [diff] [review]
Disable synthMouseMove for mobile

yep, I think GFX define is bad way to disable it and not flexible...
Comment 7 Boris Zbarsky [:bz] 2011-05-20 11:52:32 PDT
Comment on attachment 533317 [details] [diff] [review]
Pref version of synthMouseMove disable

This looks fine, but why not just make that a live pref using a bool pref cache?
Comment 8 Oleg Romashin (:romaxa) 2011-05-20 13:04:45 PDT
Created attachment 534080 [details] [diff] [review]
Version with cacheBool var
Comment 9 Boris Zbarsky [:bz] 2011-05-20 13:37:41 PDT
Comment on attachment 534080 [details] [diff] [review]
Version with cacheBool var

This is going to add a new cache on each DidReflow call.
Comment 10 Oleg Romashin (:romaxa) 2011-05-20 15:34:57 PDT
Created attachment 534135 [details] [diff] [review]
Version with cacheBool var v2

Did not found any static initialize function in PresShell, so I hope it is ok to reuse static condition in PresShell ctor. registeredReporter
Comment 11 Boris Zbarsky [:bz] 2011-05-20 15:37:01 PDT
Comment on attachment 534135 [details] [diff] [review]
Version with cacheBool var v2

r=me
Comment 12 Daniel Holbert [:dholbert] 2011-05-22 09:58:41 PDT
http://hg.mozilla.org/projects/cedar/rev/dfeab223be6d
Comment 13 Daniel Holbert [:dholbert] 2011-05-22 13:31:14 PDT
The push from comment 12 appears to have caused orange in mochitest-chrome on all platforms:
> layout/style/test/chrome/test_hover.html | Test timed out.
Sample log:
http://tinderbox.mozilla.org/showlog.cgi?log=Cedar/1306088180.1306089707.19816.gz
http://tbpl.mozilla.org/?tree=Cedar&rev=dfeab223be6d

Backed out:
  http://hg.mozilla.org/projects/cedar/rev/dfeab223be6d
Comment 14 Daniel Holbert [:dholbert] 2011-05-22 13:32:16 PDT
(In reply to comment #13)
> Backed out:
>   http://hg.mozilla.org/projects/cedar/rev/dfeab223be6d

er sorry, that's the original push. backout push was:
http://hg.mozilla.org/projects/cedar/rev/8176003df609
Comment 15 Timothy Nikkel (:tnikkel) 2011-05-22 16:01:35 PDT
If the pref isn't specified does it maybe default to returning false?
Comment 16 Boris Zbarsky [:bz] 2011-05-22 20:06:25 PDT
Er, yes.  There's an optional third argument (I hate optional arguments) for what the default value should be, defaulting to false.  I should have caught that... :(
Comment 17 Oleg Romashin (:romaxa) 2011-05-23 07:39:36 PDT
Created attachment 534436 [details] [diff] [review]
Added default value

Hm, I saw in other implementations static variable initially initialized with some default value, and I though that  AddBoolPrefVarCache will take value pointer and check it's value and set it as default (we had this before somewhere in GTK API)..

But ok, here is one more default value passed
Comment 18 Dão Gottwald [:dao] 2011-05-25 02:53:32 PDT
http://hg.mozilla.org/mozilla-central/rev/27ff48b7cf61

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