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
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
The issue is that when replacing frame loaders for a non-remote browser,
mFocusedElement is not that <browser> element. It instead points to the
(potentially null) focused element in mFocusedWindow.
If mFocusedWindow is about to be destroyed (or put into the bfcache), clear it
and point mFocusedElement to the <browser> instead, so that we can focus the
new frame loader if needed later.
If we don't do this, the focused window gets cleared when the window is nuked
in nsFocusManager::WasNuked (but by that point it's too late to do any fix up,
as we've lost the context that we're swapping, and of our frameloader owner,
and everything else).
Test in D232437.
Differential Revision: https://phabricator.services.mozilla.com/D233444
Modify the frame recursion check to only happen when a load is triggered
from a src or data attribute (depending on if it's an iframe, embed or
an object element). This prevents recursion checks from blocking
ordinary navigations and navigations with a target.
Differential Revision: https://phabricator.services.mozilla.com/D229503
When we're called from IPC, we can race the child for frame loader
destruction, which means we can wind up with a null `mFrameLoader` here.
Adding a null check is consistent with the other similar null checks higher
and lower in the call chain.
Differential Revision: https://phabricator.services.mozilla.com/D176286
This is a large refactoring of the DocumentChannel process switch codepath,
with the end goal of being better able to support future process switch
requirements such as dynamic isolation on android, as well as the immediate
requirement of null principal handling.
The major changes include:
1. The logic is in C++ and has less failure cases, meaning it should be harder
for us to error out unexpectedly and not process switch.
2. Process selection decisions are more explicit, and tend to rely less on
state such as the current remoteType when possible. This makes reasoning
about where a specific load will complete easier.
3. Additional checks are made after a "WebContent" behavior is selected to
ensure that if an existing document in the same BCG is found, the load will
finish in the required content process. This should make dynamic checks such
as Android's logged-in site isolation easier to implement.
4. ProcessIsolation logging is split out from DocumentChannel so that it's
easier to log just the information related to process selection when
debugging.
5. Null result principal precursors are considered when performing process
selection.
Other uses of E10SUtils for process selection have not yet been migrated to the
new design as they have slightly different requirements. This will be done in
follow-up bugs.
Differential Revision: https://phabricator.services.mozilla.com/D120673
This is a large refactoring of the DocumentChannel process switch codepath,
with the end goal of being better able to support future process switch
requirements such as dynamic isolation on android, as well as the immediate
requirement of null principal handling.
The major changes include:
1. The logic is in C++ and has less failure cases, meaning it should be harder
for us to error out unexpectedly and not process switch.
2. Process selection decisions are more explicit, and tend to rely less on
state such as the current remoteType when possible. This makes reasoning
about where a specific load will complete easier.
3. Additional checks are made after a "WebContent" behavior is selected to
ensure that if an existing document in the same BCG is found, the load will
finish in the required content process. This should make dynamic checks such
as Android's logged-in site isolation easier to implement.
4. ProcessIsolation logging is split out from DocumentChannel so that it's
easier to log just the information related to process selection when
debugging.
5. Null result principal precursors are considered when performing process
selection.
Other uses of E10SUtils for process selection have not yet been migrated to the
new design as they have slightly different requirements. This will be done in
follow-up bugs.
Differential Revision: https://phabricator.services.mozilla.com/D120673
I'm unsure if this is the cause of our crashes, but there is a chance that we
are somehow attempting to re-initialize a nsFrameLoader after it has been
destroyed, and thus re-connecting to a process which has has an
already-destroyed browser. This adds a flag to prevent initialization from
being attempted multiple times for a nsFrameLoader.
Differential Revision: https://phabricator.services.mozilla.com/D121688
This is a large refactoring of the DocumentChannel process switch codepath,
with the end goal of being better able to support future process switch
requirements such as dynamic isolation on android, as well as the immediate
requirement of null principal handling.
The major changes include:
1. The logic is in C++ and has less failure cases, meaning it should be harder
for us to error out unexpectedly and not process switch.
2. Process selection decisions are more explicit, and tend to rely less on
state such as the current remoteType when possible. This makes reasoning
about where a specific load will complete easier.
3. Additional checks are made after a "WebContent" behavior is selected to
ensure that if an existing document in the same BCG is found, the load will
finish in the required content process. This should make dynamic checks such
as Android's logged-in site isolation easier to implement.
4. ProcessIsolation logging is split out from DocumentChannel so that it's
easier to log just the information related to process selection when
debugging.
5. Null result principal precursors are considered when performing process
selection.
Other uses of E10SUtils for process selection have not yet been migrated to the
new design as they have slightly different requirements. This will be done in
follow-up bugs.
Differential Revision: https://phabricator.services.mozilla.com/D120673
This is a large refactoring of the DocumentChannel process switch codepath,
with the end goal of being better able to support future process switch
requirements such as dynamic isolation on android, as well as the immediate
requirement of null principal handling.
The major changes include:
1. The logic is in C++ and has less failure cases, meaning it should be harder
for us to error out unexpectedly and not process switch.
2. Process selection decisions are more explicit, and tend to rely less on
state such as the current remoteType when possible. This makes reasoning
about where a specific load will complete easier.
3. Additional checks are made after a "WebContent" behavior is selected to
ensure that if an existing document in the same BCG is found, the load will
finish in the required content process. This should make dynamic checks such
as Android's logged-in site isolation easier to implement.
4. ProcessIsolation logging is split out from DocumentChannel so that it's
easier to log just the information related to process selection when
debugging.
5. Null result principal precursors are considered when performing process
selection.
Other uses of E10SUtils for process selection have not yet been migrated to the
new design as they have slightly different requirements. This will be done in
follow-up bugs.
Differential Revision: https://phabricator.services.mozilla.com/D120673
This should pretty much match our non-fission behavior. I found the
original approach of keeping the frameloader from nsSubDocumentFrame
quite hard to get green on try unfortunately, because among other
things, the new frameloader wouldn't get the right viewport sizes /
position / etc.
Differential Revision: https://phabricator.services.mozilla.com/D121198
This is a large refactoring of the DocumentChannel process switch codepath,
with the end goal of being better able to support future process switch
requirements such as dynamic isolation on android, as well as the immediate
requirement of null principal handling.
The major changes include:
1. The logic is in C++ and has less failure cases, meaning it should be harder
for us to error out unexpectedly and not process switch.
2. Process selection decisions are more explicit, and tend to rely less on
state such as the current remoteType when possible. This makes reasoning
about where a specific load will complete easier.
3. Additional checks are made after a "WebContent" behavior is selected to
ensure that if an existing document in the same BCG is found, the load will
finish in the required content process. This should make dynamic checks such
as Android's logged-in site isolation easier to implement.
4. ProcessIsolation logging is split out from DocumentChannel so that it's
easier to log just the information related to process selection when
debugging.
5. Null result principal precursors are considered when performing process
selection.
Other uses of E10SUtils for process selection have not yet been migrated to the
new design as they have slightly different requirements. This will be done in
follow-up bugs.
Differential Revision: https://phabricator.services.mozilla.com/D120673
The crash happens currently when there are more than one loading session history entries at the same time and when
the latter load then accesses current active entry, it has already the bfcached frameloader.
I have tried and failed to write a testcase for this. The Pernosco record is from running
https://searchfox.org/mozilla-central/source/devtools/client/webconsole/test/browser/browser_webconsole_message_categories.js
and the crash has happened when there is a race condition between a load initiated in the parent process and another load initiated in a child
process.
The patch tries to make the setup rather safe. If active entry has changed or it has gotten frameloader, don't try to bfcache.
Differential Revision: https://phabricator.services.mozilla.com/D116410
Previously, we would need to suspend progress events from the previous
BrowserParent, as otherwise we would receive STATE_STOP progress notifications
from the previous browser when it is destroyed, which would throw off frontend
code. With the new checks added by part 1, we will now catch these cases by
detecting that the current window global has changed, and we can get rid of this
explicit override.
Differential Revision: https://phabricator.services.mozilla.com/D105557
The change to dom/base/nsFrameLoaderOwner.cpp is to log about the issues but still ensure we don't crash.
I'd prefer to not put error loads to bfcache.
Differential Revision: https://phabricator.services.mozilla.com/D107300
The name RemotenessChangeState uses same the convention as the related methods, even though there might
not be a remoteness change happening, only a browsing context switch. But the naming
inconsistency exists there even without any bfcache work.
RemotenessChangeState will be renamed to RemotenessChangeOptions in a followup.
Differential Revision: https://phabricator.services.mozilla.com/D105229
Olli Pettay