This is the main part to address bug 1701368.
Before this patch, `nsAvailableMemoryWatcher` directly broadcasted a memory-pressure
event when we enter into a low-memory situation and `TabUnloader` unloaded a tab in
response to the memory-pressure message. We want to decouple `TabUnloader` from
memory-pressure listeners because unloading a tab may solve a low-memory situation
alone.
With this patch, if `nsAvailableMemoryWatcher` detects a low-memory situation,
it invokes `TabUnloader` synchronously via an XPCOM interface. If `TabUnloader`
unloads a tab, we don't do any further action. If there is no discardable tab,
`TabUnloader` notifies back `nsAvailableMemoryWatcher` via another XPCOM interface,
so that `nsAvailableMemoryWatcher` can notify of a memory-pressure event.
Differential Revision: https://phabricator.services.mozilla.com/D117673
This patch introduces an XPCOM object which is represented by the single instance of
`nsAvailableMemoryWatcherBase` so that `nsAvailableMemoryWatcher` can synchronously
access `TabUnloader`.
We currently implement a watcher class for Windows only. For other platforms, what
we need to do is to define a class inherinting `nsAvailableMemoryWatcherBase` and
a simple factory method `CreateAvailableMemoryWatcher()` returning an instance of
that class.
Differential Revision: https://phabricator.services.mozilla.com/D118393
This patch removes dependency on the available physical memory.
With this patch, `nsAvailableMemoryWatcher` triggers `OnLowMemory` when the available
commit space is low, and triggers `OnHighMemory` when the available commit space is
no longer low.
The key part of this change is the `if` block in `nsAvailableMemoryWatcher::Notify`,
where we use a single condition `IsCommitSpaceLow()` to declare either Low or High.
After this change, `OnLowMemory` is called not only in the main thread but also in
a worker thread. So `StartPollingIfUserInteracting` also needs a lock to protect
`mPolling`.
Differential Revision: https://phabricator.services.mozilla.com/D117672
We had `NS_DispatchMemoryPressure` and `NS_DispatchEventualMemoryPressure`
to dispatch a memory-pressure event which took `MemPressure_New` and
`MemPressure_Ongoing` to translate into "low-memory" and "low-memory-ongoing"
message respectively.
With that model, we could end up sending a wrong message if somebody
called the API with `MemPressure_Ongoing` without sending `MemPressure_New`.
To avoid that, this patch removes `MemPressure_Ongoing` and makes
the API decide whether it should dispatch a "new" event or "ongoing" event.
Differential Revision: https://phabricator.services.mozilla.com/D119122
1. Use `nsAutoHandle` instead of a raw `HANDLE`
2. Add a dtor with `MOZ_ASSERT`
3. Prevent double init
4. Cache `nsAvailableMemoryWatcher::mObserverSvc`
Differential Revision: https://phabricator.services.mozilla.com/D117670
This patch splits `nsAvailableMemoryWatcher` into 1) an nsISupports-derived class
`nsAvailableMemoryWatcherBase` and 2) a platform-specific class `nsAvailableMemoryWatcher`,
taking out the 2) part as a new file AvailableMemoryWatcherWin.cpp without any change.
Test cases for `nsAvailableMemoryWatcher` will be added by a subsequent patch.
Differential Revision: https://phabricator.services.mozilla.com/D117669
`nsAvailableMemoryWatcher::mTimer` was nullptr when `LowMemoryCallback()` tried to
initialize a timer via `OnLowMemory()`. There are two possible reasons.
The first case is that NS_NewTimer() returned nullptr because the available memory was
already low when initializing `nsAvailableMemoryWatcher`. In this case, we should not
register the callback.
The second case is the low-memory callback was queued while shutting down
`nsAvailableMemoryWatcher` (just before we unregiter the callback). We do refcount
the object and use the mutex correctly, but we touch the nulled out member. We should
make an early return if the object was shut down.
Differential Revision: https://phabricator.services.mozilla.com/D118745
Currently the XPCOM thead pool creates one less thread than the number of
cores. The JS helper thread pool creates an equal number.
I tested increasing the number of threads to match the number of cores and
found it resolved this regression.
Differential Revision: https://phabricator.services.mozilla.com/D118327
This increases the stack size used for task controller threads to the size
previously used for JS helper threads. Some parsing use cases can use a lot of
stack.
Differential Revision: https://phabricator.services.mozilla.com/D118184
This adds plumbing to make the JS engine set the stack quota based on the
actual stack size for external thread pool threads (and internal thread pool
ones).
The quota is calculated as 90% of the size, which is currently hardcoded into
the constants.
Differential Revision: https://phabricator.services.mozilla.com/D118183
This particular race is a tricky one - there's no perfect solution to protecting the timer thread from being called in `cancel` while being dereferenced. This ensures that we won't run into that problem by locking all of our TimerThread calls behind a mutex inside a wrapper class. Then we hold onto the wrapper class until after we shutdown `nsThreadManager`, in which case no background thread pools should be active anymore to call `nsTimerImpl::Cancel`.
For reference, the worst-case scenario happens when another thread dereferences `gThread` [between these two calls](https://searchfox.org/mozilla-central/rev/98a9257ca2847fad9a19631ac76199474516b31e/xpcom/threads/nsTimerImpl.cpp#402-403), in which case we will get stuck on a dereferenced mutex. By putting the check and the actual call into `gThread` behind a mutex maybe we can prevent this issue.
Differential Revision: https://phabricator.services.mozilla.com/D115453
The "magic static" objects created in `nsThread::ThreadList` and
`nsThread::ThreadListMutex` have non-trivial destructors, so they end up
setting `atexit` hooks.
I'd like to switch these over to be `StaticLocalAutoPtr`s which were added
specifically for the purpose of holding magic statics, and have trivial
destructors.
Differential Revision: https://phabricator.services.mozilla.com/D117549
Specifically:
For "bullets", i.e. 'list-style-type:disc|circle|square|
disclosure-closed|disclosure-open', we use a built-in font
(-moz-bullet-font, which has glyphs for those symbols + space) to
retain mostly backwards compatible rendering for those. Authors may
override that with an explicit 'font-family' ::marker style though.
We also use this font for 'list-style-image' in case it would
fallback to one of the above when the image fails to load (so that
we get the same width space).
When the -moz-bullet-font is used we also set 'font-synthesis' to
avoid synthesizing italic/bold for this font. Authors may override
this with an explicit ::marker declaration.
We also set 'letter-spacing' and 'word-spacing' to the initial value
for bullets for web-compat reasons. Again, authors may override
this with an explicit ::marker declaration. (This breaks backwards-
compat slightly but makes us compatible with Chrome. We used to
ignore these for list-style-type:<string> too.)
Differential Revision: https://phabricator.services.mozilla.com/D111693
Specifically:
For "bullets", i.e. 'list-style-type:disc|circle|square|
disclosure-closed|disclosure-open', we use a built-in font
(-moz-bullet-font, which has glyphs for those symbols + space) to
retain mostly backwards compatible rendering for those. Authors may
override that with an explicit 'font-family' ::marker style though.
We also use this font for 'list-style-image' in case it would
fallback to one of the above when the image fails to load (so that
we get the same width space).
When the -moz-bullet-font is used we also set 'font-synthesis' to
avoid synthesizing italic/bold for this font. Authors may override
this with an explicit ::marker declaration.
We also set 'letter-spacing' and 'word-spacing' to the initial value
for bullets for web-compat reasons. Again, authors may override
this with an explicit ::marker declaration. (This breaks backwards-
compat slightly but makes us compatible with Chrome. We used to
ignore these for list-style-type:<string> too.)
Differential Revision: https://phabricator.services.mozilla.com/D111693
ATK, Windows and XPCOM expect aria attribute keys to be stripped of
their aria- prefix. We should still store the item using the aria_ atom
and then strip the prefix when converting the key to a string.
Differential Revision: https://phabricator.services.mozilla.com/D116786
Before this patch, we use InputPriorityController::DidRunTask to change
InputVsyncState which is problematic.
Consider this scenario
1. Two events are in the queue, vsync(V1) and input(I1)
2. I1 runs and starts an inner event loop (We only expect one input
event to be run because there's only one input event)
3. Another input event(I2) arrives
4. Inner event loop picks I2 to run
5. When I2 is finished, it sets the InputVsyncState to `RunVsync`
6. I1's DidRunTask is called and crashed because the state shouldn't
be `RunVsync`.
This patch moves the code which checks InputVsyncState from `DidRunTask`
to `WillRunTask` so that the state is correctly checked and updated
before the input task is about to run.
Differential Revision: https://phabricator.services.mozilla.com/D117336
Currently parallel Wasm compilation requires at least two threads for
architectural reasons. This patch updates the TaskController thread policy such
that there are always two threads available, even on single core systems.
Differential Revision: https://phabricator.services.mozilla.com/D117002
Bug 1711610 moved `AvailableMemoryTracker::Init()` from `NS_InitXPCOM` to `XRE_mainRun`,
but it caused memory degradation because `AvailableMemoryTracker` was no longer initialized
in the child processes.
I made that part for `nsAvailableMemoryWatcher` to cache the pref value in the earlier design,
but it's not needed at all in the current design because `nsAvailableMemoryWatcher` loads
a mirror value every time.
This patch reverts `AvailableMemoryTracker::Init()` back to `NS_InitXPCOM`.
Differential Revision: https://phabricator.services.mozilla.com/D116742
The JS helper thread system needs to know how many threads are available, in
particular because parallel Wasm compilation needs at least two threads to
avoid deadlock. This adds a method to get the count from TaskController and
passes it through to the JS engine when setting up the thread pool.
Differential Revision: https://phabricator.services.mozilla.com/D116220
The JS helper thread system needs to know how many threads are available, in
particular because parallel Wasm compilation needs at least two threads to
avoid deadlock. This adds a method to get the count from TaskController and
passes it through to the JS engine when setting up the thread pool.
Differential Revision: https://phabricator.services.mozilla.com/D116220
This patch does change the extension-related special case in SVGContextPaint::IsAllowedForImageFromURI
to check the "internal:svgContextPropertiesAllowed" extension permission, and move the existing
criteria in the Extension class (which takes care of adding the internal permission when those
criteria as met).
This patch does not contain yet a new explicit test case for the new internal permission (which is
part of the patch build on top of this one and attached to the same bugzilla issue), but it does
pass the existing mochitest-chrome (test_chrome_ext_svg_context_fill.html).
Differential Revision: https://phabricator.services.mozilla.com/D115835
If IdleTaskManager is suspended and non-idle task _is_run_, nothing seems to guarantee idle tasks get run later.
Calling UpdateCachedIdleDeadline triggers child->parent->child ipc messages if needed and ends up enabling
IdleTaskManager.
Differential Revision: https://phabricator.services.mozilla.com/D116316
This involves changing moveToAnchor to be allowed while the popup is showing. This change allows the buttons within the tab to use the normal algorithm for determining the tooltip position. This also fixes bug 1695900 so that tooltips for items in bookmarks menus also appear offset as well. Only the main tab and bookmarks on the toolbar appear aligned with the button's bottom edge.
Differential Revision: https://phabricator.services.mozilla.com/D115558
This patch adds application info (Name and Publisher for now) in the
about:third-party page if a module is a part of an installed application,
which is registered in the registry and shown in Windows Control Panel.
To achieve this, we parse the registry to collect installed applications
in the background task.
Differential Revision: https://phabricator.services.mozilla.com/D109306
In the gtest for these patches, we can encounter a hang in the following steps:
- Monitor waits in the main thread (for async steps to finish)
- Offthread, monitor waits for the timer to fire
- Timer notifies the waiting monitors
- Monitor on the main thread continues first, wrapping up the test. It assumes all async steps are finished.
- The offthread monitor wait follows, but at this point the main thread has dereferenced the monitor. Because of this race, we run into a UAF and hang on the offthread monitor.
My solution for this is to use two monitors, and notify the outer one after we have=completed all of the async steps including the wait for the timer notification. This should avoid a race between the inner `monitor.wait()` and the free at the end of the tests.
Tentative try push for the fix: https://treeherder.mozilla.org/jobs?repo=try&revision=307960b2899b320ef5a82d276b86d633a9653941
Differential Revision: https://phabricator.services.mozilla.com/D116163
DelayedDispatch, in all current implementations, will set up a timer sync and
then Dispatch() a runnable. Since the timer is set up before the Dispatch, there
is a theoretical chance that the timer fires and dispatches a TimerEvent to the
target thread before DelayedDispatch managed to do so. When this happens the
internal DelayedDispatch runnable exits early, i.e., in practice it never runs.
The chance increases dramatically if the Dispatch() to the target in question is
tail dispatched, since the time between DelayedDispatch and the tail could be
non-trivial.
This patch removes the assert that checks that all DelayedRunnables that have
run have also been scheduled, since per the above no such guarantee exists.
Differential Revision: https://phabricator.services.mozilla.com/D112876
This patch adds application info (Name and Publisher for now) in the
about:third-party page if a module is a part of an installed application,
which is registered in the registry and shown in Windows Control Panel.
To achieve this, we parse the registry to collect installed applications
in the background task.
Differential Revision: https://phabricator.services.mozilla.com/D109306
This patch adds application info (Name and Publisher for now) in the
about:third-party page if a module is a part of an installed application,
which is registered in the registry and shown in Windows Control Panel.
To achieve this, we parse the registry to collect installed applications
in the background task.
Differential Revision: https://phabricator.services.mozilla.com/D109306
This patch adds application info (Name and Publisher for now) in the
about:third-party page if a module is a part of an installed application,
which is registered in the registry and shown in Windows Control Panel.
To achieve this, we parse the registry to collect installed applications
in the background task.
Differential Revision: https://phabricator.services.mozilla.com/D109306
Various code was passing 'this' to mozilla::DropJSObjects in unlink, but that's
the CC participant. The right object to pass is 'tmp'. I also added static
asserts in mozilla::Hold/DropJSObjects to block this in the future.
Differential Revision: https://phabricator.services.mozilla.com/D115884
This patch reinstate the code to call `SaveMemoryReport` when we enter a low-memory
situation from a normal state, which was removed by bug 1586236.
Differential Revision: https://phabricator.services.mozilla.com/D115960
After bug 1586236, we use the memory resource notification object to detect a low
memory situation on Windows, which is signaled when the available physical memory
is low. If the available physical memory is low, however, it's possible that there
is still commit space enough for the application to run. In such a situation, we
don't want to make aggressive efforts to reduce memory usage.
This patch makes sure we send the memory pressure event (both New and Ongoing) only
when the available commit space is lower than the threshold value defined by the pref
"browser.low_commit_space_threshold_mb". Its default value is set to 200MB
based on our telemetry data indicating ~60% of OOM crashes with <100MB, ~75% with <300MB.
To use the pref in `nsAvailableMemoryWatcher`, this patch moves the call to
`AvailableMemoryTracker::Init()` to `XRE_mainRun()`. It was in `NS_InitXPCOM`
because the old initialization code hooked APIs and needed to be done while
the process has only one thread (bug 741540). The current `AvailableMemoryTracker`
does not use hooks, so it doesn't have to be initialized that early.
Differential Revision: https://phabricator.services.mozilla.com/D115605
cf. bug 1710755, touching idl files regenerates xptdata.cpp and
xptdata.h, and the latter currently triggers tons of things to be
rebuilt because xptdata.h is included from xptinfo.h, which is directly
or indirectly included in many other places.
But there's only one thing defined in xptdata.h: nsXPTInterface, and
there's actually only one place that actively uses it:
StaticComponents.cpp.in, via a function defined in xptinfo.h.
Using a forward declaration in xptinfo.h allows to only include
xptdata.h from StaticComponents.cpp.in.
While here, add an include guard to xptdata.h.
Differential Revision: https://phabricator.services.mozilla.com/D115329
In the Itanium C++ ABI VTables contain extra fields before the virtual function
pointer list for RTTI information, and these fields are not eliminated even
when RTTI is disabled using -fno-rtti. The two relevant fields for rust-xpcom's
vtables are the "offset to top" field, which contains the displacement to the
top of the object from the corresponding vtable pointer as a `ptrdiff_t`
(`isize`), and the "typeinfo pointer" field which points to the typeinfo object
and is null when building with -fno-rtti. The VTable pointer points after these
fields to the beginning of the virtual function pointer list.
While these extra fields would generally not be accessed in -fno-rtti
situations, gcc and clang still support `dynamic_cast<void*>` even when
-fno-rtti is passed, meaning that the "offset to top" field should be present.
Although I was unable to find documentation for the C++ ABI used on Darwin, it
appears to behave the same as the Itanium C++ ABI when it comes to VTable
layout.
In order to include these fields in the manual vtables built by the rust-xpcom
macros, a `&'static VTableExtra<VTableType>` is used instead of directly using
the vtable type, and the vtable reference stored in the struct is offset into
the allocation.
As the only platform I know of to not include these extra fields is Windows,
they are generated on all non-Windows platforms.
Differential Revision: https://phabricator.services.mozilla.com/D115085
This is for a few reasons:
* The leak logging isn't as useful as other types of logging, as
nsTArray_base is frequently relocated without invoking a constructor,
such as when stored within another nsTArray. This means that
XPCOM_MEM_LOG_CLASSES cannot be used to identify specific leaks of
nsTArray objects.
* The nsTArray type is layout compatible with the ThinVec crate with the
correct flags, and ThinVec does not currently perform leak logging.
This means that if a large number of arrays are transferred between rust
and C++ code using ThinVec, for example within another ThinVec, they
will not be logged correctly and might appear as e.g. negative leaks.
* Leaks which have been found thanks to the leak logging added by this
type have often not been significant, and/or have needed to be
circumvented using some other mechanism. Most leaks found with this type
in them also include other types which will continue to be tracked.
Differential Revision: https://phabricator.services.mozilla.com/D115087
This change adds the ground work to share content provided by the JS engine of
the Parent process to initialize the JS engine of other threads and Content
processes.
The singleton class xpc::SelfHostedShmem is used to wrap the logic behind
holding the memory. The memory is initialized with `InitFromParent` or
`InitFromChild`. The memory is accessible using either the `Content` or
`Handle`.
The shared memory is transfered through the command line using
`mozilla::ipc::ExportSharedJSInit` and read using
`mozilla::ipc::ImportSharedJSInit` functions. The command line is used, as we
need the shared memory to be avilable for the JS engine initialization. The
command line is composed of a single command named `-jsInit` which is followed
by the handle (on Windows) and the length of the shared content.
The memory associated with the shared memory is cleared in `ShutdownXPCOM` after
closing all threads, and shuting down the JS engine. This is necessary as we
expect the JS engine to borrow content from the shared memory.
Differential Revision: https://phabricator.services.mozilla.com/D110576
This allows the `mozilla::Queue` type to be used more easily and without
causing assertion failures, leaks, or unsafe behaviour.
Differential Revision: https://phabricator.services.mozilla.com/D112763
This warning previously fired loudly from the ports logic being added in bug
1706374 were acquired. As the warning also occasionally fires in existing code
and provides no safety guarantees, I think we should consider removing it.
Differential Revision: https://phabricator.services.mozilla.com/D112762
Win32 errors ERROR_DEVICE_HARDWARE_ERROR, ERROR_DEVICE_NOT_CONNECTED, ERROR_DISK_FULL need a mapping.
NS_ERROR_FILE_DISK_FULL is duplicate to NS_ERROR_FILE_NO_DEVICE_SPACE
Drive by: RejectJSPromise lacked some NS_ERROR_* mappings
Differential Revision: https://phabricator.services.mozilla.com/D113974
We hook several file APIs to record I/O performance data. Since TLS is not
allocated in ntdll's loader worker thread, however, if someone does a file
operation, we hit read AV because `WinIOAutoObservation` uses `nsString` and
a thread local variable.
Currently we can see this crash happens only when a DLL rule of AppLocker is
defined, but theoretically this can happen when any module loaded in a worker
thread does file operation in its entrypoint.
The proposed fix is to skip `WinIOAutoObservation` if TLS is not available.
Differential Revision: https://phabricator.services.mozilla.com/D113032
This assertion currently only happens on debug builds, but attempts to dispatch delayed runnables after we have started or finished cancelling all delayed runnables in a TaskQueue may be causing shutdown hangs.
Differential Revision: https://phabricator.services.mozilla.com/D113839
Toshihito Kikuchi
Jeff Muizelaar
Florian Quèze
Jon Coppeard
Nika Layzell
Alexandru Michis
Kris Maglione
Butkovits Atila
R Aravind
Andrew Osmond
Brindusan Cristian
kriswright
Iulian Moraru
Sean Feng
Aaron Klotz
Mats Palmgren
Gabriele Svelto
Michelle Goossens
Eitan Isaacson
Dorel Luca
Andrey Bienkowski
Doug Thayer
Csoregi Natalia
Emilio Cobos Álvarez
Sandor Molnar
Luca Greco
Edgar Chen
Mozilla Releng Treescript
Olli Pettay
Neil Deakin
Noemi Erli
Andreas Pehrson
Peter Van der Beken
Andi-Bogdan Postelnicu
Valentin Gosu
Henrik Skupin
Agi Sferro
Mike Hommey
Nicolas B. Pierron
Paul Bone
Alexandre Lissy
Andreea Pavel
Jens Stutte
Christian Holler
Jesse Schwartzentruber