Closed Bug 836920 Opened 7 years ago Closed 7 years ago

Move Optional to BindingDeclarations.h

Categories

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

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Details

Attachments

(1 file, 1 obsolete file)

Right now it's in BindingUtils.h, but we no longer include that in binding headers.  So if your interface name starts with a letter before 'P' (so that you come before PrimitiveConversions.h in the include list in the .cpp) and there's a dictionary involved (which uses Optional<> members in the header), you lose.
Comment on attachment 708799 [details] [diff] [review]
Move dom::Optional<> and dom::Sequence to BindingDeclarations.h so using dictionaries in interfaces whose name comes before 'PrimitiveConversions' alphabetically does not break.

Actually, this doesn't build; need to move FakeDependentString too, and check whether that pulls in anything too evil.
Attachment #708799 - Attachment is obsolete: true
Attachment #708799 - Flags: review?(peterv)
Yeah, that doesn't work because xpcpublic.h includes BindingDeclarations.h and xpcshell.cpp includes xpcpublic.h.

Let me play with this stuff a bit more, I guess...
Yeah, and various other external-string code (e.g. TestAppShellSteadyState.cpp) includes nsIDocument.h, which includes DocumentBinding.h, which includes BindingDeclarations.h.

And the problem there, of course, is that FakeDependentString relies on the internal version of nsDependentString...
Comment on attachment 708883 [details] [diff] [review]
Move dom::Optional<> and dom::Sequence to BindingDeclarations.h so using dictionaries in interfaces whose name comes before 'PrimitiveConversions' alphabetically does not break.  the code is just moving except the Optional<nsAString>::operator=

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

::: dom/bindings/BindingDeclarations.h
@@ +214,5 @@
>  };
>  
> +// Class for representing optional arguments.
> +template<typename T>
> +class Optional {

While you're here, want to move {'s on their own line, following the style of this file?
Attachment #708883 - Flags: review?(peterv) → review+
Done and https://hg.mozilla.org/integration/mozilla-inbound/rev/88601eca5f69

Then https://hg.mozilla.org/integration/mozilla-inbound/rev/830784a5e43e to fix warnings-as-errors bustage.
Flags: in-testsuite?
Whiteboard: [need review]
Target Milestone: --- → mozilla21
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.