Closed Bug 906877 Opened 8 years ago Closed 8 years ago
Remove observer messages in Browser
Element Panning .js that have been obsoleted by multi-apzc
Now that subframes are async-scrollable, some messages sent from BrowserElementPanning.js to APZC, such as detect-scrollable-subframe, are no longer used. These messages, and the corresponding APZC methods, can be removed.
Actually we can get rid of cancel-default-pan-zoom as well. I forgot but was recently reminded by mbrubeck that the code path to cancel pan/zoom via touch events is at http://mxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/AsyncPanZoomController.cpp#1297 (The ContentReceivedTouch codepath).
I believe this removes all the things that are unnecessary with APZC now handling subframe scrolling as well. Metro is planning on using BrowserElementPanning.js for scrolling subframes (I think) but it will be mutually exclusive with APZC, so it shouldn't affect removal of this code. Jim, can you check that applying this patch doesn't break the intended behaviour of bug 901607? I'm not sure I know exactly how to test that.
Comment on attachment 794230 [details] [diff] [review] Patch Works great, will post an update to my work in the other bug.
Attachment #794230 - Flags: feedback?(jmathies) → feedback+
Comment on attachment 794230 [details] [diff] [review] Patch Requesting review from Vivien for the BrowserElementPanning.js bits (and anything else you care to look at) and BenWa for the rest. It's mostly code deletion.
Comment on attachment 794230 [details] [diff] [review] Patch Less code, yeah! As a nit I would remove the _asyncPanZoomForViewportFrame and replace it directly by a call to docshell.asyncPanZoomEnabled since it is use in only one place.
Attachment #794230 - Flags: review?(21) → review+
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Backed out for causing Marionette failures that I didn't notice until after the merge. https://hg.mozilla.org/mozilla-central/rev/6a2fa1dcddf0 https://tbpl.mozilla.org/php/getParsedLog.php?id=27339664&tree=B2g-Inbound
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla26 → ---
8 years ago
Component: Graphics: Layers → Panning and Zooming
8 years ago
I finally got around to debugging this, turns out my patch to BrowserElementPanning.js removed a non-dead line of code (the return value was dead but the function call had side-effects). Putting that back fixed it for me locally. Carrying r+ on the new patch, pushed to try; will land it once the tree reopens.
Try was green at https://tbpl.mozilla.org/?tree=Try&rev=cb39e3c2d164, so pushed to b2g-inbound: https://hg.mozilla.org/integration/b2g-inbound/rev/b1941b03d0a6
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
I don't understand why you removed mDisableNextTouchBatch.
(In reply to Jim Mathies [:jimm] from comment #13) > I don't understand why you removed mDisableNextTouchBatch. I've brought this back and improved the tracking a bit. Will post updated patches in bug 918273.
(In reply to Jim Mathies [:jimm] from comment #13) > I don't understand why you removed mDisableNextTouchBatch. Well it was only ever set to true in a function that was also being removed, so I ripped out all the dependent code too. But yeah, you might need to bring that back as well.
You need to log in before you can comment on or make changes to this bug.