Depends on D103325
When we stop watching for resources, if there are no other watchers currently in activity, we will unregister the JSWindowActors, with following stack:
```
unregisterJSWindowActor@resource://devtools/server/actors/watcher/WatcherRegistry.jsm:344:17
maybeUnregisteringJSWindowActor@resource://devtools/server/actors/watcher/WatcherRegistry.jsm:280:7
unwatchTargets@resource://devtools/server/actors/watcher.js:199:21
handler@resource://devtools/shared/protocol/Actor.js:172:37
onPacket@resource://devtools/server/devtools-server-connection.js:379:58
send/<@resource://devtools/shared/transport/local-transport.js:68:25
exports.makeInfallible/<@resource://devtools/shared/ThreadSafeDevToolsUtils.js:103:22
DevToolsUtils.executeSoon*exports.executeSoon@resource://devtools/shared/DevToolsUtils.js:54:21
send@resource://devtools/shared/transport/local-transport.js:56:21
send@resource://devtools/shared/protocol/Front.js:272:30
generateRequestMethods/</frontProto[name]@resource://devtools/shared/protocol/Front/FrontClassWithSpec.js:42:14
stopListening@resource://devtools/shared/resources/target-list.js:363:29
destroy@resource://devtools/shared/resources/target-list.js:612:10
```
This will lead to destroy all the currently created JSWindowActors, which will also destroy the corresponding target actors.
However RDM will still try to perform some cleanups on the current target after destroying the target-list. These calls will fail as soon as RDM uses a JSWindowActor based target.
This will only start happening in Bug 1644397, but then it will make the test browser_tab_remoteness_change_fission_switch_target.js fail on shutdown.
Differential Revision: https://phabricator.services.mozilla.com/D103326
RDM uses a dedicated client, target-list etc. Which means that we can use a local tab descriptor and remove the target switching logic from RDM.
New test is added in D103688
Differential Revision: https://phabricator.services.mozilla.com/D103325
Streams serialized with the default nsIInputStream serializer will be serialized
with delayedStart, meaning that when sent from the parent process to the content
process it will be serialized as a RemoteLazyInputStream.
In the specific case of SendOpenStream this causes issues, as the content
process code depends on the synchronous nature of nsIFileInputStream to allow it
to be wrapped in a SnappyUncompressInputStream, which requires a sync stream.
Relying on this property is not generally correct and only works because we know
only nsFileInputStream will be sent by the parent process. Other types of sync
streams may be received as async if they are sufficiently large, such as
nsStringInputStream.
Differential Revision: https://phabricator.services.mozilla.com/D103227
SetData will check that `mStartedReading` has not been set yet to ensure the
stream is in a non-partially-read state. This can lead to assertion failures
when sending a stream not seeked to the beginning over IPC.
Differential Revision: https://phabricator.services.mozilla.com/D103226
Previously if `Clone()` was called on a closed nsPipeInputStream, it could cause
crashes due to the already-closed nsPipeInputStream being added to mInputList,
violating internal nsPipe invariants. Skipping adding the stream to that list
should avoid this edge-case, as the pipe is already closed.
Differential Revision: https://phabricator.services.mozilla.com/D101807
nsMIMEInputStream has a requirement that the inner nsIInputStream object
implement nsISeekableStream, which is usually enforced by the SetData method.
This check was bypassed by the Deserialize method, which unfortunately meant
that non-seekable IPC payloads could end up within a nsMIMEInputStream when sent
from another process (e.g. due to large nsStringStreams being serialized as
nsPipes over IPC).
This patch uses the SeekableStreamWrapper introduced in the previous patch to
wrap the inner stream when deserializing nsMIMEInputStream, avoiding the
previously mentioned issue.
Differential Revision: https://phabricator.services.mozilla.com/D101806
This patch introduces a new SeekableStreamWrapper class which handles adapting
nsIInputStreams which support being cheaply cloned using nsICloneableInputStream
into seekable input streams by operating on a clone of the original stream, and
re-cloning that stream when seeking backwards.
This wrapper is generally intended to be used with nsPipeInputStream as that
type supports both a fairly cheap clone operation, and keeping a large internal
buffer which is fairly cheap to seek using this method, but should also work
with other types such as RemoteLazyInputStream or nsStringStream.
An alternate strategy was considered where nsPipe was given internal support for
a mSeekable flag to be set on creation. This flag would then have a similar
effect, except with additional optimizations due to being visible within the
implementation of the nsPipe, rather than relying on an unadvanced
nsPipeInputStream to keep the buffer alive.
I ended up choosing this approach instead for a few reasons:
* The seekable adapter can be applied to an already-created nsPipeInputStream,
such as one received from IPC. With the nsPipe approach, making an IPC stream
seekable either requires telling IPCStreamDestination to use a seekable pipe
ahead of time, or performing a NS_AsyncCopy from the IPC-provided pipe into a
different seekable pipe, which is likely wasted effort and would prevent
optimizations such as RemoteLazyInputStream and DelayedStart streams.
* The adapter can support other features of the underlying stream, such as
nsIInputStreamLength, without resorting to adding additional adapter layers
on top of the returned nsPipe.
* The performance is unlikely to be substantially different in the most common
case, which is using Seek(NS_SEEK_SET, 0) to return to the beginning of the
stream.
* Less additional complexity is added to the already-complicated internals of
nsPipe, and instead it is kept in a separate wrapper stream, which is easier
to review.
Using nsStorageStream, as is used by EnsureUploadStreamIsCloneable, was also
considered, but was rejected as it has similar problems to the seekable nsPipe
approach and also doesn't implement nsIAsyncStream, meaning that one must wait
for the NS_AsyncCopy to be completed before reading the stream.
It may actually be possible to replace the existing uses of nsStorageStream with
a wrapped nsPipe in the future, but that is left as follow-up material, and may
have memory overhead implications due to nsPipe not resizing the final segment,
unlike nsStorageStream.
Differential Revision: https://phabricator.services.mozilla.com/D101805
The current set of cases where a nsIInputStream is directly serialized over IPC
is fairly limited, and includes cases where the IPDL struct may be directly
stored within a content process, having the nsIInputStream left unused.
By switching this serialization to DelayedStart, we can avoid performing
unnecessary streaming copies of IPC data when sending postdata streams related
to session history and document navigation, while also avoiding issues like this
coming up again due to delayed start being the default.
Differential Revision: https://phabricator.services.mozilla.com/D101804
When aDelayedStart is specified, if the created IPCStream contains an internal
IPC stream actor, an actor is used under the hood to send raw data between
processes when needed. This is usually done to reduce overhead in cases where
the target process may not use the nsIInputStream immediately, if at all.
By switching to using RemoteLazyInputStream when sending DelayedStart actors
from parent to content, the amount of data sent in the initial payload is
reduced even further, and the stream is optimized to allow sending it back to
the parent process without streaming the data through the content process again.
Normal delayed start streams are still used when sending payloads from other
processes, as RemoteLazyInputStream is only supported for nsIInputStreams
originating in the parent process.
Differential Revision: https://phabricator.services.mozilla.com/D101803
Previously, we would apply the InputStreamLengthWrapper before
DelayedStartInputStream, which meant that a stream serialized with aDelayedStart
would not correctly implement nsIInputStreamLength. By inverting the order of
these wrappers, the nsIInputStreamLength implementation is visible, without
impacting the functionality of the DelayedStartInputStream wrapper.
Differential Revision: https://phabricator.services.mozilla.com/D101802
In a later part of this patch series, this will be necessary to handle cases
when BackgroundParentImpl* (or other PBackgroundParent subclasses) are passed to
RemoteLazyInputStreamParent::Create, as previously this would've caused an
undefined linker symbol error. By using an explicit overload set, we can move
the subclass conversion to the caller, and avoid this issue.
Differential Revision: https://phabricator.services.mozilla.com/D101801
Without the other patches in this series, this test fails with both with and
without Fission enabled, for two different reasons.
With Fission disabled, the second reload request appears as empty, due to us
being unable to rewind the postData nsIInputStream. With Fission enabled, the
second reload request causes crashes due to the nsMIMEInputStream's invariant of
requiring a seekable stream is invalidated, causing the nsICloneableInputStream
implementation to misbehave.
Differential Revision: https://phabricator.services.mozilla.com/D101800
Using the same url as the following link is a slippery slope because
the test doesn't really wait for the load to finish, so some of the
targets for iframes and such seem to never run and the test times out
very intermittently.
I've confirmed this is an issue unrelated to the changes in the patch.
MANUAL PUSH: Unreviewed one-liner test fix.
Previously, there was undefined behaviour when validating enum values in
EnumSerializer, which were actually invalid (which was hit during fuzzing, e.g.),
because the invalid integral value was cast to the enum type for comparison.
This patch changes the comparison to cast the valid values to their integral
values instead and compare those.
Differential Revision: https://phabricator.services.mozilla.com/D103449
Instead of collection both origins exceeding their group limit and the global
limit before clearing any origin, do that separately. This removes the need
to adjust the global usage while determining which origins to evict simulating
the effects of evicting the origins exceeding their group limit.
This requires moving the lock into the GetOrigin* functions (thus removing
"Locked" from their names). Also, the arguments to GetOriginInfosExceedingGlobalLimit
are removed.
As an effect, the predicate passed into MaybeInsertOriginInfos is the same in
both uses, and just checks the IsPersisted property, so the predicate argument
is removed and the function is renamed to MaybeInsertNonPersistedOriginInfos.
Differential Revision: https://phabricator.services.mozilla.com/D103629
Instead of inserting all elements into OriginInfo array, and then removing
or skipping irrelevant entries, only insert the relevant ones in the first
place, using a new utility function template MaybeAppendOriginInfos.
Sort them by in least-recently used order according to the access timestamp.
Iterate over them to identify the threshold.
Finally, truncate the array to the threshold position.
Differential Revision: https://phabricator.services.mozilla.com/D101146
Extracts parts of CheckTemporaryStorageLimits into the following new private
member functions of QuotaManager:
* LockedGetOriginInfosExceedingGroupLimit
* LockedGetOriginInfosExceedingGlobalLimit
* GetOriginInfosExceedingLimits
* ClearOrigins
Differential Revision: https://phabricator.services.mozilla.com/D101145
Julian Descottes
Jeff Muizelaar
Kris Maglione
Nick Alexander
Jon Bauman
Nika Layzell
Tooru Fujisawa
Emilio Cobos Álvarez
Emma Malysz
Nan Jiang
Mihai Alexandru Michis
Simon Giesecke
Iain Ireland
Chris H-C