Figure out a plan for ChromeWindow in the WebIDL world

RESOLVED FIXED in mozilla30

Status

()

Core
DOM
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

unspecified
mozilla30
x86
Mac OS X
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

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.
Addons mxr shows no hits for ChromeWindow.prototype, fwiw.
Peter points out that nsGlobalWindow::IsChromeWindow exists; we should use that in the Func instead.  And maybe in the HasInstance code.
Blocks: 789261
Created attachment 8367131 [details] [diff] [review]
part 1.  Add a WebIDL ChromeWindow interface for use in instanceof once Window is on WebIDL bindings.
Attachment #8367131 - Flags: review?(peterv)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Created attachment 8367132 [details] [diff] [review]
part 2.  Add a static nsGlobalWindow::IsChromeWindow test function for use in Func annotations.
Attachment #8367132 - Flags: review?(peterv)
Created attachment 8367133 [details] [diff] [review]
part 3.  Define WebIDL quickstubs for ChromeWindow.
Attachment #8367133 - Flags: review?(peterv)
Whiteboard: [need review]
Created attachment 8367824 [details] [diff] [review]
part 3.  Define WebIDL quickstubs for ChromeWindow.
Attachment #8367824 - Flags: review?(peterv)
Attachment #8367133 - Attachment is obsolete: true
Attachment #8367133 - Flags: review?(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.
Attachment #8367131 - Flags: review?(peterv) → review+
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?
Attachment #8367132 - Flags: review?(peterv) → review+
Attachment #8367824 - Flags: review?(peterv) → review+
> 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.
You need to log in before you can comment on or make changes to this bug.