Closed Bug 1637159 Opened 2 years ago Closed 2 years ago

remove public DOMEventTargetHelper::BindToOwner() rebind methods

Categories

(Core :: DOM: Events, task)

task

Tracking

()

RESOLVED FIXED
mozilla78
Tracking Status
firefox78 --- fixed

People

(Reporter: karlt, Assigned: karlt)

References

Details

Attachments

(6 files)

DOMEventTargetHelper::BindToOwner() documentation says that it "is only called on rebind", but

  1. Rebinding is never performed, and
  2. It is called for cases that are not rebinds.

https://hg.mozilla.org/mozilla-central/rev/0c35057e2bb41d3440942338b5ad3c7d9c2a3345#l8.64 introduced the BindToOwner() methods, including support for rebinding to a different window, but all of the call sites seem to have bound a new object.

https://hg.mozilla.org/integration/mozilla-inbound/rev/cda31f55d8ca1e84bee52d9b5ca2f6820c08cf50 for https://bugzilla.mozilla.org/show_bug.cgi?id=1450266#c17 added invocations of BindToOwner() from nsGlobalWindowOuter::SetNewDocument().
At that revision also, all other calls seem to be on new objects.

https://hg.mozilla.org/integration/mozilla-inbound/rev/e3b3fac9a495#l3.47 moved BindToOwner() calls from SetNewDocument() to nsGlobalWindowInner::MigrateStateForDocumentOpen().
https://bugzilla.mozilla.org/show_bug.cgi?id=1451913#c55 said
"Note, my hope is that one day we can implement bug 1449992 and this entire mechanism can go away."
https://hg.mozilla.org/integration/mozilla-inbound/rev/3baa317d82f5#l2.63 introduced the documentation.

https://hg.mozilla.org/mozilla-central/rev/a01586b62cf5#l1.92 removed MigrateStateForDocumentOpen().

BindToOwner() is now unnecessary as all remaining uses can be covered by passing the relevant object to the DOMEventTargetHelper constructor.

See Also: → 1625372
Severity: -- → S3

WorkerGlobalScopeBase has no override for BindToOwner(), so there is no need
for the virtual call.

The DOMEventTargetHelper(nsIGlobalObject*) constructor is not suitable here
because the nsIGlobalObject would not have been constructed when the
DOMEventTargetHelper constructor would need it.

Depends on D75037

This method can be and is used by derived classes as well as DOMEventTargetHelper
itself, so it does not need to be "Internal".

Pushed by ktomlinson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3c7e244b3d5b
specify DOMEventTargetHelper owner for IDBFileHandle via constructor r=smaug
https://hg.mozilla.org/integration/autoland/rev/96df7fe7497f
provide XMLHttpRequest owner via DOMEventTargetHelper constructor r=smaug
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
Pushed by ktomlinson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7289b7a7367a
use BindToOwnerInternal() from WorkerGlobalScopeBase r=smaug
https://hg.mozilla.org/integration/autoland/rev/c5e22093726a
remove now-unused public DOMEventTargetHelper::BindToOwner() rebind methods r=smaug
https://hg.mozilla.org/integration/autoland/rev/c1c2ee6e3742
remove unused unbind code from BindToOwnerInternal() r=smaug
https://hg.mozilla.org/integration/autoland/rev/ef1634b33aad
rename BindToOwnerInternal() to BindToOwner() r=smaug
See Also: → 1642849
See Also: 1625372
You need to log in before you can comment on or make changes to this bug.