The main reason to not do this would be performance (avoiding the
addref/release), but there are two main mitigating factors:
1) All calls to UnwrapReflectorToISupports that pass in a Web IDL object
already do the addref (and in fact QI). So this only affects the
XPCWrappedNative case.
2) The vast majority of the callers proceed to QI on the pointer anyway, and a
second addref is cheap; it's the first addref after a CC that can be
expensive on a cycle-collected object.
Going through the changes one by one:
* In GlobalObject::GetAsSupports, we do have a change that slightly slows down
precisely in the XPCWrappedNative global case. That's the message managers
and the backstagepass. And this really only affects calls to Web IDL statics
from those globals.
* In UnwrapArgImpl we're talking about a Web IDL method taking an "external
interface" type, and the UnwrapReflectorToISupports call is immediately
followed by QI anyway.
* In UnwrapXPConnectImpl we're talking about the case when we have a
non-WebIDL-object implementation of a Web IDL interface. Again, this is the
message manager globals, for EventTarget. And we have a QI call immediately
after the UnwrapReflectorToISupports.
* In the generated HasInstance hook for EventTarget we will be slightly slower
when the LHS of the instanceof is an XPCWrappedNative. And not much slower,
because again there's an immediate QI.
* In InstallXBLField we're never going to have an XPCWrappedNative as thisObj;
it's always an Element in practice. So this is no more expensive than before.
* In sandbox's GetPrincipalOrSOP we now have an extra addref. But it was
followed by various QIs anyway.
* In XPCConvert::JSValToXPCException we have an extra addref if someone throws
an XPCWrappedNative, which is fairly unlikely; our actual Exception objects
are on Web IDL bindings. Plus we have an immediate QI.
* In xpc::HasInstance we have an extra addred if the LHS of instanceof is an
XPCWrappedNative. But, again, there's an immediated QI after the
UnwrapReflectorToISupports.
* In xpcJSWeakReference::Init we are likely doing an extra addref, but again
immediately followed by QI.
I think it's worth making this change just to remove the footgun and that the
perf impact, if any, is pretty minimal.
Most of these changes are just replacements of GetNativeOfWrapper with
UnwrapReflectorToISupports, which is all it did under the hood.
The other changes are as follows:
* In nsDOMClassInfo, we really care whether we have a window, so we can just
UNWRAP_OBJECT to the Window interface, since Window is always on Web IDL
bindings now. Also, the weird compartment check hasn't been needed ever since
GetNativeOfWrapper stopped returning things off the passed-in object's
prototype chain (Firefox 22, bug 658909).
* The only use of do_QueryWrapper was to get a Window in nsDocument; again we
can UNWRAP_OBJECT.
* In XPCJSRuntime, we again just want to check for a Window, so UNWRAP_OBJECT.
The idea is that CastableObjectUnwrapper will want to have a MutableHandle for
the thing it's unwrapping whenever its target is a raw pointer. Since we have
convenient MutableHandle<Value> in most cases, it's easier to switch
CastableObjectUnwrapper to working with MutableHandle<Value> or Handle<Value>
instead of trying to get MutableHandle<JSObject*> in the right places.
There are basically two changes here:
1) Make CastableObjectUnwrapper work with at thing that looks like a Value.
2) Change various callsites to pass in MutableHandle<Value>, in addition to a
Handle<Value>, into the JS-to-C++ conversion templates. The
MutableHandle<Value> is passed as ${maybeMutableVal}. It may not actually
end up being a MutableHandle in some cases.
The reason for passing both a Handle and a MutableHandle is that when the thing
we actually have is a Rooted named "foo" the Handle will be "foo" but the
MutableHandle is most naturally written as "&foo". This is not a problem if
you're just passing it through, but if you want to test whether it's an object,
say, you have a problem. Writing "foo.isObject()" is ok, but "&foo.isObject()"
is not, and neither is "(&foo).isObject()". This could be worked around by
passing the MutableHandle as "JS::MutableHandle<JS::Value>(&foo)" or something,
and then "JS::MutableHandle<JS::Value>(&foo).isObject()" does work. But it
makes the code very hard to read.
So we just pass both things along; ${val} should be used for readonly access and
${maybeMutableVal} any time you really want a MutableHandle.
Starting from 52, NativeKey::HandleCharMessage() ignores all control characters. However, some keyboard layout utilities may send WM_CHAR message whose wParam is '\r' for emulating pressing Enter key. For supporting such utilities, we should dispatch Enter keypress event when HandleCharMessage() receives such event.
Note that this patch does NOT support a pair of WM_KEYDOWN and WM_CHAR whose wParam is '\r' but the WM_KEYDOWN isn't VK_RETURN. If there is such case, we need to support it too. However, it needs a lot of code changes. So, we shouldn't support it until such bug is filed actually.
MozReview-Commit-ID: CWyvBtLmXgg
--HG--
extra : rebase_source : 6e9b27dc2fb9eb75626cdd5ab339d348a7be3040
This is the Servo side of [bug 1372464](https://bugzilla.mozilla.org/show_bug.cgi?id=1372464).
Source-Repo: https://github.com/servo/servo
Source-Revision: 95a85a301f1d014c45448a4d5d4f5a8497904af6
--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : be1244951c8d868b79ee09b37d1f26615f6ec89f
Enable stylo in Firefox Nightly builds for macOS so we can
get testing feedback until we have addressed issues blocking
enabling it for developer builds.
MozReview-Commit-ID: 3y0QT4oiBpt
--HG--
extra : rebase_source : 531162da334a6d124dcd7328f45d677cb4933551
Thanks to Manish for help in reflecting this idiomatically in rust.
MozReview-Commit-ID: 8tB7vsc5yxc
--HG--
extra : transplant_source : F%87%16%82.P%BD%F3%B1%A4%19%BA%F0%3DQ%F6%ED%BD%95%60
I believe this should fix some incorrect clearing of F_CLASS_FIXED.
MozReview-Commit-ID: 4ga2NEM9M5Z
--HG--
extra : transplant_source : %ECF%CF%D0%F6%19%9F%24%86%EFR%CAVZ%ED%60%D5nU%D8
<!-- Please describe your changes on the following line: -->
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).
<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____
<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Source-Repo: https://github.com/servo/servo
Source-Revision: 82f65f6eb38400affcc8e8bbc9023f345baf3942
--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 7bc5b0604156f6622162331e8d4e05ebb01b41e6
People apparently do all sorts of silly stuff with stylesheets, see Google Inbox
in bug 1379203.
Bug: 1379433
Reviewed-By: heycam
MozReview-Commit-ID: 4x2d3glOFu8
Source-Repo: https://github.com/servo/servo
Source-Revision: 00e366ac82ee3fe3119ecffc68d895974f33c5e0
--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 460637505da4995e4b93e6eac36c81d097d96a5b
