Last Comment Bug 758361 - Viewport resets during page load
: Viewport resets during page load
Status: VERIFIED FIXED
[viewport]
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All Android
: -- normal (vote)
: Firefox 16
Assigned To: Kartikaya Gupta (email:kats@mozilla.com)
:
: Sebastian Kaspari (:sebastian)
Mentors:
: 753092 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-24 13:45 PDT by Kartikaya Gupta (email:kats@mozilla.com)
Modified: 2012-07-17 07:31 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
verified
verified
verified
.N+


Attachments
Backtraces (12.82 KB, text/plain)
2012-05-25 09:41 PDT, Kartikaya Gupta (email:kats@mozilla.com)
no flags Details
Add a guard for before-first-paint (2.19 KB, patch)
2012-05-25 10:01 PDT, Kartikaya Gupta (email:kats@mozilla.com)
bzbarsky: review+
Details | Diff | Splinter Review
Move the before-first-paint into EnsureVisible (7.67 KB, patch)
2012-05-29 10:05 PDT, Kartikaya Gupta (email:kats@mozilla.com)
no flags Details | Diff | Splinter Review
Move the before-first-paint into UnsuppressAndInvalidate (5.48 KB, patch)
2012-06-04 06:58 PDT, Kartikaya Gupta (email:kats@mozilla.com)
bzbarsky: review+
mark.finkle: approval‑mozilla‑aurora+
blassey.bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Kartikaya Gupta (email:kats@mozilla.com) 2012-05-24 13:45:08 PDT
STR:
1. Load CNN.com
2. While it's loading, zoom in and scroll down a bit

Expected results:
- When the page is done loading, it remains zoomed in

Actual results:
- It zooms out during page load

Seeing this on a Galaxy Nexus on m-c code, not sure if it's happening in aurora as well.
Comment 1 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-24 13:46:34 PDT
The reason this is happening is because the before-first-paint event is getting dispatched multiple times. The first time it is dispatched via the normal code path:

#0  DocumentViewerImpl::Show (this=<optimized out>) at /Users/kats/zspace/mozilla-git/layout/base/nsDocumentViewer.cpp:2031
#1  0x623846f2 in nsPresContext::EnsureVisible (this=0x616e3c00) at /Users/kats/zspace/mozilla-git/layout/base/nsPresContext.cpp:1811
#2  0x6238fc5a in PresShell::UnsuppressAndInvalidate (this=0x6183ed00) at /Users/kats/zspace/mozilla-git/layout/base/nsPresShell.cpp:3530
#3  0x62391a3e in PresShell::UnsuppressPainting (this=0x6183ed00) at /Users/kats/zspace/mozilla-git/layout/base/nsPresShell.cpp:3580
#4  0x6238ab1c in PresShell::sPaintSuppressionCallback (aTimer=<optimized out>, aPresShell=<optimized out>) at /Users/kats/zspace/mozilla-git/layout/base/nsPresShell.cpp:1709
#5  0x62da4882 in nsTimerImpl::Fire (this=0x658e1b00) at /Users/kats/zspace/mozilla-git/xpcom/threads/nsTimerImpl.cpp:473
#6  0x62da4a84 in nsTimerEvent::Run (this=0x5c6e2200) at /Users/kats/zspace/mozilla-git/xpcom/threads/nsTimerImpl.cpp:556
#7  0x62da0af2 in nsThread::ProcessNextEvent (this=0x5c6b10f0, mayWait=<optimized out>, result=0x5c4a87cf) at /Users/kats/zspace/mozilla-git/xpcom/threads/nsThread.cpp:624
#8  0x62d6a6e8 in NS_ProcessNextEvent_P (thread=0x5c60a0cc, mayWait=false) at /Users/kats/zspace/mozilla-git/obj-android-debug/xpcom/build/nsThreadUtils.cpp:213
#9  0x62cb2ad0 in mozilla::ipc::MessagePump::Run (this=0x5c6b3220, aDelegate=0x5c6d80e0) at /Users/kats/zspace/mozilla-git/ipc/glue/MessagePump.cpp:82
#10 0x62dd29a2 in MessageLoop::RunInternal (this=0x5c6d80e0) at /Users/kats/zspace/mozilla-git/ipc/chromium/src/base/message_loop.cc:208
#11 0x62dd2a02 in RunHandler (this=<optimized out>) at /Users/kats/zspace/mozilla-git/ipc/chromium/src/base/message_loop.cc:201
#12 MessageLoop::Run (this=0x5c6d80e0) at /Users/kats/zspace/mozilla-git/ipc/chromium/src/base/message_loop.cc:175
#13 0x62c04fae in nsBaseAppShell::Run (this=0x5f3303e0) at /Users/kats/zspace/mozilla-git/widget/xpwidgets/nsBaseAppShell.cpp:163
#14 0x62abfe7a in nsAppStartup::Run (this=0x5fd5a340) at /Users/kats/zspace/mozilla-git/toolkit/components/startup/nsAppStartup.cpp:256
#15 0x621ecef2 in XREMain::XRE_mainRun (this=0x5c4a8a74) at /Users/kats/zspace/mozilla-git/toolkit/xre/nsAppRunner.cpp:3765
#16 0x621ef5cc in XREMain::XRE_main (this=0x5c4a8a74, argc=<optimized out>, argv=0x5c6bf048, aAppData=0x5b6cda2c) at /Users/kats/zspace/mozilla-git/toolkit/xre/nsAppRunner.cpp:3842
#17 0x621ef756 in XRE_main (argc=7, argv=0x5c6bf048, aAppData=0x5b6cda2c) at /Users/kats/zspace/mozilla-git/toolkit/xre/nsAppRunner.cpp:3918
#18 0x621f443a in GeckoStart (data=0x1800308, appData=0x5b6cda2c) at /Users/kats/zspace/mozilla-git/toolkit/xre/nsAndroidStartup.cpp:73
#19 0x5b6b5aea in Java_org_mozilla_gecko_GeckoAppShell_nativeRun (jenv=0x1617d90, jc=<optimized out>, jargs=0x25f00005) at /Users/kats/zspace/mozilla-git/mozglue/android/APKOpen.cpp:966
#20 0x40843c34 in dvmPlatformInvoke () from /Users/kats/android/jdb/moz-gdb/lib/01466E640801401C/system/lib/libdvm.so
#21 0x4087deee in dvmCallJNIMethod(unsigned int const*, JValue*, Method const*, Thread*) () from /Users/kats/android/jdb/moz-gdb/lib/01466E640801401C/system/lib/libdvm.so


On subsequent dispatches the backtrace looks different, like this:

#0  DocumentViewerImpl::Show (this=<optimized out>) at /Users/kats/zspace/mozilla-git/layout/base/nsDocumentViewer.cpp:2031
#1  0x62a42a64 in nsDocShell::SetVisibility (this=<optimized out>, aVisibility=<optimized out>) at /Users/kats/zspace/mozilla-git/docshell/base/nsDocShell.cpp:5002
#2  0x6254d728 in nsFrameLoader::Show (this=0x6552f280, marginWidth=<optimized out>, marginHeight=<optimized out>, scrollbarPrefX=<optimized out>, scrollbarPrefY=2, frame=0x67eb9858)
    at /Users/kats/zspace/mozilla-git/content/base/src/nsFrameLoader.cpp:789
#3  0x62410e1a in nsSubDocumentFrame::ShowViewer (this=0x67eb9858) at /Users/kats/zspace/mozilla-git/layout/generic/nsSubDocumentFrame.cpp:197
#4  0x62410e64 in AsyncFrameInit::Run (this=<optimized out>) at /Users/kats/zspace/mozilla-git/layout/generic/nsSubDocumentFrame.cpp:113
#5  0x62514c1c in nsContentUtils::RemoveScriptBlocker () at /Users/kats/zspace/mozilla-git/content/base/src/nsContentUtils.cpp:4760
#6  0x62344f5e in nsAutoScriptBlocker::~nsAutoScriptBlocker (this=0x659952fc, __in_chrg=<optimized out>) at ../../dist/include/nsContentUtils.h:2163
#7  0x6239a5ae in PresShell::FlushPendingNotifications (this=0x6183ed00, aType=Flush_Style) at /Users/kats/zspace/mozilla-git/layout/base/nsPresShell.cpp:3800
#8  0x6239c72a in nsRefreshDriver::Notify (this=0x5fd04580, aTimer=<optimized out>) at /Users/kats/zspace/mozilla-git/layout/base/nsRefreshDriver.cpp:375
#9  0x62da488e in nsTimerImpl::Fire (this=0x658e1ab0) at /Users/kats/zspace/mozilla-git/xpcom/threads/nsTimerImpl.cpp:476
#10 0x62da4a84 in nsTimerEvent::Run (this=0x5c6e2460) at /Users/kats/zspace/mozilla-git/xpcom/threads/nsTimerImpl.cpp:556
#11 0x62da0af2 in nsThread::ProcessNextEvent (this=0x5c6b10f0, mayWait=<optimized out>, result=0x5c4a87cf) at /Users/kats/zspace/mozilla-git/xpcom/threads/nsThread.cpp:624
#12 0x62d6a6e8 in NS_ProcessNextEvent_P (thread=0x659952fc, mayWait=false) at /Users/kats/zspace/mozilla-git/obj-android-debug/xpcom/build/nsThreadUtils.cpp:213
#13 0x62cb2ad0 in mozilla::ipc::MessagePump::Run (this=0x5c6b3220, aDelegate=0x5c6d80e0) at /Users/kats/zspace/mozilla-git/ipc/glue/MessagePump.cpp:82
#14 0x62dd29a2 in MessageLoop::RunInternal (this=0x5c6d80e0) at /Users/kats/zspace/mozilla-git/ipc/chromium/src/base/message_loop.cc:208
#15 0x62dd2a02 in RunHandler (this=<optimized out>) at /Users/kats/zspace/mozilla-git/ipc/chromium/src/base/message_loop.cc:201
#16 MessageLoop::Run (this=0x5c6d80e0) at /Users/kats/zspace/mozilla-git/ipc/chromium/src/base/message_loop.cc:175
#17 0x62c04fae in nsBaseAppShell::Run (this=0x5f3303e0) at /Users/kats/zspace/mozilla-git/widget/xpwidgets/nsBaseAppShell.cpp:163
#18 0x62abfe7a in nsAppStartup::Run (this=0x5fd5a340) at /Users/kats/zspace/mozilla-git/toolkit/components/startup/nsAppStartup.cpp:256
#19 0x621ecef2 in XREMain::XRE_mainRun (this=0x5c4a8a74) at /Users/kats/zspace/mozilla-git/toolkit/xre/nsAppRunner.cpp:3765
#20 0x621ef5cc in XREMain::XRE_main (this=0x5c4a8a74, argc=<optimized out>, argv=0x5c6bf048, aAppData=0x5b6cda2c) at /Users/kats/zspace/mozilla-git/toolkit/xre/nsAppRunner.cpp:3842
#21 0x621ef756 in XRE_main (argc=7, argv=0x5c6bf048, aAppData=0x5b6cda2c) at /Users/kats/zspace/mozilla-git/toolkit/xre/nsAppRunner.cpp:3918
#22 0x621f443a in GeckoStart (data=0x1800308, appData=0x5b6cda2c) at /Users/kats/zspace/mozilla-git/toolkit/xre/nsAndroidStartup.cpp:73
#23 0x5b6b5aea in Java_org_mozilla_gecko_GeckoAppShell_nativeRun (jenv=0x1617d90, jc=<optimized out>, jargs=0x25f00005) at /Users/kats/zspace/mozilla-git/mozglue/android/APKOpen.cpp:966
#24 0x40843c34 in dvmPlatformInvoke () from /Users/kats/android/jdb/moz-gdb/lib/01466E640801401C/system/lib/libdvm.so
#25 0x4087deee in dvmCallJNIMethod(unsigned int const*, JValue*, Method const*, Thread*) () from /Users/kats/android/jdb/moz-gdb/lib/01466E640801401C/system/lib/libdvm.so
Comment 2 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-24 14:50:06 PDT
Sorry, the second backtrace above is wrong; that gets fired on a different document and is ignored. The correct backtrace looks like this:

#0  nsBeforeFirstPaintDispatcher::Run (this=0x672b1e00) at /Users/kats/zspace/mozilla-git/layout/base/nsDocumentViewer.cpp:4386
#1  0x6125198a in nsContentUtils::AddScriptRunner (aRunnable=0x672b1e00) at /Users/kats/zspace/mozilla-git/content/base/src/nsContentUtils.cpp:4781
#2  0x610af558 in DocumentViewerImpl::Show (this=<optimized out>) at /Users/kats/zspace/mozilla-git/layout/base/nsDocumentViewer.cpp:2039
#3  0x610bf742 in nsPresContext::EnsureVisible (this=0x661eec00) at /Users/kats/zspace/mozilla-git/layout/base/nsPresContext.cpp:1811
#4  0x618b217e in nsPluginInstanceOwner::Init (this=0x661f8900, aContent=0x65738c60) at /Users/kats/zspace/mozilla-git/dom/plugins/base/nsPluginInstanceOwner.cpp:3219
#5  0x618a6c7c in nsPluginHost::InstantiateEmbeddedPluginInstance (this=<optimized out>, aMimeType=0x6903c138 "application/x-shockwave-flash", aURL=0x65927b40, aContent=<optimized out>, 
    aOwner=0x65738d1c) at /Users/kats/zspace/mozilla-git/dom/plugins/base/nsPluginHost.cpp:905
#6  0x612aa234 in nsObjectLoadingContent::InstantiatePluginInstance (this=0x65738cb0, aMimeType=0x6903c138 "application/x-shockwave-flash", aURI=0x65927b40)
    at /Users/kats/zspace/mozilla-git/content/base/src/nsObjectLoadingContent.cpp:683
#7  0x612aa4ba in nsObjectLoadingContent::SyncStartPluginInstance (this=0x65738cb0) at /Users/kats/zspace/mozilla-git/content/base/src/nsObjectLoadingContent.cpp:2097
#8  0x6146a236 in nsHTMLPluginObjElementSH::GetPluginInstanceIfSafe (wrapper=<optimized out>, obj=<optimized out>, _result=0x5c3a3404) at /Users/kats/zspace/mozilla-git/dom/base/nsDOMClassInfo.cpp:9582
#9  0x6146f756 in nsHTMLPluginObjElementSH::NewResolve (this=0x65dab680, wrapper=0x66342c80, cx=0x5dd52da0, obj=0x66036b20, id=..., flags=5, objp=0x5c3a3510, _retval=0x5c3a3517)
    at /Users/kats/zspace/mozilla-git/dom/base/nsDOMClassInfo.cpp:9903
#10 0x6170de62 in XPC_WN_Helper_NewResolve (cx=0x5dd52da0, obj=..., id=..., flags=<optimized out>, objp=0x5c3a3594) at /Users/kats/zspace/mozilla-git/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1069
#11 0x61e4f68c in CallResolveOp (recursedp=<optimized out>, propp=<optimized out>, objp=<optimized out>, flags=<optimized out>, id=<optimized out>, obj=<optimized out>, start=0x0, cx=<optimized out>)
    at /Users/kats/zspace/mozilla-git/js/src/jsobj.cpp:4625
#12 LookupPropertyWithFlagsInline (cx=0x5dd52da0, obj=..., id=..., flags=1628072405, objp=0x5c3a35e8, propp=0x5c3a361c) at /Users/kats/zspace/mozilla-git/js/src/jsobj.cpp:4675
#13 0x61e55e9e in js_GetPropertyHelperInline (vp=<optimized out>, getHow=<optimized out>, id_=<optimized out>, receiver=<optimized out>, obj=<optimized out>, cx=<optimized out>)
    at /Users/kats/zspace/mozilla-git/js/src/jsobj.cpp:5010
#14 js::GetPropertyHelper (cx=0x672b1e00, obj=..., id=<optimized out>, getHow=1, vp=0x5c3a3b28) at /Users/kats/zspace/mozilla-git/js/src/jsobj.cpp:5096
#15 0x61e2458a in GetPropertyOperation (vp=<optimized out>, lval=<optimized out>, pc=<optimized out>, cx=<optimized out>) at /Users/kats/zspace/mozilla-git/js/src/jsinterpinlines.h:231
#16 js::Interpret (cx=<optimized out>, entryFrame=<optimized out>, interpMode=<optimized out>) at /Users/kats/zspace/mozilla-git/js/src/jsinterp.cpp:2408
#17 0x61e2f33c in js::RunScript (cx=0x5dd52da0, script=<optimized out>, fp=0x5e100038) at /Users/kats/zspace/mozilla-git/js/src/jsinterp.cpp:266
#18 0x61e2fc4a in js::InvokeKernel (cx=0x5dd52da0, args=..., construct=js::NO_CONSTRUCT) at /Users/kats/zspace/mozilla-git/js/src/jsinterp.cpp:326
#19 0x61e30462 in Invoke (construct=<optimized out>, args=<optimized out>, cx=<optimized out>) at /Users/kats/zspace/mozilla-git/js/src/jsinterp.h:125
#20 js::Invoke (cx=0x5dd52da0, thisv=..., fval=..., argc=<optimized out>, argv=0x5c3a4108, rval=0x5c3a4268) at /Users/kats/zspace/mozilla-git/js/src/jsinterp.cpp:358
#21 0x61d811a0 in JS_CallFunctionValue (cx=0x5dd52da0, obj=0x671ee180, fval=..., argc=1, argv=0x5c3a4108, rval=0x5c3a4268) at /Users/kats/zspace/mozilla-git/js/src/jsapi.cpp:5491
#22 0x61702402 in nsXPCWrappedJSClass::CallMethod (this=0x5e609e20, wrapper=<optimized out>, methodIndex=<optimized out>, info=0x5de5f320, nativeParams=0x5c3a4350)
    at /Users/kats/zspace/mozilla-git/js/xpconnect/src/XPCWrappedJSClass.cpp:1474
#23 0x616fd2f2 in nsXPCWrappedJS::CallMethod (this=0x65441ec0, methodIndex=3, info=0x5de5f320, params=<optimized out>) at /Users/kats/zspace/mozilla-git/js/xpconnect/src/XPCWrappedJS.cpp:579
#24 0x61af0db6 in PrepareAndDispatch (self=<optimized out>, methodIndex=<optimized out>, args=0x5c3a440c) at /Users/kats/zspace/mozilla-git/xpcom/reflect/xptcall/src/md/unix/xptcstubs_arm.cpp:105
#25 0x61af03ec in SharedStub () from /Users/kats/zspace/mozilla-git/obj-android-debug/dist/bin/libxul.so
#26 0x6131e454 in nsEventListenerManager::HandleEventSubType (this=<optimized out>, aListenerStruct=<optimized out>, aListener=0x63cf06d0, aDOMEvent=0x65b9b4c0, aCurrentTarget=0x65a0cc00, 
    aPhaseFlags=6, aPusher=0x5c3a4594) at /Users/kats/zspace/mozilla-git/content/events/src/nsEventListenerManager.cpp:809
#27 0x6131e5de in nsEventListenerManager::HandleEventInternal (this=0x65899650, aPresContext=<optimized out>, aEvent=0x65b9b580, aDOMEvent=0x5c3a4584, aCurrentTarget=0x65a0cc00, aFlags=6, 
    aEventStatus=0x5c3a4588, aPusher=0x5c3a4594) at /Users/kats/zspace/mozilla-git/content/events/src/nsEventListenerManager.cpp:866
#28 0x613368c4 in HandleEvent (aPusher=<optimized out>, aEventStatus=<optimized out>, aFlags=<optimized out>, aCurrentTarget=<optimized out>, aDOMEvent=<optimized out>, aEvent=<optimized out>, 
    aPresContext=<optimized out>, this=<optimized out>) at /Users/kats/zspace/mozilla-git/content/events/src/nsEventListenerManager.h:137
#29 nsEventTargetChainItem::HandleEvent (this=<optimized out>, aVisitor=..., aFlags=6, aMayHaveNewListenerManagers=<optimized out>, aPusher=0x5c3a4594)
    at /Users/kats/zspace/mozilla-git/content/events/src/nsEventDispatcher.cpp:185
#30 0x61336a30 in nsEventTargetChainItem::HandleEventTargetChain (this=<optimized out>, aVisitor=..., aFlags=6, aCallback=0x0, aMayHaveNewListenerManagers=false, aPusher=0x5c3a4594)
    at /Users/kats/zspace/mozilla-git/content/events/src/nsEventDispatcher.cpp:317
#31 0x613373f8 in nsEventDispatcher::Dispatch (aTarget=<optimized out>, aPresContext=0x661eec00, aEvent=0x65b9b580, aDOMEvent=<optimized out>, aEventStatus=0x5c3a4630, aCallback=0x0, aTargets=0x0)
    at /Users/kats/zspace/mozilla-git/content/events/src/nsEventDispatcher.cpp:643
#32 0x613376f4 in nsEventDispatcher::DispatchDOMEvent (aTarget=0x65a0cc00, aEvent=<optimized out>, aDOMEvent=0x65b9b4c0, aPresContext=0x661eec00, aEventStatus=0x5c3a4630)
    at /Users/kats/zspace/mozilla-git/content/events/src/nsEventDispatcher.cpp:706
#33 0x6129897a in nsINode::DispatchEvent (this=0x65a0cc00, aEvent=0x65b9b4c0, aRetVal=0x5c3a4687) at /Users/kats/zspace/mozilla-git/content/base/src/nsGenericElement.cpp:1132
#34 0x61246d9c in nsContentUtils::DispatchEvent (aDoc=<optimized out>, aTarget=<optimized out>, aEventName=<optimized out>, aCanBubble=<optimized out>, aCancelable=true, aTrusted=true, 
    aDefaultAction=0x5c3a4687) at /Users/kats/zspace/mozilla-git/content/base/src/nsContentUtils.cpp:3397
#35 0x61246dfe in nsContentUtils::DispatchTrustedEvent (aDoc=0x672b1e00, aTarget=0x65b9b4c0, aEventName=..., aCanBubble=241, aCancelable=<optimized out>, aDefaultAction=0x0)
    at /Users/kats/zspace/mozilla-git/content/base/src/nsContentUtils.cpp:3368
#36 0x6127e62c in nsDocument::DispatchContentLoadedEvents (this=0x65a0cc00) at /Users/kats/zspace/mozilla-git/content/base/src/nsDocument.cpp:4148
#37 0x60f4b3ae in nsRunnableMethodImpl<void (nsPACMan::*)(), true>::Run (this=<optimized out>) at ../../../dist/include/nsThreadUtils.h:313
#38 0x61adbb62 in nsThread::ProcessNextEvent (this=0x5c5b10f0, mayWait=<optimized out>, result=0x5c3a47cf) at /Users/kats/zspace/mozilla-git/xpcom/threads/nsThread.cpp:624
#39 0x61aa5758 in NS_ProcessNextEvent_P (thread=0x672b1e00, mayWait=false) at /Users/kats/zspace/mozilla-git/obj-android-debug/xpcom/build/nsThreadUtils.cpp:213
#40 0x619edb40 in mozilla::ipc::MessagePump::Run (this=0x5c5b3220, aDelegate=0x5c5d80e0) at /Users/kats/zspace/mozilla-git/ipc/glue/MessagePump.cpp:82
#41 0x61b0da1a in MessageLoop::RunInternal (this=0x5c5d80e0) at /Users/kats/zspace/mozilla-git/ipc/chromium/src/base/message_loop.cc:208
#42 0x61b0da7a in RunHandler (this=<optimized out>) at /Users/kats/zspace/mozilla-git/ipc/chromium/src/base/message_loop.cc:201
#43 MessageLoop::Run (this=0x5c5d80e0) at /Users/kats/zspace/mozilla-git/ipc/chromium/src/base/message_loop.cc:175
#44 0x6194001e in nsBaseAppShell::Run (this=0x5dd303e0) at /Users/kats/zspace/mozilla-git/widget/xpwidgets/nsBaseAppShell.cpp:163
#45 0x617faeea in nsAppStartup::Run (this=0x5e65a340) at /Users/kats/zspace/mozilla-git/toolkit/components/startup/nsAppStartup.cpp:256
#46 0x60f27ef2 in XREMain::XRE_mainRun (this=0x5c3a4a74) at /Users/kats/zspace/mozilla-git/toolkit/xre/nsAppRunner.cpp:3765
#47 0x60f2a5cc in XREMain::XRE_main (this=0x5c3a4a74, argc=<optimized out>, argv=0x5c5bf048, aAppData=0x5b4cda2c) at /Users/kats/zspace/mozilla-git/toolkit/xre/nsAppRunner.cpp:3842
#48 0x60f2a756 in XRE_main (argc=7, argv=0x5c5bf048, aAppData=0x5b4cda2c) at /Users/kats/zspace/mozilla-git/toolkit/xre/nsAppRunner.cpp:3918
#49 0x60f2f43a in GeckoStart (data=0x19750c0, appData=0x5b4cda2c) at /Users/kats/zspace/mozilla-git/toolkit/xre/nsAndroidStartup.cpp:73
#50 0x5b4b5aea in Java_org_mozilla_gecko_GeckoAppShell_nativeRun (jenv=0x1762020, jc=<optimized out>, jargs=0x25f00005) at /Users/kats/zspace/mozilla-git/mozglue/android/APKOpen.cpp:966
#51 0x40843c34 in dvmPlatformInvoke () from /Users/kats/android/jdb/moz-gdb/lib/01466E640801401C/system/lib/libdvm.so
#52 0x4087deee in dvmCallJNIMethod(unsigned int const*, JValue*, Method const*, Thread*) () from /Users/kats/android/jdb/moz-gdb/lib/01466E640801401C/system/lib/libdvm.so
#53 0x01762128 in ?? ()
#54 0x01762128 in ?? ()
Comment 3 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-24 14:58:26 PDT
snorp, can you shed any light on the backtrace in comment 2? Specifically if anything in that path changed recently (last ~2 weeks or so) that might be causing DocumentViewerImpl::Show to get called when it didn't before? I just want to understand the problem better. It's likely that sticking in a guard somewhere to not show the document twice will fix this problem, but I want to make sure there's no other underlying problems.
Comment 4 James Willcox (:snorp) (jwillcox@mozilla.com) 2012-05-25 07:09:50 PDT
I'm not aware of any changes to that code, but I don't really keep tabs on the general plugin stuff. I think maybe there were some click-to-play changes, but not sure if they touched this path.
Comment 5 James Willcox (:snorp) (jwillcox@mozilla.com) 2012-05-25 07:15:31 PDT
This bug really irritates me.

Looking at it briefly it does indeed seem like there should be a guard in nsDocumentViewer::Show. I don't see anything obvious to use for that, though...
Comment 6 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-25 09:41:12 PDT
Created attachment 627258 [details]
Backtraces

Putting the backtraces in an attachment so they're easier to read.
Comment 7 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-25 10:01:17 PDT
Created attachment 627270 [details] [diff] [review]
Add a guard for before-first-paint

This is smallest guard that gets the job done. :bz, I'm open to suggestions for better fixes for this - I just don't know this code very well and figured it would be easier to discuss once I verified this fixes the problem (which it does).
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2012-05-25 10:06:14 PDT
What's the expected behavior if a viewer is shown, then hidden, then shown again?
Comment 9 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-25 10:17:45 PDT
The browser.js code that uses the event expects the event to come only once over the entire lifetime of a document. So if the viewer is shown, then hidden, then shown again, I don't think the second shown should trigger the event.

However I don't know all the circumstances under which the viewer might be hidden. Is there an easy way to know what those are?
Comment 10 Boris Zbarsky [:bz] (still a bit busy) 2012-05-25 10:32:27 PDT
Sure.  display:none on the containing frame (or anything that triggers a frame reconstruct on it) will do it.
Comment 11 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-25 10:39:26 PDT
In those cases we don't want the before-first-paint event to fire.
Comment 12 Boris Zbarsky [:bz] (still a bit busy) 2012-05-25 10:44:09 PDT
Comment on attachment 627270 [details] [diff] [review]
Add a guard for before-first-paint

OK.  What about the case when the hide is due to going into bfcache and the show is due to coming out of bfcache?
Comment 13 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-25 10:54:02 PDT
Hmm.. yeah in that case we want the event.
Comment 14 Johnathan Nightingale [:johnath] 2012-05-25 11:55:53 PDT
Soft, but yesplz
Comment 15 Boris Zbarsky [:bz] (still a bit busy) 2012-05-25 20:47:42 PDT
Comment on attachment 627270 [details] [diff] [review]
Add a guard for before-first-paint

Then this doesn't sound like what you want....
Comment 16 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-28 11:10:20 PDT
Is there some way to detect that case? Or a better place/API to hook for this purpose? There must be some point during page load at which the active document is switched from the old one to the new one (where "active" is defined as "the document that will be painted if a paint is requested").
Comment 17 Boris Zbarsky [:bz] (still a bit busy) 2012-05-28 20:41:07 PDT
> Is there some way to detect that case?

Probably, with enough work.  Most simply, by changing the signature of Show() and fixing all callers as needed.

> There must be some point during page load at which the active document is switched from
> the old one to the new one (where "active" is defined as "the document that will be
> painted if a paint is requested").

There is, and that point is Show().  It's just that this point is also reached in various other cases (e.g. when a document switches from display:none to not).  Preexisting consumers don't care about the differences between those two cases; they just care that the document used to not need to be painted, and now it needs to be paintes.
Comment 18 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-29 10:05:40 PDT
Created attachment 628003 [details] [diff] [review]
Move the before-first-paint into EnsureVisible

Modifying the Show() function to take a parameter requires IDL changes and will have far-reaching implications, so I'd like to avoid that. The attached patch moves the dispatching of the before-first-paint event up a level in the call stack, so that it only happens on the code path with the UnsuppressAndInvalidate call, rather than also happening on the plugin codepath.

Based on your previous comments I suspect this will get triggered on frame reconstruction as well, but I realized that in practice this doesn't matter because (1) our browser.js listener only cares about these events for the top-level content document, not iframes and (2) I don't believe there is any way to really trigger a frame reconstruction on the top-level content document in Fennec.

Does that seem reasonable?
Comment 19 Brad Lassey [:blassey] (use needinfo?) 2012-05-31 12:25:14 PDT
bz, review ping?
Comment 20 Boris Zbarsky [:bz] (still a bit busy) 2012-05-31 14:19:30 PDT
Working on it.  I've been in meetings most of the last two days.  :(  Should hopefully happen tonight after the kids are in bed.
Comment 21 Boris Zbarsky [:bz] (still a bit busy) 2012-05-31 17:54:09 PDT
Comment on attachment 628003 [details] [diff] [review]
Move the before-first-paint into EnsureVisible

This will also get triggered by fullscreen, possibly.

In any case, why are you passing false in the nsPluginInstanceOwner code?
Comment 22 Kartikaya Gupta (email:kats@mozilla.com) 2012-06-01 14:01:19 PDT
(In reply to Boris Zbarsky (:bz) from comment #21)
> This will also get triggered by fullscreen, possibly.

It doesn't appear to get triggered by fullscreening/unfullscreening. At least it didn't hit the breakpoint I set in EnsureVisible.

> In any case, why are you passing false in the nsPluginInstanceOwner code?

So that we don't fire the before-first-paint event on that codepath. As I understand it, that codepath may only execute after the document is already active.
Comment 23 Boris Zbarsky [:bz] (still a bit busy) 2012-06-01 14:10:56 PDT
> It doesn't appear to get triggered by fullscreening/unfullscreening.

Just on Fennec, right?  I would expect it to trigger for fullscreen in b2g or a XUL UI.

> As I understand it, that codepath may only execute after the document is already active.

If that were the case, the call to EnsureVisible() would not be needed at all, I would think.  The fact that it's needed exactly indicates that the document is not "already active", unless I'm really missing something.
Comment 24 Kartikaya Gupta (email:kats@mozilla.com) 2012-06-01 17:47:44 PDT
(In reply to Boris Zbarsky (:bz) from comment #23)
> Just on Fennec, right?  I would expect it to trigger for fullscreen in b2g
> or a XUL UI.

I only tested that on Fennec, so yes.

> If that were the case, the call to EnsureVisible() would not be needed at
> all, I would think.  The fact that it's needed exactly indicates that the
> document is not "already active", unless I'm really missing something.

Based on the comment right above the call to EnsureVisible() it sounds like a workaround to make sure plugins get destroyed properly. A workaround that, as a side-effect, could end up calling EnsureVisible on a document that is already active. That's the entire problem in this bug, in fact - while loading CNN I see EnsureVisible get called two or three times for the top-level document, which is what is triggering the viewport reset.
Comment 25 Boris Zbarsky [:bz] (still a bit busy) 2012-06-01 18:17:08 PDT
What the comment says is that we can reach that code for a plug-in when the document of that plug-in is loading but has not yet been made active.  _If_ that happens, we have to make sure we make the previous document inactive (and destroy any plug-ins in it) before we proceed with plug-in setup for the new document.  To make the previous document inactive, we call EnsureVisible() on the document of the plug-in, on the assumption that it's a no-op if the document is already visible and will hide the old document if the new one is not visible yet.
Comment 26 Kartikaya Gupta (email:kats@mozilla.com) 2012-06-01 20:57:47 PDT
(In reply to Boris Zbarsky (:bz) from comment #25)
> To make the previous document inactive, we call
> EnsureVisible() on the document of the plug-in, on the assumption that it's
> a no-op if the document is already visible

Right, exactly. The problem is that it's not a no-op for us, because it ends up firing the before-first-paint event again.

> and will hide the old document if
> the new one is not visible yet.

In this case there will still be another call to EnsureVisible from the usual code path (i.e. from UnsuppressAndInvalidate -- see the first backtrace in comment #1). As long as exactly one of these two calls to EnsureVisible triggers the before-first-paint event, it does what we want. Passing in false from the plug-in code and true from the other code makes that happen.
Comment 27 Boris Zbarsky [:bz] (still a bit busy) 2012-06-01 22:08:22 PDT
Hmm, ok.  But then why not just put the event dispatch in UnsuppressAndInvalidate, if that's what you really want?
Comment 28 Kartikaya Gupta (email:kats@mozilla.com) 2012-06-02 05:24:43 PDT
I wanted to check the return value from Show() to make sure the event isn't dispatched if Show() fails. Although if you're ok with me changing EnsureVisible() to return false when Show() fails then I can move the dispatch to UnsuppressAndInvalidate.
Comment 29 Boris Zbarsky [:bz] (still a bit busy) 2012-06-03 18:22:46 PDT
I think I would be fine with that, yes.  That sounds like the best approach here.  Thanks for talking this through!
Comment 30 Kartikaya Gupta (email:kats@mozilla.com) 2012-06-04 06:58:55 PDT
Created attachment 629759 [details] [diff] [review]
Move the before-first-paint into UnsuppressAndInvalidate

Thank you as well!

This moves the call out into PresShell::UnsuppressAndInvalidate and runs it if and only if the call to EnsureVisible happens and returns true.
Comment 31 Boris Zbarsky [:bz] (still a bit busy) 2012-06-04 08:33:54 PDT
Comment on attachment 629759 [details] [diff] [review]
Move the before-first-paint into UnsuppressAndInvalidate

r=me
Comment 32 Kartikaya Gupta (email:kats@mozilla.com) 2012-06-04 09:28:17 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/43c1e66ef550
Comment 33 Kartikaya Gupta (email:kats@mozilla.com) 2012-06-04 14:40:45 PDT
This didn't make the merge today.
Comment 34 Geoff Lankow (:darktrojan) 2012-06-05 06:10:49 PDT
https://hg.mozilla.org/mozilla-central/rev/43c1e66ef550
Comment 35 Kartikaya Gupta (email:kats@mozilla.com) 2012-06-05 10:43:44 PDT
Comment on attachment 629759 [details] [diff] [review]
Move the before-first-paint into UnsuppressAndInvalidate

[Approval Request Comment]
Bug caused by (feature/regressing bug #): unknown
User impact if declined: on some pages with plugins, during page load the viewport jumps back up to the top of the page and zooms out
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): should affect mobile only, because mobile is the only code that listens for this event. however the code change itself affects all platforms. there is a small risk of it not fixing the bug completely
String or UUID changes made by this patch: none
Comment 36 Joe Drew (not getting mail) 2012-06-05 11:57:14 PDT
note: we're not OK with this landing in beta due to possible risk, but Aurora is OK by us and akeybl.
Comment 37 Alex Keybl [:akeybl] 2012-06-05 11:57:15 PDT
Comment on attachment 629759 [details] [diff] [review]
Move the before-first-paint into UnsuppressAndInvalidate

[Triage Comment]
Mobile triage decided that because this was only a soft blocker and carries non-zero risk, we'd approve for Aurora 15 but not Beta 14.
Comment 38 Kartikaya Gupta (email:kats@mozilla.com) 2012-06-05 12:18:39 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/809a7e55a042
Comment 39 Brad Lassey [:blassey] (use needinfo?) 2012-06-14 17:49:59 PDT
re-nom'ing and suggesting .N+ because it fixed the brightcove CDN problem
Comment 40 Brad Lassey [:blassey] (use needinfo?) 2012-06-15 11:44:03 PDT
Comment on attachment 629759 [details] [diff] [review]
Move the before-first-paint into UnsuppressAndInvalidate

Let's get this landed on mozilla-beta's default branch
Comment 41 Kartikaya Gupta (email:kats@mozilla.com) 2012-06-15 12:16:13 PDT
https://hg.mozilla.org/releases/mozilla-beta/rev/e9908430b490
Comment 42 Adrian Tamas (:AdrianT) 2012-06-19 07:33:00 PDT
I am not seeing any viewport jump on Nightly 16.0a1 2012-06-19 or Aurora 15.0a2 2012-06-19 on HTC Desire running Android 2.2. The patch has not landed in Firefox Mobile 14 Beta 7 so leaving the bug opened for verification for Beta 8.
Comment 43 Paul Feher 2012-06-26 04:52:47 PDT
Verified/Fixed on:
Firefox Mobile 14.0b8 2012-06-22
HTC Desire Z - Android (2.3.3)
Comment 44 Kartikaya Gupta (email:kats@mozilla.com) 2012-07-17 07:31:53 PDT
*** Bug 753092 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.