1391110 - Convert FrameLoader bindings to WebIDL

Status: RESOLVED FIXED

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

Description

[Kris Maglione [:kmag]]

(and possibly MessageManager too)

I'm looking at profiles of tab code where 13ms of a 50ms block is XPConnect overhead from frameloaders and webnavigation objects, which should drop to approximately 0 for WebIDL.

Presumably this shows up in a lot of other places, too.

Comment 1

[Kris Maglione [:kmag]]

Splitting this into two bugs, since the docShell case is much more complicated.

Comment 2

[Kris Maglione [:kmag]]

Attachment: Bug 1391110: Part 1 - Convert FrameLoader bindings to WebIDL.

PConnect wrapper overhead for this interface has been showing up heavily in a
lot of my profiles, in some places accounting for 50ms of the 80ms we spend
getting getting <browser> messageManagers. This improves the situation
considerably.

Review commit: https://reviewboard.mozilla.org/r/170460/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/170460/

Comment 3

[Kris Maglione [:kmag]]

Attachment: Bug 1391110: Part 2 - Reparent FrameLoader wrapper when owner content changes.

We allow swapping frameloaders between unrelated documents, so we need to
reparent wrappers when the owner content changes.

Review commit: https://reviewboard.mozilla.org/r/170462/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/170462/

Comment 4

[Kris Maglione [:kmag]]

Attachment: Bug 1391110: Part 3 - Add nsIWebBrowserPersistable to FrameLoader bindings.

Review commit: https://reviewboard.mozilla.org/r/170464/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/170464/

Comment 5

[Kris Maglione [:kmag]]

Attachment: Bug 1391110: Part 4 - Remove unnecessary nsIFrameLoaderOwner QIs.

The FrameLoaderOwner interface has been implemented in WebIDL for several
years now, so these QIs are simply unnecessary overhead.

Review commit: https://reviewboard.mozilla.org/r/170466/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/170466/

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8899195 [details]
Bug 1391110: Part 4 - Remove unnecessary nsIFrameLoaderOwner QIs.

https://reviewboard.mozilla.org/r/170466/#review175638

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8899193 [details]
Bug 1391110: Part 2 - Reparent FrameLoader wrapper when owner content changes.

https://reviewboard.mozilla.org/r/170462/#review175640

scary.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8899194 [details]
Bug 1391110: Part 3 - Add nsIWebBrowserPersistable to FrameLoader bindings.

https://reviewboard.mozilla.org/r/170464/#review175642

::: dom/base/nsFrameLoader.cpp:3878
(Diff revision 1)
>    }
>  }
>  
> +void
> +nsFrameLoader::StartPersistence(uint64_t aOuterWindowID,
> +                     nsIWebBrowserPersistDocumentReceiver* aRecv,

Could you align latter two params under the first one.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Btw, do you have links to some profiles where all this is showing up badly?

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8899192 [details]
Bug 1391110: Part 1 - Convert FrameLoader bindings to WebIDL.

https://reviewboard.mozilla.org/r/170460/#review175644

Comment 11

[Kris Maglione [:kmag]]

(In reply to Olli Pettay [:smaug] from comment #9)
> Btw, do you have links to some profiles where all this is showing up badly?

Not for most of them (perf-html links are hard to keep track of), but here's the most relevant one for frameloader performance in my recent history:

https://perfht.ml/2vjtOLR

Which is based on a testcase add-on replicating the performance issues from bug 1385618. They also show up in Andy's profiles in that bug (comment 6), particularly the WebNavigation issues:

https://perfht.ml/2v0SEVg

Comment 12

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6763889f2682541de1443960bb1e0e0785a7491a
Bug 1391110: Part 1 - Convert FrameLoader bindings to WebIDL. r=smaug

https://hg.mozilla.org/integration/mozilla-inbound/rev/44dfa6fc4ba628358ef1fa51cdd400dabf68deb2
Bug 1391110: Part 2 - Reparent FrameLoader wrapper when owner content changes. r=smaug

https://hg.mozilla.org/integration/mozilla-inbound/rev/5a801710d5f2daa3cc19f1b13c37900b5a594b7c
Bug 1391110: Part 3 - Add nsIWebBrowserPersistable to FrameLoader bindings. r=smaug

https://hg.mozilla.org/integration/mozilla-inbound/rev/4b9198e1725289baac765a7b0398ff97db87da38
Bug 1391110: Part 4 - Remove unnecessary nsIFrameLoaderOwner QIs. r=smaug

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6763889f2682
https://hg.mozilla.org/mozilla-central/rev/44dfa6fc4ba6
https://hg.mozilla.org/mozilla-central/rev/5a801710d5f2
https://hg.mozilla.org/mozilla-central/rev/4b9198e17252