Removing these checks causes crashes when trying to upgrade a <= 17 db to >= 23:
(A) upgradeDatabaseFrom17to18 calls createReadingListTable, and we create the table using the new (>=23 schema).
This schema has no "read" column.
(B) upgradeDatabaseFrom22to23 migrates the same table. As part of the migration it tries to select the "read"
column, and we crash because that doesn't exist. This was prevented by an early return if didCreateReadingListTable
was set.
It looks like removing the didCreateTablsTable checks is OK because the migration only adds a foreign-key constraint,
and doesn't depend on any columns that didn't exist in the initial version of the migration. However it seems wasteful
to run the migration on a table that is already in the expected state. Moreover not having table-created checks is
not safe in most cases, and having these checks should be the default pattern - especially in case any future migrations
affecting the same table are added.
MozReview-Commit-ID: 4j1PlQc6LLN
Users who have this pref set to a non-default value have a different slow
script experience than the others.
This preference is generally set as a flag: it is only ever changed at most
one time in a profile, so it shouldn't appreciably increase subsession splits.
MozReview-Commit-ID: FoQXvd9eSh
The underlying intent of the assertion was to guarantee that pending errors wouldn't be overwritten.
This patch makes that intent more clear while addressing the fact that the original assertion was
not necessarily true.
The basic idea here is as follows:
* Rule nodes are reference-counted, but releasing them adds them to a linked
list rather than freeing them. This allows for the reuse that motivated the
original GC scheme.
* We get rid of the marking, and instead rely on the reference count.
* Sweeping no longer requires a complicated traversal. We just pop items
off the free list until it's empty. When a child is destroyed, its parent
may go onto the free list.
* We remove special handling for the root node, and use a regular reference-counted
edge from the style set.
* The free list automatically asserts that it's empty (meaning all nodes have been
freed) in its destructor, which runs when the style set is destroyed.
* We get rid of the list of style context roots on the style set. We still need
a count though, because of the HasCachedStyleData check.
Additionally, added WeakReferences to the SEM in its callbacks so we can
GC ASAP if the Activity (and thus the SEM) gets GC'd. This is important
since we hold a reference to Context which can be a rather large object.
Furthermore, I added some related thread annotations where I felt they
were useful.
MozReview-Commit-ID: KaWlw14uOoN
The default search engine attribute may be null in the core ping if we haven't
been able to retrieve the value yet. It's unclear when this might be, but the
possibility is in the javadoc of `SearchEngineManager.getEngine`.
MozReview-Commit-ID: IrJB6GyjyTO
As best as I can tell, this code runs before Gecko is initialized. via
bug 1255657 comment 8:
To confirm this, GeckoProfile is created before the GeckoThread is
finished launching but it seems almost coincidental:
GeckoApp.onCreate
GeckoThread.launch
GeckoThread.start
...
GeckoThread.run
GeckoThread.getGeckoArgs (notably before GeckoLader.nativeRun)
GeckoThread.addCustomProfileArg
(if GAP.getGeckoInterface != null, which is set earlier in GeckoApp.onCreate)
EITHER
GeckoProfile.getDir() (if in guest mode)
GeckoProfile.forceCreate
GeckoProfile.forceCreate (if not in guest mode)
forceCreate opens the times.json file.
That being said, if this code path changes, forceCreate is also called
when the GeckoView is initialized (which occurs after the GeckoThread.launch
call, but is likely to happen before Gecko finishes launching).
---
If we wanted GeckoProfile initialization timing to be seem less coincidental,
we should consider initializing the profile non-lazily: bug 1262625.
MozReview-Commit-ID: LGluC021CTg
