The 'Object.prototype.watch' function has wonky, incomplete semantics, is non-standard, and serves as a ready source of serious bugs because it is available to content. The Debug object (bug 636907) should provide watchpoint functionality instead. Once it is good enough, Object.prototype.watch should be deleted.

The debug object probably won't be available to web content (see bug 636907 comment 10). So we might as well disable Object.prototype.watch *for web content* right now. Web content can use standard techniques such as Object.defineProperty for most non-"debugger" uses of watch. What debuggers currently use Object.prototype.watch?

See also bug 630370 for self-hosting Object.prototype.watch.

ES6 is going to define Object.observe.

Can we remove this in 58? The warning is in 57 (bug 934669).

I put this in a different patch, because only this part really touches code outside js/ and the rest is relatively self-contained.

I was almost certain I would have to touch a lot more places, but unless I missed something obvious the interactions with the JITs etc are actually quite limited.

I kind of followed these steps before deleting some tests: 1) Test name or summary mentions watch/unwatch -> delete 2) Test is very short and watch is a major part -> delete (We had like 50 tests just for __defineGetter__ and watch ...) 3) Complex test using watch -> tried to keep it working In general I mostly made my life easier by aggressively removing fuzz tests using watch instead of trying to somehow fix them by hand.

I am still working on the patch to remove watch/unwatch from DOM and other tests, mostly because I don't want to run all of these locally.

Nick please review the devtools test. Boris, I found the codegen.py hunk while fixing the tests, I could move it to the first path.

Comment on attachment 8920577 [details] [diff] [review] v1 - Remove watch class-hook and proxy trap r=me on the parts outside js/

Comment on attachment 8920674 [details] [diff] [review] v1 - Remove or fix tests outside JS using watch/unwatch r=me on the dom/bindings and js/xpconnect changes.

Comment on attachment 8920674 [details] [diff] [review] v1 - Remove or fix tests outside JS using watch/unwatch Review of attachment 8920674 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!

Comment on attachment 8920577 [details] [diff] [review] v1 - Remove watch class-hook and proxy trap Review of attachment 8920577 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/vm/EnvironmentObject.cpp @@ +1,1 @@ > + Accidental blank line.

Comment on attachment 8920580 [details] [diff] [review] Mostly remove JS tests using watch/unwatch Review of attachment 8920580 [details] [diff] [review]: ----------------------------------------------------------------- Thank you.

Glad to see the end of this! I mentioned in #devtools on IRC that we were removing this stuff. I said that doing this now does not make it much harder to add watchpoints to the Debugger later, if the devtools team ever gets around to that. At worst, we could just add this stuff back. At best, we would want something even less intrusive. Either way, most of the work would be testing, updating the Debugger protocol, etc. And it seems to me most of the risk is getting GC right. The testing patch reinforces this. Most of the tests we're deleting are fuzztests from earlier days, when watchpoints were implemented using JSPropertyOps---which was faster, but intrusive and prone to bugs.

Pushed by evilpies@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/3f8f9e5f2858 Remove watch class-hook and proxy trap r=jorendorff,bz https://hg.mozilla.org/integration/mozilla-inbound/rev/e4f864ad5779 Remove the guts of the watch/unwatch implementation. r=jorendorff https://hg.mozilla.org/integration/mozilla-inbound/rev/31bc65f1acab Mostly remove JS tests using watch/unwatch. r=jorendorff https://hg.mozilla.org/integration/mozilla-inbound/rev/a124f4901430 Remove or fix tests outside JS using watch/unwatch. r=bz,fitzgen

Posted the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2017/object-prototype-watch-has-been-removed/

https://hg.mozilla.org/mozilla-central/rev/3f8f9e5f2858 https://hg.mozilla.org/mozilla-central/rev/e4f864ad5779 https://hg.mozilla.org/mozilla-central/rev/31bc65f1acab https://hg.mozilla.org/mozilla-central/rev/a124f4901430

Developer release notes: https://developer.mozilla.org/en-US/Firefox/Releases/58#JavaScript Reference pages: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/watch https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/unwatch Compat data: https://github.com/mdn/browser-compat-data/pull/590