Closed
Bug 1514697
Opened 6 years ago
Closed 6 years ago
Lazy loading for URL-Classifier features
Categories
(Toolkit :: Safe Browsing, enhancement, P1)
Toolkit
Safe Browsing
Tracking
()
RESOLVED
FIXED
mozilla66
Tracking | Status | |
---|---|---|
firefox66 | --- | fixed |
People
(Reporter: baku, Assigned: baku)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
dimi
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•6 years ago
|
||
Attachment #9031812 -
Flags: review?(dlee)
Updated•6 years ago
|
Priority: -- → P1
Comment 2•6 years ago
|
||
Comment on attachment 9031812 [details] [diff] [review]
feature_lazy.patch
Review of attachment 9031812 [details] [diff] [review]:
-----------------------------------------------------------------
Hi Baku,
Do we want to check preference before initializing Features in this patch?
tp : "privacy.trackingprotection.enabled" and "privacy.trackingprotection.pbmode.enabled"
tp annotate: "privacy.trackingprotection.annotate_channels"
flash: "plugins.flashBlock.enabled"
Comment 3•6 years ago
|
||
(In reply to Dimi Lee[:dimi][:dlee] from comment #2)
>
> Hi Baku,
> Do we want to check preference before initializing Features in this patch?
If we want to do this, I'll suggest include the changes in this patch, otherwise, we will have to wait until 2019 :)
Assignee | ||
Comment 4•6 years ago
|
||
> Do we want to check preference before initializing Features in this patch?
This is already done.
UrlClassifierFeatureFlash::MaybeCreate checks StaticPrefs::plugins_flashBlock_enabled()
UrlClassifierFeatureTrackingAnnotation::MaybeCreate checks StaticPrefs::privacy_trackingprotection_annotate_channels()
UrlClassifierFeatureTrackingProtection::MaybeCreate checks loadContext->UseTrackingProtection(), which calls nsDocShell::GetUseTrackingProtection
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(dlee)
Assignee | ||
Comment 5•6 years ago
|
||
Now I see! This is the wrong patch. I already applied these changes.
Attachment #9031812 -
Attachment is obsolete: true
Attachment #9031812 -
Flags: review?(dlee)
Attachment #9032375 -
Flags: review?(dlee)
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(dlee)
Comment 6•6 years ago
|
||
Comment on attachment 9032375 [details] [diff] [review]
feature_lazy.patch
Review of attachment 9032375 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good!
::: netwerk/url-classifier/UrlClassifierFeatureFlash.cpp
@@ +48,5 @@
> EmptyCString()) // aPrefSkipHosts
> ,
> mDenying(sFlashFeaturesMap[aId].mDenying) {}
>
> +/* static */ void UrlClassifierFeatureFlash::MaybeInitialize() {
Check XRE_IsParentProcess() here and other places?
@@ +106,5 @@
>
> bool is3rdParty =
> nsContentUtils::IsThirdPartyWindowOrChannel(nullptr, aChannel, nullptr);
>
> + MaybeInitialize();
Put this before bool is3rdParty =
@@ +120,5 @@
> }
>
> /* static */ already_AddRefed<nsIUrlClassifierFeature>
> UrlClassifierFeatureFlash::GetIfNameMatches(const nsACString& aName) {
> + MaybeInitialize();
MOZ_ASSERT(sFlashFeaturesMap[0].mFeature) to sync coding style with other features
If we only init features for the parent process then we should just null return after MaybeInitial
Attachment #9032375 -
Flags: review?(dlee) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dfd83fc53601
Lazy loading for URL-Classifier features, r=dimi
Comment 8•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox66:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in
before you can comment on or make changes to this bug.
Description
•