Bug 1249642 introduces remote channels for newtab. This bug introduces a check to allow testing environments to disable content-signatures on remote newtab.

checking browser.newtabpage.remote.mode pref to disable content signature checks.

Comment on attachment 8730122 [details] [diff] [review] cs_honour_channel_mode.patch Review of attachment 8730122 [details] [diff] [review]: ----------------------------------------------------------------- One question: how are we testing that content signature checking works? As test and test2 are for mochitest, how do we do the checks then? My r+ depends on answering that questin. ::: netwerk/protocol/http/nsHttpHandler.cpp @@ +89,5 @@ > #define TELEMETRY_ENABLED "toolkit.telemetry.enabled" > #define ALLOW_EXPERIMENTS "network.allow-experiments" > #define SAFE_HINT_HEADER_VALUE "safeHint.enabled" > #define SECURITY_PREFIX "security." > +#define NEWTAB_CHANNEL "browser.newtabpage.remote.mode" this definitely can't have a name "NEWTAB_CHANNEL" NEW_TAB_REMOTE_MODE maybe? @@ +1643,5 @@ > + // remote content-signature testing option > + if (PREF_CHANGED(NEWTAB_CHANNEL)) { > + nsAutoCString channel; > + prefs->GetCharPref(NEWTAB_CHANNEL, getter_Copies(channel)); > + if (channel.EqualsASCII("test") || EqualsLiteral please @@ +1645,5 @@ > + nsAutoCString channel; > + prefs->GetCharPref(NEWTAB_CHANNEL, getter_Copies(channel)); > + if (channel.EqualsASCII("test") || > + channel.EqualsASCII("test2") || > + channel.EqualsASCII("dev")) { I really like the names.. [sarcasm] | | ::: netwerk/protocol/http/nsHttpHandler.h @@ +336,5 @@ > SpdyInformation *SpdyInfo() { return &mSpdyInfo; } > bool IsH2MandatorySuiteEnabled() { return mH2MandatorySuiteEnabled; } > > + // returns true if content-signature test pref is set > + bool NewTabCSDisabled() { return mNewTabCSDisabled; } please don't use cryptic abbreviations. what "CS" stands for? hard to figure out when this is used in the code

Thanks for your quick review. I'll fix the things you mentioned tomorrow. (In reply to Honza Bambas (:mayhemer) from comment #2) > Comment on attachment 8730122 [details] [diff] [review] > cs_honour_channel_mode.patch > > Review of attachment 8730122 [details] [diff] [review]: > ----------------------------------------------------------------- > > One question: how are we testing that content signature checking works? As > test and test2 are for mochitest, how do we do the checks then? > > My r+ depends on answering that questin. The tests for content-signatures predate those channel mode definitions and thus forced remote newtab and thus content-signature verification in a different way. We'll update the tests later. Another channel mode gets added for that (probably test3...), which enforces content-signatures. But I wanted to get this one landed to avoid breaking other tests that won't work if content-signatures are always enforced.