Closed Bug 1299306 Opened 3 years ago Closed 3 years ago

Fix Location to return false from [[SetPrototypeOf]] instead of throwing hardcoded exceptions

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(4 files)

Right now we have hardcoding in js::SetPrototype for classes named "Location".
Note to self: setting .__proto__ and Object.setPrototypeOf should throw, but Reflect.setPrototypeOf should just return false.  Because that is the Spec Way and it Shalt Not Be Questioned.
Fix for this will depend on bug 1297717 and some of the refactoring it does.
Depends on: 1297717
Comment on attachment 8786604 [details] [diff] [review]
part 3.  Always support immutable prototypes

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

rs=me
Attachment #8786604 - Flags: review?(jwalden+bmo) → review+
Attachment #8786608 - Flags: review?(jwalden+bmo) → review+
Attachment #8786601 - Flags: review?(peterv) → review+
Comment on attachment 8786602 [details] [diff] [review]
part 2.  Call JS_SetImmutablePrototype on Location instances to make their prototype immutable in a more spec-compliant way

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

::: dom/bindings/Codegen.py
@@ +3496,5 @@
> +        bool succeeded;
> +        if (!JS_SetImmutablePrototype(aCx, aReflector, &succeeded)) {
> +          ${failureCode}
> +        }
> +        MOZ_ASSERT(succeeded,

Just to make sure that I understand why this is ok, succeeded would be false if we ended up in BaseProxyHandler::setImmutablePrototype, but we don't get there because obj->hasDynamicPrototype() is false in SetImmutablePrototype?
Attachment #8786602 - Flags: review?(peterv) → review+
> succeeded would be false if we ended up in BaseProxyHandler::setImmutablePrototype, but we don't get there because obj->hasDynamicPrototype() is false 

Yes.
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b20981109d6
part 1.  Refactor the error handling in CGWrapNonWrapperCacheMethod and CGWrapWithCacheMethod to have less duplication.  r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/f3c975a2bc7e
part 2.  Call JS_SetImmutablePrototype on Location instances to make their prototype immutable in a more spec-compliant way.  r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d6d85635ddb
part 3.  Always support immutable prototypes.  r=waldo
https://hg.mozilla.org/integration/mozilla-inbound/rev/f52595d3b9f3
part 4.  Remove the existing Location hardcoding in js::SetPrototype now that Location handles that itself.  r=waldo
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.