Skip to content

Commit

Permalink
Backed out 5 changesets (bug 1843568) for causing xpc failures in tes…
Browse files Browse the repository at this point in the history
…t_extension_permissions_migration.js CLOSED TREE

Backed out changeset a4cb1e2b9e3d (bug 1843568)
Backed out changeset 26047645c009 (bug 1843568)
Backed out changeset 50d6b858ee6e (bug 1843568)
Backed out changeset 3597df96ba38 (bug 1843568)
Backed out changeset 8faeb75f1161 (bug 1843568)
  • Loading branch information
nerli1 committed Jul 19, 2023
1 parent a799855 commit ec44190
Show file tree
Hide file tree
Showing 16 changed files with 27 additions and 153 deletions.
4 changes: 0 additions & 4 deletions mfbt/RefCounted.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#define mozilla_RefCounted_h

#include <utility>
#include <type_traits>

#include "mozilla/AlreadyAddRefed.h"
#include "mozilla/Assertions.h"
Expand Down Expand Up @@ -257,9 +256,6 @@ class RefCounted {
}
}

using HasThreadSafeRefCnt =
std::integral_constant<bool, Atomicity == AtomicRefCount>;

// Compatibility with wtf::RefPtr.
void ref() { AddRef(); }
void deref() { Release(); }
Expand Down
2 changes: 0 additions & 2 deletions mfbt/ThreadSafeWeakPtr.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,6 @@ class SupportsThreadSafeWeakPtr : public detail::SupportsThreadSafeWeakPtrBase {
return cnt;
}

using HasThreadSafeRefCnt = std::true_type;

// Compatibility with wtf::RefPtr
void ref() { AddRef(); }
void deref() { Release(); }
Expand Down
8 changes: 4 additions & 4 deletions toolkit/components/kvstore/nsIKeyValue.idl
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ interface nsIKeyValuePair;
/**
* The key/value service. Enables retrieval of handles to key/value databases.
*/
[scriptable, builtinclass, rust_sync, uuid(46c893dd-4c14-4de0-b33d-a1be18c6d062)]
[scriptable, uuid(46c893dd-4c14-4de0-b33d-a1be18c6d062)]
interface nsIKeyValueService : nsISupports {
/**
* Get a handle to an existing database or a newly-created one
Expand All @@ -55,7 +55,7 @@ interface nsIKeyValueService : nsISupports {
* The types of the callbacks vary, but they can all be implemented in JS
* via an object literal with the relevant methods.
*/
[scriptable, builtinclass, rust_sync, uuid(c449398e-174c-425b-8195-da6aa0ccd9a5)]
[scriptable, uuid(c449398e-174c-425b-8195-da6aa0ccd9a5)]
interface nsIKeyValueDatabase : nsISupports {
/**
* Write the specified key/value pair to the database.
Expand Down Expand Up @@ -145,7 +145,7 @@ interface nsIKeyValueDatabase : nsISupports {
/**
* A key/value pair. Returned by nsIKeyValueEnumerator.getNext().
*/
[scriptable, builtinclass, rust_sync, uuid(bc37b06a-23b5-4b32-8281-4b8479601c7e)]
[scriptable, uuid(bc37b06a-23b5-4b32-8281-4b8479601c7e)]
interface nsIKeyValuePair : nsISupports {
readonly attribute AUTF8String key;
readonly attribute nsIVariant value;
Expand All @@ -160,7 +160,7 @@ interface nsIKeyValuePair : nsISupports {
* which is another reason why you should use the kvstore.jsm module from JS
* instead of accessing this API directly.
*/
[scriptable, builtinclass, rust_sync, uuid(b9ba7116-b7ff-4717-9a28-a08e6879b199)]
[scriptable, uuid(b9ba7116-b7ff-4717-9a28-a08e6879b199)]
interface nsIKeyValueEnumerator : nsISupports {
bool hasMoreElements();
nsIKeyValuePair getNext();
Expand Down
60 changes: 11 additions & 49 deletions xpcom/base/nsISupportsImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -396,16 +396,6 @@ class ThreadSafeAutoRefCnt {
std::atomic<nsrefcnt> mValue;
};

namespace detail {

// Type trait indicating whether a given XPCOM interface class may only be
// implemented by types with threadsafe refcounts. This is specialized for
// classes with the `rust_sync` annotation within XPIDL-generated header files,
// and checked within macro-generated QueryInterface implementations.
template <typename T>
class InterfaceNeedsThreadSafeRefCnt : public std::false_type {};

}
} // namespace mozilla

///////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -1127,30 +1117,6 @@ void ProxyDeleteVoid(const char* aRunnableName,

///////////////////////////////////////////////////////////////////////////////

namespace mozilla::detail {

// Helper which is roughly equivalent to NS_GET_IID, which also performs static
// assertions that `Class` is allowed to implement the given XPCOM interface.
//
// These assertions are done like this to allow them to be used within the
// `NS_INTERFACE_TABLE_ENTRY` macro, though they are also used in
// `NS_IMPL_QUERY_BODY`.
template <typename Class, typename Interface>
constexpr const nsIID& GetImplementedIID() {
if constexpr (mozilla::detail::InterfaceNeedsThreadSafeRefCnt<
Interface>::value) {
static_assert(Class::HasThreadSafeRefCnt::value,
"Cannot implement a threadsafe interface with "
"non-threadsafe refcounting!");
}
return NS_GET_TEMPLATE_IID(Interface);
}

template <typename Class, typename Interface>
constexpr const nsIID& kImplementedIID = GetImplementedIID<Class, Interface>();

}

/**
* There are two ways of implementing QueryInterface, and we use both:
*
Expand Down Expand Up @@ -1187,13 +1153,13 @@ nsresult NS_FASTCALL NS_TableDrivenQI(void* aThis, REFNSIID aIID,

#define NS_INTERFACE_TABLE_BEGIN static const QITableEntry table[] = {
#define NS_INTERFACE_TABLE_ENTRY(_class, _interface) \
{&mozilla::detail::kImplementedIID<_class, _interface>, \
{&NS_GET_IID(_interface), \
int32_t( \
reinterpret_cast<char*>(static_cast<_interface*>((_class*)0x1000)) - \
reinterpret_cast<char*>((_class*)0x1000))},

#define NS_INTERFACE_TABLE_ENTRY_AMBIGUOUS(_class, _interface, _implClass) \
{&mozilla::detail::kImplementedIID<_class, _interface>, \
{&NS_GET_IID(_interface), \
int32_t(reinterpret_cast<char*>(static_cast<_interface*>( \
static_cast<_implClass*>((_class*)0x1000))) - \
reinterpret_cast<char*>((_class*)0x1000))},
Expand Down Expand Up @@ -1248,35 +1214,31 @@ nsresult NS_FASTCALL NS_TableDrivenQI(void* aThis, REFNSIID aIID,
"QueryInterface requires a non-NULL destination!"); \
nsISupports* foundInterface;

#define NS_IMPL_QUERY_BODY_IID(_interface) \
mozilla::detail::kImplementedIID<std::remove_reference_t<decltype(*this)>, \
_interface>

#define NS_IMPL_QUERY_BODY(_interface) \
if (aIID.Equals(NS_IMPL_QUERY_BODY_IID(_interface))) \
foundInterface = static_cast<_interface*>(this); \
#define NS_IMPL_QUERY_BODY(_interface) \
if (aIID.Equals(NS_GET_IID(_interface))) \
foundInterface = static_cast<_interface*>(this); \
else

#define NS_IMPL_QUERY_BODY_CONDITIONAL(_interface, condition) \
if ((condition) && aIID.Equals(NS_IMPL_QUERY_BODY_IID(_interface))) \
foundInterface = static_cast<_interface*>(this); \
#define NS_IMPL_QUERY_BODY_CONDITIONAL(_interface, condition) \
if ((condition) && aIID.Equals(NS_GET_IID(_interface))) \
foundInterface = static_cast<_interface*>(this); \
else

#define NS_IMPL_QUERY_BODY_AMBIGUOUS(_interface, _implClass) \
if (aIID.Equals(NS_IMPL_QUERY_BODY_IID(_interface))) \
if (aIID.Equals(NS_GET_IID(_interface))) \
foundInterface = static_cast<_interface*>(static_cast<_implClass*>(this)); \
else

// Use this for querying to concrete class types which cannot be unambiguously
// cast to nsISupports. See also nsQueryObject.h.
#define NS_IMPL_QUERY_BODY_CONCRETE(_class) \
if (aIID.Equals(NS_IMPL_QUERY_BODY_IID(_class))) { \
if (aIID.Equals(NS_GET_IID(_class))) { \
*aInstancePtr = do_AddRef(static_cast<_class*>(this)).take(); \
return NS_OK; \
} else

#define NS_IMPL_QUERY_BODY_AGGREGATED(_interface, _aggregate) \
if (aIID.Equals(NS_IMPL_QUERY_BODY_IID(_interface))) \
if (aIID.Equals(NS_GET_IID(_interface))) \
foundInterface = static_cast<_interface*>(_aggregate); \
else

Expand Down
12 changes: 0 additions & 12 deletions xpcom/docs/xpidl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -199,18 +199,6 @@ invoked on property calls instead of an object with the given property
This interface is usable by JavaScript classes. Must inherit from a
``scriptable`` interface.

``rust_sync``
`````````````

This interface is safe to use from multiple threads concurrently. All child
interfaces must also be marked with this property. Interfaces marked this way
must be either non-scriptable or ``builtinclass``, and must use threadsafe
reference counting.

Interfaces marked as ``rust_sync`` will implement the ``Sync`` trait in Rust.
For more details on what that means, read the trait's documentation:
https://doc.rust-lang.org/nightly/std/marker/trait.Sync.html.

Methods and Attributes
~~~~~~~~~~~~~~~~~~~~~~

Expand Down
16 changes: 1 addition & 15 deletions xpcom/idl-parser/xpidl/header.py
Original file line number Diff line number Diff line change
Expand Up @@ -390,9 +390,7 @@ def write_webidl(p, fd):

iface_epilog = """};
NS_DEFINE_STATIC_IID_ACCESSOR(%(name)s, %(defname)s_IID)"""

iface_decl = """
NS_DEFINE_STATIC_IID_ACCESSOR(%(name)s, %(defname)s_IID)
/* Use this macro when declaring classes that implement this interface. */
#define NS_DECL_%(macroname)s """
Expand Down Expand Up @@ -436,13 +434,6 @@ class doesn't implement the interface. This is useful for forwarding. */
}
"""

iface_threadsafe_tmpl = """\
namespace mozilla::detail {
template <>
class InterfaceNeedsThreadSafeRefCnt<%(name)s> : public std::true_type {};
}
"""


def infallibleDecl(member):
isattr = isinstance(member, xpidl.Attribute)
Expand Down Expand Up @@ -610,11 +601,6 @@ def write_attr_decl(a):

fd.write(iface_epilog % names)

if iface.attributes.rust_sync:
fd.write(iface_threadsafe_tmpl % names)

fd.write(iface_decl % names)

def writeDeclaration(fd, iface, virtual):
declType = "NS_IMETHOD" if virtual else "nsresult"
suffix = " override" if virtual else ""
Expand Down
22 changes: 4 additions & 18 deletions xpcom/idl-parser/xpidl/rust.py
Original file line number Diff line number Diff line change
Expand Up @@ -437,15 +437,13 @@ def print_rust_bindings(idl, fd, relpath):
#[repr(C)]
pub struct %(name)s {
vtable: &'static %(name)sVTable,
vtable: *const %(name)sVTable,
/// This field is a phantomdata to ensure that the VTable type and any
/// struct containing it is not safe to send across threads by default, as
/// XPCOM is generally not threadsafe.
/// struct containing it is not safe to send across threads, as XPCOM is
/// generally not threadsafe.
///
/// If this type is marked as [rust_sync], there will be explicit `Send` and
/// `Sync` implementations on this type, which will override the inherited
/// negative impls from `Rc`.
/// XPCOM interfaces in general are not safe to send across threads.
__nosync: ::std::marker::PhantomData<::std::rc::Rc<u8>>,
// Make the rust compiler aware that there might be interior mutability
Expand Down Expand Up @@ -511,15 +509,6 @@ def print_rust_bindings(idl, fd, relpath):
"""


sendsync_tmpl = """\
// This interface is marked as [rust_sync], meaning it is safe to be transferred
// and used from multiple threads silmultaneously. These override the default
// from the __nosync marker type allowng the type to be sent between threads.
unsafe impl Send for %(name)s {}
unsafe impl Sync for %(name)s {}
"""


wrapper_tmpl = """\
// The implementations of the function wrappers which are exposed to rust code.
// Call these methods rather than manually calling through the VTable struct.
Expand Down Expand Up @@ -579,9 +568,6 @@ def write_interface(iface, fd):
printComments(fd, iface.doccomments, "")
fd.write(struct_tmpl % names)

if iface.attributes.rust_sync:
fd.write(sendsync_tmpl % {"name": iface.name})

if iface.base is not None:
fd.write(
deref_tmpl
Expand Down
3 changes: 0 additions & 3 deletions xpcom/idl-parser/xpidl/rust_macros.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ def methodAsMethodStruct(iface, m):
Interface {
name: "%(name)s",
base: %(base)s,
sync: %(sync)s,
methods: %(methods)s,
},
"""
Expand Down Expand Up @@ -74,7 +73,6 @@ def write_interface(iface, fd):
% {
"name": iface.name,
"base": base,
"sync": "true" if iface.attributes.rust_sync else "false",
"methods": "Ok(&[\n%s])" % methods,
}
)
Expand All @@ -84,7 +82,6 @@ def write_interface(iface, fd):
% {
"name": iface.name,
"base": base,
"sync": "false",
"methods": 'Err("%s")' % reason,
}
)
Expand Down
24 changes: 0 additions & 24 deletions xpcom/idl-parser/xpidl/xpidl.py
Original file line number Diff line number Diff line change
Expand Up @@ -833,23 +833,6 @@ def resolve(self, parent):
"builtinclass '%s'" % (self.name, self.base),
self.location,
)

if realbase.attributes.rust_sync and not self.attributes.rust_sync:
raise IDLError(
"interface '%s' is not rust_sync but derives from rust_sync '%s'"
% (self.name, self.base),
self.location,
)

if (
self.attributes.rust_sync
and self.attributes.scriptable
and not self.attributes.builtinclass
):
raise IDLError(
"interface '%s' is rust_sync but is not builtinclass" % self.name,
self.location,
)
elif self.name != "nsISupports":
raise IDLError(
"Interface '%s' must inherit from nsISupports" % self.name,
Expand Down Expand Up @@ -930,7 +913,6 @@ class InterfaceAttributes(object):
builtinclass = False
function = False
main_process_scriptable_only = False
rust_sync = False

def setuuid(self, value):
self.uuid = value.lower()
Expand All @@ -947,17 +929,13 @@ def setbuiltinclass(self):
def setmain_process_scriptable_only(self):
self.main_process_scriptable_only = True

def setrust_sync(self):
self.rust_sync = True

actions = {
"uuid": (True, setuuid),
"scriptable": (False, setscriptable),
"builtinclass": (False, setbuiltinclass),
"function": (False, setfunction),
"object": (False, lambda self: True),
"main_process_scriptable_only": (False, setmain_process_scriptable_only),
"rust_sync": (False, setrust_sync),
}

def __init__(self, attlist, location):
Expand Down Expand Up @@ -992,8 +970,6 @@ def __str__(self):
l.append("\tfunction\n")
if self.main_process_scriptable_only:
l.append("\tmain_process_scriptable_only\n")
if self.rust_sync:
l.append("\trust_sync\n")
return "".join(l)


Expand Down
Loading

0 comments on commit ec44190

Please sign in to comment.