1464263 - Update content module settings on initialization

Status: RESOLVED FIXED

(GeckoView :: General, defect, P1)

Description

[Eugen Sawin [:esawin]]

In bug 1459759 we've fixed setting propagation to content modules on initialization, but have since regressed that in bug 1460047 by only updating settings for enabled modules.

Comment 1

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1464263-1.0-Always-update-settings-on-initializa.patch

Some modules need to apply setting during initialization (e.g., for tracking protection), I don't think this should depend on the enabled state.

Comment 2

[Eugen Sawin [:esawin]]

Attachment: 0003-Bug-1464263-1.1-Update-settings-for-non-enabled-modu.patch

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8980677 [details] [diff] [review]
0003-Bug-1464263-1.1-Update-settings-for-non-enabled-modu.patch

Review of attachment 8980677 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with nit.

::: mobile/android/chrome/geckoview/geckoview.js
@@ +62,5 @@
>  
>      this.forEach(module => {
>        module.onInit();
>      });
> +    this._updateSettings(this._settings);

Instead of calling `_updateSettings` here, I think it's better to call `this._impl.onSettingsUpdate();` inside `ModuleInfo.onInit()`, right after `this._impl.onInit();`

Comment 4

[Eugen Sawin [:esawin]]

Attachment: 0003-Bug-1464263-1.2-Update-settings-for-non-enabled-modu.patch

Addressed comment.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/352257f10f20
[1.2] Update settings for non-enabled modules. r=jchen

Comment 6

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/352257f10f20