Right now, NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR expects singleton
constructors to return already-addrefed raw pointers, and while it accepts
constructors that return already_AddRefed, most existing don't do so.
Meanwhile, the convention elsewhere is that a raw pointer return value is
owned by the callee, and that the caller needs to addref it if it wants to
keep its own reference to it.
The difference in convention makes it easy to leak (I've definitely caused
more than one shutdown leak this way), so it would be better if we required
the singleton getters to return an explicit already_AddRefed, which would
behave the same for all callers.
This also cleans up several singleton constructors that left a dangling
pointer to their singletons when their initialization methods failed, when
they released their references without clearing their global raw pointers.
MozReview-Commit-ID: 9peyG4pRYcr
--HG--
extra : rebase_source : 2f5bd89c17cb554541be38444672a827c1392f3f
This lets us replace moz_xstrdup() of string literals with AssignLiteral(),
among other improvements.
--HG--
extra : rebase_source : 9994d8ccb4f196cf63564b0dac2ae6c4370defb4
NS_ProxyRelease will immediately release the provided reference if already on
the right thread unless aAlwaysRelease=true (the default is false). We need to
set it to be true to avoid Close() and its potential to, out of desperation
when faced with misuse, spin a nested event loop.
--HG--
extra : rebase_source : 91c70a6fa081fdf20a8dfabf5d69bccbef1ba33c
The NS_LITERAL_CSTRING macro creates a temporary nsLiteralCString to encapsulate the string literal and its length, but AssignLiteral() can determine the string literal's length at compile-time without nsLiteralCString.
MozReview-Commit-ID: B5Y8KyExPQ8
--HG--
extra : rebase_source : e27b266c145daa5acd887e998c6d5b408101e1db
extra : source : 33f49977a33cbdb1c7127871b940eefccc018f65
This is for the trunk/nightly landing only, and approval was explicitly
requested from #sheriffs to land without tests as a preliminary stop-gap.
Tests and a proper fix will land as part of bug 1389660, but that will be a few
days.
--HG--
extra : rebase_source : 5849f2f37d735387e13ac0486f1fae7906725111
extra : source : 908a4bb55a039985473e1565d88abebfe024c44c
The observer references were the only thing guranteed to keep the Service
alive, leading to potential use-after-free during the iteration loop to
make sure all the connections were closed. (Ironically, if they were
fully closed and their instances destroyed, that's when bad things would
happen.)
--HG--
extra : rebase_source : 6c8d6f9e0b75751b10166d7e2c63d5a3cb27d28e
This patch parameterizes nsAuto[C]String, renames them as nsAuto[C]StringN, and
redefines nsAuto[C]String as typedefs for nsAuto[C]StringN<64>.
(The alternative would be to templatize nsAuto[C]String and use a default
parameter, but that would require writing "nsAuto[C]String<>" everywhere.)
This patch makes the following changes to the macros.
- Removes PROFILER_LABEL_FUNC. It's only suitable for use in functions outside
classes, due to PROFILER_FUNCTION_NAME not getting class names, and it was
mostly misused.
- Removes PROFILER_FUNCTION_NAME. It's no longer used, and __func__ is
universally available now anyway.
- Combines the first two string literal arguments of PROFILER_LABEL and
PROFILER_LABEL_DYNAMIC into a single argument. There was no good reason for
them to be separate, and it forced a '::' in the label, which isn't always
appropriate. Also, the meaning of the "name_space" argument was interpreted
in an interesting variety of ways.
- Adds an "AUTO_" prefix to PROFILER_LABEL and PROFILER_LABEL_DYNAMIC, to make
it clearer they construct RAII objects rather than just being function calls.
(I myself have screwed up the scoping because of this in the past.)
- Fills in the 'js::ProfileEntry::Category::' qualifier within the macro, so
the caller doesn't need to. This makes a *lot* more of the uses fit onto a
single line.
The patch also makes the following changes to the macro uses (beyond those
required by the changes described above).
- Fixes a bunch of labels that had gotten out of sync with the name of the
class and/or function that encloses them.
- Removes a useless PROFILER_LABEL use within a trivial scope in
EventStateManager::DispatchMouseOrPointerEvent(). It clearly wasn't serving
any useful purpose. It also serves as extra evidence that the AUTO_ prefix is
a good idea.
- Tweaks DecodePool::SyncRunIf{Preferred,Possible} so that the labelling is
done within them, instead of at their callsites, because that's a more
standard way of doing things.
--HG--
extra : rebase_source : 318d1bc6fc1425a94aacbf489dd46e4f83211de4
Per bug: it can take around 200ms to shrink memory for
Places.sqlite. This can end up janking the browser if we hit a
memory-pressure. This patch simply removes the #if DEBUG guards
around the mAsyncExecutionThreadIsAlive boolean which is already
being updated and exposes it so that we can shrink memory off
the main thread when possible.
MozReview-Commit-ID: LoDGKrOXs8u
--HG--
extra : rebase_source : adf03f187e7297835566f3bac92a8be8a6147131
Introduce a new SpinningSynchronousClose API that allows to synchronously close
a connection that executed asynchronous statements, by spinning the events loop.
This is expected to be used rarely in particular cases like database corruption.
It is currently [noscript] since the only consumer is cpp, in the future we can
evaluate removing that, if we find more uses for it.
MozReview-Commit-ID: 7SPqutoF9jJ
--HG--
extra : rebase_source : d053e308192f335dfdca089668e56115b068ff8c
Avoids a thread-safety race condition on shutdown where we could try to finalize a statement twice.
Allows the async thread to be referenced until ShutdownAsyncThread, so async finalizers can make use of it.
Removes the no more useful mAsyncExecutionThreadIsAlive.
Nullifies the sqlite3_mutex pointer when the connection is closed, since it would be a dangling pointer.
Use a ScopeExit to ensure the connection and the mutex pointers are always nullified on failure.
Makes asyncClose bailout early if a Close method was already invoked before.
Makes AsyncInitDatabase not use AsyncClose to just shutdown the async thread.
Fixes various unfinalized statements in consumers code.
Makes mConnectionClosed better follow mDBConn status.
Replaces some mutex locking isClosed() calls with lockless isConnectionReadyOnThisThread.
MozReview-Commit-ID: 6sftFehsQTt
--HG--
extra : rebase_source : 2bf3088b500376e58e62e8f078d9950588adc649
This function is arguably nicer than calling NS_ProcessNextEvent
manually, is slightly more efficient, and will enable better auditing
for NS_ProcessNextEvent when we do Quantum DOM scheduling changes.
Change mozilla::Smprintf and friends to return a UniquePtr, rather than
relying on manual memory management. (Though after this patch there are
still a handful of spots needing SmprintfFree.)
MozReview-Commit-ID: COa4nzIX5qa
--HG--
extra : rebase_source : ab4a11b4d2e758099bd0794d5c25d799a7e42680
There's an antipattern where nsLiteralString is used as an unnecessary intermediary in converting from CharT* to CharT*,
e.g. CallAFunctionThatTakesACharPointer(NS_LITERAL_CSTRING("foo").get());
or
NS_NAMED_LITERAL_STRING(foo, "abc");
CallAFunctionThatTakesACharPointer(foo.get());
This patch rewrites the callsites that can be trivially changed to use char*/char16_t*.
I'd somewhat like to remove nsTLiteralString::get() altogether, but in code that's less straightforward than these examples, get() is useful enough to keep.
MozReview-Commit-ID: Kh1rUziVllo
--HG--
extra : rebase_source : c21a65694d6e1c42fd88f73632f7ac8f38d005ae
There's an antipattern where nsLiteralString is used as an unnecessary intermediary in converting from CharT* to CharT*,
e.g. CallAFunctionThatTakesACharPointer(NS_LITERAL_CSTRING("foo").get());
or
NS_NAMED_LITERAL_STRING(foo, "abc");
CallAFunctionThatTakesACharPointer(foo.get());
This patch rewrites the callsites that can be trivially changed to use char*/char16_t*.
I'd somewhat like to remove nsTLiteralString::get() altogether, but in code that's less straightforward than these examples, get() is useful enough to keep.
MozReview-Commit-ID: Kh1rUziVllo
--HG--
extra : rebase_source : c21a65694d6e1c42fd88f73632f7ac8f38d005ae
Changed |print("enum ID : uint32_t {", file=output)| to |print("enum HistogramID : uint32_t {", file=output)| at line 53 of the file |toolkit/components/telemetry/gen-histogram-enum.py|, and then replaced all the textual occurrences of |Telemetry::ID| to |Telemetry::HistogramID| and |ID| to |HistogramID| in 43 other files.
nsIXPCScriptable flags handling in xpc_map_end.h is a bit of a mess.
- Half the flags relate to whether various functions are defined (PreCreate,
GetProperty, etc). These are set using the XPC_MAP_WANT_* macros;
for each one xpc_map_end.h inserts the corresponding flag using the
preprocessor (see XPC_MAP_CLASSNAME::GetScriptableFlags()).
- The other half of the flags relate to other things (IS_GLOBAL_OBJECT,
DONT_REFLECT_INTERFACE_NAMES, etc). These are set using the XPC_MAP_FLAGS
macro.
Having two similar but different mechanisms to set the flags for a class is
confusing. (Indeed, until recently we had some classes where a single flag was
redundantly specified via both mechanisms.) Note also that the classes done in
dom/base/nsIDOMClassInfo.h also specify all the flags in a single value,
similar to how XPC_MAP_FLAGS works.
This patch removes the XPC_MAP_WANT_* macros. All flags are now set
via XPC_MAP_FLAGS. This is a significant simplification to xpc_map_end.h and
all the places that use it.
The downside of this change is that I had to change the flag constants from
class constants (i.e. nsIXPCScriptable::FOO) to macros (i.e.
NSIXPCSCRIPTABLE_FOO) because they need to be used in #if statements like this
in xpc_map_end.h:
#if !((XPC_MAP_FLAGS) & NSIXPCSCRIPTABLE_WANT_PRECREATE)
and you can't use a '::'-qualified name inside a #if. I think this downside is
outweighed by the simplification described above.
Overall the patch removes 80 lines of code.
--HG--
extra : rebase_source : 6d5c341d0deba8f1529d81c17bb8819e09620b05
This change is mostly straightforward, except for the following.
- It removes all the printing from the do_check_* macros because gtest macros
do appropriate printing.
- test_StatementCache.cpp needs some special gtest magic for the type
parameterization.
- It merges the four tests in test_unlock_notify.cpp because they rely on being
executed in order, and so aren't independent.
- storage_test_harness_tail.h is no longer necessary because gtest provides the
test looping functionality.
- It uses #include and the preprocessor to remove the duplication between
test_deadlock_detector.cpp and xpcom/tests/DeadlockDetector.cpp.
- It makes the test in test_service_init_background_thread.cpp a death test to
force it to be the first storage gtest, because it fails otherwise.
- It adds code to undo the SQLite mutex hooking as necessary, so that tests
don't interfere with each other.
- It de-virtualizes Spinner's destructor (as identified in bug 1318282).
--HG--
rename : storage/test/storage_test_harness.h => storage/test/gtest/storage_test_harness.h
rename : storage/test/test_AsXXX_helpers.cpp => storage/test/gtest/test_AsXXX_helpers.cpp
rename : storage/test/test_StatementCache.cpp => storage/test/gtest/test_StatementCache.cpp
rename : storage/test/test_asyncStatementExecution_transaction.cpp => storage/test/gtest/test_asyncStatementExecution_transaction.cpp
rename : storage/test/test_async_callbacks_with_spun_event_loops.cpp => storage/test/gtest/test_async_callbacks_with_spun_event_loops.cpp
rename : storage/test/test_binding_params.cpp => storage/test/gtest/test_binding_params.cpp
rename : storage/test/test_deadlock_detector.cpp => storage/test/gtest/test_deadlock_detector.cpp
rename : storage/test/test_file_perms.cpp => storage/test/gtest/test_file_perms.cpp
rename : storage/test/test_mutex.cpp => storage/test/gtest/test_mutex.cpp
rename : storage/test/test_service_init_background_thread.cpp => storage/test/gtest/test_service_init_background_thread.cpp
rename : storage/test/test_statement_scoper.cpp => storage/test/gtest/test_statement_scoper.cpp
rename : storage/test/test_transaction_helper.cpp => storage/test/gtest/test_transaction_helper.cpp
rename : storage/test/test_true_async.cpp => storage/test/gtest/test_true_async.cpp
rename : storage/test/test_unlock_notify.cpp => storage/test/gtest/test_unlock_notify.cpp
extra : rebase_source : dbb695c112564efa1945116be1a8435988982e74
shindli
Gabriele Svelto
Andrew Sutherland
Andrew McCreight
Mark Banner
Kris Maglione
Nicholas Nethercote
Bob Owen
Sebastian Hengst
Kai Engert
Chris Peterson
Ryan VanderMeulen
Marco Bonardo
Jan de Mooij
Phil Ringnalda
Masatoshi Kimura
Nathan Froyd
Andrea Marchesini
Olli Pettay
Sylvestre Ledru
Bill McCloskey
Carsten "Tomcat" Book
Florian Quèze
Wes Kocher
Doug Thayer
Bevis Tseng
Mike Hommey
Tom Tromey
Sajid Ahmed
Randell Jesup
David Major
Iris Hsiao
Joel Maher
Vedant Sareen
Zibi Braniecki
Jan Beich
Jared Wein
Markus Stange
Tomislav Jurin