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)

Product:
Core
Component:
Panning and Zooming
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla64
Tracking Flags:
Tracking Status
firefox63 + verified
firefox64 --- verified

People

(Reporter: botond, Assigned: botond)

References

Details

Attachments

(2 files)

Bug 1495055 - Accept layout viewport updates from the main thread right away on Android. r?kats
(deleted), text/x-phabricator-request
kats
: review+
pascalc
: approval-mozilla-beta+
pascalc
: approval-mozilla-release+
Details
Bug 1495055 - Adjust the composited layout viewport in AdjustScrollForSurfaceShift(). r?kats
(deleted), text/x-phabricator-request
kats
: review+
Details 
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
status-firefox63: --- → affected
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.
Depends on D7368
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
https://hg.mozilla.org/mozilla-central/rev/ed9b268816b4
https://hg.mozilla.org/mozilla-central/rev/fd895bb95b99
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox64: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
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
status-firefox64: fixed → ---
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.
tracking-firefox63: --- → +
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
status-firefox64: --- → affected
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
https://hg.mozilla.org/mozilla-central/rev/604682f515cc
https://hg.mozilla.org/mozilla-central/rev/3445b06f9ae9
Status: REOPENED → RESOLVED
Closed: 6 years ago → 6 years ago
status-firefox64: affected → fixed
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
status-firefox63: fixed → affected
status-firefox64: fixed → affected
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 ago → 6 years ago
Flags: needinfo?(botond)
Resolution: --- → FIXED
See Also: → https://bugzilla.mozilla.org/show_bug.cgi?id=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
status-firefox63: fixed → verified
status-firefox64: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: