1393954 - Roll Safe Browsing back to V2 in the first RC for 56

Status: VERIFIED FIXED

(Toolkit :: Safe Browsing, defect, P1)

Description

[François Marier [:francois]]

For capacity planning reason on the Google side, we will be doing a staged roll-out of the Safe Browsing V4 feature (bug 1387651).

To avoid accidentally letting 100% of users on release ping the V4 server, we will roll back everyone on Beta 56 to the old V2 server.

Comment 1

[François Marier [:francois]]

Attachment: Patch to be uplifted in the last beta of the 56 cycle

Comment 2

[François Marier [:francois]]

This needs to be tracked by release management so that it lands in the last beta of the 56 cycle.

Comment 3

[Liz Henry (:lizzard) (relman/hg->git project)]

Thanks, I'll mark it as a blocker for release to ensure we see it.

Comment 4

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8902939 [details] [diff] [review]
Patch to be uplifted in the last beta of the 56 cycle

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

::: browser/app/profile/firefox.js
@@ -1707,5 @@
>  // Whether or not to restore a session with lazy-browser tabs.
>  pref("browser.sessionstore.restore_tabs_lazily", true);
>  
> -// Enable safebrowsing v4 tables (suffixed by "-proto") update.
> -pref("urlclassifier.malwareTable", "goog-malware-proto,goog-unwanted-proto,test-malware-simple,test-unwanted-simple");

Is it intentional that the test-malware-simple and test-unwanted-simple tables are gone?

@@ -1708,5 @@
>  pref("browser.sessionstore.restore_tabs_lazily", true);
>  
> -// Enable safebrowsing v4 tables (suffixed by "-proto") update.
> -pref("urlclassifier.malwareTable", "goog-malware-proto,goog-unwanted-proto,test-malware-simple,test-unwanted-simple");
> -pref("urlclassifier.phishTable", "goog-phish-proto,test-phish-simple");

Same question for test-phish-simple.

Comment 5

[Gian-Carlo Pascutto [:gcp]]

How confident are we that V2 still fully works on 56? I don't want to find out the answer to that question on release.

Comment 6

[François Marier [:francois]]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #4)
> Comment on attachment 8902939 [details] [diff] [review]
> Patch to be uplifted in the last beta of the 56 cycle
> 
> Review of attachment 8902939 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/app/profile/firefox.js
> @@ -1707,5 @@
> >  // Whether or not to restore a session with lazy-browser tabs.
> >  pref("browser.sessionstore.restore_tabs_lazily", true);
> >  
> > -// Enable safebrowsing v4 tables (suffixed by "-proto") update.
> > -pref("urlclassifier.malwareTable", "goog-malware-proto,goog-unwanted-proto,test-malware-simple,test-unwanted-simple");
> 
> Is it intentional that the test-malware-simple and test-unwanted-simple
> tables are gone?

They're not actually gone. The above is overriding the pref that's set in https://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2fe7d7b6e75ad6b6b5da223/modules/libpref/init/all.js#5349

So by removing the override, we're going back to the default (which is what Fennec is using).

(In reply to Gian-Carlo Pascutto [:gcp] from comment #5)
> How confident are we that V2 still fully works on 56? I don't want to find
> out the answer to that question on release.

We'll be running an experiment on all of beta to compare the crash rate of V2 and V4 (bug 1377267). In addition to the crashes, we'll also get lots of telemetry from this (most of which is now keyed to the provider).

Also, I'm running Nightly with a bunch of extra V2 lists (Yandex, Baidu and MozCN) and haven't noticed any problems so far. Not a huge surprise given how much common code the two versions share.

However, you raise a good point and I'll see if we can get Soft Vision to repeat their validation tests on V2 as well.

Comment 7

[François Marier [:francois]]

Attachment: Patch to be uplifted in the last beta of the 56 cycle

Rebased on latest beta. Carrying review from GCP.

Comment 8

[François Marier [:francois]]

(In reply to François Marier [:francois] from comment #6)
> However, you raise a good point and I'll see if we can get Soft Vision to
> repeat their validation tests on V2 as well.

Soft Vision is in the middle of running their pre-release tests on V2.

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

François, can you request uplift now (for beta 12 build tomorrow)

Thanks!

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

From discussion on IRC. It's fine to land this after we do the beta 12 build but it means we will need to uplift to m-r, since we are doing the m-c to m-r merge early on Thursday this week. Just to be clear, we will build the release candidate (RC) on Monday  from the mozilla-release branch and then push it to beta users.

Comment 11

[François Marier [:francois]]

Comment on attachment 8906197 [details] [diff] [review]
Patch to be uplifted in the last beta of the 56 cycle

Approval Request Comment
[Feature/Bug causing the regression]: Gradual roll-out of Safe Browsing V4 requires this (bug 1387651)
[User impact if declined]: Google has asked us to do a gradual rollout, they are not ready for all of our users at once. Safe Browsing could be broken for all of our users
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]: Yes, SV has manually verified both V2 and V4 in Nightly and Beta.
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: No
[Why is the change risky/not risky?]: Just a pref flip
[String changes made/needed]: None

Comment 12

[Gerry Chang [:gchang]]

Comment on attachment 8906197 [details] [diff] [review]
Patch to be uplifted in the last beta of the 56 cycle

This needs to be in m-r before 56.0-build1.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/b75624820200

Comment 14

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

We verified this issue using Fx 56.0-build1 on Widows 7 x64, Windows 10 x64, Ubuntu 16.04 LTS x86 and mac OS X 10.12.

Cheers!