Closed
Bug 1422633
Opened 8 years ago
Closed 8 years ago
Inline nsDocument::ThreadSafeGetDocumentState.
Categories
(Core :: DOM: Core & HTML, enhancement, P2)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: emilio, Assigned: emilio)
References
Details
Attachments
(2 files)
It's a totally unnecessary function call, that shows up on stylo-chrome profiles.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 3•8 years ago
|
||
mozreview-review |
Comment on attachment 8934021 [details]
Bug 1422633: Inline the Rust bits of GetDocumentState too.
https://reviewboard.mozilla.org/r/204948/#review210388
Attachment #8934021 -
Flags: review?(xidorn+moz) → review+
Comment 4•8 years ago
|
||
mozreview-review |
Comment on attachment 8934020 [details]
Bug 1422633: Inline nsIDocument::ThreadSafeGetDocumentState().
https://reviewboard.mozilla.org/r/204946/#review210548
::: dom/base/nsIDocument.h:2644
(Diff revision 1)
> /**
> * Returns the document state.
> * Document state bits have the form NS_DOCUMENT_STATE_* and are declared in
> * nsIDocument.h.
> */
> - virtual mozilla::EventStates GetDocumentState() = 0;
> + mozilla::EventStates GetDocumentState() {
{ goes to its own line
::: dom/base/nsIDocument.h:2652
(Diff revision 1)
> + }
> +
> + // GetDocumentState() mutates the state due to lazy resolution;
> + // and can't be used during parallel traversal. Use this instead,
> + // and ensure GetDocumentState() has been called first.
> + // This will assert if the state is stale.
Not about this bug, since you just move code, but what asserts anything here?
::: dom/base/nsIDocument.h:2653
(Diff revision 1)
> +
> + // GetDocumentState() mutates the state due to lazy resolution;
> + // and can't be used during parallel traversal. Use this instead,
> + // and ensure GetDocumentState() has been called first.
> + // This will assert if the state is stale.
> + mozilla::EventStates ThreadSafeGetDocumentState() const {
{ goes to its own line
Attachment #8934020 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 5•8 years ago
|
||
(In reply to Olli Pettay [:smaug] (ni?s and f?s processed after r?s) from comment #4)
> ::: dom/base/nsIDocument.h:2652
> (Diff revision 1)
> > + }
> > +
> > + // GetDocumentState() mutates the state due to lazy resolution;
> > + // and can't be used during parallel traversal. Use this instead,
> > + // and ensure GetDocumentState() has been called first.
> > + // This will assert if the state is stale.
>
> Not about this bug, since you just move code, but what asserts anything here?
Hmm, yeah, I'd have bet this used to have a `mGotDocumentState` assertion... I think I could break it though... I think keeping it asynchronously is not super-useful... I'll file a followup.
Updated•8 years ago
|
Priority: -- → P2
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5cf2158f9740
Inline nsIDocument::ThreadSafeGetDocumentState(). r=smaug
https://hg.mozilla.org/integration/autoland/rev/d3afac5121cc
Inline the Rust bits of GetDocumentState too. r=xidorn
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5cf2158f9740
https://hg.mozilla.org/mozilla-central/rev/d3afac5121cc
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in
before you can comment on or make changes to this bug.
Description
•