925902 - Setting the displayport seems to often fail

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

While investigating bug 919039 I often end up in a state where the displayport is wrong. This is clear if you have the layer borders turned on because you can see the purple and green borders for the layer's "gecko-visible region" and displayport.

I also added some logging and the displayport requested in RequestContentRepaint was fine but the displayport that came back in NotifyLayersUpdated was wrong. I tracked this down to the code in APZCCallbackHelper.cpp which tries to get the element from the scrollId and noticed that the nsLayoutUtils::FindContentFor call was returning null. Not sure why yet.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Ah! The content associated with the ROOT_SCROLL_ID in the nsLayoutUtils content map is getting cleared if the device locks (which happens after a timeout of a few seconds of inactivity). So now I have proper STR:

1. Open the browser and load a page e.g. people.mozilla.com/~nhirata/html_tp/
2. Hit the power button to lock the device
3. Unlock the device
4. Pan/zoom. You will notice that the displayport top-left is always the same as the scroll position, so if you zoom in and pan upwards or to the left, you immediately see whitespace rather than having a buffer of painted content.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

The ROOT_SCROLL_ID element is getting removed from the content map here:

(gdb) bt
#0  DestroyViewID (aObject=0x4462fac0, aPropertyName=0x435285c0, aPropertyValue=0x441440f8, aData=0x0) at /Users/kats/zspace/mozilla-git/layout/base/nsLayoutUtils.cpp:477
#1  0x409c39ec in nsPropertyTable::PropertyList::DeletePropertyFor (this=0x44137b80, aObject=...) at /Users/kats/zspace/mozilla-git/content/base/src/nsPropertyTable.cpp:340
#2  0x409c3a3a in nsPropertyTable::DeleteAllPropertiesFor (this=<value optimized out>, aObject=...) at /Users/kats/zspace/mozilla-git/content/base/src/nsPropertyTable.cpp:86
#3  0x40999d7c in nsIDocument::DeleteAllPropertiesFor (this=0x4413f800, aNode=0x4462fac0) at /Users/kats/zspace/mozilla-git/content/base/src/nsDocument.cpp:1994
#4  0x409bcbce in nsNodeUtils::LastRelease (aNode=0x4462fac0) at /Users/kats/zspace/mozilla-git/content/base/src/nsNodeUtils.cpp:210
#5  0x4096b74c in mozilla::dom::FragmentOrElement::Release (this=0x4462fac0) at /Users/kats/zspace/mozilla-git/content/base/src/FragmentOrElement.cpp:1748
#6  0x40960836 in mozilla::dom::Comment::Release (this=0x4462fac0) at /Users/kats/zspace/mozilla-git/content/base/src/Comment.cpp:24
#7  0x406d36cc in ~nsCOMPtr_base (this=0x43aaf008) at ../../../dist/include/nsCOMPtr.h:430
#8  ~nsCOMPtr (this=0x43aaf008) at ../../../dist/include/nsCOMPtr.h:473
#9  nsTArrayElementTraits<nsCOMPtr<nsIContent> >::Destruct (this=0x43aaf008) at ../../../dist/include/nsTArray.h:534
#10 nsTArray_Impl<nsCOMPtr<nsIContent>, nsTArrayInfallibleAllocator>::DestructRange (this=0x43aaf008) at ../../../dist/include/nsTArray.h:1569
#11 nsTArray_Impl<nsCOMPtr<nsIContent>, nsTArrayInfallibleAllocator>::RemoveElementsAt (this=0x43aaf008) at ../../../dist/include/nsTArray.h:1286
#12 nsTArray_Impl<nsCOMPtr<nsIContent>, nsTArrayInfallibleAllocator>::Clear (this=0x43aaf008) at ../../../dist/include/nsTArray.h:1297
#13 0x4096bc7e in ContentUnbinder::Run (this=0x43aaf000) at /Users/kats/zspace/mozilla-git/content/base/src/FragmentOrElement.cpp:1086
#14 0x4107f96c in nsThread::ProcessNextEvent (this=0x403168e0, mayWait=<value optimized out>, result=0xbea74027) at /Users/kats/zspace/mozilla-git/xpcom/threads/nsThread.cpp:622
#15 0x4105eff8 in NS_ProcessNextEvent (thread=0x43aaf000, mayWait=false) at /Users/kats/zspace/mozilla-git/xpcom/glue/nsThreadUtils.cpp:238
#16 0x40e13238 in mozilla::ipc::MessagePump::Run (this=0x40301b20, aDelegate=0xbea74934) at /Users/kats/zspace/mozilla-git/ipc/glue/MessagePump.cpp:85
#17 0x40e132ea in mozilla::ipc::MessagePumpForChildProcess::Run (this=0x40301b20, aDelegate=0xbea74934) at /Users/kats/zspace/mozilla-git/ipc/glue/MessagePump.cpp:250
#18 0x4109aeb8 in MessageLoop::RunInternal (this=0x1000000) at /Users/kats/zspace/mozilla-git/ipc/chromium/src/base/message_loop.cc:220
#19 0x4109af2e in MessageLoop::RunHandler (this=0xbea74934) at /Users/kats/zspace/mozilla-git/ipc/chromium/src/base/message_loop.cc:213
#20 MessageLoop::Run (this=0xbea74934) at /Users/kats/zspace/mozilla-git/ipc/chromium/src/base/message_loop.cc:187
#21 0x40dc20c0 in nsBaseAppShell::Run (this=0x40358280) at /Users/kats/zspace/mozilla-git/widget/xpwidgets/nsBaseAppShell.cpp:161
#22 0x406b8102 in XRE_RunAppShell () at /Users/kats/zspace/mozilla-git/toolkit/xre/nsEmbedFunctions.cpp:714
#23 0x40e132b8 in mozilla::ipc::MessagePumpForChildProcess::Run (this=0x40301b20, aDelegate=0xbea74934) at /Users/kats/zspace/mozilla-git/ipc/glue/MessagePump.cpp:217
#24 0x4109aeb8 in MessageLoop::RunInternal (this=0x40358280) at /Users/kats/zspace/mozilla-git/ipc/chromium/src/base/message_loop.cc:220
#25 0x4109af2e in MessageLoop::RunHandler (this=0xbea74934) at /Users/kats/zspace/mozilla-git/ipc/chromium/src/base/message_loop.cc:213
#26 MessageLoop::Run (this=0xbea74934) at /Users/kats/zspace/mozilla-git/ipc/chromium/src/base/message_loop.cc:187
#27 0x406b8576 in XRE_InitChildProcess (aArgc=<value optimized out>, aArgv=0xbea74a3c, aProcess=3198634692) at /Users/kats/zspace/mozilla-git/toolkit/xre/nsEmbedFunctions.cpp:551
#28 0x000086a0 in main (argc=6, argv=0xbea74ac4) at /Users/kats/zspace/mozilla-git/ipc/app/MozillaRuntimeMain.cpp:116

Comment 3

[Kartikaya Gupta (email:[e-mail])]

So, the element being destroyed is actually the *previous* page's root element. The real STR are:

1. Open the browser and load a page e.g. people.mozilla.com/~nhirata/html_tp/
2. Reload the page
3. Hit the power button to lock the device (this is when the first page load gets GC'd and the ROOT_SCROLL_ID property is deleted)
4. Unlock the device
5. Pan/zoom. You will notice that the displayport top-left is always the same as the scroll position, so if you zoom in and pan upwards or to the left, you immediately see whitespace rather than having a buffer of painted content.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 1 - Add some more logging fanciness

I was using this to debug bug 919039 but then it revealed this problem. Seems kind of handy to keep in the tree.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 2 - Clear the property on the old ROOT_SCROLL_ID content before setting a new one

This fixes the problem where at some random point in the future a GC runs and destroys the content map entry for ROOT_SCROLL_ID because some previous ROOT_SCROLL_ID content is getting GC'd.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Regression from part 2 of bug 895905, requesting uplift to koi.

Comment 7

[Timothy Nikkel (:tnikkel)]

Another reason why ROOT_SCROLL_ID is a bad idea.

Comment 8

[Botond Ballo [:botond]]

Comment on attachment 816077 [details] [diff] [review]
Part 1 - Add some more logging fanciness

Review of attachment 816077 [details] [diff] [review]:
-----------------------------------------------------------------

I'd like to propose an alternate patch that puts the code in APZC_LOG_FM into a function rather than a macro.

Comment 9

[Botond Ballo [:botond]]

Comment on attachment 816077 [details] [diff] [review]
Part 1 - Add some more logging fanciness

Review of attachment 816077 [details] [diff] [review]:
-----------------------------------------------------------------

Turns out that writing APZC_LOG_FM as a function isn't nearly as simple as I thought (in fact, it cannot be done in a portable way), so let's just go with the macro.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/87e987f2aedf
remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/71909c62bc14

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/87e987f2aedf
https://hg.mozilla.org/mozilla-central/rev/71909c62bc14

Comment 13

[Jason Smith [:jsmith]]

Blocking+ for the dupe being a blocker.

Comment 14

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ea8e3d9d10ca