Assertion failure: "should have already reflowed the kid" with DRAWWINDOW_DO_NOT_FLUSH

RESOLVED FIXED in mozilla25

Status

()

Core
SVG
--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: heycam)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
mozilla25
x86_64
Mac OS X
assertion, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
Created attachment 766554 [details]
testcase

1. Install https://www.squarefree.com/extensions/domFuzzLite3.xpi 
2. Set
     user_pref("svg.text.css-frames.enabled", true);
3. Load the testcase

Assertion failure: !(((kid)->GetStateBits() & ((nsFrameState(1) << (10)) | (nsFrameState(1) << (12)))) != 0) (should have already reflowed the kid), at layout/svg/nsSVGTextFrame2.cpp:4837

This assertion was added in bug 885642.

DRAWWINDOW_DO_NOT_FLUSH is used by Firefox's thumbnail service, IIRC.  So there's probably a way to trigger this bug without the extension.
(Reporter)

Comment 1

5 years ago
Created attachment 766555 [details]
stack (gdb)
I think the best we can do is just not paint the text, which is what we are doing now (but also asserting).  It'd be good to keep the assertion in there for other cases where we know we should have reflowed the text.  I wonder if there's a way to detect that we're in a DRAWWINDOW_DO_NOT_FLUSH?
roc, would it be OK to store a bit (only #ifdef DEBUG) in PresShell::mRenderFlags while we are painting the window under DrawWindow(..., DRAWWINDOW_DO_NOT_FLUSH) that nsSVGTextFrame2::PaintSVG could then check to see whether it can skip the assertion that the text frames are not dirty?

Or otherwise, have a flag on the nsDisplayListBuilder that nsDisplaySVGText::Paint can look at?
Flags: needinfo?(roc)
(In reply to Cameron McCormack (:heycam) from comment #3)
> roc, would it be OK to store a bit (only #ifdef DEBUG) in
> PresShell::mRenderFlags while we are painting the window under
> DrawWindow(..., DRAWWINDOW_DO_NOT_FLUSH) that nsSVGTextFrame2::PaintSVG
> could then check to see whether it can skip the assertion that the text
> frames are not dirty?

Yes I think that's reasonable.
Flags: needinfo?(roc)
Created attachment 767027 [details] [diff] [review]
patch

I wasn't sure that it was really worth the clutter of wrapping things in #ifdefs in the end.
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #767027 - Flags: review?(roc)
Comment on attachment 767027 [details] [diff] [review]
patch

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

::: testing/specialpowers/content/specialpowersAPI.js
@@ +1134,5 @@
> +      flags |= ctx.DRAWWINDOW_DRAW_CARET;
> +    }
> +    if (options.DRAWWINDOW_DO_NOT_FLUSH) {
> +      flags |= ctx.DRAWWINDOW_DO_NOT_FLUSH;
> +    }

I think you can write something like
  for (var option in options) {
    flags |= ctx[option];
  }
Hooray Javascript!
Attachment #767027 - Flags: review?(roc) → review+
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #7)
> I think you can write something like
>   for (var option in options) {
>     flags |= ctx[option];
>   }
> Hooray Javascript!

Good one. :)

https://hg.mozilla.org/integration/mozilla-inbound/rev/d7c237784ce9
https://hg.mozilla.org/integration/mozilla-inbound/rev/c965b82024eb

I had to do:

  for (var option in options) {
    flags |= options[option] && ctx[option];
  }

to make { DRAWWINDOW_DO_NOT_FLUSH: false } not set that flag.
https://hg.mozilla.org/mozilla-central/rev/c965b82024eb
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.