Closed Bug 1495055 Opened 6 years ago Closed 6 years ago

"Aa" button in reader mode jumps slightly when dynamic toolbar appears/disappears

Categories

(Core :: Panning and Zooming, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla64
Tracking Status
firefox63 + verified
firefox64 --- verified

People

(Reporter: botond, Assigned: botond)

References

Details

Attachments

(2 files)

Quoting from bug 1465616 comment 180: ==== STR: 1. Add a page to the reading list; 2. Scroll up and down the content of the page; 3. Pay attention to 'Aa' option. Expected result: Scroll down - "Aa" option not displayed. Scroll up - "Aa" option displayed and stays in a fixed position Actual result: When the "Aa" option is displayed it scrolls up with the page, sometimes jumps up and down before remaining in a fixed position. ==== See also the video from that comment and bug 1465616 comment 182.
The jump seems to happen to any fixed element when the dynamic toolbar transitions from shown to hidden or vice versa.
Assignee: nobody → botond
I think the issue here is that when the transition of the dynamic toolbar causes the RCD-RSF's visual viewport height to change, the corresponding update to the height of the layout viewport doesn't happen until a couple of frames later, due to this code [1]. As a result, we get a couple of rendered frames where fixed elements are attached to a layout viewport whose dimensions are out of date. [1] https://searchfox.org/mozilla-central/rev/819cd31a93fd50b7167979607371878c4d6f18e8/gfx/layers/apz/src/AsyncPanZoomController.cpp#4221
(In reply to Botond Ballo [:botond] from comment #2) > I think the issue here is that when the transition of the dynamic toolbar > causes the RCD-RSF's visual viewport height to change, the corresponding > update to the height of the layout viewport doesn't happen until a couple of > frames later, due to this code [1]. > > As a result, we get a couple of rendered frames where fixed elements are > attached to a layout viewport whose dimensions are out of date. I don't have any better ideas for how to fix this besides just removing this delay in updating the layout viewport. It was introduced ~5 years ago in the B2G days and it may well simply not be necessary any more. There is also a second issue, only related to the APZ frame delay, where basically the fix applied in bug 1400440 needs to be applie to the layout viewport offset as well. These two changes fix the issue for me locally. Let's see how they fare on Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0ab306ef4ddaba1bdc7fa01a8a74880c9d8b871d
Previously we would wait at least one transaction from the time the composition bounds (visual viewport) was updated, but this causes problems due to the visual and layout viewport being out of sync after a screen resize, such as one due to dynamic toolbar transitions in Fennec.
Comment on attachment 9013495 [details] Bug 1495055 - Accept layout viewport updates from the main thread right away on Android. r?kats Kartikaya Gupta (email:kats@mozilla.com) has approved the revision.
Attachment #9013495 - Flags: review+
Comment on attachment 9013496 [details] Bug 1495055 - Adjust the composited layout viewport in AdjustScrollForSurfaceShift(). r?kats Kartikaya Gupta (email:kats@mozilla.com) has approved the revision.
Attachment #9013496 - Flags: review+
Pushed by bballo@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ed9b268816b4 Accept layout viewport updates from the main thread right away. r=kats https://hg.mozilla.org/integration/autoland/rev/fd895bb95b99 Adjust the composited layout viewport in AdjustScrollForSurfaceShift(). r=kats
Depends on: 1496369
Backed out for frequently asserting on FrameLayerBuilder.cpp in crashtest on OSX. Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=running,pending,success,testfailed,busted,exception,runnable&classifiedState=unclassified&searchStr=e10s,test-macosx64%2Fdebug-crashtest-e10s,r-e10s(c)&selectedJob=203407646&revision=fd895bb95b99a0f0b1d8995279141773aa741703 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=203577944&repo=autoland&lineNumber=51638 Backout link: https://hg.mozilla.org/mozilla-central/rev/9f5ea3c3f1160fc88c53bd4d74ccc3b137c71896 23:11:55 INFO - REFTEST PROCESS-CRASH | file:///Users/cltbld/tasks/task_1538719176/build/tests/reftest/tests/layout/base/crashtests/1458121.html (finished) | application crashed [@ mozilla::ContainerState::ProcessDisplayItems(nsDisplayList*)] 23:11:55 INFO - Crash dump filename: /var/folders/np/1nwrkgjx0d78r3j8gzrktlj400000w/T/tmpeyIGu5.mozrunner/minidumps/64B43240-C067-45D8-8A34-87753362588C.dmp 23:11:55 INFO - Operating system: Mac OS X 23:11:55 INFO - 10.10.5 14F27 23:11:55 INFO - CPU: amd64 23:11:55 INFO - family 6 model 69 stepping 1 23:11:55 INFO - 4 CPUs 23:11:55 INFO - 23:11:55 INFO - GPU: UNKNOWN 23:11:55 INFO - 23:11:55 INFO - Crash reason: EXC_BAD_ACCESS / KERN_INVALID_ADDRESS 23:11:55 INFO - Crash address: 0x0 23:11:55 INFO - Process uptime: 422 seconds 23:11:55 INFO - 23:11:55 INFO - Thread 0 (crashed) 23:11:55 INFO - 0 XUL!mozilla::ContainerState::ProcessDisplayItems(nsDisplayList*) [FrameLayerBuilder.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 5208 + 0x0] 23:11:55 INFO - rax = 0x0000000000000000 rdx = 0x00007fff7a9e91f8 23:11:55 INFO - rcx = 0x0000000000000000 rbx = 0x00007fff5ab7f068 23:11:55 INFO - rsi = 0x00edcf0000edcf00 rdi = 0x00edce0000edcf03 23:11:55 INFO - rbp = 0x00007fff5ab7ee90 rsp = 0x00007fff5ab7ea70 23:11:55 INFO - r8 = 0x00007fff5ab7ea20 r9 = 0x00007fff7aa94300 23:11:55 INFO - r10 = 0x00007fff9387f5d2 r11 = 0x00007fff9387f421 23:11:55 INFO - r12 = 0x000000016611a400 r13 = 0x000000010b9825b4 23:11:55 INFO - r14 = 0x00007fff5ab7f068 r15 = 0x0000000167bf3c20 23:11:55 INFO - rip = 0x0000000108f07572 23:11:55 INFO - Found by: given as instruction pointer in context 23:11:55 INFO - 1 XUL!mozilla::FrameLayerBuilder::BuildContainerLayerFor(nsDisplayListBuilder*, mozilla::layers::LayerManager*, nsIFrame*, nsDisplayItem*, nsDisplayList*, mozilla::ContainerLayerParameters const&, mozilla::gfx::Matrix4x4Typed<mozilla::gfx::UnknownUnits, mozilla::gfx::UnknownUnits> const*, unsigned int) [FrameLayerBuilder.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 6707 + 0xf] 23:11:55 INFO - rbp = 0x00007fff5ab7f900 rsp = 0x00007fff5ab7eea0 23:11:55 INFO - rip = 0x0000000108f0f41e 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 2 XUL!nsDisplayFilters::BuildLayer(nsDisplayListBuilder*, mozilla::layers::LayerManager*, mozilla::ContainerLayerParameters const&) [nsDisplayList.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 10109 + 0x12] 23:11:55 INFO - rbp = 0x00007fff5ab7f9b0 rsp = 0x00007fff5ab7f910 23:11:55 INFO - rip = 0x0000000108f6a7b3 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 3 XUL!mozilla::FrameLayerBuilder::AddPaintedDisplayItem(mozilla::PaintedLayerData*, mozilla::AssignedDisplayItem&, mozilla::ContainerState&, mozilla::layers::Layer*) [FrameLayerBuilder.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 5730 + 0x2] 23:11:55 INFO - rbp = 0x00007fff5ab7fbe0 rsp = 0x00007fff5ab7f9c0 23:11:55 INFO - rip = 0x0000000108f09fce 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 4 XUL!mozilla::PaintedLayerDataNode::PopAllPaintedLayerData() [FrameLayerBuilder.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 3770 + 0xb] 23:11:55 INFO - rbp = 0x00007fff5ab7fdf0 rsp = 0x00007fff5ab7fbf0 23:11:55 INFO - rip = 0x0000000108ef9ca2 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 5 XUL!mozilla::PaintedLayerDataNode::Finish(bool) [FrameLayerBuilder.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 3343 + 0x8] 23:11:55 INFO - rbp = 0x00007fff5ab7fe20 rsp = 0x00007fff5ab7fe00 23:11:55 INFO - rip = 0x0000000108ef8e1f 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 6 XUL!mozilla::PaintedLayerDataNode::FinishAllChildren(bool) [FrameLayerBuilder.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 3332 + 0x8] 23:11:55 INFO - rbp = 0x00007fff5ab7fe60 rsp = 0x00007fff5ab7fe30 23:11:55 INFO - rip = 0x0000000108ef8ef5 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 7 XUL!mozilla::PaintedLayerDataTree::Finish() [FrameLayerBuilder.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 3341 + 0xa] 23:11:55 INFO - rbp = 0x00007fff5ab7fe90 rsp = 0x00007fff5ab7fe70 23:11:55 INFO - rip = 0x0000000108efba84 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 8 XUL!mozilla::ContainerState::Finish(unsigned int*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, nsDisplayList*) [FrameLayerBuilder.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 6306 + 0x5] 23:11:55 INFO - rbp = 0x00007fff5ab7ff10 rsp = 0x00007fff5ab7fea0 23:11:55 INFO - rip = 0x0000000108f0dc99 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 9 XUL!mozilla::FrameLayerBuilder::BuildContainerLayerFor(nsDisplayListBuilder*, mozilla::layers::LayerManager*, nsIFrame*, nsDisplayItem*, nsDisplayList*, mozilla::ContainerLayerParameters const&, mozilla::gfx::Matrix4x4Typed<mozilla::gfx::UnknownUnits, mozilla::gfx::UnknownUnits> const*, unsigned int) [FrameLayerBuilder.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 6714 + 0xb] 23:11:55 INFO - rbp = 0x00007fff5ab80980 rsp = 0x00007fff5ab7ff20 23:11:55 INFO - rip = 0x0000000108f0f4ff 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 10 XUL!nsDisplayList::BuildLayers(nsDisplayListBuilder*, mozilla::layers::LayerManager*, unsigned int, bool) [nsDisplayList.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 2583 + 0x22] 23:11:55 INFO - rbp = 0x00007fff5ab80d10 rsp = 0x00007fff5ab80990 23:11:55 INFO - rip = 0x0000000108f436ff 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 11 XUL!nsDisplayList::PaintRoot(nsDisplayListBuilder*, gfxContext*, unsigned int) [nsDisplayList.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 2808 + 0x14] 23:11:55 INFO - rbp = 0x00007fff5ab80e60 rsp = 0x00007fff5ab80d20 23:11:55 INFO - rip = 0x0000000108f44acf 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 12 XUL!nsLayoutUtils::PaintFrame(gfxContext*, nsIFrame*, nsRegion const&, unsigned int, nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags) [nsLayoutUtils.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 3834 + 0x15] 23:11:55 INFO - rbp = 0x00007fff5ab838d0 rsp = 0x00007fff5ab80e70 23:11:55 INFO - rip = 0x0000000108be9f29 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 13 XUL!mozilla::PresShell::Paint(nsView*, nsRegion const&, unsigned int) [PresShell.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 6350 + 0x17] 23:11:55 INFO - rbp = 0x00007fff5ab83a70 rsp = 0x00007fff5ab838e0 23:11:55 INFO - rip = 0x0000000108b756b7 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 14 XUL!nsViewManager::ProcessPendingUpdatesPaint(nsIWidget*) [nsViewManager.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 480 + 0x16] 23:11:55 INFO - rbp = 0x00007fff5ab83ad0 rsp = 0x00007fff5ab83a80 23:11:55 INFO - rip = 0x00000001088917e0 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 15 XUL!nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) [nsViewManager.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 412 + 0x8] 23:11:55 INFO - rbp = 0x00007fff5ab83b40 rsp = 0x00007fff5ab83ae0 23:11:55 INFO - rip = 0x000000010889130e 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 16 XUL!nsViewManager::ProcessPendingUpdates() [nsViewManager.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 1102 + 0xd] 23:11:55 INFO - rbp = 0x00007fff5ab83b60 rsp = 0x00007fff5ab83b50 23:11:55 INFO - rip = 0x0000000108892ccc 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 17 XUL!nsRefreshDriver::Tick(mozilla::TimeStamp) [nsRefreshDriver.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 2046 + 0x8] 23:11:55 INFO - rbp = 0x00007fff5ab83e40 rsp = 0x00007fff5ab83b70 23:11:55 INFO - rip = 0x0000000108b42066 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 18 XUL!mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) [nsRefreshDriver.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 325 + 0x8] 23:11:55 INFO - rbp = 0x00007fff5ab83e70 rsp = 0x00007fff5ab83e50 23:11:55 INFO - rip = 0x0000000108b47663 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 19 XUL!mozilla::RefreshDriverTimer::Tick(mozilla::TimeStamp) [nsRefreshDriver.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 318 + 0xb] 23:11:55 INFO - rbp = 0x00007fff5ab83e90 rsp = 0x00007fff5ab83e80 23:11:55 INFO - rip = 0x0000000108b47592 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 20 XUL!mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) [nsRefreshDriver.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 756 + 0xb] 23:11:55 INFO - rbp = 0x00007fff5ab83ec0 rsp = 0x00007fff5ab83ea0 23:11:55 INFO - rip = 0x0000000108b48a5a 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 21 XUL!mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) [nsRefreshDriver.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 572 + 0xb] 23:11:55 INFO - rbp = 0x00007fff5ab83f10 rsp = 0x00007fff5ab83ed0 23:11:55 INFO - rip = 0x0000000108b48628 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 22 XUL!mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) [VsyncChild.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 78 + 0x2] 23:11:55 INFO - rbp = 0x00007fff5ab83f30 rsp = 0x00007fff5ab83f20 23:11:55 INFO - rip = 0x0000000108eb08fb 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 23 XUL!mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) [PVsyncChild.cpp: : 167 + 0xd] 23:11:55 INFO - rbp = 0x00007fff5ab83fc0 rsp = 0x00007fff5ab83f40 23:11:55 INFO - rip = 0x0000000105db47a0 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 24 XUL!mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) [PBackgroundChild.cpp: : 2280 + 0xc] 23:11:55 INFO - rbp = 0x00007fff5ab84110 rsp = 0x00007fff5ab83fd0 23:11:55 INFO - rip = 0x0000000105d087d1 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 25 XUL!mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) [MessageChannel.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 2248 + 0x9] 23:11:55 INFO - rbp = 0x00007fff5ab84170 rsp = 0x00007fff5ab84120 23:11:55 INFO - rip = 0x0000000105aca7eb 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 26 XUL!mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) [MessageChannel.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 2175 + 0xb] 23:11:55 INFO - rbp = 0x00007fff5ab84220 rsp = 0x00007fff5ab84180 23:11:55 INFO - rip = 0x0000000105ac8c52 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 27 XUL!mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) [MessageChannel.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 2012 + 0xb] 23:11:55 INFO - rbp = 0x00007fff5ab84280 rsp = 0x00007fff5ab84230 23:11:55 INFO - rip = 0x0000000105ac969d 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 28 XUL!mozilla::ipc::MessageChannel::MessageTask::Run() [MessageChannel.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 2045 + 0x8] 23:11:55 INFO - rbp = 0x00007fff5ab842a0 rsp = 0x00007fff5ab84290 23:11:55 INFO - rip = 0x0000000105ac9caa 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 29 XUL!nsThread::ProcessNextEvent(bool, bool*) [nsThread.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 1231 + 0x6] 23:11:55 INFO - rbp = 0x00007fff5ab84800 rsp = 0x00007fff5ab842b0 23:11:55 INFO - rip = 0x00000001054745ff 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 30 XUL!NS_ProcessNextEvent(nsIThread*, bool) [nsThreadUtils.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 530 + 0xd] 23:11:55 INFO - rbp = 0x00007fff5ab84820 rsp = 0x00007fff5ab84810 23:11:55 INFO - rip = 0x0000000105478367 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 31 XUL!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) [MessagePump.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 97 + 0xa] 23:11:55 INFO - rbp = 0x00007fff5ab84870 rsp = 0x00007fff5ab84830 23:11:55 INFO - rip = 0x0000000105ace00e 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 32 XUL!MessageLoop::Run() [message_loop.cc:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 318 + 0x5] 23:11:55 INFO - rbp = 0x00007fff5ab848a0 rsp = 0x00007fff5ab84880 23:11:55 INFO - rip = 0x0000000105a7ca57 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 33 XUL!nsBaseAppShell::Run() [nsBaseAppShell.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 158 + 0x8] 23:11:55 INFO - rbp = 0x00007fff5ab848c0 rsp = 0x00007fff5ab848b0 23:11:55 INFO - rip = 0x00000001088cd309 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 34 XUL!nsAppShell::Run() [nsAppShell.mm:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 742 + 0x8] 23:11:55 INFO - rbp = 0x00007fff5ab84900 rsp = 0x00007fff5ab848d0 23:11:55 INFO - rip = 0x000000010893facf 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 35 XUL!XRE_RunAppShell() [nsEmbedFunctions.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 939 + 0x6] 23:11:55 INFO - rbp = 0x00007fff5ab84950 rsp = 0x00007fff5ab84910 23:11:55 INFO - rip = 0x000000010a0819e3 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 36 XUL!mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) [MessagePump.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 269 + 0x5] 23:11:55 INFO - rbp = 0x00007fff5ab84980 rsp = 0x00007fff5ab84960 23:11:55 INFO - rip = 0x0000000105ace852 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 37 XUL!MessageLoop::Run() [message_loop.cc:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 318 + 0x5] 23:11:55 INFO - rbp = 0x00007fff5ab849b0 rsp = 0x00007fff5ab84990 23:11:55 INFO - rip = 0x0000000105a7ca57 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 38 XUL!XRE_InitChildProcess(int, char**, XREChildData const*) [nsEmbedFunctions.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 765 + 0x8] 23:11:55 INFO - rbp = 0x00007fff5ab84c70 rsp = 0x00007fff5ab849c0 23:11:55 INFO - rip = 0x000000010a08150e 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 39 plugin-container!main [plugin-container.cpp:1bcc72890955ece4a5b54ee17fdffcfb4ed1401d : 50 + 0x13] 23:11:55 INFO - rbp = 0x00007fff5ab84cb0 rsp = 0x00007fff5ab84c80 23:11:55 INFO - rip = 0x000000010507af39 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 40 libdyld.dylib + 0x35c9 23:11:55 INFO - rbp = 0x00007fff5ab84cc8 rsp = 0x00007fff5ab84cc0 23:11:55 INFO - rip = 0x00007fff9387f5c9 23:11:55 INFO - Found by: previous frame's frame pointer 23:11:55 INFO - 41 libdyld.dylib + 0x35c9 23:11:55 INFO - rbp = 0x00007fff5ab84cc8 rsp = 0x00007fff5ab84cc8 23:11:55 INFO - rip = 0x00007fff9387f5c9 23:11:55 INFO - Found by: stack scanning
Status: RESOLVED → REOPENED
Flags: needinfo?(botond)
QA Contact: kats
Resolution: FIXED → ---
Target Milestone: mozilla64 → ---
QA Contact: kats
Tracking for 63 as this is the likely cause of bug 1493742 which affects top sites.
I was able to reproduce this issue by performing different steps. And the pushlog is the same as in bug https://bugzilla.mozilla.org/show_bug.cgi?id=1465616#c182. Environment: Device: Motorola Nexus 6(Android 7.1.1), Xiaomi Mi4i(Android 5.0.2); Build: Nightly 64.0a1 (2018-10-10), 63.0b11; Steps to reproduce: 1. Launch Fennec and go to espn.com, cnn.com. newyorker.com; 2. Change device orientation from portrait to landscape a couple of times; 3. Optional: Don't interact with the message "about cookies". Expected result: No visual issues when orientation is changed. Actual result: Visual issues are displayed when device orientation is changed (header from the site is flickering). Video: https://drive.google.com/open?id=1Q7m3m_rF4O6xZyTi9lIFcrVOjsGq6FGr Pushlog:https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c6e7b65bf8b02a32a6c1d583eb1d79e3116d692d&tochange=bac4139e4ff9b3071e1ce17113ac65ed1d8e8598
I was able to reproduce this issue by performing different steps. And the pushlog is the same as in bug https://bugzilla.mozilla.org/show_bug.cgi?id=1465616#c182. Environment: Device: Motorola Nexus 6(Android 7.1.1), Xiaomi Mi4i(Android 5.0.2); Build: Nightly 64.0a1 (2018-10-10), 63.0b11; Steps to reproduce: 1. Launch Fennec and go to espn.com, cnn.com. newyorker.com; 2. Change device orientation from portrait to landscape a couple of times; 3. Optional: Don't interact with the message "about cookies". Expected result: No visual issues when orientation is changed. Actual result: Visual issues are displayed when device orientation is changed (header from the site is flickering). Video: https://drive.google.com/open?id=1Q7m3m_rF4O6xZyTi9lIFcrVOjsGq6FGr Pushlog:https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c6e7b65bf8b02a32a6c1d583eb1d79e3116d692d&tochange=bac4139e4ff9b3071e1ce17113ac65ed1d8e8598
(In reply to Sorina Florean [:sorina] from comment #12) > I was able to reproduce this issue by performing different steps. If the steps are different, the issue is different. The steps you're describing have nothing in common with comment 0, so please file a new bug for this. And anyway the patch was backed out so it's expected that the issue will still be present.
This patch has caused a number of regressions: (1) An intermittent mochitest failure (bug 1452820) (2) An intermittent Mac-only crashtest failure (bug 1496392) (3) Talos regressions (bug 1496369) (1) and (2) are both pre-existing bugs that were surfaced by this patch, I think because it changed some timings. The talos regressions are likely also related to that. (1) has been fixed; (2) and (3) remain to be investigated. ((2) is difficult for me to investigate because it's Mac-only; if someone with a Mac would like to debug it locally, that would be very helpful.) As this is being tracked for 63, I'd like to pursue a more limited fix that fixes the bug while hopefully avoiding most of the timing-related changes that are having these side effects.
Flags: needinfo?(botond)
I also realized that the original patch doesn't fix the issue fully (and the new patch I'm working on doesn't either): quickly scrolling back and forth so as to cause the dynamic toolbar to disappear and reappear in quick succession, you can still trigger the bug. However, this doesn't happen during normal scrolling, and so the patch still alleviates the bug to a significant degree.
Attachment #9013495 - Attachment description: Bug 1495055 - Accept layout viewport updates from the main thread right away. r?kats → Bug 1495055 - Update the viewport right away when the dynamic toolbar is moving. r?kats
No longer blocks: 1497762
Attachment #9013495 - Attachment description: Bug 1495055 - Update the viewport right away when the dynamic toolbar is moving. r?kats → Bug 1495055 - Accept layout viewport updates from the main thread right away on Android. r?kats
Pushed by bballo@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/604682f515cc Accept layout viewport updates from the main thread right away on Android. r=kats https://hg.mozilla.org/integration/autoland/rev/3445b06f9ae9 Adjust the composited layout viewport in AdjustScrollForSurfaceShift(). r=kats
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Boton, can you request an uplift to beta today?
Flags: needinfo?(botond)
Comment on attachment 9013495 [details] Bug 1495055 - Accept layout viewport updates from the main thread right away on Android. r?kats Note: uplift request applies to both patches. [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: Bug 1465616 User impact if declined: position:fixed and sticky elements can flicker / temporarily be incorrectly positioned in Firefox for Android after dynamic toolbar transitions and screen orientation changes. In addition to the STR in this bug, this fix is expected to resolve bug 1493742, bug 1497762, and bug 1497884. Is this code covered by automated tests?: No Has the fix been verified in Nightly?: Yes Needs manual test from QE?: No If yes, steps to reproduce: List of other uplifts needed: None Risk to taking this patch: Medium Why is the change risky/not risky? (and alternatives if risky): I would rank this as a moderate to high risk uplift at this stage. It's hard to be confident about all the implications of this patch and rule out unexpected side effects. On the plus side, the effects are limited to Android, and it does fix rendering issues that affect top sites. There is an alternative fix [1] which is more limited in scope, in that it only affects dynamic toolbar transitions, and not other types of viewport size changes. However, it's not clear that it's any less risky, because it's more complex and complexity adds risk. It also doesn't fix bug 1497884. [1] https://phabricator.services.mozilla.com/D7368?id=23171 String changes made/needed:
Flags: needinfo?(botond)
Attachment #9013495 - Flags: approval-mozilla-beta?
Comment on attachment 9013495 [details] Bug 1495055 - Accept layout viewport updates from the main thread right away on Android. r?kats I am taking this patch in our last fennec beta although it is evaluated to a medium risk patch as it fixes rendering issues on several top sites, is a small patch, only affects Android, has decent code coverage and we haven't had regressions reported on Nightly since it landed 3 days ago.
Attachment #9013495 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 9013495 [details] Bug 1495055 - Accept layout viewport updates from the main thread right away on Android. r?kats I am resetting the approval request until we have finished the Beta to Release merge and I will approve again for both beta & release branches after the merge so as that we have the patch on the final release. (please disregard the default uplift form below automatically inserted by bugzilla when resetting the flag) [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: None User impact if declined: Is this code covered by automated tests?: Yes Has the fix been verified in Nightly?: Yes Needs manual test from QE?: Yes If yes, steps to reproduce: List of other uplifts needed: None Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): String changes made/needed:
Attachment #9013495 - Flags: approval-mozilla-beta+ → approval-mozilla-beta?
Comment on attachment 9013495 [details] Bug 1495055 - Accept layout viewport updates from the main thread right away on Android. r?kats The Beta to Release merge is done. Uplift approved for 63.0b15 and 63rc1.
Attachment #9013495 - Flags: approval-mozilla-release+
Attachment #9013495 - Flags: approval-mozilla-beta?
Attachment #9013495 - Flags: approval-mozilla-beta+
(In reply to Botond Ballo [:botond] from comment #16) > I also realized that the original patch doesn't fix the issue fully (and the > new patch I'm working on doesn't either): quickly scrolling back and forth > so as to cause the dynamic toolbar to disappear and reappear in quick > succession, you can still trigger the bug. However, this doesn't happen > during normal scrolling, and so the patch still alleviates the bug to a > significant degree. Will use bug 1493742 to track this remaining issue.
Devices: - Google Pixel (Android 9) - OnePlus 5T (Android 8.1.0) - Huawei P9 Lite (Android 6) Hello, While testing the issue described in Comment 0 is still reproducible for both Beta (63.0b15) and Nightly (2018-10-16).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Flags: needinfo?(botond)
Please see comment 16 and comment 26; the remaining issue is tracked in bug 1493742.
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Flags: needinfo?(botond)
Resolution: --- → FIXED
See Also: → 1493742
@Botond how could QA verify this issue since it is not clear to me what has been fixed from the comments, since following the steps provided in Comment 0 will be treated in bug 1493742, are there any clear steps to verify this particular fix?
Flags: needinfo?(botond)
After this fix, the jumping should only happen when scrolling back and forth (i.e. switching directions) in quick succession. Before this fix, it would also occur during more normal scrolling patterns, e.g. with several scroll gestures in the same direction and a pause before switching, or even just reaching the bottom of the page.
Flags: needinfo?(botond)
Confirmed as fixed testing the scenario described in Comment 30. Marking as verified, thank you for the clarification Botond!
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: