[:fabrice] Fabrice Desré Reporter
	Description • 11 years ago

That's a performance hit we should not have to take. Let's move the support for :active and double clicks to APZC and make it so that the system app can also use apzc.

	[:fabrice] Fabrice Desré Reporter
	Comment 1 • 11 years ago

kats, is that something you could do?

	Doug Sherk (:drs) (inactive)
	Comment 2 • 11 years ago

Do you think the perf gains on this are good enough that we should land it on 1.3 if I take it and can finish it on time?

	[:fabrice] Fabrice Desré Reporter
	Comment 3 • 11 years ago

No, there is no way this will make 1.3
It may make it to 1.3t if it's worth it (ie not too risky, clear improvements, etc.)

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 4 • 11 years ago

It's something I can do, but I'm still working my way through 1.3 blockers and have other things lined up afterwards. We can discuss how to prioritize it based on the expected performance gain.

Note also that the code in BEP.js needs to be run on the main thread of the child process to access DOM/layout properties, so it would really just move into C++ code but still be called from TabChild.cpp. I guess we could put it in APZCCallbackHelper or something to be more reusable across platforms.

	Ben Kelly [:bkelly, not reviewing]
	Comment 5 • 11 years ago

So I saw this contributing to checkerboarding over in bug 975831.  I think it might be worth fixing for 1.4 if possible.

Kats, do you think this is something I could if you point me in the right direction?  Or do I need to have deep down knowledge of the gfx subsystem?

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 6 • 11 years ago

So one thing to make clear is that until we enable APZ in the root process (bug 950934) we can't actually get *rid* of BrowserElementPanning.js.

However, in processes that already have APZ enabled, BEP.js only does two useful things, as far as I know: it sets the active flag on elements that get a touchstart (and clears it when appropriate), and it handles double-tap events.

If we can move the active element stuff into TabChild or APZCCallbackHelper, we should be able to stop listening for the touch events in BEP.js in APZ-enabled processes. That should be sufficient to alleviate the perf problems that are contributing to checkerboarding.

I would suggest filing a separate bug for that issue and making it block this one, as it is a smaller and simpler piece of work. It shouldn't require too much knowledge of the gfx subsystem; it's mostly just porting code from JS to C++. The code in BEP.js that is relevant here is [1] and all the codepaths that call it.

Note also that Vivien recently landed bug 972081 which moved some of the code from BEP.js to TabChild so he can probably provide some guidance on this as well.

[1] http://mxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementPanning.js?rev=e75276f3bcc0#465

	Ben Kelly [:bkelly, not reviewing]
	Comment 7 • 11 years ago

Ok, I will tackle that over in bug 976605.

	Botond Ballo [:botond]
	Comment 8 • 10 years ago

In bug 995394, we are moving the parts of BEP.js that are only used when APZ is disabled to a separate file.

The remaining parts of BEP.js serve two functions:

  1) Resetting the hover state when the observer events "BEC:ShownModalPrompt",
     "Activity:Success", "Activity:Error", or a "visibilitychange" event is
     fired.

  2) Handling the "Gesture:DoubleTap" message.

I'll file bugs for porting these functions to C++.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 9 • 8 years ago

We can delete this code now. All the relevant code from BEP.js has been migrated to C++ or is not necessary. See the last few comments in bug 1131358.

	Kan-Ru Chen [:kanru] (UTC+9)
	Comment 11 • 8 years ago
mozreview-review

Comment on attachment 8798432 [details]
Bug 970125 - Rip out the BrowserElementPanning JS code that's not used any more.

https://reviewboard.mozilla.org/r/83936/#review82694

Thanks!

	Pulsebot
	Comment 12 • 8 years ago

Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/66764f6f8a7c
Rip out the BrowserElementPanning JS code that's not used any more. r=kanru

	Phil Ringnalda (:philor)
	Comment 13 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/66764f6f8a7c