The callsites now mostly use MakeAndAddRef (which just stuffs a new object into
a RefPtr and calls `forget()` to yield an already_AddRefed).
In cases where the callsite constructs the object earlier and retains a strong
reference, they'll now use do_AddRef to explicitly generate a strong reference
(as an already_AddRefed) to pass into RegisterStrongMemoryReporter.
Differential Revision: https://phabricator.services.mozilla.com/D299042
This reverts commit 1ab110ab2f.
Revert "Bug 2037761: Change RegisterStrongMemoryReporter to take already_AddRefed instead of a raw pointer, and update callsites. r=dom-core,smaug"
This reverts commit 2e187ddbc8.
The callsites now mostly use MakeAndAddRef (which just stuffs a new object into
a RefPtr and calls `forget()` to yield an already_AddRefed).
In cases where the callsite constructs the object earlier and retains a strong
reference, they'll now use do_AddRef to explicitly generate a strong reference
(as an already_AddRefed) to pass into RegisterStrongMemoryReporter.
Differential Revision: https://phabricator.services.mozilla.com/D299042
The changeset has been generated automatically through:
./mach static-analysis check --output ./clang-tidy-bugprone.json --format json --fix
followed by manual adjustment when needed or when the auto-fixer bailed out.
This paves the way for Bug 2023527
Differential Revision: https://phabricator.services.mozilla.com/D287941
The changeset has been generated automatically through:
./mach static-analysis check --output ./clang-tidy-bugprone.json --format json --fix
followed by manual adjustment when needed or when the auto-fixer bailed out.
This paves the way for Bug 2023527
Differential Revision: https://phabricator.services.mozilla.com/D287941
This patch is to remove the ClonedMessageData type which is currently
frequently used for sending a StructuredCloneData over IPC manually. Instead,
under the new approach, the StructuredCloneData is transferred directly using
its serializer.
This allows for simplification of various types across the codebase which
previously needed complex handling for this manual {de-}serialization step,
especially around keeping the borrowed-from buffers alive in some cases.
As part of this, the type has also been changed to be a threadsafe refcounted
object. This is in part possible due to earlier parts, which have made it safe
to Read() from the same StructuredCloneData object concurrently from multiple
threads (so long as it does not support transferrables).
The underlying data storage within StructuredCloneData was previously already
reference counted (due to SharedJSAllocatedData).
In addition to StructuredCloneData, other data structures (e.g.
nsStructuredCloneContainer and SharedMessageBody) have also been updated to be
serialized directly, avoiding now-unnecessary intermediate objects.
Differential Revision: https://phabricator.services.mozilla.com/D281115
Previously, we would copy the data and deserialize it once for each listener.
This interacts poorly with the actual data which can be sent over the message
manager, as it is possible to include transferables (which must only be
deserialized once).
This new approach ensures that the data structure is always deserialized once
in the shared scope, and then passed to all of the listeners, avoiding issues
with deserializing the same transferable multiple times.
Differential Revision: https://phabricator.services.mozilla.com/D281553
This was never used outside of tests, and adds a substantial amount of
complexity to the nsFrameMessageManager's ReceiveMessage implementation, which
is going to be further rewritten in the next part.
Differential Revision: https://phabricator.services.mozilla.com/D281552
This already would not have worked (as you cannot transfer a transferrable to
multiple destinations at the same time). This makes it explicit, preventing
future code from being added and breaking things, and ensuring that codepaths
in StructuredCloneHolder which mutate the object during Read will not.
We already disable transferrables in this way for BroadcastChannel, but this
codepath was likely missed due to only being used by internal system JS.
Differential Revision: https://phabricator.services.mozilla.com/D281114
This is the last member of StructuredCloneHolder which is mutated when reading
a StructuredCloneHolder which does not support transfers.
This does change behaviour, as the Read functions no longer clobber every
exception thrown by the StructuredClone implementation with a DataCloneError
exception, instead stealing the exceptions as they are thrown. The behaviour of
the Write function has been changed less substantially, as it already avoided
clobbering existing exceptions.
This change in behaviour is not a complete shift. Our Structured Clone custom
callbacks have historically not reliably thrown exceptions on failure, so
uncatchable exceptions are replaced with generic exceptions before being
returned to the JS engine to avoid throwing uncatchable exceptions in error
cases where we did not previously.
It's worth noting that this change assumes that the JS implementation of
structured clone reliably throws exceptions itself. In general SpiderMonkey
code is better about this (due to better-established patterns), than Gecko code
using SpiderMonkey calling conventions.
The new implementations of Read/Write use StealExceptionFromJSContext over
NoteJSContextException, as SuppressExceptions/IgnoreErrors/IgnoredErrorResult
are frequently used with StructuredCloneHolder, and JSContext exceptions are
not suppressed when SuppressExceptions is called.
Differential Revision: https://phabricator.services.mozilla.com/D281113
This reverts commit 72c32e25bf.
Revert "Bug 2013389 - Part 11: Make attachment arrays more consistent in StructuredCloneHolder, r=asuth"
This reverts commit c0912428c7.
Revert "Bug 2013389 - Part 10: Fix version tracking in nsStructuredCloneContainer, r=asuth,extension-reviewers,spidermonkey-reviewers,sfink,robwu"
This reverts commit f2e9691831.
Revert "Bug 2013389 - Part 9: Remove the now-unused ReadFromBuffer method, r=asuth"
This reverts commit 6eb034458e.
Revert "Bug 2013389 - Part 8: Avoid the unnecessary ClonedMessageData intermediate, r=asuth,mccr8,extension-reviewers,dom-worker-reviewers,robwu"
This reverts commit 4dd6f1d6ea.
Revert "Bug 2013389 - Part 7: Only deserialize once in nsFrameMessageManager, r=asuth"
This reverts commit 65d16af619.
Revert "Bug 2013389 - Part 6: Remove weak listener support from MessageManager, r=smaug"
This reverts commit 66f4b308a0.
Revert "Bug 2013389 - Part 5: Broadcasting MessageManagers cannot support transferrables, r=asuth"
This reverts commit e95e1a5a6c.
Revert "Bug 2013389 - Part 4: Avoid depending on mErrorMessage when deserializing StructuredCloneHolder, r=asuth,dom-worker-reviewers"
This reverts commit ae78c69357.
Revert "Bug 2013389 - Part 3: Remove aGlobal argument from StructuredCloneHolder::Read, r=asuth,dom-worker-reviewers"
This reverts commit 066bd37bfd.
Revert "Bug 2013389 - Part 2: Stop tracking mGlobal within StructuredCloneHolder::Read, r=asuth"
This reverts commit 6b4f3a43c0.
Revert "Bug 2013389 - Part 1: Add an Adopt API to StructuredCloneHolder, r=asuth"
This reverts commit 8624db71c2.
This patch is to remove the ClonedMessageData type which is currently
frequently used for sending a StructuredCloneData over IPC manually. Instead,
under the new approach, the StructuredCloneData is transferred directly using
its serializer.
This allows for simplification of various types across the codebase which
previously needed complex handling for this manual {de-}serialization step,
especially around keeping the borrowed-from buffers alive in some cases.
As part of this, the type has also been changed to be a threadsafe refcounted
object. This is in part possible due to earlier parts, which have made it safe
to Read() from the same StructuredCloneData object concurrently from multiple
threads (so long as it does not support transferrables).
The underlying data storage within StructuredCloneData was previously already
reference counted (due to SharedJSAllocatedData).
In addition to StructuredCloneData, other data structures (e.g.
nsStructuredCloneContainer and SharedMessageBody) have also been updated to be
serialized directly, avoiding now-unnecessary intermediate objects.
Differential Revision: https://phabricator.services.mozilla.com/D281115
Previously, we would copy the data and deserialize it once for each listener.
This interacts poorly with the actual data which can be sent over the message
manager, as it is possible to include transferables (which must only be
deserialized once).
This new approach ensures that the data structure is always deserialized once
in the shared scope, and then passed to all of the listeners, avoiding issues
with deserializing the same transferable multiple times.
Differential Revision: https://phabricator.services.mozilla.com/D281553
This was never used outside of tests, and adds a substantial amount of
complexity to the nsFrameMessageManager's ReceiveMessage implementation, which
is going to be further rewritten in the next part.
Differential Revision: https://phabricator.services.mozilla.com/D281552
This already would not have worked (as you cannot transfer a transferrable to
multiple destinations at the same time). This makes it explicit, preventing
future code from being added and breaking things, and ensuring that codepaths
in StructuredCloneHolder which mutate the object during Read will not.
We already disable transferrables in this way for BroadcastChannel, but this
codepath was likely missed due to only being used by internal system JS.
Differential Revision: https://phabricator.services.mozilla.com/D281114
This is the last member of StructuredCloneHolder which is mutated when reading
a StructuredCloneHolder which does not support transfers.
This does change behaviour, as the Read functions no longer clobber every
exception thrown by the StructuredClone implementation with a DataCloneError
exception, instead stealing the exceptions as they are thrown. The behaviour of
the Write function has been changed less substantially, as it already avoided
clobbering existing exceptions.
This change in behaviour is not a complete shift. Our Structured Clone custom
callbacks have historically not reliably thrown exceptions on failure, so
uncatchable exceptions are replaced with generic exceptions before being
returned to the JS engine to avoid throwing uncatchable exceptions in error
cases where we did not previously.
It's worth noting that this change assumes that the JS implementation of
structured clone reliably throws exceptions itself. In general SpiderMonkey
code is better about this (due to better-established patterns), than Gecko code
using SpiderMonkey calling conventions.
The new implementations of Read/Write use StealExceptionFromJSContext over
NoteJSContextException, as SuppressExceptions/IgnoreErrors/IgnoredErrorResult
are frequently used with StructuredCloneHolder, and JSContext exceptions are
not suppressed when SuppressExceptions is called.
Differential Revision: https://phabricator.services.mozilla.com/D281113
This reverts commit 5c5cf7465b.
Revert "Bug 2013389 - Part 10: Fix version tracking in nsStructuredCloneContainer, r=asuth,extension-reviewers,spidermonkey-reviewers,sfink,robwu"
This reverts commit c87a327d86.
Revert "Bug 2013389 - Part 9: Remove the now-unused ReadFromBuffer method, r=asuth"
This reverts commit 5adf49e5d8.
Revert "Bug 2013389 - Part 8: Avoid the unnecessary ClonedMessageData intermediate, r=asuth,mccr8,extension-reviewers,dom-worker-reviewers,robwu"
This reverts commit f6d12563fe.
Revert "Bug 2013389 - Part 7: Only deserialize once in nsFrameMessageManager, r=asuth"
This reverts commit 44c1325d76.
Revert "Bug 2013389 - Part 6: Remove weak listener support from MessageManager, r=smaug"
This reverts commit 1d1b495c91.
Revert "Bug 2013389 - Part 5: Broadcasting MessageManagers cannot support transferrables, r=asuth"
This reverts commit e2eb75ff8f.
Revert "Bug 2013389 - Part 4: Avoid depending on mErrorMessage when deserializing StructuredCloneHolder, r=asuth,dom-worker-reviewers"
This reverts commit d9a234ad7b.
Revert "Bug 2013389 - Part 3: Remove aGlobal argument from StructuredCloneHolder::Read, r=asuth,dom-worker-reviewers"
This reverts commit 48279cea50.
Revert "Bug 2013389 - Part 2: Stop tracking mGlobal within StructuredCloneHolder::Read, r=asuth"
This reverts commit 27162e5046.
Revert "Bug 2013389 - Part 1: Add an Adopt API to StructuredCloneHolder, r=asuth"
This reverts commit e3a9f32716.
This patch is to remove the ClonedMessageData type which is currently
frequently used for sending a StructuredCloneData over IPC manually. Instead,
under the new approach, the StructuredCloneData is transferred directly using
its serializer.
This allows for simplification of various types across the codebase which
previously needed complex handling for this manual {de-}serialization step,
especially around keeping the borrowed-from buffers alive in some cases.
As part of this, the type has also been changed to be a threadsafe refcounted
object. This is in part possible due to earlier parts, which have made it safe
to Read() from the same StructuredCloneData object concurrently from multiple
threads (so long as it does not support transferrables).
The underlying data storage within StructuredCloneData was previously already
reference counted (due to SharedJSAllocatedData).
In addition to StructuredCloneData, other data structures (e.g.
nsStructuredCloneContainer and SharedMessageBody) have also been updated to be
serialized directly, avoiding now-unnecessary intermediate objects.
Differential Revision: https://phabricator.services.mozilla.com/D281115
Previously, we would copy the data and deserialize it once for each listener.
This interacts poorly with the actual data which can be sent over the message
manager, as it is possible to include transferables (which must only be
deserialized once).
This new approach ensures that the data structure is always deserialized once
in the shared scope, and then passed to all of the listeners, avoiding issues
with deserializing the same transferable multiple times.
Differential Revision: https://phabricator.services.mozilla.com/D281553
This was never used outside of tests, and adds a substantial amount of
complexity to the nsFrameMessageManager's ReceiveMessage implementation, which
is going to be further rewritten in the next part.
Differential Revision: https://phabricator.services.mozilla.com/D281552
This already would not have worked (as you cannot transfer a transferrable to
multiple destinations at the same time). This makes it explicit, preventing
future code from being added and breaking things, and ensuring that codepaths
in StructuredCloneHolder which mutate the object during Read will not.
We already disable transferrables in this way for BroadcastChannel, but this
codepath was likely missed due to only being used by internal system JS.
Differential Revision: https://phabricator.services.mozilla.com/D281114
This is the last member of StructuredCloneHolder which is mutated when reading
a StructuredCloneHolder which does not support transfers.
This does change behaviour, as the Read functions no longer clobber every
exception thrown by the StructuredClone implementation with a DataCloneError
exception, instead stealing the exceptions as they are thrown. The behaviour of
the Write function has been changed less substantially, as it already avoided
clobbering existing exceptions.
This change in behaviour is not a complete shift. Our Structured Clone custom
callbacks have historically not reliably thrown exceptions on failure, so
uncatchable exceptions are replaced with generic exceptions before being
returned to the JS engine to avoid throwing uncatchable exceptions in error
cases where we did not previously.
It's worth noting that this change assumes that the JS implementation of
structured clone reliably throws exceptions itself. In general SpiderMonkey
code is better about this (due to better-established patterns), than Gecko code
using SpiderMonkey calling conventions.
The new implementations of Read/Write use StealExceptionFromJSContext over
NoteJSContextException, as SuppressExceptions/IgnoreErrors/IgnoredErrorResult
are frequently used with StructuredCloneHolder, and JSContext exceptions are
not suppressed when SuppressExceptions is called.
Differential Revision: https://phabricator.services.mozilla.com/D281113
System add-ons can be installed in the profile directory as .xpi JAR files that
are updated independently of the application. Prior to this patch, when these JARs
are updated independently from the rest of the application, the ScriptPreloader
and StartupCache can serve stale bytecode compiled from the previous version of
the JAR, causing the old code to execute even though the JAR has been replaced.
This fix here adds a helper to checking if a script's JAR file is in one of
the omni.ja files before using cached bytecode.
Scripts from the omni.ja files continue to benefit from bytecode caching as
before.
Differential Revision: https://phabricator.services.mozilla.com/D280333
This reverts commit 74f72acbbb.
Revert "Bug 2012009 - Part 1: Don't retrieve scripts from non omni.ja JAR files from the ScriptPreloader or StartupCache. r=tcampbell"
This reverts commit 25dbc84378.
System add-ons can be installed in the profile directory as .xpi JAR files that
are updated independently of the application. Prior to this patch, when these JARs
are updated independently from the rest of the application, the ScriptPreloader
and StartupCache can serve stale bytecode compiled from the previous version of
the JAR, causing the old code to execute even though the JAR has been replaced.
This fix here adds a helper to checking if a script's JAR file is in one of
the omni.ja files before using cached bytecode.
Scripts from the omni.ja files continue to benefit from bytecode caching as
before.
Differential Revision: https://phabricator.services.mozilla.com/D280333
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
While SCD doesn't have the problems that its parent classes have in terms
of moving, people have done a poor job of keeping the move constructor
updated as they add new auxiliary arrays. This led to strange behavior where
something would work when being sent between processes, but would crash
with a bounds error when being sent within a process.
This patch addresses the problem by preventing SCD from being moved. Instead,
UniquePtr<StructuredCloneData> must be used to move them around.
The dropping of the "using" of SCD from PBrowser means a few related places
have to explicitly qualify SCD with the ipc namespace.
Differential Revision: https://phabricator.services.mozilla.com/D262718
This reverts commit cd2962e220.
Revert "Bug 1977139, part 1 - Use macro for HasClonedDOMObjects and StructuredCloneHolder::Read. r=asuth"
This reverts commit 2cc5ec76dd.
While SCD doesn't have the problems that its parent classes have in terms
of moving, people have done a poor job of keeping the move constructor
updated as they add new auxiliary arrays. This led to strange behavior where
something would work when being sent between processes, but would crash
with a bounds error when being sent within a process.
This patch addresses the problem by preventing SCD from being moved. Instead,
UniquePtr<StructuredCloneData> must be used to move them around.
The dropping of the "using" of SCD from PBrowser means a few related places
have to explicitly qualify SCD with the ipc namespace.
Differential Revision: https://phabricator.services.mozilla.com/D262718
This changes a lot of places, but it's essentially just find and
replace. We had to change this old marker API, because the new API
uses this one to serialize the marker schema fields.
This patch removes the AddKey{Label,}FormatSearchable methods and
switches to use the original AddKey{Label,}Format methods with an
optional parameter instead to make sure that we can pass multiple flags
instead of just Searchable.
For Rust, since we can't have default argument values, we had to create
a new method for them.
Differential Revision: https://phabricator.services.mozilla.com/D260465
Most changes are automated. Relevant bit is dom/.clang-format. I had to
manually disable clang-format in a few places where importing external
files and so on that aren't self-contained, and fix a couple includes /
forward declarations.
Differential Revision: https://phabricator.services.mozilla.com/D256800
In D220213 we tried to change `Symbol.unscopables` so that it's only supported on
syntactic `with` environments (for performance reasons). This failed some WPT tests
for event handlers because they use non-syntactic with-environments but have to
support `Symbol.unscopables`.
This patch adds a `JS::EnvironmentChain` class and uses it instead of an object vector
for some JS APIs. This class also lets the embedder specify whether `Symbol.unscopables`
must be supported. The `evaluate` shell function now has an option for this too.
Differential Revision: https://phabricator.services.mozilla.com/D226139
In D220213 we tried to change `Symbol.unscopables` so that it's only supported on
syntactic `with` environments (for performance reasons). This failed some WPT tests
for event handlers because they use non-syntactic with-environments but have to
support `Symbol.unscopables`.
This patch adds a `JS::EnvironmentChain` class and uses it instead of an object vector
for some JS APIs. This class also lets the embedder specify whether `Symbol.unscopables`
must be supported. The `evaluate` shell function now has an option for this too.
Differential Revision: https://phabricator.services.mozilla.com/D226139
Daniel Holbert