[FIX]Text inputs cause reentry into frame construction ("WARNING: recurring into frame construction: 'mPresContext->mLayoutPhaseCount[eLayoutPhase_FrameC] == 0'")

RESOLVED FIXED in mozilla1.9.2a1

Status

()

P4
normal
RESOLVED FIXED
13 years ago
10 years ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

(Blocks: 3 bugs)

Trunk
mozilla1.9.2a1
x86
Linux
Points:
---
Dependency tree / graph
Bug Flags:
wanted-next +
wanted1.9.1 +
wanted1.9 -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [dbaron-1.9:RsCe], URL)

Attachments

(2 attachments, 1 obsolete attachment)

STEPS TO REPRODUCE:

1)  Enable assert mentioned in bug 334460
2)  Load the testcase URL in this bug

What happens is that once we finish setting up the text control frame we do editor init, while still inside frame construction in general.  Editor messes with our anonymous content, causing reentry.  Note that bug 217201 made the current setup suck less than it used to, but ideally we'd defer the editor init to after we're all done with frame construction.
Summary: Text inputs cause reentry into frame construction → Text inputs cause reentry into frame construction ("WARNING: recurring into frame construction: 'mPresContext->mLayoutPhaseCount[eLayoutPhase_FrameC] == 0'")

Updated

13 years ago
Blocks: 341986

Updated

12 years ago
Flags: blocking1.9?
This bug seems to be a leading cause of console spewage.
Unfortunately, it's kinda hard to fix as things stand.  See bug 221820 for my last attempt.
Depends on: 221820

Updated

12 years ago
No longer blocks: 334460
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 334450
This is not a dup of bug 334450. That bug is about nsTextBoxFrame, this one is 
about nsTextControlFrame
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

Updated

12 years ago
Blocks: 334460
Targeting/blocking M9 per request by dbaron.
Target Milestone: --- → mozilla1.9 M9
Not going to make M9, realistically, but we should really try to get it done pretty soon.
Status: REOPENED → NEW
Target Milestone: mozilla1.9 M9 → ---
I'll try taking a look at this and seeing if there's something I can come up with...
Assignee: nobody → dbaron
Flags: wanted1.9-
Flags: wanted1.9+
Flags: wanted-next+
Flags: wanted1.9.1?
Flags: wanted1.9.1? → wanted1.9.1+

Updated

10 years ago
Blocks: 457537
Created attachment 357427 [details] [diff] [review]
Proposed fix

This basically takes a large chunk of the patch in bug 221820, but NOT the actual lazy init of the editor.  Instead, we set up a bunch of our things (like our text listener, etc) under CreateAnonymousContent, and then put off editor init (a combination of the current InitEditor and the current CreateFrameFor) to a script runner.  That will make sure that it runs as soon as the scriptblocker (which should be in place, since we're in frame construction!) gets removed.

A lot of the patch is just moving code around without changing it, but it could still use a somewhat careful read.  I haven't found any issues with the new ordering, but this is somewhat fragile code.  :(
Assignee: dbaron → bzbarsky
Status: NEW → ASSIGNED
Attachment #357427 - Flags: superreview?(roc)
Attachment #357427 - Flags: review?(mats.palmgren)
(Assignee)

Updated

10 years ago
Summary: Text inputs cause reentry into frame construction ("WARNING: recurring into frame construction: 'mPresContext->mLayoutPhaseCount[eLayoutPhase_FrameC] == 0'") → [FIX]Text inputs cause reentry into frame construction ("WARNING: recurring into frame construction: 'mPresContext->mLayoutPhaseCount[eLayoutPhase_FrameC] == 0'")
(Assignee)

Updated

10 years ago
Blocks: 221820
No longer depends on: 221820
Attachment #357427 - Flags: superreview?(roc) → superreview+
Comment on attachment 357427 [details] [diff] [review]
Proposed fix

r=mats

> layout/forms/nsTextControlFrame.cpp

You can just remove nsTextControlFrame::CreateFrameFor() since
"return nsnull" is the default behaviour.

> nsTextControlFrame::AttributeChanged
> {
>   if (!mEditor || !mSelCon) 
>-    return NS_ERROR_NOT_INITIALIZED;
>+    return NS_OK;

Why not "return nsStackFrame::AttributeChanged(...)" ?


> layout/forms/nsTextControlFrame.h

s/NSIGFXTEXTCONTROLFRAME2/NSITEXTCONTROLFRAME/g

>+  void DelayedEditorInit();

I'd prefer if it were non-public.


> layout/generic/nsIAnonymousContentCreator.h

You can remove "class nsPresContext;" while you're here.

Also, this comment seems out of date:

> HTML frames like nsFileControlFrame currently use this as well as XUL frames
> like nsScrollbarFrame and nsSliderFrame.

Neither nsScrollbarFrame/nsSliderFrame use it AFAICT.
Attachment #357427 - Flags: review?(mats.palmgren) → review+
Created attachment 359367 [details] [diff] [review]
Updated to comments

> You can just remove nsTextControlFrame::CreateFrameFor()

Good catch.  Done.

> Why not "return nsStackFrame::AttributeChanged(...)" ?

We never call that, actually, and even the call to nsBoxFrame looks weird...  I'll make this call nsBoxFrame, though.  Should be safe enough.

> s/NSIGFXTEXTCONTROLFRAME2/NSITEXTCONTROLFRAME/g

Done.

> I'd prefer if it were non-public.

OK.  Moved to a friend class, I guess.

> Also, this comment seems out of date:

Fixed.
Attachment #357427 - Attachment is obsolete: true
Pushed http://hg.mozilla.org/mozilla-central/rev/0c39d79738c4
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Depends on: 477700
Depends on: 478219
Depends on: 492387
You need to log in before you can comment on or make changes to this bug.