1514697 - Lazy loading for URL-Classifier features

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, enhancement, P1)

Comment 1

[Andrea Marchesini [:baku]]

Attachment: feature_lazy.patch

Comment 2

[Dimi Lee [ooo 16.04 - 21.04］]

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

[Dimi Lee [ooo 16.04 - 21.04］]

(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 :)

Comment 4

[Andrea Marchesini [:baku]]

> 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

Comment 5

[Andrea Marchesini [:baku]]

Attachment: feature_lazy.patch

Now I see! This is the wrong patch. I already applied these changes.

Comment 6

[Dimi Lee [ooo 16.04 - 21.04］]

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dfd83fc53601
Lazy loading for URL-Classifier features, r=dimi

Comment 8

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/dfd83fc53601