Opening our Omnijars can be expensive, and it should be deferrable until after
startup is completed, provided we have a startup cache. In a previous patch in this
stack, we implemented caching of the zip central directory for omnijars, but we
still have to open the file in order to hand the object off to various omnijar
consumers. In a later patch, we will wrap nsZipArchive access in a class which
will allow us to transparently cache nsZipArchive results. These two get us
most of the way to not needing to read from the underlying omnijar files during
startup, but there are still nontrivial pieces, like nsZipFind for instance,
which we don't want to just duplicate inside of a wrapper class, so we would
like to sort out a way in which we can use an nsZipArchive class, but not
actually back it up with the real underlying file until we really need data
from it which we can't find in a cache.
Depends on D77633
Differential Revision: https://phabricator.services.mozilla.com/D78584
We need to be able to init StartupCache before the Omnijar in order to cache
all of the Omnijar contents we access. This patch implements that.
Depends on D77632
Differential Revision: https://phabricator.services.mozilla.com/D77633
We would like to be able to defer opening the omnijar files until after startup
if the StartupCache has already been populated. Opening the omnijar files takes
a nontrivial time, at least on Windows, and almost everything in the omnijar
should be fairly compressible, and thus makes sense to live in the StartupCache.
See the last patch in this series for a little more discussion on numbers, but
tl;dr: we saw a 12% improvement in time to about:home being finished on reference
hardware with these changes together with the changes from the descendant patches.
Differential Revision: https://phabricator.services.mozilla.com/D77632
NS_INLINE_DECL_REFCOUNTING macro doesn't properly work when the object is used on a thread that isn't backed by a single PRThread (such as TaskQueue). See bug 1648031.
The resolution of this issue is rather complex, and outside the scope of this series of change.
So for now, we create a new macro NS_INLINE_DECL_REFCOUNTING_ONEVENTTHREAD which will use a different mechanism to ensure the thread-safe usage of a class.
Differential Revision: https://phabricator.services.mozilla.com/D81269
This should be a relatively straightforward patch. Essentially, we implement
a wrapper class (and friends) around nsZipArchive (and friends), which transparently
caches entries from the underlying zip archive in the StartupCache. This will break
without changes to the StartupCache, made in the patch after this, which allow it
to be used off of the main thread, and outside the main process.
Depends on D77635
Differential Revision: https://phabricator.services.mozilla.com/D77634
The overall goal of this patch is to make the StartupCache accessible anywhere.
There's two main pieces to that equation:
1. Allowing it to be accessed off main thread, which means modifying the
mutex usage to ensure that all data accessed from non-main threads is
protected.
2. Allowing it to be accessed out of the chrome process, which means passing
a handle to a shared cache buffer down to child processes.
Number 1 is somewhat fiddly, but it's all generally straightforward work. I'll
hope that the comments and the code are sufficient to explain what's going on
there.
Number 2 has some decisions to be made:
- The first decision was to pass a handle to a frozen chunk of memory down to
all child processes, rather than passing a handle to an actual file. There's
two reasons for this: 1) since we want to compress the underlying file on
disk, giving that file to child processes would mean they have to decompress
it themselves, eating CPU time. 2) since they would have to decompress it
themselves, they would have to allocate the memory for the decompressed
buffers, meaning they cannot all simply share one big decompressed buffer.
- The drawback of this decision is that we have to load and decompress the
buffer up front, before we spawn any child processes. We attempt to
mitigate this by keeping track of all the entries that child processes
access, and only including those in the frozen decompressed shared buffer.
- We base our implementation of this approach off of the shared preferences
implementation. Hopefully I got all of the pieces to fit together
correctly. They seem to work in local testing and on try, but I think
they require a set of experienced eyes looking carefully at them.
- Another decision was whether to send the handles to the buffers over IPC or
via command line. We went with the command line approach, because the startup
cache would need to be accessed very early on in order to ensure we do not
read from any omnijars, and we could not make that work via IPC.
- Unfortunately this means adding another hard-coded FD, similar to
kPrefMapFileDescriptor. It seems like at the very least we need to rope all
of these together into one place, but I think that should be filed as a
follow-up?
Lastly, because this patch is a bit of a monster to review - first, thank you
for looking at it, and second, the reason we're invested in this is because we
saw a >10% improvement in cold startup times on reference hardware, with a p
value less than 0.01. It's still not abundantly clear how reference hardware
numbers translate to numbers on release, and they certainly don't translate
well to Nightly numbers, but it's enough to convince me that it's worth some
effort.
Depends on D78584
Differential Revision: https://phabricator.services.mozilla.com/D77635
Opening our Omnijars can be expensive, and it should be deferrable until after
startup is completed, provided we have a startup cache. In a previous patch in this
stack, we implemented caching of the zip central directory for omnijars, but we
still have to open the file in order to hand the object off to various omnijar
consumers. In a later patch, we will wrap nsZipArchive access in a class which
will allow us to transparently cache nsZipArchive results. These two get us
most of the way to not needing to read from the underlying omnijar files during
startup, but there are still nontrivial pieces, like nsZipFind for instance,
which we don't want to just duplicate inside of a wrapper class, so we would
like to sort out a way in which we can use an nsZipArchive class, but not
actually back it up with the real underlying file until we really need data
from it which we can't find in a cache.
Depends on D77633
Differential Revision: https://phabricator.services.mozilla.com/D78584
We need to be able to init StartupCache before the Omnijar in order to cache
all of the Omnijar contents we access. This patch implements that.
Depends on D77632
Differential Revision: https://phabricator.services.mozilla.com/D77633
We would like to be able to defer opening the omnijar files until after startup
if the StartupCache has already been populated. Opening the omnijar files takes
a nontrivial time, at least on Windows, and almost everything in the omnijar
should be fairly compressible, and thus makes sense to live in the StartupCache.
See the last patch in this series for a little more discussion on numbers, but
tl;dr: we saw a 12% improvement in time to about:home being finished on reference
hardware with these changes together with the changes from the descendant patches.
Differential Revision: https://phabricator.services.mozilla.com/D77632
We want it to returning the actual nsThread if that's where the MessageLoop would dispatch its tasks; otherwise return the MessageLoop's EventTarget
Depends on D80357
Differential Revision: https://phabricator.services.mozilla.com/D80811
NS_INLINE_DECL_REFCOUNTING macro doesn't properly work when the object is used on a thread that isn't backed by a single PRThread (such as TaskQueue). See bug 1648031.
The resolution of this issue is rather complex, and outside the scope of this series of change.
So for now, we create a new macro NS_INLINE_DECL_REFCOUNTING_ONEVENTTHREAD which will use a different mechanism to ensure the thread-safe usage of a class.
Differential Revision: https://phabricator.services.mozilla.com/D81269
We want it to returning the actual nsThread if that's where the MessageLoop would dispatch its tasks; otherwise return the MessageLoop's EventTarget
Depends on D80357
Differential Revision: https://phabricator.services.mozilla.com/D80811
Apparently I added these in the initial commit for RecursiveMutex. I'm
not quite sure what I was thinking, but we don't need them for the
RecursiveMutex itself. (We have them on the corresponding `*Auto*Lock`
classes, which are also `MOZ_RAII`.)
Differential Revision: https://phabricator.services.mozilla.com/D81345
This just spins the event loop during fast shutdown until all queued
cert_storage tasks have completed. The patch achieves this by simply
adding a counter which will be incremented and decremented on the
main thread via tying into the tasks' `new` and `done` methods. A
slightly more performant solution would use a condvar and sleep the
main thread waiting on pending operations to complete, but given the
low frequency of these occuring during shutdown, such an approach
would be overkill.
Differential Revision: https://phabricator.services.mozilla.com/D80906
We were storing LogModule::mName as an &'static str in the map... That's
not fine.
The Arc shenanigans were also more complicated than they need to be IMO.
This uses a plain atomic bool to keep the fast path snappy.
Differential Revision: https://phabricator.services.mozilla.com/D81007
On mobile platform, "ENTER" key on software keyboard key can sometimes
change to "Next" and etc. Although Firefox OS/Firefox Android have
`mozactionhint` attribute for its behaviour, This is standardized version
of it.
Chrome and Safari already support it now, so I would like to support this
on Firefox/GeckoView Nightly.
Differential Revision: https://phabricator.services.mozilla.com/D79641
This will allow to remove AbstractThread::Current() as GetCurrentSerialEventTarget TLS value will be set whenever a task dispatched on the XPCOMThreadWrapper is run.
Differential Revision: https://phabricator.services.mozilla.com/D80355
Before P1, GetCurrentThreadSerialEventTarget would have always returned the same data as NS_GetCurrentThread, making the comment incorrect Now it will properly return the running TaskQueue if any.
This change of name more clearly exposes what they are doing, as we aren't always dealing with threads directly; but a nsISerialEventTarget
Differential Revision: https://phabricator.services.mozilla.com/D80354
In the future, we may want to extend GetCurrentThreadSerialEventTarget to return the actual nsISerialEventTarget used to dispatch the task.
Differential Revision: https://phabricator.services.mozilla.com/D80353
The crash reports indicate a situation where a non-default heap is destroyed
after we retrieve a handle is happening on a regular basis.
We've decided to count only the default heap, ignoring all non-default heaps
which we don't manage.
Differential Revision: https://phabricator.services.mozilla.com/D80299
When TaskQueue was first conceived; it was only used with AbstractThreads and with tail dispatch.
By default, AbstractThread::Dispatch dropped the flags , as it was dispatching all tasks via the tail dispatcher.
It was an oversight, there's no use-case where we wouldn't want the dispatch flags to be carried forward.
It also simplifies the code and TaskQueue's use.
Depends on D80351
Differential Revision: https://phabricator.services.mozilla.com/D80352
We have a clang-plugin check that wants to prefer `SprintfLiteral` over `snprintf`, but for some reason this wasn't caught before clang-11. I _think_ it has to do with previous versions not being able to see that `buflen` was constant, but I'm not really sure.
Differential Revision: https://phabricator.services.mozilla.com/D80021
This also extracts new private member functions AssignOwned and AssignNonDependent.
Furthermore, it fixes an inconsistency for the dependent case: Formerly,
this caused the substring tuple to be materialized into a linear string
infallibly, although the function was a fallible one. It only assigned the
resulting string fallibly, but this would never have failed, since the
allocation already happened before.
Differential Revision: https://phabricator.services.mozilla.com/D78694
Caching filenames in 32-entry LRU array covers >95% of calls, and makes the average `Filename()` call 5 to 10 times cheaper.
browser_start_content_mainthreadio.js needed to be updated to handle operations that now have a filename thanks to the cache.
Differential Revision: https://phabricator.services.mozilla.com/D79767
This also extracts new private member functions AssignOwned and AssignNonDependent.
Furthermore, it fixes an inconsistency for the dependent case: Formerly,
this caused the substring tuple to be materialized into a linear string
infallibly, although the function was a fallible one. It only assigned the
resulting string fallibly, but this would never have failed, since the
allocation already happened before.
Differential Revision: https://phabricator.services.mozilla.com/D78694
This removes variants of "blacklist" from the xpcom directory. The preference name "network.file.path_blacklist" is left in place and will need a more thorough plan for removal. Instances of `MOZ_ASAN_BLACKLIST` remain as well and should be replaced in a larger modifcation of the `#define` in the mfbt component.
Differential Revision: https://phabricator.services.mozilla.com/D80098
mozilla::ReverseIterator is required for "stashing iterators", but otherwise
std::reverse_iterator can be used and is potentially more efficient.
Also fix some other glitches in iterator type definitions.
Differential Revision: https://phabricator.services.mozilla.com/D79489
When chaining a MozPromise set to be dispatched via the direct task queue (or synchronous), it makes sense for the chained promise to be dispatched in the same fashion.
All MozPromises generated by the IPC bindings are set to use the direct task queue in order to prevent the then runnable to run out of order with other IPC tasks.
We want to preserve that task ordering by default.
Differential Revision: https://phabricator.services.mozilla.com/D78178
This prevent being able to assert in the AutoTaskDispatcher that we are using it from the right thread/taskqueue as the SimpleTaskQueue object isn't thread-safe.
We want to assert that all nsIDirectTaskDispatcher methods are only ever accessed on the underlying thread. To do so require that the scope of AutoTaskDispatcher to terminate prior the AutoTaskGuard one.
Differential Revision: https://phabricator.services.mozilla.com/D79096
We extract the dealing of direct tasks from the TailDispatcher object and move into to the existing nsThread and TaskQueue classes.
TaskQueue is made to work with do_QueryInterface.
We continue accessing the direct tasks dispatcher via the TailDispatcher for now, which itself will forward the dealing of such tasks to the underlying thread.
Differential Revision: https://phabricator.services.mozilla.com/D79093
When chaining a MozPromise set to be dispatched via the direct task queue (or synchronous), it makes sense for the chained promise to be dispatched in the same fashion.
All MozPromises generated by the IPC bindings are set to use the direct task queue in order to prevent the then runnable to run out of order with other IPC tasks.
We want to preserve that task ordering by default.
Differential Revision: https://phabricator.services.mozilla.com/D78178
This patch will
- create a chrome-only webdil interface `MediaController`
- expose supported keys via `MediaController` webidl interface
The advantage of doing so are
- to have a dedicated interface that is only used for MediaController that can be used for testing and our future plan (media hub)
More Details :
Currently, we access media controller's from `ChromeUtils` [1], but it causes a problem of creating a duplicated enum of the enum which we want to expose into Chrome JS.
Instead, we should create a media controller interface to access all its attibutes, which is more easier and clean.
In addition, we're planning to have a something like Chrome's media hub [2]. In order to do that, we have to expose some JS methods to allow us to control playback directly from Chrome JS.
[1] https://searchfox.org/mozilla-central/rev/559b25eb41c1cbffcb90a34e008b8288312fcd25/dom/chrome-webidl/ChromeUtils.webidl#485-493
[2] https://blog.google/products/chrome/manage-audio-and-video-in-chrome/
Differential Revision: https://phabricator.services.mozilla.com/D77757
This makes it easier to consider special conditions when checking the pref. It
currently returns false if safe mode is enabled.
Differential Revision: https://phabricator.services.mozilla.com/D78798
This patch is a fairly mechanical conversion. The old `nullptr` gets converted
to ScrollOrigin::NotSpecified, and all the other possible values get corresponding
values in the new ScrollOrigin enum. A few switch statements are introduced to
clean up big if statements, but other than that, additional cleanups will happen
in later patches.
Differential Revision: https://phabricator.services.mozilla.com/D78438
After investigating the potential to reduce the nsTerminator's crash timeout from
1 min, to 20s, and then finally 40s, we have decided to this does not provide
significant gains to justify increasing the amount of shutdown hang crashes
and potential to lose data. We should maintain the crash timeout at 1 min.
Differential Revision: https://phabricator.services.mozilla.com/D77939
Trying to use nsTHashtable<SheetLoadDataHashKey> yields the following
error:
0:01.28 /home/emilio/src/moz/gecko-2/obj-debug-no-sccache/dist/include/nsTHashtable.h:288:8: error: multiple overloads of 'RemoveEntry' instantiate to the same signature 'void (mozilla::SheetLoadDataHashKey *)'
0:01.28 void RemoveEntry(EntryType* aEntry) { mTable.RemoveEntry(aEntry); }
0:01.28 ^
0:01.28 /home/emilio/src/moz/gecko-2/obj-debug-no-sccache/dist/include/mozilla/css/Loader.h:600:38: note: in instantiation of template class 'nsTHashtable<mozilla::SheetLoadDataHashKey>' requested here
0:01.28 nsTHashtable<SheetLoadDataHashKey> mLoadsPerformed;
0:01.28 ^
0:01.28 /home/emilio/src/moz/gecko-2/obj-debug-no-sccache/dist/include/nsTHashtable.h:264:8: note: previous declaration is here
0:01.28 void RemoveEntry(KeyType aKey) {
I tried and failed to std::enable_if my way to victory, but then
realized I could probably use nsTHashtable<const SheetLoadDataHashKey>,
so that the compiler wouldn't complain about the overload (they're
identical, so they don't really matter).
But doesn't work either, unless I have this patch, because:
0:12.57 /home/emilio/src/moz/gecko-2/obj-debug-no-sccache/dist/include/nsTHashtable.h:504:36: error: call to implicitly-deleted copy constructor of 'const mozilla::SheetLoadDataHashKey'
0:12.57 new (mozilla::KnownNotNull, aTo) EntryType(std::move(*fromEntry));
0:12.57 ^ ~~~~~~~~~~~~~~~~~~~~~
0:12.57 /home/emilio/src/moz/gecko-2/obj-debug-no-sccache/dist/include/nsTHashtable.h:478:13: note: in instantiation of member function 'nsTHashtable<const mozilla::SheetLoadDataHashKey>::s_CopyEntry' requested here
0:12.57 : s_CopyEntry,
0:12.57 ^
0:12.57 /home/emilio/src/moz/gecko-2/obj-debug-no-sccache/dist/include/nsTHashtable.h:163:16: note: in instantiation of member function 'nsTHashtable<const mozilla::SheetLoadDataHashKey>::Ops' requested here
0:12.57 : mTable(Ops(), sizeof(EntryType), PLDHashTable::kDefaultInitialLength) {}
0:12.57 ^
0:12.57 /home/emilio/src/moz/gecko-2/layout/style/Loader.cpp:359:9: note: in instantiation of member function 'nsTHashtable<const mozilla::SheetLoadDataHashKey>::nsTHashtable' requested here
0:12.57 Loader::Loader()
0:12.57 ^
0:12.57 /home/emilio/src/moz/gecko-2/obj-debug-no-sccache/dist/include/mozilla/css/Loader.h:97:3: note: copy constructor is implicitly deleted because 'SheetLoadDataHashKey' has a user-declared move constructor
0:12.57 SheetLoadDataHashKey(SheetLoadDataHashKey&& toMove)
0:12.57 ^
0:13.08 1 error generated
This one was easier to fix though. Not too proud of this one.
Differential Revision: https://phabricator.services.mozilla.com/D77830
People seemed to be taking the comment as indicative that Services.py
wasn't providing everything they needed, so make it clearer that you can
still get `nsIXULAppInfo`, you just have to work a little harder.
Differential Revision: https://phabricator.services.mozilla.com/D77477
Native Windows apps (including but not limited to IE, old Edge, and Notepad) does not automatically resolve shell links (.lnk), so this patch follows their behavior.
Differential Revision: https://phabricator.services.mozilla.com/D73546
This unifies toolbarbutton-text and toolbarbutton-multiline-text. We now
always use toolbarbutton-text for the button's text, but can either use
textContent or the value attribute, depending on the value of the wrap
attribute. This reduces DOM size and consumer complexity, at the cost of
adding some logic to toolbarbutton.js itself.
Differential Revision: https://phabricator.services.mozilla.com/D76383
There's no use case for stateful comparators, so they can be just plain
function pointers.
This is used in some hot places like CSS selector matching.
Differential Revision: https://phabricator.services.mozilla.com/D77084
This removes all docshell nsISecureBrowserUI and mixed content properties, and moves them into CanonicalBrowsingContext/WindowGlobalParent. It makes the mixed content blocker just compute the state for the current load, and then send the results to the parent process, where we update the security state accordingly.
I think we could in the future remove onSecurityChange entirely, and instead just fire an event to the <browser> element notifying it of changes to the queryable securityUI.
Unfortunately we have a lot of existing code that depends on specific ordering between onSecurityChange and onLocationChange, so I had to hook into the RemoteWebProgress implementation in BrowserParent to mimic the same timings.
Differential Revision: https://phabricator.services.mozilla.com/D75447
This removes all docshell nsISecureBrowserUI and mixed content properties, and moves them into CanonicalBrowsingContext/WindowGlobalParent. It makes the mixed content blocker just compute the state for the current load, and then send the results to the parent process, where we update the security state accordingly.
I think we could in the future remove onSecurityChange entirely, and instead just fire an event to the <browser> element notifying it of changes to the queryable securityUI.
Unfortunately we have a lot of existing code that depends on specific ordering between onSecurityChange and onLocationChange, so I had to hook into the RemoteWebProgress implementation in BrowserParent to mimic the same timings.
Differential Revision: https://phabricator.services.mozilla.com/D75447
This drops `followLinks` as 1) it has been no-op on UNIX for a long time and 2) its Windows implementation never had a proper symlink support.
Differential Revision: https://phabricator.services.mozilla.com/D75569
This drops `followLinks` as 1) it has been no-op on UNIX for a long time and 2) its Windows implementation never had a proper symlink support.
Differential Revision: https://phabricator.services.mozilla.com/D75569
This matches how the `Dispatch(already_AddRefed<nsIRunnable>)`
overloads work in C++: `Dispatch` takes ownership of the runnable, and
leaks it if dispatch fails—because the thread manager is shutting down,
for instance. This avoids a race where a runnable can be released on
either the owning or target thread.
Rust doesn't allow arbitrary `Self` types yet (see
rust-lang/rust#44874), so we need to change `dispatch` and
`dispatch_with_options` to be associated methods.
Differential Revision: https://phabricator.services.mozilla.com/D75858
Some background information on why we want to flush writes: we're
currently working on a "fast shutdown" project, which will simply
call _exit(0) once a certain phase (controlled via pref) in
shutdown is hit. This will allow us to skip a lot of ceremony,
like freeing memory, removing listeners on observer notifications,
etc. However, we don't want to be cavalier about data loss, so
for the time being we want to ensure that all file writes which we
previously had some assurance would complete will continue to
complete. This means explicitly calling them immediately before a
fast shutdown. We also want to call these before we turn on late
write checking, which we control via another pref. This late write
checking surfaces writes via telemetry which we didn't notice with
local testing. However we want to ensure that we flush our writes
before this point so we don't have lots of noise in the telemetry.
Lina I'm requesting review from you since it looks like you did the
reviews on most of the initial code. However, please let me know if
there's someone more suitable.
Depends on D75389
Differential Revision: https://phabricator.services.mozilla.com/D75572
When performing the last GC, remaining state watchers would dispatch direct tasks ; this was exposed with P3. Ensure this never happens.
Differential Revision: https://phabricator.services.mozilla.com/D75499
During the last cycle collection, state watchers may attempt to dispatch tasks to the current AbstractThread, so we can't clear the TLS entry until that step has completed.
We want the object to be deleted last; ordering in call to ClearOnShutdown() can't guarantee it.
Differential Revision: https://phabricator.services.mozilla.com/D75498
In this bug we're moving away from monolithic JNI headers to class-specific
headers so that we don't have to rebuild the world every time we make a change
to a JNI interface.
Differential Revision: https://phabricator.services.mozilla.com/D75366
We already ensure that we finish our write before doing a fast
shutdown, but right now there's just too much noise in the late
write checking telemetry because of this.
Differential Revision: https://phabricator.services.mozilla.com/D75389
Gecko Idle detection relies heavily on RefreshDriver. GC/CC scheduling, including when to run GC or CC slices, and the length of the slices, is mostly based on idle time. As WebXR isn't using normal RefreshDriver, the content process thinks it's idle and GC and CC get basically up to 50 ms slices.
Not having WebXR idle detection is causing stutter frames during immersive presentation. This patch implements idle deadline hint and sets the correct GC slices budgets during WebXR presentation.
Differential Revision: https://phabricator.services.mozilla.com/D74426
Bug 1536796 introduces "\\?\" prefix (DOS device specifier) to Windows file
paths. At the moment, the prefix is only prepended to the file paths that start
with a disk designator and a backslash.
On the other hands, IsBlockedUNCPath blocks file paths that start with "\\" in
Windows and that includes DOS device paths (the cases mentioned above).
Thus, this patch prevents DOS device paths from being treating as UNC paths in
IsBlockedUNCPath.
Differential Revision: https://phabricator.services.mozilla.com/D73621
This wraps the current thread such that a call to Dispatch will be done as direct task dispatch instead so that the task will be run in the current event loop.
This allows for have similar (but not identical to) the microtask semantics of JS promises
Differential Revision: https://phabricator.services.mozilla.com/D69995
If set, the callback will be dispatched via a direct task.
Direct tasks are run via the current thread's tail dispatcher.
This mechanism is only available if both the caller and the target are on the same thread and an AbstractThread is available
Differential Revision: https://phabricator.services.mozilla.com/D71592
In Python 3, iteration order over the contents of a native `dict` is always unpredictable, which results in bugs like bug 1635755 where `xptdata.cpp` has irreproducible content. To avoid this, we be sure to always write out JSON in a fixed, deterministic order.
Differential Revision: https://phabricator.services.mozilla.com/D74443
already_AddRefed destructor assert that it's nullptr.
DelayedDispatch check that the value passed isn't 0 and return an error code, leaving the already_AddRefed untouched.
Differential Revision: https://phabricator.services.mozilla.com/D73821
already_AddRefed destructor assert that it's nullptr.
DelayedDispatch check that the value passed isn't 0 and return an error code, leaving the already_AddRefed untouched.
Differential Revision: https://phabricator.services.mozilla.com/D73821
As of bug 1621451 this argument was ignored, but it just silently runs your code with `python3` if you pass it anyway. Ensure this doesn't happen any more, and protect against any other unexpected arguments as well.
Differential Revision: https://phabricator.services.mozilla.com/D73485
`ply`, [by design](https://github.com/dabeaz/ply/issues/79), does not produce reproducible table files; hence bug 1633156. (Note that this was *always* true, but only became a problem once we switched to Python 3, which has more unpredictable dict iteration order than Python 2.7, at least prior to [3.7](https://docs.python.org/3/whatsnew/3.7.html#summary-release-highlights).)
In any other circumstance I would consider submitting a patch to `ply` to fix this, but as of the [in-progress version 4.0 of the library](https://github.com/dabeaz/ply/blob/master/CHANGES), it doesn't even emit this cached data any more, and indeed the [latest version of the code](1fac9fed64/ply) doesn't even call `open()` at all except to do logging or to read the text data to be parsed from `stdin`. So if we were going to pin our future on `ply` and upgrade to later versions of the library in the future, we would have to live in a world where `ply` doesn't generate cached table files for us anyway.
Emitting the cached table files so later build steps can consume them is an "optimization", but it's not clear exactly how much actual value that optimization provides overall. Quoth the `CHANGES` file from that repository:
```
PLY no longer writes cached table files. Honestly, the use of
the cached files made more sense when I was developing PLY on
my 200Mhz PC in 2001. It's not as much as an issue now. For small
to medium sized grammars, PLY should be almost instantaneous.
```
In practice, I have found this to be true; namely, `./mach build pre-export export` takes just about as long on my machine after this patch as it did before, and in a try push I performed, there's no noticeable performance regression from applying this patch. In local testing I also found that generating the LALR tables in calls to `yacc()` takes about 0.01s on my machine generally, and we generate these tables a couple dozen times total over the course of the `export` tier now. This isn't *nothing*, but in my opinion it's also not nearly long enough where it would be a concern given how long `export` already takes.
That `CHANGES` file also stresses that if caching this data is important, we have the option of doing so via `pickle`. If and when we decide that re-enabling this optimization is valuable for us, we should take control of this process and perform the generation in such a way that we can guarantee reproducibility.
Differential Revision: https://phabricator.services.mozilla.com/D73484
This file is used by the searchfox indexer using python2, so maintaining
py2-compatibility is desirable if not unduly burdensome.
Differential Revision: https://phabricator.services.mozilla.com/D74224
The place where it's done right now is just unsound. It modifies
attributes during frame construction and it causes assertions to fire
with lazy frame-construction enabled.
Differential Revision: https://phabricator.services.mozilla.com/D74029
already_AddRefed destructor assert that it's nullptr.
DelayedDispatch check that the value passed isn't 0 and return an error code, leaving the already_AddRefed untouched.
Differential Revision: https://phabricator.services.mozilla.com/D73821
We currently generate absolute paths in all of our XPIDL-generated
source files, which is not so great for several reasons (deterministic
generation of files across machines, Searchfox analysis logic, shared
compilation caches, etc.). Let's generate paths that still indicate
where you should be looking, but are identical across compilations,
objdirs, etc.
Differential Revision: https://phabricator.services.mozilla.com/D73747
We currently generate absolute paths in all of our XPIDL-generated
source files, which is not so great for several reasons (deterministic
generation of files across machines, Searchfox analysis logic, shared
compilation caches, etc.). Let's generate paths that still indicate
where you should be looking, but are identical across compilations,
objdirs, etc.
Differential Revision: https://phabricator.services.mozilla.com/D73747
Raw Cr.ERROR don't get stack information, same as throwing JS literals instead
of `new Error()`s.
This was done automatically with a new eslint rule that will be introduced in
the next commit. One instance of a raw Cr.ERROR was not replaced since it is
used in a test that specifically checks the preservation of raw Cr values in
XPCJS. The rule will be disabled for that instance.
Differential Revision: https://phabricator.services.mozilla.com/D28073
Doug Thayer
Jean-Yves Avenard
Mihai Alexandru Michis
Simon Giesecke
Bas Schouten
Zeke Medley
Honza Bambas
Brindusan Cristian
Narcis Beleuzu
Nihanth Subramanya
Nathan Froyd
Razvan Maries
Mozilla Releng Treescript
lougeniac64
Andrea Marchesini
Csoregi Natalia
Emilio Cobos Álvarez
Coroiu Cristina
Chris Fronk
Butkovits Atila
Makoto Kato
Toshihito Kikuchi
David Major
Geoff Brown
Gijs Kruitbosch
Cosmin Sabou
Gerald Squelart
Eric Rahm
Nicholas Nethercote
Valentin Gosu
alwu
Kashav Madan
Dana Keeler
Kartikaya Gupta
Kris Taeleman
Emma Malysz
Bogdan Tara
ssengupta
Dorel Luca
Kagami Sascha Rosylight
Sylvestre Ledru
Jonathan Kingston
Dan Minor
Matt Woodrow
Andrew McCreight
Cameron McCormack
Lina Cambridge
Nika Layzell
Kershaw Chang
Nicklas Boman
Aaron Klotz
Sebastian Streich
Imanol Fernandez
Tom Tung
Ricky Stewart
Edgar Chen
Mirko Brodesser
Bill Gianopoulos
Bob Owen
Chris Peterson
Ian Moody