1342883 - Disabled recipes are executed by Shield System Addon

Status: VERIFIED FIXED

(Firefox :: Normandy Client, defect)

Description

[Adrian Florinescu [:aflorinescu]]

[Description:]
Even if the recipes are disabled in Normandy Control Center, the recipes are still downloaded and executed by the System Addon.

[Prerequisites]:
Open FF/ about:config and
1. Set the extensions.shield-recipe-client.dev_mode preference to true to run recipes immediately on startup.
2. Set the extensions.shield-recipe-client.logging.level preference to 0 to enable more logging.
3. Set the security.content.signature.root_hash preference to DB:74:CE:58:E4:F9:D0:9E:E0:42:36:BE:6C:C5:C4:F6:6A:E7:74:7D:C0:21:42:7A:03:BC:2F:57:0C:8B:9B:90.




[Steps v1:]
1. Connect to VPN.
2. Open Control Center.
3. Create a Recipe. 
4. Disable the recipe.
5. Open Firefox with the prerequisites] to be able to get recipes. (varies depending on the normandy server)
6. Open Browser Console.

[Steps v2:]
1. Connect to VPN.
2. Open Control Center.
3. Create a Recipe. 
4. Open Firefox with the prerequisites] to be able to get recipes. (varies depending on the normandy server)
5. Open browser console and the recipe created at step 3 will be fetched by the Shield System addon.
6. Close FF.
7. From Control Center disable the recipe created at step3.
8. Reopen FF and open browser console.

[Actual Result:]
The disabled recipe is identified by the System Addon as a valid recipe and will be fetched:
extensions.shield-recipe-client.normandy-api	DEBUG	Fetched 4 recipes from the server: disabled_recipe, ...

[Expected Result:]
When disabled the recipe is not fetched by the Shield System Addon.


[Note:]
Both versions of the steps(v1 and v2) are listed above in order be taken into consideration for the fix verification.

Comment 1

[Osmose [:osmose, :mkelly]]

Github issue: https://github.com/mozilla/normandy/issues/544

Comment 2

[Osmose [:osmose, :mkelly]]

Patch landed in mozilla-central in the sync from bug 1349348. This will be deployed to Beta 53 and Release 53 as a system add-on (bug 1351454).

Comment 3

[Adrian Florinescu [:aflorinescu]]

Verified as fixed on Beta53 across Ubuntu 16.04, Win10, OSX 10.12.

Comment 4

[Benjamin Chen [:bechen]]

Attachment: Bug 1342883 - part1: When nsDocument receive unload event from nsDocumentViewer, nofity the MediaElements.

Review commit: https://reviewboard.mozilla.org/r/183102/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/183102/

Comment 5

[Benjamin Chen [:bechen]]

Attachment: Bug 1342883 - part2: Once a MediaElement receive document unload event, remove all MediaElements in gElementTable with the same uri.

Review commit: https://reviewboard.mozilla.org/r/183104/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/183104/

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8911705 [details]
Bug 1342883 - part1: When nsDocument receive unload event from nsDocumentViewer, nofity the MediaElements.

https://reviewboard.mozilla.org/r/183102/#review188312

Wrong bug?

Comment 7

[Benjamin Chen [:bechen]]

Comment on attachment 8911706 [details]
Bug 1342883 - part2: Once a MediaElement receive document unload event, remove all MediaElements in gElementTable with the same uri.

https://reviewboard.mozilla.org/r/183104/#review188998

::: dom/html/HTMLMediaElement.cpp:6377
(Diff revision 1)
>        ShutdownDecoder();
>      }
>    }
>  
>    if (aDocumentUnload && mDecoder) {
> +    RemoveMediaElementFromURITable(true);

If the unload event comes through OnPageHide() eventually, no matter F5, ctrl+F5, normal close ..., this implementation seems not working for non-e10s. One process multiple tabs