1572508 - Convert the prefs in PositionedEventTargeting.cpp

Status: RESOLVED FIXED

(Core :: Preferences: Backend, task)

Description

[Kris Wright :KrisWright]

There are a bunch of prefs in PositionedEventTargeting.cpp, stored as EventRadiusPrefs[1], that could be converted to static prefs or code constants. These are a bit tricky because they are dynamically generated or selected in a call to GetPrefsFor [2]. Converting these to static prefs will require reworking how they are used a bit - instead of dynamically selecting the prefs as is, the code could use a boolean flag to select which pref from the struct to use.

Another option is to eschew using prefs entirely, and either use the general callback mechanism or just use code constants to set the same values.

[1] https://searchfox.org/mozilla-central/rev/9ae20497229225cb3fa729a787791f424ff8087b/layout/base/PositionedEventTargeting.cpp#79
[2] https://searchfox.org/mozilla-central/rev/9ae20497229225cb3fa729a787791f424ff8087b/layout/base/PositionedEventTargeting.cpp#95

Comment 1

[Nicholas Nethercote [inactive]]

I thought of another option: convert all the relevant prefs to static prefs, and then change EventRadiusPrefs so it holds a bunch of function pointers. sMouseEventRadiusPrefs would hold function pointers to the getters for the mouse prefs, and sTouchEventRadiusPrefs would hold function pointers to the getters for the touch prefs. Both of these structs would be able to be constructed statically.

I still would prefer to replace the prefs in favour of code constants, but if that's not possible, I think the above is the next best option, because it wouldn't require major logic changes that the boolean flags approach would require.

Comment 2

[Kris Wright :KrisWright]

Attachment: Bug 1572508 - Convert the VarCache prefs in PositionedEventTargeting to callbacks

These prefs should eventually be converted to static prefs or just use code constants, but rather than gut the tests that use these pref values I decided to go for the least invasive route and set callbacks for the two structs.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: Bug 1572508 - Convert `ui.touch.*` and `ui.mouse.*` VarCache prefs to static prefs. r=KrisWright,kats.

This is a weird conversion because the existing VarCache mirror prefs are in a
two static structs, one for touch prefs and one for the equivalent mouse prefs,
and we dynamically select the appropriate struct in GetPrefsFor().

(But note that ui.mouse.radius.reposition and
ui.mouse.radius.inputSource.touchonly do not have corresponding touch
prefs! For touch events they are always considered to be false.)

The commit make the following changes.

• It makes the prefs into static prefs.
• It changes GetPrefsFor() so it copies the values of the static prefs into a
  temporary struct, which is then read from.
• It renames some fields in EventRadiusPrefs to make them more closely match
  the pref names.

Comment 4

[Nicholas Nethercote [inactive]]

Kris: here's my attempt, which is more invasive but avoids callbacks. I ended up just making copies of the static pref values instead of using function pointers like I suggested in comment 1.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f719a8ab74c6
Convert `ui.touch.*` and `ui.mouse.*` VarCache prefs to static prefs. r=KrisWright,kats,geckoview-reviewers,agi.

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/f719a8ab74c6