search

KawaiiBASIC / classilla

Automatically exported from code.google.com/p/classilla
0 stars 0 forks source link

Make layout abort reliable until the new layout engine is constructed [paypal, others] #62

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Paypal used to work in 9.0, then they changed something and it started
freezing as reported by users. I fixed this in 9.0.4 (unsure what) and now
Paypal is freezing again.

Stack trace indicates a long, ?infinite reflow loop.

Original issue reported on code.google.com by classi...@floodgap.com on 15 Oct 2009 at 1:39

GoogleCodeExporter commented 9 years ago 
This bug is really hard to track down. Paypal will intermittently come up, and
sometimes it won't.

Scrollframes are being involved, however.

Original comment by classi...@floodgap.com on 20 Oct 2009 at 4:28

GoogleCodeExporter commented 9 years ago 
In nsBlockFrame::ReflowInlineFrames, there is code for aborting on 
mReflowDepth, plus
a breaker for spins. It may be better to simply NS_ABORT() if we get over 30 
and at
least save Classilla, even if we don't know why this is happening. It does not 
look
like a low memory situation, especially not on the 1.5GB development box.

Original comment by classi...@floodgap.com on 18 Jan 2010 at 2:13

GoogleCodeExporter commented 9 years ago 
Ran it again and ::Reflow is being called 10 or 15 times.

This is not any better with overflow branch.

I think I need to solve this for 9.1, if with nothing else than calling the 
frame
complete and bailing out.

Original comment by classi...@floodgap.com on 30 Jan 2010 at 7:04

GoogleCodeExporter commented 9 years ago 
mReflowState.mReflowDepth contains our current reflow depth. It is also checked 
in
nsFrame::IsFrameTreeTooDeep, where MAX_FRAME_DEPTH is defined as 
MAX_REFLOW_DEPTH+4.
MAX_REFLOW_DEPTH is defined as 75 in content/nsIHTMLContentSink.h, where bug 
55095
reduced it to 75 (from 200 for Windows/Linux). Perhaps we need to reduce it 
again,
but let's see what M55095 was doing.

Original comment by classi...@floodgap.com on 12 Feb 2010 at 7:13

GoogleCodeExporter commented 9 years ago 
However, M42138 also implies that IsFrameTreeTooDeep() has side-effects, and 
defines
#define MAX_DEPTH_FOR_LIST_RENUMBERING 200 in nsBlockFrame.cpp. I suspect that 
this
is not our bug, however.

Serendipitously, nsBlockFrame *does* call IsFrameTreeTooDeep() as part of 
::Reflow,
and does correctly set NS_FRAME_COMPLETE if it indicates it is. 
nsInlineFrame.cpp
does the same thing, so pending M55095 I think the temporary solution is to 
reduce
MAX_REFLOW_DEPTH.

Original comment by classi...@floodgap.com on 12 Feb 2010 at 7:17

GoogleCodeExporter commented 9 years ago 
NOT the solution. mReflowDepth at worst gets to around 15 before it hangs up, 
when
examining the stack. Setting it lower than that ruins many sites that work 
normally,
and causes large portions of content not to appear. We'll have to find another
workaround.

Original comment by classi...@floodgap.com on 12 Feb 2010 at 7:45

GoogleCodeExporter commented 9 years ago 
Report from Shay of another site affected by this. The bug is mercifully rare 
but
unrecoverable without Force Quitting and I can't figure out what the reflow 
system is
getting stuck on. This may not be fixable without replacing layout entirely.

In the meantime, I hacked in this workaround to 
nsBlockFrame::ReflowDirtyLines():

  for ( ; line != line_end; ++line, aState.AdvanceToNextLine()) {

  // Classilla issue 62
  // This doesn't fix our reflow problem, but it gives us an escape hatch.
  // Essentially we simply declare the reflow done as long as the user holds the
  // Cmd-. combination down, allowing reflow to gracefully slide to an incomplete
  // but stable halt. This is the best we can do until I can fix layout definitively
  // (or update it to a later Mozilla).
    KeyMap keymap;
    GetKeys(keymap);
    if (keymap[1] == 8421376) {
        rv = NS_OK;
        // Turn everything off so we take as quick an exit as possible
        keepGoing = PR_FALSE;
        reflowedFloat = PR_FALSE;
        lastLineMovedUp = PR_FALSE;
        needToRecoverState = PR_FALSE;
        repositionViews = PR_FALSE;
        foundAnyClears = PR_FALSE;
        goto skip_it_all; // at the bottom of the line iterator
    }
    // end issue

This solves the issue and even makes layout look reasonably sane on his 
reported site
(the URL was

http://h30434.www3.hp.com/t5/Monitors-and-video/2159m-problem/m-p/111553

). On Paypal, the scrollframe handling the account display on the main page 
won't
show, but the site is now useable, so I am downgrading this to high and changing
summary. Still, this might not be repairable in Classilla as it stands.

Original comment by classi...@floodgap.com on 25 Feb 2010 at 6:02

GoogleCodeExporter commented 9 years ago 
Er, it solves the freeze. It does not solve the underlying problem. Natch.

Original comment by classi...@floodgap.com on 25 Feb 2010 at 6:07

GoogleCodeExporter commented 9 years ago 
Sakuya reports there are still some situations where layout abort does not 
work. I
probably have to throw some GetKeys() in a couple other places. The test case is

http://www.bhphotovideo.com

where adding an item to the cart causes the cart to display and the browser to 
hang
in reflow. MacsBug stack trace looks like this:

  Calling chain using A6/R1 links
   Back chain  ISA  Caller
   00000000    PPC  295F2410
   2E612280    PPC  295DABF4  main+00144
   2E612220    PPC  295DA2E8  main1(int, char**, nsISupports*)+00A08
   2E612080    PPC  28A4E508  nsAppShellService::Run()+00018
   2E612040    PPC  28A08888  nsAppShell::Run()+00038
   2E612000    PPC  28A090DC  nsMacMessagePump::DoMessagePump()+0003C
   2E611FB0    PPC  28A093C4  nsMacMessagePump::DispatchEvent(int, EventRecord*)
+00154
   2E611F70    PPC  2EFAA8D0  Repeater::DoRepeaters(const EventRecord&)+00030
   2E611F30    PPC  289F274C  nsMacNSPREventQueueHandler::RepeatAction(const Eve
ntRecord&)+0000C
   2E611EF0    PPC  289F28A8  nsMacNSPREventQueueHandler::ProcessPLEventQueue()+
00058
   2E611EA0    PPC  2947C548  nsEventQueueImpl::ProcessPendingEvents()+00038
   2E611E50    PPC  294D8688  PL_ProcessPendingEvents+00088
   2E611E10    PPC  294D881C  PL_HandleEvent+0001C
   2E611DD0    PPC  28B82D60  HandlePLEvent(ReflowEvent*)+000C0
   2E611D70    PPC  28B833B0  PresShell::ProcessReflowCommands(int)+00200
   2E611C90    PPC  28B71294  IncrementalReflow::Dispatch(nsIPresContext*, nsHTM
LReflowMetrics&, const nsSize&, nsIRenderingContext&)+00164
   2E611B60    PPC  28C4F40C  ViewportFrame::Reflow(nsIPresContext*, nsHTMLReflo
wMetrics&, const nsHTMLReflowState&, unsigned int&)+0013C
   2E611910    PPC  28B5A0F8  nsContainerFrame::ReflowChild(nsIFrame*, nsIPresCo
ntext*, nsHTMLReflowMetrics&, const nsHTMLReflowState&, int, int, unsigned int, 
unsigned int&)+00088
   2E6118B0    PPC  28CB2C34  nsGfxScrollFrame::Reflow(nsIPresContext*, nsHTMLRe
flowMetrics&, const nsHTMLReflowState&, unsigned int&)+00044
   2E611860    PPC  28C800DC  nsBoxFrame::Reflow(nsIPresContext*, nsHTMLReflowMe
trics&, const nsHTMLReflowState&, unsigned int&)+001DC
   2E6117A0    PPC  28CC3318  nsBox::Layout(nsBoxLayoutState&)+00038
   2E611760    PPC  28CB3DA0  nsGfxScrollFrame::DoLayout(nsBoxLayoutState&)+0004
0
   2E611710    PPC  28CB41C8  nsGfxScrollFrameInner::Layout(nsBoxLayoutState&)+0
01F8
   2E611550    PPC  28CB3CE8  nsGfxScrollFrameInner::LayoutBox(nsBoxLayoutState&
MORE (you've seen 23%):
, nsIBox*, const nsRect&)+00018
   2E611510    PPC  28CCBF04  nsContainerBox::LayoutChildAt(nsBoxLayoutState&, n
sIBox*, const nsRect&)+00114
   2E6114B0    PPC  28CC3318  nsBox::Layout(nsBoxLayoutState&)+00038
   2E611470    PPC  28CF4B68  nsScrollBoxFrame::DoLayout(nsBoxLayoutState&)+0020
8
   2E611320    PPC  28CC3318  nsBox::Layout(nsBoxLayoutState&)+00038
   2E6112E0    PPC  28CC8958  nsBoxToBlockAdaptor::DoLayout(nsBoxLayoutState&)+0
0158
   2E611210    PPC  28CC8F50  nsBoxToBlockAdaptor::Reflow(nsBoxLayoutState&, nsI
PresContext*, nsHTMLReflowMetrics&, const nsHTMLReflowState&, unsigned int&, int
, int, int, int, int)+003A0
   2E611090    PPC  28D0B7B4  CanvasFrame::Reflow(nsIPresContext*, nsHTMLReflowM
etrics&, const nsHTMLReflowState&, unsigned int&)+001B4
   2E610F00    PPC  28B5A0F8  nsContainerFrame::ReflowChild(nsIFrame*, nsIPresCo
ntext*, nsHTMLReflowMetrics&, const nsHTMLReflowState&, int, int, unsigned int, 
unsigned int&)+00088
   2E610EA0    PPC  28C08E28  nsBlockFrame::Reflow(nsIPresContext*, nsHTMLReflow
Metrics&, const nsHTMLReflowState&, unsigned int&)+00498
   2E610C10    PPC  28C0AFEC  nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&
)+0033C
   2E610AC0    PPC  28C0B904  nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLi
neList_iterator, int*, int)+000B4
   2E610990    PPC  28C0D504  nsBlockFrame::ReflowBlockFrame(nsBlockReflowState&
, nsLineList_iterator, int*)+00574
   2E6105F0    PPC  28C32874  nsBlockReflowContext::ReflowBlock(const nsRect&, i
nt, nsCollapsingMargin&, int, int, nsMargin&, nsHTMLReflowState&, unsigned int&)
+00554
   2E610560    PPC  28C08E28  nsBlockFrame::Reflow(nsIPresContext*, nsHTMLReflow
Metrics&, const nsHTMLReflowState&, unsigned int&)+00498
   2E6102D0    PPC  28C0AFEC  nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&
)+0033C
   2E610180    PPC  28C0B904  nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLi
neList_iterator, int*, int)+000B4
   2E610050    PPC  28C0D504  nsBlockFrame::ReflowBlockFrame(nsBlockReflowState&
, nsLineList_iterator, int*)+00574
   2E60FCB0    PPC  28C32874  nsBlockReflowContext::ReflowBlock(const nsRect&, i
nt, nsCollapsingMargin&, int, int, nsMargin&, nsHTMLReflowState&, unsigned int&)
+00554

Maybe something in ReflowLine would be nice and central.

Original comment by classi...@floodgap.com on 7 Apr 2010 at 1:03

GoogleCodeExporter commented 9 years ago

Original comment by classi...@floodgap.com on 7 Apr 2010 at 1:04

GoogleCodeExporter commented 9 years ago

Original comment by classi...@floodgap.com on 7 Apr 2010 at 1:17

GoogleCodeExporter commented 9 years ago 
CSSCompressedDataBlock's RuleInfo mapping also is a common source of freezes. 
Flickr
with 9.2-internal's new JavaScript comes to mind. I wonder if we can simply 
degrade
the node or something, or add this code to it, at least.

Original comment by classi...@floodgap.com on 18 May 2010 at 5:07

GoogleCodeExporter commented 9 years ago 
Added code to the MapRuleInfoInto so that it cannot exceed a maximum number of
iterations. This solves the problem for now, albeit with a performance hit on
affected pages.

Original comment by classi...@floodgap.com on 23 May 2010 at 9:00

GoogleCodeExporter commented 9 years ago 
No layout crashes or freezes since. Closing as verified, since we'll be
reimplementing issue 1 for 9.3 anyway.

Original comment by classi...@floodgap.com on 30 May 2010 at 10:18

GoogleCodeExporter commented 9 years ago 
One last note: added a hole to ReflowLine() with a check there too. This 
catches MSN
and a few other sites.

Original comment by classi...@floodgap.com on 4 Jun 2010 at 4:16

GoogleCodeExporter commented 9 years ago 
Sakuya:

> Issue 62 - Make layout abort reliable - is marked as verified, but the 
> test case still works for me in 9.2 and 9.2.1 - adding an item to the 
> cart at bhphotovideo.com and subsequently displaying it hangs the 
> browser and command-period isn't succeeding.

Fix for 9.2.2

Original comment by classi...@floodgap.com on 29 Aug 2010 at 5:40

GoogleCodeExporter commented 9 years ago

Original comment by classi...@floodgap.com on 29 Aug 2010 at 5:44

GoogleCodeExporter commented 9 years ago 
They must have changed their site, because I can't reproduce this. Switching 
back to Fixed.

Original comment by classi...@floodgap.com on 6 Feb 2011 at 7:01