Closed Bug 877654 Opened 6 years ago Closed 6 years ago

Remove thisptr offset tables

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: peterv, Assigned: peterv)

References

Details

Attachments

(7 files, 1 obsolete file)

Almost everything that we used them for is on new bindings.
Attached patch Split off calls to base QI v1 (obsolete) — Splinter Review
This introduces a new macro that just does the call to the base class' QueryInterface and the call to DOMQueryInterface for elements. We still want to do that even when removing the offset table.
Attachment #755932 - Flags: review?(Ms2ger)
Attachment #755932 - Attachment is obsolete: true
Attachment #755932 - Flags: review?(Ms2ger)
Attachment #755933 - Flags: review?(Ms2ger)
This stops supporting thisptr offset tables for HTML elements. They shouldn't be used anymore from XPConnect anyway (except maybe for form elements, but we'll just use regular QIs for those now).
Attachment #755934 - Flags: review?(Ms2ger)
Replacing macros that should be equivalent after the previous patches in the stack.
Attachment #755935 - Flags: review?(Ms2ger)
More replacing of equivalent macros.
Attachment #755936 - Flags: review?(Ms2ger)
This removes thisptr offset tables from other objects. Again, they should be unused and we'll just fall back to regular QIs if not.
Attachment #755938 - Flags: review?(Ms2ger)
Just removing unused macros.
Attachment #755942 - Flags: review?(Ms2ger)
Begone.
Attachment #755952 - Flags: review?(Ms2ger)
Comment on attachment 755933 [details] [diff] [review]
Split off calls to base QI v1

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

::: content/html/content/src/nsGenericHTMLElement.h
@@ +1452,5 @@
>  #define NS_HTML_CONTENT_INTERFACE_TABLE_TO_MAP_SEGUE_AMBIGUOUS(_class, _base, \
>                                                                 _base_if)      \
> +  NS_OFFSET_AND_INTERFACE_TABLE_TO_MAP_SEGUE
> +
> +#define NS_HTML_CONTENT_INTERFACES_AMBIGUOUS(_class, _base, _base_if)         \

It doesn't look like this uses _class. Please remove the argument. (Can be in a separate patch on top of those, if you prefer.)
Attachment #755933 - Flags: review?(Ms2ger) → review+
Comment on attachment 755934 [details] [diff] [review]
Replace HTML element offset table macros with normal QI table macros v1

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

Could you remove the NS_HTML_CONTENT_INTERFACE_TABLEn macros in this patch, please?

::: content/html/content/src/HTMLAudioElement.cpp
@@ +36,5 @@
>  NS_INTERFACE_TABLE_HEAD(HTMLAudioElement)
>  NS_HTML_CONTENT_INTERFACES(HTMLAudioElement, HTMLMediaElement)
> +NS_INTERFACE_TABLE_INHERITED4(HTMLAudioElement, nsIDOMHTMLMediaElement,
> +                              nsIDOMHTMLAudioElement, nsITimerCallback,
> +                              nsIAudioChannelAgentCallback)

Want to fix up the indentation here too?

::: content/html/content/src/HTMLIFrameElement.cpp
@@ +37,5 @@
>  // QueryInterface implementation for HTMLIFrameElement
>  NS_INTERFACE_TABLE_HEAD(HTMLIFrameElement)
>    NS_HTML_CONTENT_INTERFACES(HTMLIFrameElement, nsGenericHTMLFrameElement)
> +  NS_INTERFACE_TABLE_INHERITED2(HTMLIFrameElement, nsIDOMHTMLIFrameElement,
> +                                nsIDOMGetSVGDocument)

I'd keep the separate lines

::: content/html/content/src/HTMLSharedElement.cpp
@@ -36,5 @@
>    NS_HTML_CONTENT_INTERFACES_AMBIGUOUS(HTMLSharedElement, nsGenericHTMLElement,
>                                         nsIDOMHTMLParamElement)
> -  NS_HTML_CONTENT_INTERFACE_TABLE_AMBIGUOUS_BEGIN(HTMLSharedElement,
> -                                                  nsIDOMHTMLParamElement)
> -  NS_OFFSET_AND_INTERFACE_TABLE_END

No NS_INTERFACE_TABLE_INHERITED0?

::: content/html/content/src/HTMLSharedListElement.cpp
@@ -33,5 @@
>                                         nsGenericHTMLElement,
>                                         nsIDOMHTMLOListElement)
> -  NS_HTML_CONTENT_INTERFACE_TABLE_AMBIGUOUS_BEGIN(HTMLSharedListElement,
> -                                                  nsIDOMHTMLOListElement)
> -  NS_OFFSET_AND_INTERFACE_TABLE_END

Ditto
Attachment #755934 - Flags: review?(Ms2ger) → review+
Comment on attachment 755935 [details] [diff] [review]
Replace HTML element table to map segue macros with generic macro v1

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

Remove NS_HTML_CONTENT_INTERFACE_TABLE_TO_MAP_SEGUE in this patch, please.
Attachment #755935 - Flags: review?(Ms2ger) → review+
Comment on attachment 755936 [details] [diff] [review]
Replace HTML element QI map end macro with generic element QI map end macro v1

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

Remove NS_HTML_CONTENT_INTERFACE_MAP_END in this patch

::: content/html/content/src/HTMLAnchorElement.cpp
@@ -50,5 @@
>                                  nsIDOMHTMLAnchorElement,
>                                  nsILink,
>                                  Link)
> -  NS_HTML_CONTENT_INTERFACE_TABLE_TO_MAP_SEGUE(HTMLAnchorElement,
> -                                               nsGenericHTMLElement)

Move this into the previous patch
Attachment #755936 - Flags: review?(Ms2ger) → review+
Comment on attachment 755938 [details] [diff] [review]
Remove offset tables from non-HTMLElement objects v1

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

::: content/base/src/nsDocument.cpp
@@ +1521,5 @@
>  }
>  
>  NS_INTERFACE_TABLE_HEAD(nsDocument)
>    NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
> +  NS_INTERFACE_TABLE_BEGIN

Feel free to add NS_INTERFACE_TABLE18

::: content/svg/content/src/SVGMPathElement.cpp
@@ +45,5 @@
>  NS_IMPL_ADDREF_INHERITED(SVGMPathElement,SVGMPathElementBase)
>  NS_IMPL_RELEASE_INHERITED(SVGMPathElement,SVGMPathElementBase)
>  
>  NS_INTERFACE_TABLE_HEAD_CYCLE_COLLECTION_INHERITED(SVGMPathElement)
> +  NS_INTERFACE_TABLE_INHERITED4(SVGMPathElement, nsIDOMNode, nsIDOMElement,

I wonder if we should add something like NS_IMPL_ISUPPORTS_INHERITEDn that also handles CC

::: content/svg/content/src/SVGStyleElement.cpp
@@ +26,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGStyleElement, SVGStyleElementBase)
>  
>  NS_INTERFACE_TABLE_HEAD_CYCLE_COLLECTION_INHERITED(SVGStyleElement)
> +  NS_INTERFACE_TABLE_INHERITED6(SVGStyleElement, nsIDOMNode, nsIDOMElement,
> +                                nsIDOMSVGElement, nsIDOMLinkStyle,

You can drop nsIDOMNode, nsIDOMElement, nsIDOMSVGElement; nsSVGElement handles that.

::: content/svg/content/src/SVGSwitchElement.cpp
@@ +33,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGSwitchElement,SVGSwitchElementBase)
>  
>  NS_INTERFACE_TABLE_HEAD_CYCLE_COLLECTION_INHERITED(SVGSwitchElement)
> +  NS_INTERFACE_TABLE_INHERITED3(SVGSwitchElement, nsIDOMNode, nsIDOMElement,
> +                                nsIDOMSVGElement)

Ditto

::: content/svg/content/src/SVGUseElement.cpp
@@ +62,5 @@
>  NS_IMPL_RELEASE_INHERITED(SVGUseElement,SVGUseElementBase)
>  
>  NS_INTERFACE_TABLE_HEAD_CYCLE_COLLECTION_INHERITED(SVGUseElement)
> +  NS_INTERFACE_TABLE_INHERITED4(SVGUseElement, nsIDOMNode, nsIDOMElement,
> +                                nsIDOMSVGElement, nsIMutationObserver)

Ditto

::: content/xml/content/src/nsXMLElement.cpp
@@ +25,2 @@
>    NS_ELEMENT_INTERFACE_TABLE_TO_MAP_SEGUE
>    NS_DOM_INTERFACE_MAP_ENTRY_CLASSINFO(Element)

Does this still need CI?
Attachment #755938 - Flags: review?(Ms2ger) → review+
Comment on attachment 755952 [details] [diff] [review]
Remove thisptr table support from XPConnect v1

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

::: js/xpconnect/src/XPCQuickStubs.cpp
@@ +600,5 @@
>              return rv;
>      } else if (cur) {
>          nsISupports *native;
> +        if (!mozilla::dom::UnwrapDOMObjectToISupports(cur, native)) {
> +          if (IS_SLIM_WRAPPER(cur)) {

Four-space indentation here

::: xpcom/components/nsIClassInfo.idl
@@ -9,5 @@
> -%{C++
> -/**
> - * Calling QueryInterface with this special IID will return a null-terminated
> - * table of QITableEntry's. Not all objects support this.
> - * Note that this breaks XPCOM rules a bit (the table doesn't derive from

A bit, eh.
Attachment #755952 - Flags: review?(Ms2ger) → review+
Comment on attachment 755942 [details] [diff] [review]
Remove node offset table macros v1

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

This was folded into other patches.
Attachment #755942 - Flags: review?(Ms2ger)
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.