Closed Bug 970125 Opened 6 years ago Closed 3 years ago

Get rid of BrowserElementPanning.js

Categories

(Core :: DOM: Content Processes, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: fabrice, Assigned: kats)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
kats, is that something you could do?
Flags: needinfo?(bugmail.mozilla)
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?
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.)
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.
Flags: needinfo?(bugmail.mozilla)
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?
blocking-b2g: --- → 1.4?
Flags: needinfo?(bugmail.mozilla)
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
Flags: needinfo?(bugmail.mozilla)
Ok, I will tackle that over in bug 976605.
blocking-b2g: 1.4? → ---
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++.
Depends on: 995394
Depends on: 1131358
Depends on: 1131359
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.
Assignee: nobody → bugmail
Component: General → DOM: Content Processes
Product: Firefox OS → Core
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!
Attachment #8798432 - Flags: review?(kchen) → review+
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
https://hg.mozilla.org/mozilla-central/rev/66764f6f8a7c
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
OS: Gonk (Firefox OS) → All
No longer blocks: 1369194
You need to log in before you can comment on or make changes to this bug.