This change splits PLDHashTable::Iterator::NextEntry() into two separate
functions, which allow you to get the current element and advance the iterator
separately, which means you can use a for-loop to iterate instead of a
while-loop.
As part of this change, the internals of PLDHashTable::Iterator were
significantly changed and simplified (and modelled after js::HashTable's
equivalent code). It's no longer duplicating code from PL_DHashTableEnumerator.
The chaos mode code was a casualty of this, but given how unreliable that code
has proven to be (see bug 1173212, bug 1174046) this is for the best. (We can
reimplement chaos mode once PLDHashTable::Iterator is back on more solid
footing again, if we think it's important.)
All these changes will make it much easier to add an alternative Iterator that
removes elements, which was turning out to be difficult with the prior code.
In order to make the for-loop header usually fit on a single line, I
deliberately renamed a bunch of things to have shorter names.
In summary, you used to write this:
PLDHashTable::Iterator iter(&table);
while (iter.HasMoreEntries()) {
auto entry = static_cast<FooEntry*>(iter.NextEntry());
// ... do stuff with |entry| ...
}
// iter's scope extends beyond here
and now you write this:
for (auto iter = table.Iter(); !iter.Done(); iter.Next()) {
auto entry = static_cast<FooEntry*>(iter.Get());
// ... do stuff with |entry| ...
}
// iter's scope doesn't reach here
--HG--
extra : rebase_source : fa5cac2fc50b1ab7624030bced4763131280f4d8
The old code attempted to deal with any OOMs during this enumeration --
OOMs are possible because it's growing an nsCOMArray -- but failed to do so
correctly.
- It didn't check the return value of AppendObject().
- It did check that EntryCount() matched the return value of
PL_DHashTableEnumerate(), but that's always (and vacuously) true.
The new code just returns NS_ERROR_OUT_OF_MEMORY if AppendObject() fails; this
is trivial now that it uses an iterator and doesn't have to call out to another
function.
This is straightforward mapping of PR_LOG levels to their LogLevel
counterparts:
PR_LOG_ERROR -> LogLevel::Error
PR_LOG_WARNING -> LogLevel::Warning
PR_LOG_WARN -> LogLevel::Warning
PR_LOG_INFO -> LogLevel::Info
PR_LOG_DEBUG -> LogLevel::Debug
PR_LOG_NOTICE -> LogLevel::Debug
PR_LOG_VERBOSE -> LogLevel::Verbose
Instances of PRLogModuleLevel were mapped to a fully qualified
mozilla::LogLevel, instances of PR_LOG levels in #defines were mapped to a
fully qualified mozilla::LogLevel::* level, and all other instances were
mapped to us a shorter format of LogLevel::*.
Bustage for usage of the non-fully qualified LogLevel were fixed by adding
|using mozilla::LogLevel;| where appropriate.
|mOps| is always non-null now, and there's no longer any distinction between
and uninitialized and initialized table. Yay.
--HG--
extra : rebase_source : 3d1fb72aee4dd21ff20db0ff3166d4e932ade897
This is straightforward mapping of PR_LOG levels to their LogLevel
counterparts:
PR_LOG_ERROR -> LogLevel::Error
PR_LOG_WARNING -> LogLevel::Warning
PR_LOG_WARN -> LogLevel::Warning
PR_LOG_INFO -> LogLevel::Info
PR_LOG_DEBUG -> LogLevel::Debug
PR_LOG_NOTICE -> LogLevel::Debug
PR_LOG_VERBOSE -> LogLevel::Verbose
Instances of PRLogModuleLevel were mapped to a fully qualified
mozilla::LogLevel, instances of PR_LOG levels in #defines were mapped to a
fully qualified mozilla::LogLevel::* level, and all other instances were
mapped to us a shorter format of LogLevel::*.
Bustage for usage of the non-fully qualified LogLevel were fixed by adding
|using mozilla::LogLevel;| where appropriate.
This is straightforward mapping of PR_LOG levels to their LogLevel
counterparts:
PR_LOG_ERROR -> LogLevel::Error
PR_LOG_WARNING -> LogLevel::Warning
PR_LOG_WARN -> LogLevel::Warning
PR_LOG_INFO -> LogLevel::Info
PR_LOG_DEBUG -> LogLevel::Debug
PR_LOG_NOTICE -> LogLevel::Debug
PR_LOG_VERBOSE -> LogLevel::Verbose
Instances of PRLogModuleLevel were mapped to a fully qualified
mozilla::LogLevel, instances of PR_LOG levels in #defines were mapped to a
fully qualified mozilla::LogLevel::* level, and all other instances were
mapped to us a shorter format of LogLevel::*.
Bustage for usage of the non-fully qualified LogLevel were fixed by adding
|using mozilla::LogLevel;| where appropriate.
CLOSED TREE
Backed out changeset 12ce98475c6e (bug 1119980)
Backed out changeset bdb8d05f8870 (bug 1119980)
Backed out changeset a68a18840492 (bug 1119980)
This patch simplifies nsStaticCaseInsensitiveNameTable greatly by taking
advantage of the following observations.
- |new| is infallible, so |new nsStaticCaseInsensitiveNameTable()| calls don't
need their return value checked.
- nsStaticCaseInsensitiveNameTable::Init() checks if any of the added entries
differ only in case, so the callers of that function don't need to do the
same check.
- nsStaticCaseInsensitiveNameTable::Init() never returns false because
moz_xmalloc() is infallible. (Its callers never check the return value
anyway.) So it can be merged into the constructor. And
~nsStaticCaseInsensitiveNameTable() need not null-check |mNameArray|.
- PLDHashTable now has an initializing constructor and destructor, so these can
be used in nsStaticCaseInsensitiveNameTable, thus avoiding manual
PLD_HashTable{Init,Finish}() calls.
This patch converts easy cases, i.e. where the PL_DHashTableInit() call occurs
in a constructor and the PL_DHashTableFinish() call occurs in a destructor.
--HG--
extra : rebase_source : d8dc450f80ac23b8455141b471cc9ae823e1e384
Due to Android startup regressions (bug 1163066) and plugin crashes (bug
1165155).
--HG--
extra : rebase_source : 380f79e67dff4c4eaa2614f286a4d0669666b652
This patch converts easy cases, i.e. where the PL_DHashTableInit() call occurs
in a constructor and the PL_DHashTableFinish() call occurs in a destructor.
They are kept around for the sake of the standalone glue, which is used
for e.g. webapprt, which doesn't have direct access to jemalloc, and thus
still needs a wrapper to go through the xpcom function list and get to
jemalloc from there.
It's no longer needed now that entry storage isn't allocated there. (The other
possible causes of failures in that function are less interesting and simply
crashing is a reasonable thing to do for them.)
This also makes PL_DNewHashTable() infallible, so I removed some
now-unnecessary checks of its result.
--HG--
extra : rebase_source : 4c6ab0c449bc18e8bace8bf036b5bd78d3a2f1c4
The QI in xpc_TryUnmarkWrappedGrayObject() might somehow trigger a GC,
which in turn could destroy an observer, that would in turn call
RemoveObserver() which mutates the hash table that we're enumerating.
I kept all the existing PL_DHashTableAdd() calls fallible, in order to be
conservative, except for the ones in nsAtomTable.cpp which already were
followed immediately by an abort on failure.
--HG--
extra : rebase_source : 526d96ab65e4d7d71197b90d086d19fbdd79b7b5
I kept all the existing PL_DHashTableAdd() calls fallible, in order to be
conservative, except for the ones in nsAtomTable.cpp which already were
followed immediately by an abort on failure.
--HG--
extra : rebase_source : eeba14d732077ef2e412f4caca852de6b6b85f55
Because it's no longer needed now that entry storage isn't allocated there.
(The other possible causes of failures are much less interesting and simply
crashing is a reasonable thing to do for them.)
This also makes PL_DNewHashTable() infallible.
--HG--
extra : rebase_source : 848cc9bbdfe434525857183b8370d309f3acbf49
It feels safer to use a function with a new name, rather than just changing the
behaviour of the existing function.
For most of these cases the PL_DHashTableLookup() result was checked with
PL_DHASH_ENTRY_IS_{FREE,BUSY} so the conversion was easy. A few of them
preceded that check with a useless null check, but the intent of these was
still easy to determine.
I'll do the trickier ones in subsequent patches.
--HG--
extra : rebase_source : ab37a7a30be563861ded8631771181aacf054fd4
Currently the setting of PLDHashTable::ops is very haphazard.
- PLDHashTable has no constructor, so it's not auto-nulled, so lots of places
null it themselves.
- In the fallible PLDHashTable::Init() function, if the entry storage
allocation fails we'll be left with a table that has |ops| set -- indicating
it's been initialized -- but has null entry storage. I'm not certain this can
cause problems but it feels unsafe, and some (but not all) callers of Init()
null it on failure.
- PLDHashTable does not null |ops| in Finish(), so some (but not all) callers
do this themselves.
This patch makes things simpler.
- It adds a constructor that zeroes |ops|.
- It modifies Init() so that it only sets |ops| once success is ensured.
- It zeroes |ops| in Finish().
- Finally, it removes all the now-unnecessary |ops| nulling done by the users
of PLDHashTable.
--HG--
extra : rebase_source : bb34979c218d152562a2f9c7e5215256c111cc5b
This patch adds another implementation of BaseTimeDuration's ValueCalculator
template parameter that is careful to preserve Forever/-Forever values when
performing arithmetic.
It also defines a typedef for a specialization of BaseTimeDuration that uses
this new ValueCalculator definition.
This patch builds on the templatization from the previous patch to move
arithmetic on the mValue member of BaseTimeDuration to the ValueCalculator
template parameter.
This patch prepares the way for having a separate StickyTimeDuration class
by factoring TimeDuration into a templated base class: BaseTimeDuration.
BaseTimeDuration takes a templated parameter, ValueCalculator, which is a helper
object that defines how various arithmetic operations are performed on its
mValue member (an int64_t count of ticks).
This patch does not actually define or use the ValueCalculator parameter yet but
simply performs the renaming and templatization.
With regards to the templatization, arithmetic operators are defined to take
objects with the same ValueCalculator template parameter (so that we don't, for
example, apply non-safe arithmetic to a StickyTimeDuration).
However, comparison operators are defined to also operate on objects with
a different ValueCalculator template parameter since comparison should be
independent of the type of arithmetic used.
Likewise, the constructor and assignment operator are defined to operate on
objects with a different ValueCalculator template parameter so that objects can
be converted from TimeDuration to StickyTimeDuration and vice-versa.
The constructor is marked as explicit, however, so that we don't silently
convert a StickyTimeDuration to a TimeDuration and unwittingly apply
non-safe arithmetic to a StickyTimeDuration.
TimeDuration is defined as a specialization of BaseTimeDuration that uses
TimeDurationValueCalculator as its ValueCalculator type.
TimeDurationValueCalculator is filled-in in a subsequent patch.
Now that the implementation of a number of TimeDuration methods have been
delegated to a separate TimeDurationPlatformUtils class, it is easier to add
consistent handling to the parameters passed to and from those methods.
This patch adds checks when converting between TimeDurations and doubles so that
Forever (represented by a tick count of INT64_MAX) is paired with
PositiveInfinity, and INT64_MIN is paired with NegativeInfinity.
In order to have different templated versions of TimeDuration we first split out
the platform-specific code so that this code doesn't need to concern itself with
templates (and because putting template code in .cpp files is messy).
Andrew McCreight
Nicholas Nethercote
Bill McCloskey
Benoit Girard
Ted Mielczarek
Eric Rahm
Carsten "Tomcat" Book
Wes Kocher
Botond Ballo
Phil Ringnalda
Mike Hommey
David Major
Andrea Marchesini
Chris Peterson
Ryan VanderMeulen
Randall Barker
Jonathan Watt
Ehsan Akhgari
Mason Chang
Andrew McCreight
Trevor Saunders
Michael Pruett
Nathan Froyd
Brian Birtles
Jacek Caban
Honza Bambas
Birunthan Mohanathas
Kyle Huey