Closed Bug 751200 Opened 13 years ago Closed 13 years ago

Infinite loop in nsContentEventHandler (was: Touch response got worse since v14)

Categories

(Firefox for Android Graveyard :: General, defect)

14 Branch
All
Android
defect
Not set
major

Tracking

(firefox12 affected, firefox13+ affected, firefox14 verified, firefox15 verified, blocking-fennec1.0 betaN+)

VERIFIED FIXED
Firefox 15
Tracking Status
firefox12 --- affected
firefox13 + affected
firefox14 --- verified
firefox15 --- verified
blocking-fennec1.0 --- betaN+

People

(Reporter: smarteralec, Assigned: ehsan.akhgari)

References

Details

(Keywords: regression)

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.3) Gecko/20100423 Ubuntu/10.04 (lucid) Firefox/3.6.3 Build ID: 20120502042005 Steps to reproduce: Since change from Aurora 13.n to 14.n there is a noticeable drop in responsiveness of the touch response. (smaller focus/response area?) Actual results: Many touches on links produce no response from Aurora even though link may become highlit as it does when it's normally going to open. Haptic feedback does occur from device. Repeated touches may not work, but zooming in to a ridiculous level and then touching almost always works. This became apparent immediately with installation of Aurora 14.n - did not have this problem with 13.02. Doesn't seem to affect text-input fields such as this, in selecting where edit cursor should move to (or at least not as problematic.)
Samsung Galaxy Tab 10.1 GT-P7510R
Which pages do you see this on?
Bugzilla according to bug 751324.
blocking-fennec1.0: --- → +
Assignee: nobody → wjohnston
OS: Linux → Android
Hardware: x86 → All
Confirmed. Sometimes I click on a link and it doesn't go anywhere
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to Aaron Train [:aaronmt] from comment #2) > Which pages do you see this on? It happens on a variety of pages, and might be related to the amount of Javascript the page uses. It's most noticeable when logged into and using Yahoo mail, eg. trying to change folders, select commands like "Empty Spam", are the most problematic.
Also bug 752939 being fixed could definitely help things here.
placing in qawanted for figuring out a reduce test case.
Keywords: qawanted
Congrats developers, it's gotten better over the past week's dailies. The issue of a nearby link, rather than the selected link, opening has gone away completely on my Samsung 10.1. (I still don't like getting haptic feedback by pressing anywhere on a page, but it's not a performance issue, merely seems confusing to endusers, as it's kind of unique :)
Are you still seeing any issues with clicks not working? I tried changing folders and emptying spam in Yahoo mail (the mobile interface, which is what yahoo serves to the Galaxy Tab) in a recent build and it seemed to work fine. The haptic feedback issue is being tracked by bug 755549. If that's the only issue you're seeing then I would like to mark this bug closed.
Yes, still seeing the problem (not as bad?) Sometimes a click will respond only after a 5-8 second lag. Responses get stacked if pressed again before the first response. (Using the new Mail is -much- worse than the Classic.) However a new worse problem with today's build. (5-17) From within Yahoo Mail, 14.0a2 hangs repeatedly trying to close an open message. (Just tried again to confirm: yes, does it always, but only in new Mail) NOTE: I'm not using the mobile interface; I use Phony to set Desktop Firefox as my user-agent, so Yahoo is serving me the desktop interface. (Should this freezing up be reported as a new bug?) Exact steps to reproduce: 1 login to Yahoo mail account that's using the new Mail (javascript heavy) 2 Open a message (opens in a new tab within Mail) 3 little x in right side of tab (close) doesn't show. 4 press on msg tab, x appears, but Aurora is now frozen. Pressing the "change/new tab" tab of Aurora brings up the list of open Aurora tabs, but selecting any other tab does nothing, leaving frozen Yahoo Mail tab showing. Requires two pressings of Menu -> Quit to close Aurora. Of Menu -> selections, here's the responses: Reload - no response Bookmark - works fine, both to mark and unmark Share - works fine Save as PDF - no response Clear Site Settings - no response Downloads - no response Settings - half a response. Goes to settings page, but no options are bolded/operational. 'Back' key returns to frozen page. Phony (installed plugin) - no response Quit - no response the first time; quits properly the second time.
Eric, can you try and reproduce comment 10 on your devices? reporter used a galaxy tab, but we can check phones first. attach logcat if possible.
On nightly build 15.0a1 (05-21), the mobile site is functioning just as intended. However, if we try to view desktop version of yahoo mail on fennec, the situation in comment 10 can be reproduced. Right now I could not access anything after fennec is frozen. I'm using Samsung GT-I9100.
The code goes into an infinite loop, below is a backtrace from gdb. CC'ing ehsan who was the last person to touch the loop in question, which is in frame 1 in the backtrace below. The loop is the one at http://dxr.lanedo.com/mozilla-central/content/events/src/nsContentEventHandler.cpp.html#l922 What happens is iter->GetCurrentNode returns null, so the loop does a "continue" to the next iteration. The iterator prints out this assertion: I/Gecko (26872): ###!!! ASSERTION: Null current node in an iterator that's not done!: 'mCurNode', file /Users/kats/zspace/mozilla-git/content/base/src/nsContentIterator.cpp, line 1101 and returns null, and so the loop keeps going infinitely. The full backtrace: #0 nsContentIterator::GetCurrentNode (this=0x603d5ab0) at /Users/kats/zspace/mozilla-git/content/base/src/nsContentIterator.cpp:1104 #1 0x611c9426 in nsContentEventHandler::GetFlatTextOffsetOfRange (aRootContent=<optimized out>, aNode=0x6756c9a0, aNodeOffset=0, aNativeOffset=0x5c292dc8) at /Users/kats/zspace/mozilla-git/content/events/src/nsContentEventHandler.cpp:923 #2 0x611c951e in nsContentEventHandler::GetFlatTextOffsetOfRange (aRootContent=0x0, aRange=0x609a6400, aNativeOffset=0x1) at /Users/kats/zspace/mozilla-git/content/events/src/nsContentEventHandler.cpp:956 #3 0x611c991c in nsContentEventHandler::OnQuerySelectedText (this=0x5c292844, aEvent=0x5c292d78) at /Users/kats/zspace/mozilla-git/content/events/src/nsContentEventHandler.cpp:473 #4 0x611adfd2 in nsEventStateManager::DoQuerySelectedText (this=0x675e6240, aEvent=0x5c292d78) at /Users/kats/zspace/mozilla-git/content/events/src/nsEventStateManager.cpp:5041 #5 0x611b4742 in nsEventStateManager::PreHandleEvent (this=0x675e6240, aPresContext=0x6815fc00, aEvent=0x5c292d78, aTargetFrame=0x663d3a10, aStatus=0x5c292d3c) at /Users/kats/zspace/mozilla-git/content/events/src/nsEventStateManager.cpp:1258 #6 0x60f6001a in PresShell::HandleEventInternal (this=0x683f4e00, aEvent=0x5c292d78, aStatus=<optimized out>) at /Users/kats/zspace/mozilla-git/layout/base/nsPresShell.cpp:6374 #7 0x60f61526 in PresShell::HandleEvent (this=0x683f4e00, aFrame=0x663de400, aEvent=0x5c292d78, aDontRetargetEvents=false, aEventStatus=0x5c292d3c) at /Users/kats/zspace/mozilla-git/layout/base/nsPresShell.cpp:6009 #8 0x60f607d0 in PresShell::HandleEvent (this=<optimized out>, aFrame=<optimized out>, aEvent=0x5c292d78, aDontRetargetEvents=<optimized out>, aEventStatus=0x5c292d3c) at /Users/kats/zspace/mozilla-git/layout/base/nsPresShell.cpp:5662 #9 0x612b8d58 in nsViewManager::DispatchEvent (this=<optimized out>, aEvent=0x5c292d78, aView=0x60406ac0, aStatus=0x5c292d3c) at /Users/kats/zspace/mozilla-git/view/src/nsViewManager.cpp:865 #10 0x612b645a in HandleEvent (aEvent=0x5c292d78) at /Users/kats/zspace/mozilla-git/view/src/nsView.cpp:126 #11 0x617c3a5c in nsWindow::DispatchEvent (this=0x5db58400, aEvent=0x5c292d78) at /Users/kats/zspace/mozilla-git/widget/android/nsWindow.cpp:644 #12 0x617c5372 in nsWindow::OnIMESelectionChange (this=0x5db58400) at /Users/kats/zspace/mozilla-git/widget/android/nsWindow.cpp:2179 #13 0x617c55d2 in nsWindow::ResetInputState (this=0x5db58400) at /Users/kats/zspace/mozilla-git/widget/android/nsWindow.cpp:2068 #14 0x611c638c in nsIMEStateManager::OnChangeFocusInternal (aPresContext=0x6815fc00, aContent=0x0, aAction=...) at /Users/kats/zspace/mozilla-git/content/events/src/nsIMEStateManager.cpp:168 #15 0x611c6454 in nsIMEStateManager::OnChangeFocus (aPresContext=0x0, aContent=0x5ecf7, aCause=<optimized out>) at /Users/kats/zspace/mozilla-git/content/events/src/nsIMEStateManager.cpp:104 #16 0x612cce42 in nsFocusManager::Blur (this=0x5e501280, aWindowToClear=0x5c4ba300, aAncestorWindowToFocus=<optimized out>, aIsLeavingDocument=<optimized out>, aAdjustWidgets=true) at /Users/kats/zspace/mozilla-git/dom/base/nsFocusManager.cpp:1522 #17 0x612cf08c in nsFocusManager::ClearFocus (this=0x5e501280, aWindow=<optimized out>) at /Users/kats/zspace/mozilla-git/dom/base/nsFocusManager.cpp:548 #18 0x611d7eba in nsGenericHTMLElement::Blur (this=0x658c1ee0) at /Users/kats/zspace/mozilla-git/content/html/content/src/nsGenericHTMLElement.cpp:3158 #19 0x611f5bc4 in nsHTMLFrameElement::Blur (this=0x0) at /Users/kats/zspace/mozilla-git/content/html/content/src/nsHTMLFrameElement.cpp:36 #20 0x615aab8a in nsIDOMHTMLElement_Blur (cx=0x5db51200, argc=<optimized out>, vp=0x5e002558) at /Users/kats/zspace/mozilla-git/obj-android-debug/js/xpconnect/src/dom_quickstubs.cpp:14158 #21 0x61ca6320 in js::CallJSNative (cx=0x5db51200, native=0x615aab3d <nsIDOMHTMLElement_Blur(JSContext*, unsigned int, jsval*)>, args=...) at /Users/kats/zspace/mozilla-git/js/src/jscntxtinlines.h:397 #22 0x61cb8bde in js::InvokeKernel (cx=0x5db51200, args=..., construct=js::NO_CONSTRUCT) at /Users/kats/zspace/mozilla-git/js/src/jsinterp.cpp:310 #23 0x61ca7278 in js::Interpret (cx=<optimized out>, entryFrame=<optimized out>, interpMode=<optimized out>) at /Users/kats/zspace/mozilla-git/js/src/jsinterp.cpp:2513 #24 0x61cb8394 in js::RunScript (cx=0x5db51200, script=<optimized out>, fp=0x5e002488) at /Users/kats/zspace/mozilla-git/js/src/jsinterp.cpp:266 #25 0x61cb8ca2 in js::InvokeKernel (cx=0x5db51200, args=..., construct=js::NO_CONSTRUCT) at /Users/kats/zspace/mozilla-git/js/src/jsinterp.cpp:326 #26 0x61c6f3d4 in Invoke (args=<optimized out>, cx=<optimized out>, construct=<optimized out>) at /Users/kats/zspace/mozilla-git/js/src/jsinterp.h:125 #27 js_fun_call (cx=0x5db51200, argc=<optimized out>, vp=0x5e002428) at /Users/kats/zspace/mozilla-git/js/src/jsfun.cpp:655 #28 0x61ca6320 in js::CallJSNative (cx=0x5db51200, native=0x61c6f281 <js_fun_call(JSContext*, unsigned int, JS::Value*)>, args=...) at /Users/kats/zspace/mozilla-git/js/src/jscntxtinlines.h:397 #29 0x61cb8bde in js::InvokeKernel (cx=0x5db51200, args=..., construct=js::NO_CONSTRUCT) at /Users/kats/zspace/mozilla-git/js/src/jsinterp.cpp:310 #30 0x61ca7278 in js::Interpret (cx=<optimized out>, entryFrame=<optimized out>, interpMode=<optimized out>) at /Users/kats/zspace/mozilla-git/js/src/jsinterp.cpp:2513 #31 0x61cb8394 in js::RunScript (cx=0x5db51200, script=<optimized out>, fp=0x5e002328) at /Users/kats/zspace/mozilla-git/js/src/jsinterp.cpp:266 #32 0x61cb8ca2 in js::InvokeKernel (cx=0x5db51200, args=..., construct=js::NO_CONSTRUCT) at /Users/kats/zspace/mozilla-git/js/src/jsinterp.cpp:326 #33 0x61c6f6ee in Invoke (args=<optimized out>, cx=<optimized out>, construct=<optimized out>) at /Users/kats/zspace/mozilla-git/js/src/jsinterp.h:125 #34 js_fun_apply (cx=0x5db51200, argc=<optimized out>, vp=0x5e0022f0) at /Users/kats/zspace/mozilla-git/js/src/jsfun.cpp:731 #35 0x61ca6320 in js::CallJSNative (cx=0x5db51200, native=0x61c6f531 <js_fun_apply(JSContext*, unsigned int, JS::Value*)>, args=...) at /Users/kats/zspace/mozilla-git/js/src/jscntxtinlines.h:397 #36 0x61cb8bde in js::InvokeKernel (cx=0x5db51200, args=..., construct=js::NO_CONSTRUCT) at /Users/kats/zspace/mozilla-git/js/src/jsinterp.cpp:310 #37 0x61ca7278 in js::Interpret (cx=<optimized out>, entryFrame=<optimized out>, interpMode=<optimized out>) at /Users/kats/zspace/mozilla-git/js/src/jsinterp.cpp:2513 #38 0x61eb5a0c in UncachedInlineCall (f=..., initial=<optimized out>, pret=<optimized out>, unjittable=<optimized out>, argc=1) at /Users/kats/zspace/mozilla-git/js/src/methodjit/InvokeHelpers.cpp:343 #39 0x61eb6d30 in js::mjit::stubs::UncachedCallHelper (f=..., argc=1, lowered=false, ucr=<optimized out>) at /Users/kats/zspace/mozilla-git/js/src/methodjit/InvokeHelpers.cpp:426 ---Type <return> to continue, or q <return> to quit--- #40 0x61e9ebf8 in js::mjit::CallCompiler::update (this=0x5c295480) at /Users/kats/zspace/mozilla-git/js/src/methodjit/MonoIC.cpp:928 #41 0x61e9f41c in js::mjit::ic::Call (f=<optimized out>, ic=0x5ecf7) at /Users/kats/zspace/mozilla-git/js/src/methodjit/MonoIC.cpp:990 #42 0x61e2ed1e in JaegerStubVeneer () from /Users/kats/zspace/mozilla-git/obj-android-debug/dist/bin/libxul.so #43 0x5f02d1c4 in ?? () #44 0x5f02d1c4 in ?? ()
Severity: normal → major
Also re-nom'ing since I feel this is a betaN+ blocker now rather than a release blocker.
blocking-fennec1.0: + → ?
Summary: Touch response got worse since v14 → Infinite loop in nsContentEventHandler (was: Touch response got worse since v14)
Attached patch Possible fix? (deleted) — Splinter Review
Can you please try this patch and see if it fixes the issue? Note that I don't know why a content iterator should return null as its current node when !IsDone().
Attachment #626286 - Flags: feedback?(bugmail.mozilla)
Comment on attachment 626286 [details] [diff] [review] Possible fix? Review of attachment 626286 [details] [diff] [review]: ----------------------------------------------------------------- This does fix the infinite loop.
Attachment #626286 - Flags: feedback?(bugmail.mozilla) → feedback+
Assignee: wjohnston → bugmail.mozilla
Assignee: bugmail.mozilla → wjohnston
blocking-fennec1.0: ? → betaN+
whoops - clobbered wesj's re-assignment
Assignee: wjohnston → bugmail.mozilla
Assignee: bugmail.mozilla → ehsan
Attachment #626286 - Flags: review?(bzbarsky)
Comment on attachment 626286 [details] [diff] [review] Possible fix? r=me, but the fact that we're getting null from a !IsDone() iter is pretty weird...
Attachment #626286 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky (:bz) from comment #18) > Comment on attachment 626286 [details] [diff] [review] > Possible fix? > > r=me, but the fact that we're getting null from a !IsDone() iter is pretty > weird... Agreed. I still don't know what causes that, but as long as that is an assertion, it makes sense for us to not rely on that as loop termination condition...
https://hg.mozilla.org/integration/mozilla-inbound/rev/2d4bc03223c2 Do I need to ask for Aurora approval on blocker bugs?
Target Milestone: --- → Firefox 15
(In reply to Ehsan Akhgari [:ehsan] from comment #21) > https://hg.mozilla.org/integration/mozilla-inbound/rev/2d4bc03223c2 > > Do I need to ask for Aurora approval on blocker bugs? Yep
Comment on attachment 626286 [details] [diff] [review] Possible fix? [Approval Request Comment] Bug caused by (feature/regressing bug #): Unknown User impact if declined: See the description. :) Testing completed (on m-c, etc.): kats has tested this. Risk to taking this patch (and alternatives if risky): Minimal. String or UUID changes made by this patch: None.
Attachment #626286 - Flags: approval-mozilla-aurora?
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Attachment #626286 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
This needs to land asap on MOBILE140_2012052310_RELBRANCH as well as tip of mozilla-aurora to minimize the code change between what QA has already been testing and what we'll actually release this week.
(In reply to Alex Keybl [:akeybl] from comment #26) > This needs to land asap on MOBILE140_2012052310_RELBRANCH as well as tip of > mozilla-aurora to minimize the code change between what QA has already been > testing and what we'll actually release this week. Done: http://hg.mozilla.org/releases/mozilla-aurora/rev/a5d4e1261b2c Sorry, I did not know about this requirement.
QAwanted: we need to verify this fix esp for Beta Respin.
Tested on TB Build : https://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/mozilla-aurora-android/1337887976/ 1. go to mail.yahoo.com 2. login to mail.yahoo.com 3. play around with the tabs "Inbox", "Contacts", "Calendar" Expected: always be able to go to a link, with haptic feedback and clicked highlight that disappears Actual: Depending on how fast you click and where you click: 1) fast light tap, could produce tab highlighting or haptic feedback, but not doing anything 2) clicking close to the lettering but not on the letter, could produce haptic feedback but not go to the link 3) you can get haptic feedback, and highlighting and the going to the tab. Note: I did check that the build was after this was pushed. I suspect that the current behavior is better than when the report was made. Alec, could you check to see if this resolved the issue with you? It will be in tomorrow's nightly (as well as aurora) Could you please check with either builds tomorrow?
Naoki etc all, yes, behaviour is pretty much as expected now & problem in comment 10 is expected now. Most annoying things now are wiping out saved passwords when daily updating (started about a week ago) and Flash not working on my tablet. Great work guys! I expect FF 14 will be the break-through on Android.
**** - I need to put in an enhancement request for the Bugzilla site software to all (perhaps time-limited) ability to edit one's comment... comment 30 should read "problem in comment 10 is completely fixed now" (am typing on Android now... tiny compose-window syndrome)
Thanks, good to hear the problem has been fixed. Thanks for reporting the issue and providing all the feedback!
Verified Fixed using the various STR in this bug Nightly (05/29), Aurora (05/29) Galaxy Nexus (Android 4.0.4)
Status: RESOLVED → VERIFIED
Keywords: qawanted
This bug causes an infrequently encountered hang of Firefox 12 for desktop when inputting Japanese characters on form widgets using IME. While we haven't been able to confirm the STR, a few people still complains of such a situation. Masayuki believes this regression is the cause of those hangs and a user verified the issue couldn't be reproduced with the current Aurora channel. So it's much appreciated if you could take this fix as a 13.0.1 ride-along. Thanks.
We'll consider for 13.0.1 if we need a re-spin, although if we don't have STR for desktop, we probably won't be able to take this fix.
(In reply to Alex Keybl [:akeybl] from comment #35) > We'll consider for 13.0.1 if we need a re-spin, although if we don't have > STR for desktop, we probably won't be able to take this fix. For the STR, we need to research the actual bug of the iterator. Between 2011/12/20 - 2012/1/30, there is only one change in nsContentIterator.cpp: David Zbarsky - Bug 682611 - Part 2: Remove nsIRange; r=smaug http://hg.mozilla.org/mozilla-central/rev/3c6cfdfe7d46 However, I have no idea why it could cause this bug.
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #36) > (In reply to Alex Keybl [:akeybl] from comment #35) > > We'll consider for 13.0.1 if we need a re-spin, although if we don't have > > STR for desktop, we probably won't be able to take this fix. > > For the STR, we need to research the actual bug of the iterator. > > Between 2011/12/20 - 2012/1/30, there is only one change in > nsContentIterator.cpp: > > David Zbarsky - Bug 682611 - Part 2: Remove nsIRange; r=smaug > http://hg.mozilla.org/mozilla-central/rev/3c6cfdfe7d46 > > However, I have no idea why it could cause this bug. Yeah, I don't see anything strange in the changes. Some other bug may have caused this too. Something which just affects how contentiterator works, but doesn't actually change its code. But STR would be really really nice.
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: