Closed Bug 1322080 Opened 3 years ago Closed 3 years ago

Remove AddProperty hook

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: njn, Assigned: njn)

Details

Attachments

(3 files, 1 obsolete file)

nsIXPCScriptable::addProperty is unused. Let's remove it. Bug 1132184 and bug 1132187 did likewise with other unused hooks.
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
It's never defined, and there's a comment in
XPCWrappedNativeProto::CallPostCreatePrototype() explaining how
PostCreatePrototype() doesn't have an associated "want" check.
Attachment #8816820 - Flags: review?(peterv)
The only non-trivial implementation of this method is in nsEventTargetSH, but
it's never called, so no point keeping it around. (Similar methods were removed
in bug 1132184 and bug 1132187.)
Attachment #8816821 - Flags: review?(peterv)
Attachment #8816821 - Attachment is obsolete: true
Attachment #8816821 - Flags: review?(peterv)
Hmm.  I guess the remaining XPConnect event targers don't use WANT_ADDPROPERTY but it kinda works out because they're globals so can't get gced anyway?
Attachment #8816819 - Flags: review?(peterv) → review+
Attachment #8816820 - Flags: review?(peterv) → review+
Attachment #8816826 - Flags: review?(peterv) → review+
(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #5)
> Hmm.  I guess the remaining XPConnect event targers don't use
> WANT_ADDPROPERTY but it kinda works out because they're globals so can't get
> gced anyway?

Yes, that should be the case.
Pushed by nnethercote@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6d2cb22d85b0
(part 1) - Remove nsEventTargetSH::doCreate(). r=peterv.
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b3a011acc01
(part 2) - Remove XPC_MAP_WANT_POST_CREATE_PROTOTYPE. r=peterv.
https://hg.mozilla.org/integration/mozilla-inbound/rev/9356d8836ca8
(part 3) - Remove nsIXPCScriptable::addProperty. r=peterv.
sorry had to back this out, seems this cause perma failures on xpcshell test on mac 10.10 debug like https://treeherder.mozilla.org/logviewer.html#?job_id=40476307&repo=mozilla-inbound
Flags: needinfo?(n.nethercote)
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b84794ed0eb
Backed out changeset 9356d8836ca8 
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb61f8d03872
Backed out changeset 5b3a011acc01 
https://hg.mozilla.org/integration/mozilla-inbound/rev/0dc28cac4e7e
Backed out changeset 6d2cb22d85b0 for xpcshell failures on 10.10 debug
(In reply to Carsten Book [:Tomcat] from comment #8)
> sorry had to back this out, seems this cause perma failures on xpcshell test
> on mac 10.10 debug like
> https://treeherder.mozilla.org/logviewer.html#?job_id=40476307&repo=mozilla-
> inbound

All the failures that occurred while my patches were on m-i matched existing known failures, and the failures were different for each of the runs. To check, I just did a try push where I triggered 30 runs and got 7 failures, all of which again had been seen before:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a96ae166519482795853a3242524f8516ccf4138

So I'm confident that my patches didn't change anything here. I will re-land them without change.
Flags: needinfo?(n.nethercote)
Pushed by nnethercote@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/99be04cf3cf7
(part 1, attempt 2) - Remove nsEventTargetSH::doCreate(). r=peterv.
https://hg.mozilla.org/integration/mozilla-inbound/rev/fcbb55f3cbae
(part 2, attempt 2) - Remove XPC_MAP_WANT_POST_CREATE_PROTOTYPE. r=peterv.
https://hg.mozilla.org/integration/mozilla-inbound/rev/b08af7a04122
(part 3, attempt 2) - Remove nsIXPCScriptable::addProperty. r=peterv.
https://hg.mozilla.org/mozilla-central/rev/99be04cf3cf7
https://hg.mozilla.org/mozilla-central/rev/fcbb55f3cbae
https://hg.mozilla.org/mozilla-central/rev/b08af7a04122
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.