SHA256 is an expensive operation, we should avoid using them if
possible. SafeBrowsing prefix files are loaded during startup and
verify integrity with SHA256 which may affect the performance
especially on the low-end device.
This patch simply removes the SHA256 integrity check. CRC32 version
integrity check will be introduced in the other patch.
This patch also changes the behavior of recording
"Telemetry::URLCLASSIFIER_VLPS_LOAD_CORRUPT" a little bit.
It used to records only once per session(during startup, the first
time we load prefix set), now it records per update.
Differential Revision: https://phabricator.services.mozilla.com/D21461
--HG--
extra : moz-landing-system : lando
SafeBrowsing V4 protocol use SHA-256 as the checksum to check integrity
of update data and also the integrity of prefix files.
SafeBrowsing V2 HashStore use MD5 as the checksum to check integrity of
.sbstore
Since we are going to use CRC32 as the integrity check of V4 prefix files,
I think rename V4 "checksum" to SHA256 can improve readability.
Differential Revision: https://phabricator.services.mozilla.com/D21460
--HG--
extra : moz-landing-system : lando
SHA256 is an expensive operation, we should avoid using them if
possible. SafeBrowsing prefix files are loaded during startup and
verify integrity with SHA256 which may affect the performance
especially on the low-end device.
This patch simply removes the SHA256 integrity check. CRC32 version
integrity check will be introduced in the other patch.
This patch also changes the behavior of recording
"Telemetry::URLCLASSIFIER_VLPS_LOAD_CORRUPT" a little bit.
It used to records only once per session(during startup, the first
time we load prefix set), now it records per update.
Differential Revision: https://phabricator.services.mozilla.com/D21461
--HG--
extra : moz-landing-system : lando
SafeBrowsing V4 protocol use SHA-256 as the checksum to check integrity
of update data and also the integrity of prefix files.
SafeBrowsing V2 HashStore use MD5 as the checksum to check integrity of
.sbstore
Since we are going to use CRC32 as the integrity check of V4 prefix files,
I think rename V4 "checksum" to SHA256 can improve readability.
Differential Revision: https://phabricator.services.mozilla.com/D21460
--HG--
extra : moz-landing-system : lando
In Bug 1453038, |mUpdateInterrupted| is set in Classifer::Close() which is
called by PreShutdown to abort an ongoing update. That doesn't handle
all the cases.
The SafeBrowsing update involves two threads, worker thread, and update
thread. Update thread takes care of most of the update work, when it finishes
its task, it posts a task back to the worker thread to apply the updated database
and also do some cleanup stuff. Then the update is complete.
The fix in Bug 1453038 doesn't abort an update if the woker thread is doing
the job. This is because the |mUpdateInterrupted| flag is set in the
worker thread. The PreShutdown event which eventually sets the flag has to
wait until the worker thread's current task is done.
In this patch:
1. Check nsUrlClassifierDBService::ShutdownHasStarted() to abort shutdown.
This is set by main thread so both worker thread and update thread can
be interrupted now.
2. mIsClosed is now replaced by the mUpdateInterrupted. The semantics of
mUpdateInterrupted is now changed to abort update for any internal APIs
which should cause an update to abort.
3. Remove |mUpdateInterrupted| and |ShutdownHasStarted()| checks and
unify with |ShouldAbort()|
Differential Revision: https://phabricator.services.mozilla.com/D12229
--HG--
extra : moz-landing-system : lando
A directory-based file copy without checkpoint to abort may take lots
of time to finish. This cause an issue that if firefox is shutting down
and try to close an ongoing update thread, main-thread may be blocked
for a long time.
This patch adds a wrapper for copying an entire directory, within this
wrapper, we use file-based copy and add checkpoints to let update thread
has a chance to abort.
Differential Revision: https://phabricator.services.mozilla.com/D3414
--HG--
extra : moz-landing-system : lando
A directory-based file copy without checkpoint to abort may take lots
of time to finish. This cause an issue that if firefox is shutting down
and try to close an ongoing update thread, main-thread may be blocked
for a long time.
This patch adds a wrapper for copying an entire directory, within this
wrapper, we use file-based copy and add checkpoints to let update thread
has a chance to abort.
Differential Revision: https://phabricator.services.mozilla.com/D3414
--HG--
extra : moz-landing-system : lando
This makes it possible to use different lists for tracking protection
and for the features that rely on tracking annotations.
Differential Revision: https://phabricator.services.mozilla.com/D2484
--HG--
extra : moz-landing-system : lando
URL Classifier has a mUpdateInterrupted flag to abort an ongoing
update in several checkpoints, but we didn't use this while shutting down.
Set this flag in PreShutdown() to avoid url-classifier worker thread or
update thread takes too long to finish an update.
Differential Revision: https://phabricator.services.mozilla.com/D2157
--HG--
extra : moz-landing-system : lando
If Reset() is interleaved with a shutdown, there's no point in
finishing up and we may as well bail early.
MozReview-Commit-ID: Lhm6NfAEgSj
--HG--
extra : rebase_source : e74cc22a36d287a59425079a9f7c4676e7351eba
Explicitly specify the arguments to copy to avoid making a copy of
a dangling `this` pointer.
Convert nsUrlClassifierDBService::mClassifier to a RefPtr since
the update closure might need to continue to access its members
after it's been released by the main thread.
MozReview-Commit-ID: CPio3n9MmsK
--HG--
extra : rebase_source : d7a8b207336209ee37441f3429bc608140e404ae
Replace raw pointers to LookupResult with RefPtrs and eplace the
nsAutoPtr objects + raw pointers params with UniquePtrs.
Also remove unnecessarily paranoid OOM checks when creating single
LookupResult objects since those are pretty small.
MozReview-Commit-ID: G85RNnAat6H
--HG--
extra : rebase_source : a8f6a1ff1e24663d428c8d894cb624e1c67e1bd3
The existing mix of UniquePtr and raw pointers is confusing when
trying to figure out the exact lifetime of these objects.
MozReview-Commit-ID: Br4S7BXEFKs
--HG--
extra : rebase_source : ba35d5c2eeda0741eb4c5491a6caf03f20f3d0ce
This will prevent our holding on to this pointer incorrectly in the
future.
MozReview-Commit-ID: H8ueIOK1qAK
--HG--
extra : rebase_source : 937e9c702c5109b6dfc1684392a1204d8a1edc49
I tried to make TableUpdateArray point to const TableUpdate objects
everywhere but there were two problems:
- HashStore::ApplyUpdate() triggers a few Merge() calls which include
sorting the underlying TableUpdate object first.
- LookupCacheV4::ApplyUpdate() calls TableUpdateV4::NewChecksum() when the
checksum is missing and that sets mChecksum.
MozReview-Commit-ID: LIhJcoxo7e7
--HG--
extra : rebase_source : f6ca4bf42d1ddb897a974a0b19c7185b0b6b93af
Manually keeping tabs on the lifetime of these objects is a pain
and is the likely source of some of our crashes. I suspect we might
also be leaking memory.
This change creates an explicit copy of the main array into the
update thread to avoid using a non-thread-safe shared data
structure. This is a shallow copy. Only the pointers to the
TableUpdates are copied, which means one pointer per list (e.g. 5
in total for google4 in a new profile).
MozReview-Commit-ID: 221d6GkKt0M
--HG--
extra : rebase_source : e1b81f11bb9b41e465571a95845079f455b5868e
HashStore::ApplyUpdate() is a V2-only function and so we can be
explicit about that and remove unnecessary casts.
Add a new update error code for when we fail to cast a TableUpdate
object to the expected protocol version.
MozReview-Commit-ID: 65BBwiZJw6J
--HG--
extra : rebase_source : 3f4bb0f7594d4015e2614ef747526ec5e8168a08
RegenActiveTables() relies on mPrimed being set correctly and so
the V4 lookup cache should behave the same way as the V2 one.
The V2 lookup cache on the other hand was unnecessarily setting
mPrimed to true twice.
MozReview-Commit-ID: LwNdI9DTqZ7
--HG--
extra : rebase_source : ff7d7a051f28867be5c35c1079407bb2bfd3a490
"Include what you use."
--HG--
extra : rebase_source : 2239a380029e0efbc9dd3042459222a67c38d70f
extra : amend_source : 4453c32cc469caa592049167205666997f1a1e7b
extra : histedit_source : a533edd4a4d3d0642b08989e93674661d27baa6a%2C37d27eeef9580381ccc0de8507f60166dabf1730
Passing a value of -1 to nsCString::Truncate() converts that
value to a large integer and leads to an unnecessary 4GB
memory allocation.
MozReview-Commit-ID: Icm5iUsEgA6
--HG--
extra : rebase_source : 7c7a52762ba86b1daadf60b0f362c5740d335781
This fixes usages of `Find`, `RFind` and the equality operator that kind of
work right now but will break with the proper type checking of a templatized
version of the string classes.
For `Find` and `RFind` it appears that `nsCString::(R)Find("foo", 0)` calls
were being coerced to the `Find(char*, bool, int, int)` versions. The intent was
probably to just start searching from position zero.
For the equality operator, the type of nullptr is nullptr_t rather than
char(16_t)* so we'd need to add an operator overload that takes nullptr_t. In
this case just using `IsVoid` is probably more appropriate.
--HG--
extra : rebase_source : 50f78519084012ca669da0a211c489520c11d6b6
We have a minimum requirement of VS 2015 for Windows builds, which supports
the z length modifier for format specifiers. So we don't need SizePrintfMacros.h
any more, and can just use %zu and friends directly everywhere.
MozReview-Commit-ID: 6s78RvPFMzv
--HG--
extra : rebase_source : 009ea39eb4dac1c927aa03e4f97d8ab673de8a0e
dlee
Dorel Luca
Andrea Marchesini
Kris Maglione
arthur.iakab
Sylvestre Ledru
Gurzau Raul
Francois Marier
Sebastian Hengst
Tristan Bourvon
Jason Orendorff
Thomas Nguyen
Eric Rahm
Kartikaya Gupta
Bill McCloskey
Carsten "Tomcat" Book