1341546 - convert nsIDOMWindowUtils into a Web IDL interface

Status: NEW

(Core :: DOM: Core & HTML, defect, P3)

Description

[Cameron McCormack (:heycam)]

It's a bit annoying having to use XPIDL to define new methods on nsDOMWindowUtils.  So it might be nice to convert it to Web IDL.

Comment 1

[Bobby Holley (:bholley)]

I saw you wrote some patches for this [1]. ;-)

Have you measured the codesize relative to the reduction in typelib size? In general the WebIDL bindings improve performance and spec correctness at the expense of code size (and build times). I'm concerned that wholesale conversion of internal interfaces will cause unacceptable bloat in the binary, but measurements might show it's negligible.

[1] https://twitter.com/heycam/status/965229117972013056

Comment 2

[Bobby Holley (:bholley)]

See also https://groups.google.com/forum/#!topic/mozilla.dev.platform/ONHBIzzM7WU

Comment 3

[Cameron McCormack (:heycam)]

Per my dev-platform mail, the patches cause a 189 KiB increase in libxul.so.  The drop in size of dom_base.xpt is only around 10 KiB, so the change is not quite neglible...

Comment 4

[Bobby Holley (:bholley)]

It seems like a very large portion of the methods on this interface are test-only. Can we isolate the methods that are actually called frequently in the browser and make them [ChromeOnly] methods on [Window] instead?

Comment 5

[Bobby Holley (:bholley)]

(In reply to Bobby Holley (:bholley) from comment #4)
> It seems like a very large portion of the methods on this interface are
> test-only. Can we isolate the methods that are actually called frequently in
> the browser and make them [ChromeOnly] methods on [Window] instead?

(According to the thread, Kris has seen DOMWindowUtils show up in profiles, so there must be at least a handful).

Comment 6

[Kris Maglione [:kmag]]

(In reply to Bobby Holley (:bholley) from comment #5)
> (In reply to Bobby Holley (:bholley) from comment #4)
> > It seems like a very large portion of the methods on this interface are
> > test-only. Can we isolate the methods that are actually called frequently in
> > the browser and make them [ChromeOnly] methods on [Window] instead?
> 
> (According to the thread, Kris has seen DOMWindowUtils show up in profiles,
> so there must be at least a handful).

There are several, yes. By far the most common are outerWindowID and currentInnerWindowID. And, from browser windows, getBoundsWithoutFlushing. There are a lot of others that are used less often, but sill very often. It might be worth adding some instrumentation to figure out exactly which.

Some other points, though:

- Even if we don't convert this entire interface to WebIDL, we should add a (possibly cached) WebIDL getter to retrieve the instance. A lot of the overhead of using this interface comes from the QI/getInterface dance, and then generally re-creating the wrapper every time it's used after a GC.

- We should really probably move the test-only and test/a11y methods to their own interfaces, even if only so it's easier to tell them apart.

Comment 7

[Bobby Holley (:bholley)]

(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #6)
> (In reply to Bobby Holley (:bholley) from comment #5)
> > (In reply to Bobby Holley (:bholley) from comment #4)
> > > It seems like a very large portion of the methods on this interface are
> > > test-only. Can we isolate the methods that are actually called frequently in
> > > the browser and make them [ChromeOnly] methods on [Window] instead?
> > 
> > (According to the thread, Kris has seen DOMWindowUtils show up in profiles,
> > so there must be at least a handful).
> 
> There are several, yes. By far the most common are outerWindowID and
> currentInnerWindowID. And, from browser windows, getBoundsWithoutFlushing.
> There are a lot of others that are used less often, but sill very often. It
> might be worth adding some instrumentation to figure out exactly which.

Agreed.

> Some other points, though:
> 
> - Even if we don't convert this entire interface to WebIDL, we should add a
> (possibly cached) WebIDL getter to retrieve the instance. A lot of the
> overhead of using this interface comes from the QI/getInterface dance, and
> then generally re-creating the wrapper every time it's used after a GC.

This may not matter if we move the frequently-used things as discussed above, but seems fine either way.

> - We should really probably move the test-only and test/a11y methods to
> their own interfaces, even if only so it's easier to tell them apart.

Totally.

Comment 8

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

Since DOMWindowUtils was originally for testing only, and still is mainly,
would it make sense to move the methods which are
used for non-testing too to ChromeUtils.webidl or some such.
Or in many cases there could be perhaps [ChromeOnly] methods on Window.

Comment 9

[Cameron McCormack (:heycam)]

I'm not currently working on this but if people have identified specific XPIDL attributes and functions that would be worth moving to Window as [ChromeOnly] things, I'm happy to resurrect those parts of my patches.

Comment 10

[Kris Maglione [:kmag]]

(In reply to Cameron McCormack (:heycam) from comment #9)
> I'm not currently working on this but if people have identified specific
> XPIDL attributes and functions that would be worth moving to Window as
> [ChromeOnly] things, I'm happy to resurrect those parts of my patches.

Inner and outer window IDs would definitely be the biggest wins. getBoundsWithoutFlushing would also probably help a lot for our front-end code.