Although the default value for isPrivileged is false in the
ExtensionData constructor, let's add the explicit value (false) so that
it's obvious that the default value is intended.
Differential Revision: https://phabricator.services.mozilla.com/D139725
Concurrency bug: this method accesses mutable state but is not synchronized so
the caller isn't guaranteed to see the latest values.
We fix it by using an AtomicReference. This slightly complicates the thread
policy (which is to otherwise synchronize access to mutable state) but avoids
the increased contention that could come from using synchronized for
consistency.
This is the only concurrency bug I found throughout my thread safety
documentation.
Differential Revision: https://phabricator.services.mozilla.com/D139199
getSample's callers are synchronized in the current implementation so there are
no concurrency bugs there but it can later be called by an unsynchronized caller
so I added synchronization to it and documented the thread safety guarantees.
Differential Revision: https://phabricator.services.mozilla.com/D139197
As used, the class is effectively immutable which provides some thread safety
guarantees so I updated the declarations so the class could take advantage of
that.
Effectively immutable classes need to be safely published in order to be thread
safe. I believe we could avoid the need for safe publishing (i.e. strengthen
thread safety guarantees) if we replaced the Frame[] type with a
thread safe data type (AtomicReferenceArray, a concurrent list, etc.).
Differential Revision: https://phabricator.services.mozilla.com/D139196
Like Frame, this class was not thread safe because it exposed mutable state.
However, it was being used in an immutable manner so it was trivial to make it
thread safe.
Differential Revision: https://phabricator.services.mozilla.com/D139195
This class was not thread safe because the fields in this class were mutable
and access to mutable state must be synchronized across threads. This class was
only used in an immutable fashion, however, so it was trivial to make it
immutable and thus thread safe.
Differential Revision: https://phabricator.services.mozilla.com/D139194
Root cause: some scripts in the content process were loaded before the script
cache was available there so these scripts were never written to the cache and
thus were never loaded from cache on subsequent runs, affecting performance.
These scripts listened for the app-startup topic.
This patch introduces a new topic to indicate when the script cache is init so
scripts previously listening to app-startup can listen to the new topic instead.
Differential Revision: https://phabricator.services.mozilla.com/D138501
Historically, we wanted apps to be deliberate about setting/unsetting sessions
on a GeckoView instace because we were worried that they might leak a
GeckoSession (which would leak a tab running in Gecko), so we expected apps to
call `view.releaseSession` and collect the returned session.
We also expected apps's code to be more straightforward and clearly have a
`release`/`set` session pair that could be verifiably happening in order.
All of the above assumptions are broken in AC, where the code for
`releaseSession` and `setSession` is driven by a reactive data store.
The state management piece is also so hard for AC that they call
`releaseSession` conservatively in a lot of places and never actually look at
the return value.
To more align GeckoView with AC's architecture this patch moves into the
direction where apps only `setSession` and never `release`.
GeckoView will release the session from the old GeckoView when
appropriate.
The only problem with the above is that we might get into a state
where the app thinks a GeckoView is displaying a session while really it isn't
because the session was set to a different GeckoView.
To detect the above case we introduce a new flag called `mIsSessionPoisoned`
which is set whenever the GeckoView instance used to hold a reference to a
session which has since gotten attached to a different window. If the app tries
to display such a GeckoView, we will throw an exception in `onAttachedToWindow`.
After this patch, apps won't need to manage `release`/`set` manually and if
they somehow end up in a broken state (e.g. because the app has a bug) we still
correctly crash.
This avoids crashing if the `release`/`set` pairs are out of order (e.g.
because `onAttachedToWindow`, `onDetachFromWindow` arrive out of order) which
is our leading theory for the crash in the bug attached to this patch.
Differential Revision: https://phabricator.services.mozilla.com/D139518
There's no current use for setting `JS_BINARY` in `packager.mk`, so
remove it while we're here. I elected to make it easy to add new file
types to minify rather than to make it easy to specify `JS_BINARY`,
since the latter mechanism is strictly more general and could be used
in future for things other than minification.
Differential Revision: https://phabricator.services.mozilla.com/D138364
SelectionActionDelegateTest doesn't wok on Android 10 because Android 10
requires that application must be foreground to write clipboard item.
Also, setting foreground becomes test is slow, so this change is on Android 10+
only.
Differential Revision: https://phabricator.services.mozilla.com/D139237
If the android system kills the GPU process to free memory while the
app is in the background, then we want to avoid immediately restarting
the GPU process.
To achieve this, we make GPUProcessManager keep track of whether it is
in the foreground or background. If HandleProcessLost() gets called
while in the background then we destroy the existing compositor
sessions as before, but return early instead of immediately
relaunching the process. If the process has not been launched when the
app later gets foregrounded then we do so then.
The final part of HandleProcessLost(), which reinitializes the content
bridges and emits the "compositor-reinitialized" signal, has been
moved to a new function ReinitializeRendering(). If the GPU process
has been disabled, this gets called as-before at the end of
HandleProcessLost(). When the GPU process is enabled, however, we now
call it from OnProcessLaunchComplete(), so that it gets called
regardless of whether the process is launched immediately or after a
delay.
While we're here, rename the functions RebuildRemoteSessions() and
RebuildInProcessSessions() to DestroyRemoteCompositorSessions() and
DestroyInProcessCompositorSessions(), to better reflect what they
actually do: the "rebuilding" part occurs later on. Also update the
mega-comment documenting the restart sequence, as it was somewhat
outdated.
In case a caller of EnsureGPUReady() gets called before the foreground
signal arrives (eg in nsBaseWidget::CreateCompositorSession() due to a
refresh tick paint), make EnsureGPUReady() launch the GPU process
itself if the GPU process is enabled but not yet launched. As a
consequence, to avoid launching the GPU process unnecessarily, change
a couple callers of EnsureGPUReady() to simply check whether the
process is enabled instead.
Additionally, guard against a null pointer deref if the compositor has
been destroyed when the widget receives a memory pressure event. This
is now more likely to occur as there may be a gap between the
compositor being destroyed and recreated.
Differential Revision: https://phabricator.services.mozilla.com/D139042
This patch introduces a new system for building IPDL unit tests, which is
roughly inspired by the old cxx unit test design, however designed to work with
gtest. It re-uses the existing IPDLUnitTest process type, using static
constructors only present in xul-gtest to register child actor constructors and
ProcessChild implementations to avoid bloating xul.
The IPDLUnitTest actor is used as a backchannel to communicate test failures
from the child process back to the parent process, and manage the test's async
lifecycle more reliably.
The IPDLUnitTest process also needed to have various properties about how it was
initialized adjusted:
* The IPDLUnitTest process should now always load xul-gtest when running
gtests, by using the "Self" process type and adding a DYLD_LIBRARY_PATH override
on macOS where the "Self" process type cannot be used.
* The IPDLUnitTest process now initializes minimal XPCOM, and runs a
frankeneventloop to allow it to use XPCOM event loop structures such as
SpinEventLoopUntil in tests.
* Support for creating IPDLUnitTest processes on Android was added, as these
tests had never run on android before.
Differential Revision: https://phabricator.services.mozilla.com/D137170
Rather than lazily attempting to connect the SurfaceAllocator if
required whenever the content process tries to allocate a surface,
instead give control over the connection to the parent process. Make
it connect after the content process has been created, and
additionally after rendering has being reinitialized following a GPU
process loss. This aligns with how we reconnect various IPDL protocols
between content processes and the compositor.
This avoids content processes attempting to connect to the GPU
process' surface allocator before it has been started, which is
important for the next patch in this series where we want to delay
restarting the GPU process if the app is in the background.
If the content process attempts to allocate a surface inbetween the
old connection dying and the new connection being made then it will
gracefully fail. If the GPU process dies again whilst the connection
is being made then that is okay, a new connection will be made shortly
afterwards.
Differential Revision: https://phabricator.services.mozilla.com/D139041
:whimboo found this, apparently if something else holds a reference to
permanentKey we will leak the window. To avoid this happening we use the JSM
global instead.
For more context see Bug 1501789.
Differential Revision: https://phabricator.services.mozilla.com/D138451
Android has no full-page zoom, so we are not meaningfully changing behavior.
However, these values are exposed to the GeckoView JS api, surprisingly, yet
viewport scaling and so on can change the CSS to device pixel ratio...
Shouldn't we expose device pixels there instead? Or do the api consumers assume
that those CSS pixels are scaled to the device scale factor somehow (and that's
not working)?
Depends on D138037
Differential Revision: https://phabricator.services.mozilla.com/D138038
Android has no full-page zoom, so we are not meaningfully changing behavior.
However, these values are exposed to the GeckoView JS api, surprisingly, yet
viewport scaling and so on can change the CSS to device pixel ratio...
Shouldn't we expose device pixels there instead? Or do the api consumers assume
that those CSS pixels are scaled to the device scale factor somehow (and that's
not working)?
Depends on D138037
Differential Revision: https://phabricator.services.mozilla.com/D138038
If the android system kills the GPU process then ChildCrashHandler.jsm
gets invoked but an empty dump ID. Currently we generate a crash
report anyway, which is resulting in lots of "EMPTY: no crashing
thread identified; ERROR_NO_MINIDUMP_HEADER" crashes.
Instead, we should return early before starting the crash reporter if
the dump ID is empty. We already do this for content processes, so
this patch makes us do so for every type of process.
Differential Revision: https://phabricator.services.mozilla.com/D138766
Make the ScreenOrientation part of the screen struct, as it should. Stop
using HAL to propagate just screen orientation updates, use the more
general screen manager.
Instead of HAL observers, add a simple observer service notification,
and clean a bunch of the code.
This will simplify bug 1754802 a bit, and is generally simpler.
Shouldn't change behavior. I've tested the events and some common
orientation locking use cases like Youtube, and they behave the same.
Differential Revision: https://phabricator.services.mozilla.com/D138477
Right now the title is "Welcome to GeckoView’s documentation!" which looks out
of place given that all the other titles are just the name of the product like
"libpref" or "WebIDL"
Differential Revision: https://phabricator.services.mozilla.com/D138818
BACKGROUND and FOREGROUND priority lead to the identical oom_adj value of 11
when the app is in the background. Because all non-content processes are
restartable without losing any state, we should prioritize them lower than the
current active tab, which will be prioritized to FOREGROUND when Bug 1753700 is
fixed.
Differential Revision: https://phabricator.services.mozilla.com/D138810
In bug 1750569 we attempted to ensure that following a GPU process
crash outstanding screen pixels requests would be fulfilled. While
this usually worked there was a race condition between sending the
request to the new compositor and the content process sending the
display list to the new compositor, which meant that sometimes we
would screenshot an empty screen instead of the page content.
As a GPU process crash is an extraordinary circumstance and
screenshots are non-critical, the best solution is to simply return an
error if a GPU process crash occurs while there is an outstanding
request (or if a new request is made whilst the GPU process is
restarting). This patch also updates the junit test to check for this
error rather than expecting a screenshot to be returned.
Differential Revision: https://phabricator.services.mozilla.com/D138323
isTopLevel is used for ENV_TYPE_CONTENT_SCRIPT to let embedders know if an
iframe is sending messages to the app. For extension environments we don't need
this extra check so we can always set the value to true.
Differential Revision: https://phabricator.services.mozilla.com/D137776
Make the SurfaceAllocator save a reference to all Surfaces it has
allocated. When the connection to the remote allocator dies, mark all
of those surfaces as released. Ensure subsequent attempts to allocate
new Surfaces will connect to the new compositor process (either GPU or
parent).
For webgl, when we detect we are using a Surface that has been
released, clear the pool of Surfaces used for the swapchain so that
we automatically reallocate new ones.
For video, if we encounter an error in RemoteVideoDecoder and can see
that Surface has been released, then report a
NS_ERROR_DOM_MEDIA_NEED_NEW_DECODER in place of the potentially fatal
error code we get from the java decoder. This will ensure a new
decoder is created, which will in turn allocate a new Surface to
decode in to.
On the Android emulator (so potentially real devices too) the java
decoder doesn't actually report any errors in response to the Surface
being released. Therefore we also check for the Surface being released
in ProcessOutput(), and manually raise the NEED_NEW_ENCODER error from
there if so.
Differential Revision: https://phabricator.services.mozilla.com/D133108
There are two things that make an element not be a combobox:
* <select multiple>
* <select size=N> where N > 1
I was only handling the former. Expose the gecko-internal method we have
for this via WebIDL to get this fully right forever.
The other issue reported in this bug is not a GeckoView issue, it's an
android-components issue, will file / maybe fix there if I figure out
how.
Differential Revision: https://phabricator.services.mozilla.com/D137295
When user/App rotates screen 180 degree, `onConfigurationChanged` isn't called.
But If API is 17+, we can use `DisplayManager.DisplayListener` to detect
rotation change instead.
Also, since orientation value in `android.content.res.Configuration` won't be
updated immediately, we shouldn't use it, excepting to `onConfigurationChanged`.
Differential Revision: https://phabricator.services.mozilla.com/D136869
Two issues:
* Set the openInParentProcess flag in GeckoViewPrompt.jsm for
dropdowns. This avoids needing two taps to re-open the dropdown.
* Make the "input" event composed, see bug 1734040.
Differential Revision: https://phabricator.services.mozilla.com/D137038
WebRender retains about 50MBs of memory for every window. When switching
between lots of tabs on Android (where 1 tab = 1 window), this can cause
problems as the app will consume a significant amount of memory.
To avoid this problem, we send a memory pressure event whenever a session is
deactivated, which signals to WebRender that it should deallocate the memory.
This message is sent on a delay of 10s to avoid interfering with tab switching,
and we cancel the message if the tab becomes active again before we fire the
memory pressure event.
Co-Authored-By: Cathy Lu <calu@mozilla.com>
Co-Authored-By: Jonathan Almeida [:jonalmeida] <jonalmeida942@gmail.com>
Differential Revision: https://phabricator.services.mozilla.com/D136965
We had zero coverage for this, apparently. Upon writing tests, the only
test we had for this was `@Ignore`d :(
Write better tests for this. Add tests for <select multiple> which I
broke because the mozshowdropdown events only fire for dropdown selects.
I'm sorry anyways, I should've tested manually, but I was traveling and
found issues when trying to do an artifact build so I relied on try, oh
well.
Differential Revision: https://phabricator.services.mozilla.com/D136772
Rob Wu
Mark Banner
Michael Comella
Makoto Kato
Narcis Beleuzu
Agi Sferro
Nick Alexander
Jamie Nicol
Shane Caraveo
Marian-Vasile Laza
Nika Layzell
Cosmin Sabou
Emilio Cobos Álvarez
smolnar
Sylvestre Ledru
William Durand
Anastasios Bithas