Closed
Bug 970125
Opened 11 years ago
Closed 8 years ago
Get rid of BrowserElementPanning.js
Categories
(Core :: DOM: Content Processes, defect)
Core
DOM: Content Processes
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.
Reporter | ||
Comment 1•11 years ago
|
||
kats, is that something you could do?
Flags: needinfo?(bugmail.mozilla)
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?
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.)
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.
Flags: needinfo?(bugmail.mozilla)
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?
blocking-b2g: --- → 1.4?
Flags: needinfo?(bugmail.mozilla)
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
Flags: needinfo?(bugmail.mozilla)
Comment 7•11 years ago
|
||
Ok, I will tackle that over in bug 976605.
Assignee | ||
Updated•11 years ago
|
blocking-b2g: 1.4? → ---
Updated•11 years ago
|
Depends on: parent-process-apz
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++.
Depends on: 995394
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.
Assignee: nobody → bugmail
Component: General → DOM: Content Processes
Product: Firefox OS → Core
Comment hidden (mozreview-request) |
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!
Attachment #8798432 -
Flags: review?(kchen) → review+
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
Comment 13•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Blocks: 1308955
OS: Gonk (Firefox OS) → All
You need to log in
before you can comment on or make changes to this bug.
Description
•