Closed Bug 1295370 Opened 3 years ago Closed 3 years ago

stylo: Make nsTextControlFrame::AppendAnonymousContentTo safe to call from parallel style traversal

Categories

(Core :: Editor, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: bholley, Assigned: bholley)

References

Details

Attachments

(3 files)

In Stylo, we need to be able to find native anonymous content during parallel traversal. This involves doing a QueryFrame to nsIAnonymousContentCreator and then calling AppendAnonymousContentTo. This generally works fine, because most NAC-creating frames just cache the anonymous content as a member. But nsTextFrame does various not-thread-safe things, which is what this bug is designed to fix.

In bug 1294915, we are exploring static analysis approaches to detecting these issues a la rooting analysis.
Including this header without this causes compilation problems.
Attachment #8781331 - Flags: review?(ehsan)
This is problematic when we want to call AppendAnonymousContentTo off-main-thread.
AFAICT this doesn't change behavior, since all the callers of GetRootNode should
occur after we've already called BindToFrame. However, it makes it easier for
future static analysis to see that we don't trigger node creation from
nsTextControlFrame::AppendAnonymousContentTo.
Attachment #8781333 - Flags: review?(ehsan)
Attachment #8781332 - Flags: review?(ehsan)
Attachment #8781331 - Flags: review?(ehsan) → review+
Attachment #8781332 - Flags: review?(ehsan) → review+
Comment on attachment 8781333 [details] [diff] [review]
Part 3 - Stop lazily creating the root node in nsTextEditorState::GetRootFrame.

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

::: dom/html/nsTextEditorState.cpp
@@ +1156,5 @@
>    }
>  
>    mBoundFrame = aFrame;
>  
> +  CreateRootNode();

Can you please check the return value as well?
Attachment #8781333 - Flags: review?(ehsan) → review+
Pushed by bholley@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a07c43ca2cc
Add missing include. r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ba56b086fe3
Avoid QIing a DOM node in nsTextControlFrame::AppendAnonymousContentTo. r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/a60d54195a3a
Stop lazily creating the root node in nsTextEditorState::GetRootFrame. r=ehsan
You need to log in before you can comment on or make changes to this bug.