Closed Bug 1444580 Opened 2 years ago Closed 2 years ago

More nsIDocument devirtualization.

Categories

(Core :: DOM: Core & HTML, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: emilio, Assigned: emilio)

Details

Attachments

(45 files, 1 obsolete file)

59 bytes, text/x-review-board-request
smaug
: review+
Details
59 bytes, text/x-review-board-request
smaug
: review+
Details
59 bytes, text/x-review-board-request
smaug
: review+
Details
59 bytes, text/x-review-board-request
smaug
: review+
Details
59 bytes, text/x-review-board-request
smaug
: review+
Details
59 bytes, text/x-review-board-request
smaug
: review+
Details
59 bytes, text/x-review-board-request
smaug
: review+
Details
59 bytes, text/x-review-board-request
smaug
: review+
Details
59 bytes, text/x-review-board-request
smaug
: review+
Details
59 bytes, text/x-review-board-request
smaug
: review+
Details
59 bytes, text/x-review-board-request
xidorn
: review+
Details
59 bytes, text/x-review-board-request
smaug
: review+
Details
4.62 KB, patch
smaug
: review+
Details | Diff | Splinter Review
2.49 KB, patch
smaug
: review+
Details | Diff | Splinter Review
4.75 KB, patch
smaug
: review+
Details | Diff | Splinter Review
1.99 KB, patch
smaug
: review+
Details | Diff | Splinter Review
1.74 KB, patch
smaug
: review+
Details | Diff | Splinter Review
2.81 KB, patch
smaug
: review+
Details | Diff | Splinter Review
9.03 KB, patch
smaug
: review+
Details | Diff | Splinter Review
5.26 KB, patch
smaug
: review+
Details | Diff | Splinter Review
3.43 KB, patch
smaug
: review+
Details | Diff | Splinter Review
2.28 KB, patch
smaug
: review+
Details | Diff | Splinter Review
4.58 KB, patch
smaug
: review+
Details | Diff | Splinter Review
1.68 KB, patch
smaug
: review+
Details | Diff | Splinter Review
11.51 KB, patch
smaug
: review+
Details | Diff | Splinter Review
5.00 KB, patch
smaug
: review+
Details | Diff | Splinter Review
2.70 KB, patch
smaug
: review+
Details | Diff | Splinter Review
4.28 KB, patch
smaug
: review+
Details | Diff | Splinter Review
4.27 KB, patch
smaug
: review+
Details | Diff | Splinter Review
2.58 KB, patch
smaug
: review+
Details | Diff | Splinter Review
21.97 KB, patch
smaug
: review+
Details | Diff | Splinter Review
6.64 KB, patch
smaug
: review+
Details | Diff | Splinter Review
1.39 KB, patch
smaug
: review+
Details | Diff | Splinter Review
3.10 KB, patch
smaug
: review+
Details | Diff | Splinter Review
5.94 KB, patch
smaug
: review+
Details | Diff | Splinter Review
3.44 KB, patch
smaug
: review+
Details | Diff | Splinter Review
1.76 KB, patch
smaug
: review+
Details | Diff | Splinter Review
8.08 KB, patch
smaug
: review+
Details | Diff | Splinter Review
6.51 KB, patch
smaug
: review+
Details | Diff | Splinter Review
883 bytes, patch
smaug
: review+
Details | Diff | Splinter Review
2.41 KB, patch
smaug
: review+
Details | Diff | Splinter Review
2.95 KB, patch
smaug
: review+
Details | Diff | Splinter Review
1.74 KB, patch
smaug
: review+
Details | Diff | Splinter Review
3.61 KB, patch
smaug
: review+
Details | Diff | Splinter Review
3.87 KB, patch
smaug
: review+
Details | Diff | Splinter Review
No description provided.
Attachment #8957771 - Flags: review?(bugs)
Comment on attachment 8957771 [details]
Bug 1444580: Devirtualize more nsIDocument bits. r=smaug

Submitting multiple patches with Phabricator is a pain.
Attachment #8957771 - Attachment is obsolete: true
Attachment #8957771 - Flags: review?(bugs)
Comment on attachment 8957784 [details]
Bug 1444580: Mark JS::Value as opaque instead of hidden.

https://reviewboard.mozilla.org/r/226774/#review232534
Attachment #8957784 - Flags: review?(xidorn+moz) → review+
Arg, mozreview is choking hard at trying to push all the extra patches I have... Will upload the rest manually...
Attached patch FindImageMapSplinter Review
Attachment #8957912 - Flags: review?(bugs)
Attached patch GetStateObjectSplinter Review
Attachment #8957915 - Flags: review?(bugs)
Attachment #8957916 - Flags: review?(bugs)
Attachment #8957917 - Flags: review?(bugs)
Attached patch Plugins stuff.Splinter Review
Attachment #8957919 - Flags: review?(bugs)
Attachment #8957920 - Flags: review?(bugs)
Attached patch ScriptLoaderSplinter Review
Attachment #8957921 - Flags: review?(bugs)
Attachment #8957922 - Flags: review?(bugs)
Attachment #8957923 - Flags: review?(bugs)
Attachment #8957924 - Flags: review?(bugs)
Attachment #8957927 - Flags: review?(bugs)
Attachment #8957930 - Flags: review?(bugs)
This one was the most non-trivial I'd say.
Attachment #8957931 - Flags: review?(bugs)
Attached patch GetChannelSplinter Review
Attachment #8957933 - Flags: review?(bugs)
Attachment #8957934 - Flags: review?(bugs)
Attachment #8957936 - Flags: review?(bugs)
Attachment #8957937 - Flags: review?(bugs)
Attachment #8957938 - Flags: review?(bugs)
Attachment #8957939 - Flags: review?(bugs)
Attachment #8957941 - Flags: review?(bugs)
Attachment #8957942 - Flags: review?(bugs)
Attached patch SanitizeSplinter Review
Attachment #8957943 - Flags: review?(bugs)
Can't devirtualize because of PluginDocument. oh well.
Attachment #8957944 - Flags: review?(bugs)
See the comment for why, otherwise we fail the rust unit tests in x32, and I can't land https://github.com/rust-lang-nursery/rust-bindgen/pull/1271 in Gecko until we require Rust 1.25, which is the next stable version.
Attachment #8957961 - Flags: review?(bugs)
Since reviewing this kind of patches requires heavy searchfox usage, it isn't quite as fast as one
might expect. But I'll try to get to these real soon.
(In reply to Olli Pettay [:smaug] from comment #50)
> Since reviewing this kind of patches requires heavy searchfox usage, it
> isn't quite as fast as one
> might expect. But I'll try to get to these real soon.

Yeah, that sounds great, thanks Olli! :)
Comment on attachment 8957774 [details]
Bug 1444580: Devirtualize more nsIDocument bits.

https://reviewboard.mozilla.org/r/226754/#review232678

::: commit-message-b24d0:4
(Diff revision 1)
> +Bug 1444580: Devirtualize more nsIDocument bits. r=smaug
> +
> +Summary:
> +Sorry for this ending up being a little big, I can split if needed.

this kind of comment shouldn't be in commit message
Attachment #8957774 - Flags: review?(bugs) → review+
Comment on attachment 8957775 [details]
Bug 1444580: Devirtualize autofocus and navigation timing stuff.

https://reviewboard.mozilla.org/r/226756/#review232680
Attachment #8957775 - Flags: review?(bugs) → review+
Comment on attachment 8957776 [details]
Bug 1444580: Devirtualize PaymentRequest stuff.

https://reviewboard.mozilla.org/r/226758/#review232682
Attachment #8957776 - Flags: review?(bugs) → review+
Comment on attachment 8957777 [details]
Bug 1444580: Devirtualize IsScriptEnabled.

https://reviewboard.mozilla.org/r/226760/#review232686
Attachment #8957777 - Flags: review?(bugs) → review+
Comment on attachment 8957778 [details]
Bug 1444580: Devirtualize a few other things.

https://reviewboard.mozilla.org/r/226762/#review232688
Attachment #8957778 - Flags: review?(bugs) → review+
Comment on attachment 8957779 [details]
Bug 1444580: Devirtualize the IntersectionObserver bits.

https://reviewboard.mozilla.org/r/226764/#review232690
Attachment #8957779 - Flags: review?(bugs) → review+
Comment on attachment 8957780 [details]
Bug 1444580: Mark nsIDocument::Dispatch final.

https://reviewboard.mozilla.org/r/226766/#review232692
Attachment #8957780 - Flags: review?(bugs) → review+
Comment on attachment 8957781 [details]
Bug 1444580: Devirtualize the visibility state stuff.

https://reviewboard.mozilla.org/r/226768/#review232694
Attachment #8957781 - Flags: review?(bugs) → review+
Comment on attachment 8957782 [details]
Bug 1444580: Devirtualize the fullscreen stuff.

https://reviewboard.mozilla.org/r/226770/#review232696
Attachment #8957782 - Flags: review?(bugs) → review+
Comment on attachment 8957783 [details]
Bug 1444580: Devirtualize GetImplementation.

https://reviewboard.mozilla.org/r/226772/#review232698
Attachment #8957783 - Flags: review?(bugs) → review+
Comment on attachment 8957785 [details]
Bug 1444580: Devirtualize the IdentifierMap stuff.

https://reviewboard.mozilla.org/r/226776/#review232700

::: dom/base/nsIDocument.h:9
(Diff revision 1)
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  #ifndef nsIDocument_h___
>  #define nsIDocument_h___
>  
> +#include "jsfriendapi.h"

This is quite unfortunate.
Attachment #8957785 - Flags: review?(bugs) → review+
Attachment #8957911 - Flags: review?(bugs) → review+
Attachment #8957912 - Flags: review?(bugs) → review+
Comment on attachment 8957914 [details] [diff] [review]
The responsive content stuff.

aha, this has plenty of unrelated changes too. But fine.
Attachment #8957914 - Flags: review?(bugs) → review+
Comment on attachment 8957915 [details] [diff] [review]
GetStateObject

Includes a mysterious newline deletion, but fine.
Attachment #8957915 - Flags: review?(bugs) → review+
Attachment #8957916 - Flags: review?(bugs) → review+
Attachment #8957917 - Flags: review?(bugs) → review+
Attachment #8957943 - Flags: review?(bugs) → review+
Attachment #8957923 - Flags: review?(bugs) → review+
Attachment #8957937 - Flags: review?(bugs) → review+
Attachment #8957933 - Flags: review?(bugs) → review+
Attachment #8957941 - Flags: review?(bugs) → review+
Attachment #8957921 - Flags: review?(bugs) → review+
Attachment #8957926 - Flags: review?(bugs) → review+
Attachment #8957940 - Flags: review?(bugs) → review+
Attachment #8957920 - Flags: review?(bugs) → review+
Attachment #8957942 - Flags: review?(bugs) → review+
Attachment #8957930 - Flags: review?(bugs) → review+
Comment on attachment 8957944 [details] [diff] [review]
Move CanHavePresentation to nsIDocument.

ok, CanSavePresentation stays still virtual.

This does move also Contains*Content, apparently because nsIDocument::CanSavePresentation uses them.
Attachment #8957944 - Flags: review?(bugs) → review+
Attachment #8957927 - Flags: review?(bugs) → review+
Comment on attachment 8957961 [details] [diff] [review]
Move mExpandoAndGeneration back to nsDocument for now.

>+static void
>+IncrementExpandoGeneration(nsIDocument& doc)
aDoc
Attachment #8957961 - Flags: review?(bugs) → review+
Comment on attachment 8957961 [details] [diff] [review]
Move mExpandoAndGeneration back to nsDocument for now.

With this patch, could we not have #include "jsfriendapi.h" in nsIDocument.h ?
Attachment #8957925 - Flags: review?(bugs) → review+
Attachment #8957928 - Flags: review?(bugs) → review+
Attachment #8957934 - Flags: review?(bugs) → review+
Attachment #8957939 - Flags: review?(bugs) → review+
Attachment #8957936 - Flags: review?(bugs) → review+
Attachment #8957938 - Flags: review?(bugs) → review+
Attachment #8957919 - Flags: review?(bugs) → review+
Comment on attachment 8957935 [details] [diff] [review]
Move OnPageShow / OnPageHide to nsIDocument, and devirtualize OnPageShow

ok, OnPageShow is still virtual because ImageDocument overrides it.
So the commit message is a tad wrong.
Attachment #8957935 - Flags: review?(bugs) → review+
Attachment #8957932 - Flags: review?(bugs) → review+
Attachment #8957922 - Flags: review?(bugs) → review+
Attachment #8957918 - Flags: review?(bugs) → review+
Attachment #8957924 - Flags: review?(bugs) → review+
Comment on attachment 8957931 [details] [diff] [review]
External resource load stuff.

So nsSubDocEnumFunc is defined now twice, in different classes. I guess that is fine.
Attachment #8957931 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #67)
> Comment on attachment 8957961 [details] [diff] [review]
> Move mExpandoAndGeneration back to nsDocument for now.
> 
> With this patch, could we not have #include "jsfriendapi.h" in nsIDocument.h
> ?

Yes, move that include back to nsDocument.h
*I'll move that
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d9a86837189b
Devirtualize more nsIDocument bits. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/b6e079d22001
Devirtualize autofocus and navigation timing stuff. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f04d67a53d8
Devirtualize PaymentRequest stuff. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/a9fbfa2fc6ef
Devirtualize IsScriptEnabled. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/63372eeecb17
Devirtualize a few other things. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d36b03f0bc4
Devirtualize the IntersectionObserver bits. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/d616bcc96e36
Mark nsIDocument::Dispatch final. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/8edab3c72a62
Devirtualize the visibility state stuff. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/5abd81195c99
Devirtualize the fullscreen stuff. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/e654cbd9fd4b
Devirtualize GetImplementation. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f532b7d5997
Devirtualize the IdentifierMap stuff. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ec71c2eb977
Devirtualize nsIDocument::CreateElement / CreateElementNS. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/f22a28905f0e
Devirtualize FindImageMap. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/66f34961b2a0
Devirtualize the responsive content stuff. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/0c316647cefa
Devirtualize GetStateObject. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/13fc90a09d42
Devirtualize NotifyLayerManagerRecreated. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc1dd3bd0f3d
Devirtualize subdocument stuff. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/d32d9861f9f5
Devirtualize GetRootElementInternal, and move nsINode overrides to nsIDocument. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f414e52792a
Devirtualize plugins stuff. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/3651b4affcae
Devirtualize Web Animations stuff. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/d88013d4c8ec
Devirtualize ScriptLoader. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/c60390623fa5
Devirtualize the scroll to ref stuff. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b7ec8478e33
Devirtualize GetCurrentContentSink. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/d974c5995002
Devirtualize the preload / preconnect stuff. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/2de728da0e23
Devirtualize GetBoxObjectFor / ClearBoxObjectFor. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/67e8b8e07b83
Devirtualize GetAnonymousElementByAttribute. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/ada1aea7e99d
Devirtualize more animation stuff. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/8b49b1134947
Devirtualize event handling suppression stuff. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/f3055b17e7b5
Devirtualize GetTemplateContentsOwner. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/3058e5b00ff6
Devirtualize the external resource stuff. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/0238e1f73640
Devirtualize pointer lock and screen orientation stuff. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/7dad8de751d8
Devirtualize GetChannel. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca5e80b66c8a
Devirtualize CreateShell / DeleteShell. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/eaba06c286c2
Move OnPageShow / OnPageHide to nsIDocument, devirtualize OnPageHide. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a1ca0f7d2d2
Devirtualize NodesFromRectHelper. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/24176f48062e
Devirtualize FlushSkinBindings. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0f058e80094
Devirtualize frame loader stuff. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/325839c5d0a2
Devirtualize UnblockDOMContentLoaded. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/471a0e9c8f74
Remove reference to non-existing nsDocument::UpdateScreenOrientation. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/983c290db1d6
Devirtualize WillDispatchMutationEvent / MutationEventDispatched. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1256481c7c9
Devirtualize GetLayoutHistoryState. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/2bf3fbd4d8e9
Devirtualize Sanitize. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/c641a8b309e1
Devirtualize CanSavePresentation. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/55dc61b6ab44
Move mExpandoAndGeneration back to nsDocument for now. r=smaug
https://hg.mozilla.org/mozilla-central/rev/d9a86837189b
https://hg.mozilla.org/mozilla-central/rev/b6e079d22001
https://hg.mozilla.org/mozilla-central/rev/9f04d67a53d8
https://hg.mozilla.org/mozilla-central/rev/a9fbfa2fc6ef
https://hg.mozilla.org/mozilla-central/rev/63372eeecb17
https://hg.mozilla.org/mozilla-central/rev/0d36b03f0bc4
https://hg.mozilla.org/mozilla-central/rev/d616bcc96e36
https://hg.mozilla.org/mozilla-central/rev/8edab3c72a62
https://hg.mozilla.org/mozilla-central/rev/5abd81195c99
https://hg.mozilla.org/mozilla-central/rev/e654cbd9fd4b
https://hg.mozilla.org/mozilla-central/rev/7f532b7d5997
https://hg.mozilla.org/mozilla-central/rev/0ec71c2eb977
https://hg.mozilla.org/mozilla-central/rev/f22a28905f0e
https://hg.mozilla.org/mozilla-central/rev/66f34961b2a0
https://hg.mozilla.org/mozilla-central/rev/0c316647cefa
https://hg.mozilla.org/mozilla-central/rev/13fc90a09d42
https://hg.mozilla.org/mozilla-central/rev/cc1dd3bd0f3d
https://hg.mozilla.org/mozilla-central/rev/d32d9861f9f5
https://hg.mozilla.org/mozilla-central/rev/3f414e52792a
https://hg.mozilla.org/mozilla-central/rev/3651b4affcae
https://hg.mozilla.org/mozilla-central/rev/d88013d4c8ec
https://hg.mozilla.org/mozilla-central/rev/c60390623fa5
https://hg.mozilla.org/mozilla-central/rev/4b7ec8478e33
https://hg.mozilla.org/mozilla-central/rev/d974c5995002
https://hg.mozilla.org/mozilla-central/rev/2de728da0e23
https://hg.mozilla.org/mozilla-central/rev/67e8b8e07b83
https://hg.mozilla.org/mozilla-central/rev/ada1aea7e99d
https://hg.mozilla.org/mozilla-central/rev/8b49b1134947
https://hg.mozilla.org/mozilla-central/rev/f3055b17e7b5
https://hg.mozilla.org/mozilla-central/rev/3058e5b00ff6
https://hg.mozilla.org/mozilla-central/rev/0238e1f73640
https://hg.mozilla.org/mozilla-central/rev/7dad8de751d8
https://hg.mozilla.org/mozilla-central/rev/ca5e80b66c8a
https://hg.mozilla.org/mozilla-central/rev/eaba06c286c2
https://hg.mozilla.org/mozilla-central/rev/0a1ca0f7d2d2
https://hg.mozilla.org/mozilla-central/rev/24176f48062e
https://hg.mozilla.org/mozilla-central/rev/e0f058e80094
https://hg.mozilla.org/mozilla-central/rev/325839c5d0a2
https://hg.mozilla.org/mozilla-central/rev/471a0e9c8f74
https://hg.mozilla.org/mozilla-central/rev/983c290db1d6
https://hg.mozilla.org/mozilla-central/rev/c1256481c7c9
https://hg.mozilla.org/mozilla-central/rev/2bf3fbd4d8e9
https://hg.mozilla.org/mozilla-central/rev/c641a8b309e1
https://hg.mozilla.org/mozilla-central/rev/55dc61b6ab44
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
(In reply to Emilio Cobos Álvarez [:emilio] from comment #14)
> Comment on attachment 8957771 [details]
> Bug 1444580: Devirtualize more nsIDocument bits. r=smaug
> 
> Submitting multiple patches with Phabricator is a pain.

Is a bug filed to improve it before we are forced to use Phabricator?
You need to log in before you can comment on or make changes to this bug.