Closed Bug 801083 Opened 7 years ago Closed 7 years ago

Remove old proxy-based list bindings

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19

People

(Reporter: peterv, Assigned: peterv)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch v1 (obsolete) — Splinter Review
No description provided.
Comment on attachment 670930 [details] [diff] [review]
v1

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

::: js/xpconnect/src/nsXPConnect.cpp
@@ +1414,3 @@
>      nsISupports* supports = nullptr;
> +    mozilla::dom::UnwrapDOMObjectToISupports(aJSObj, supports);
> +    return supports;

Is this function supposed to return the canonical nsISupports pointer? I'm pretty sure this one isn't necessarily it.
For Paris bindings, it needs to be the canonical one.  Any impl in which it's not is buggy.
It isn't for nsContentList -> nsIHTMLCollection, IIRC.
(In reply to :Ms2ger from comment #1)
> Is this function supposed to return the canonical nsISupports pointer?

Yeah, I added a QI.
Attached patch v1.1Splinter Review
Attachment #670930 - Attachment is obsolete: true
Attachment #673453 - Flags: review?(bzbarsky)
Comment on attachment 673453 [details] [diff] [review]
v1.1

File a followup on nuking nsIDOMHTMLPropertiesCollection altogether if we can?

I love all the code being gone.  r=me

P.S. diffstat says: "72 files changed, 181 insertions(+), 3634 deletions(-)".
Attachment #673453 - Flags: review?(bzbarsky) → review+
Comment on attachment 673453 [details] [diff] [review]
v1.1

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

::: dom/base/nsDOMClassInfo.cpp
@@ +4535,5 @@
>  
>    sDisableGlobalScopePollutionSupport =
>      Preferences::GetBool("browser.dom.global_scope_pollution.disabled");
>  
>    // Non-proxy bindings

Fix this comment

::: js/xpconnect/src/xpcpublic.h
@@ +301,5 @@
>  namespace mozilla {
>  namespace dom {
>  
>  extern int HandlerFamily;
>  inline void* ProxyFamily() { return &HandlerFamily; }

All this stuff should probably move to dom/bindings at some point.
https://hg.mozilla.org/mozilla-central/rev/5d03feda2300
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
I'm confused. This patch removed the pref dom.new_bindings form all.js, but did not remove the code in XPCJSRuntime.cpp that uses it.
Depends on: 803872
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.