964875 - Figure out a plan for ChromeWindow in the WebIDL world

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

We could make an interface inheriting from Window, but that runs into the slight issue that [Global] does not allow interfaces inheriting from it in the spec and also that [StoreInSlot] is not supported on non-leaf interfaces.

We could try to change those, or we could try to hack around this as follows:

1)  Define an empty [ChromeOnly] ChromeWindow interface and have Window implement it.  Change the hasInstance codegen to treat that case like the "have prototype object" case, by doing a QI to nsIDOMChromeWindow and put this in Bindings.conf:

  'ChromeWindow': {
    'concrete': False,
    'headerFile': 'nsGlobalWindow.h',
    'nativeType': 'nsGlobalChromeWindow',
  },

2) Put all the nsIDOMChromeWindow bits on Window, with Func that QIs the object to nsIDOMChromeWindow guarding them.

This will give us the right instanceof behavior and members.  The one thing that would break is adding things to ChromeWindow.prototype making theme exposed on the chrome window (and in fact ChromeWindow.prototype would no longer exist).  I suspect that's OK.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Addons mxr shows no hits for ChromeWindow.prototype, fwiw.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Peter points out that nsGlobalWindow::IsChromeWindow exists; we should use that in the Func instead.  And maybe in the HasInstance code.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Add a WebIDL ChromeWindow interface for use in instanceof once Window is on WebIDL bindings.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Add a static nsGlobalWindow::IsChromeWindow test function for use in Func annotations.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Define WebIDL quickstubs for ChromeWindow.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Define WebIDL quickstubs for ChromeWindow.

Comment 7

[Peter Van der Beken [:peterv]]

Comment on attachment 8367131 [details] [diff] [review]
part 1.  Add a WebIDL ChromeWindow interface for use in instanceof once Window is on WebIDL bindings.

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

::: dom/bindings/Codegen.py
@@ +1265,5 @@
>      return true;
>    }
>    """
> +        if self.descriptor.interface.identifier.name == "ChromeWindow":
> +            setBp = "*bp = UnwrapDOMObject<nsGlobalWindow>(js::UncheckedUnwrap(instance))->IsChromeWindow()"

I know there are very few of these special cases, but how about a IsInstanceOf function templated on prototype id that does this check for ChromeWindow and just returns true for all other ids? We could use it for ModalWindow too.

Comment 8

[Peter Van der Beken [:peterv]]

Comment on attachment 8367132 [details] [diff] [review]
part 2.  Add a static nsGlobalWindow::IsChromeWindow test function for use in Func annotations.

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

::: dom/base/nsGlobalWindow.cpp
@@ +4045,5 @@
> +  nsGlobalWindow* win;
> +  nsresult rv = UNWRAP_OBJECT(Window, aObj, win);
> +  if (NS_FAILED(rv)) {
> +    nsCOMPtr<nsPIDOMWindow> piWin = do_QueryInterface(
> +      nsContentUtils::XPConnect()->GetNativeOfWrapper(aCx, aObj));

do_QueryWrapper?

Comment 9

[Boris Zbarsky [:bzbarsky]]

> how about a IsInstanceOf function templated on prototype id that does this check for
> ChromeWindow 

The prototype id in this case is prototypes::id::Window.  That's why I need the additional check; the current setup assumes that if "Foo implements Bar" then any Foo instance is a Bar instance, but for "Window implements ChromeWindow" that's not actually true...

> do_QueryWrapper?

Yes, good idea.

Comment 10

[Boris Zbarsky [:bzbarsky]]

   https://hg.mozilla.org/integration/mozilla-inbound/rev/97ee8ec34362
   https://hg.mozilla.org/integration/mozilla-inbound/rev/ef6e5cfea527
   https://hg.mozilla.org/integration/mozilla-inbound/rev/9dcc5957f89e

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/97ee8ec34362
https://hg.mozilla.org/mozilla-central/rev/ef6e5cfea527
https://hg.mozilla.org/mozilla-central/rev/9dcc5957f89e