Added prefs for content protection attestation.

Added prefs for content protection attestation.

The prefs are integrated into the PlatformVerificationFlow class.

BUG=chromium:270316
TEST=unit

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221614

[Mattias Nissler (ping if slow), 2013-09-03 14:31:25]

https://chromiumcodereview-hr.appspot.com/23765004/diff/1/chrome/browser/chro...
File chrome/browser/chromeos/attestation/platform_verification_flow.cc (right):

https://chromiumcodereview-hr.appspot.com/23765004/diff/1/chrome/browser/chro...
chrome/browser/chromeos/attestation/platform_verification_flow.cc:9: #include
"base/strings/utf_string_conversions.h"
required?

https://chromiumcodereview-hr.appspot.com/23765004/diff/1/chrome/browser/chro...
chrome/browser/chromeos/attestation/platform_verification_flow.cc:13: #include
"chrome/browser/infobars/confirm_infobar_delegate.h"
required?

https://chromiumcodereview-hr.appspot.com/23765004/diff/1/chrome/browser/chro...
chrome/browser/chromeos/attestation/platform_verification_flow.cc:259: return
false;
Shouldn't we default to true here?

https://chromiumcodereview-hr.appspot.com/23765004/diff/1/chrome/browser/chro...
chrome/browser/chromeos/attestation/platform_verification_flow.cc:296: bool
PlatformVerificationFlow::FindDomainPref(
This should probably be GetDomainPref

https://chromiumcodereview-hr.appspot.com/23765004/diff/1/chrome/browser/chro...
File chrome/browser/chromeos/attestation/platform_verification_flow.h (right):

https://chromiumcodereview-hr.appspot.com/23765004/diff/1/chrome/browser/chro...
chrome/browser/chromeos/attestation/platform_verification_flow.h:132: void
set_testing_prefs(PrefService* testing_prefs) {
Can we replace these two by just using TestingWebContents and friends?

https://chromiumcodereview-hr.appspot.com/23765004/diff/1/chrome/browser/pref...
File chrome/browser/prefs/browser_prefs.cc (right):

https://chromiumcodereview-hr.appspot.com/23765004/diff/1/chrome/browser/pref...
chrome/browser/prefs/browser_prefs.cc:396: registry);
nit: can you alphabetize this block?

https://chromiumcodereview-hr.appspot.com/23765004/diff/1/chrome/browser/rend...
File chrome/browser/renderer_host/pepper/device_id_fetcher.cc (left):

https://chromiumcodereview-hr.appspot.com/23765004/diff/1/chrome/browser/rend...
chrome/browser/renderer_host/pepper/device_id_fetcher.cc:69: // change this pref
from UNSYNCABLE to SYNCABLE.
Just confirming: Is it intentional to remove this TODO without changing sync
behavior?

https://chromiumcodereview-hr.appspot.com/23765004/diff/1/chrome/common/pref_...
File chrome/common/pref_names.cc (right):

https://chromiumcodereview-hr.appspot.com/23765004/diff/1/chrome/common/pref_...
chrome/common/pref_names.cc:2574: const char kRAConsentFirstTime[] =
"settings.privacy.ra_consent";
These are chromeos-only, so they should be in an OS_CHROMEOS block. Also, these
are profile prefs, so they should be in the section labeled PROFILE PREFS
earlier in this file (this is SERVICE PREFS), which has an OS_CHROMEOS block
already.

[Darren Krahn, 2013-09-04 12:35:05]

https://chromiumcodereview.appspot.com/23765004/diff/1/chrome/browser/chromeo...
File chrome/browser/chromeos/attestation/platform_verification_flow.cc (right):

https://chromiumcodereview.appspot.com/23765004/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/attestation/platform_verification_flow.cc:9: #include
"base/strings/utf_string_conversions.h"
On 2013/09/03 14:31:25, Mattias Nissler wrote:
> required?

Done.

https://chromiumcodereview.appspot.com/23765004/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/attestation/platform_verification_flow.cc:13: #include
"chrome/browser/infobars/confirm_infobar_delegate.h"
On 2013/09/03 14:31:25, Mattias Nissler wrote:
> required?

Done.

https://chromiumcodereview.appspot.com/23765004/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/attestation/platform_verification_flow.cc:259: return
false;
On 2013/09/03 14:31:25, Mattias Nissler wrote:
> Shouldn't we default to true here?

Done.

https://chromiumcodereview.appspot.com/23765004/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/attestation/platform_verification_flow.cc:296: bool
PlatformVerificationFlow::FindDomainPref(
On 2013/09/03 14:31:25, Mattias Nissler wrote:
> This should probably be GetDomainPref

Done.

https://chromiumcodereview.appspot.com/23765004/diff/1/chrome/browser/chromeo...
File chrome/browser/chromeos/attestation/platform_verification_flow.h (right):

https://chromiumcodereview.appspot.com/23765004/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/attestation/platform_verification_flow.h:132: void
set_testing_prefs(PrefService* testing_prefs) {
On 2013/09/03 14:31:25, Mattias Nissler wrote:
> Can we replace these two by just using TestingWebContents and friends?

Ugh... really?  This is likely possible but I think it greatly simplifies the
tests with the WebContents dependency removed.  I haven't used WebContentsTester
before and maybe it's not as bad as it looks at first glance but it looks too
heavy for just tracking a URL and pref map.

https://chromiumcodereview.appspot.com/23765004/diff/1/chrome/browser/prefs/b...
File chrome/browser/prefs/browser_prefs.cc (right):

https://chromiumcodereview.appspot.com/23765004/diff/1/chrome/browser/prefs/b...
chrome/browser/prefs/browser_prefs.cc:396: registry);
On 2013/09/03 14:31:25, Mattias Nissler wrote:
> nit: can you alphabetize this block?

Done.

https://chromiumcodereview.appspot.com/23765004/diff/1/chrome/browser/rendere...
File chrome/browser/renderer_host/pepper/device_id_fetcher.cc (left):

https://chromiumcodereview.appspot.com/23765004/diff/1/chrome/browser/rendere...
chrome/browser/renderer_host/pepper/device_id_fetcher.cc:69: // change this pref
from UNSYNCABLE to SYNCABLE.
On 2013/09/03 14:31:25, Mattias Nissler wrote:
> Just confirming: Is it intentional to remove this TODO without changing sync
> behavior?

Yes, I confirmed with wad@ that we do not want these prefs to be syncable.

https://chromiumcodereview.appspot.com/23765004/diff/1/chrome/common/pref_nam...
File chrome/common/pref_names.cc (right):

https://chromiumcodereview.appspot.com/23765004/diff/1/chrome/common/pref_nam...
chrome/common/pref_names.cc:2574: const char kRAConsentFirstTime[] =
"settings.privacy.ra_consent";
On 2013/09/03 14:31:25, Mattias Nissler wrote:
> These are chromeos-only, so they should be in an OS_CHROMEOS block. Also,
these
> are profile prefs, so they should be in the section labeled PROFILE PREFS
> earlier in this file (this is SERVICE PREFS), which has an OS_CHROMEOS block
> already.

Done.

[Mattias Nissler (ping if slow), 2013-09-04 16:01:44]

I'm still feeling uneasy about the just-for-test member fields, but LGTM given
that I don't a convincing proposal to improve this (but see my comment).

https://chromiumcodereview.appspot.com/23765004/diff/1/chrome/browser/chromeo...
File chrome/browser/chromeos/attestation/platform_verification_flow.h (right):

https://chromiumcodereview.appspot.com/23765004/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/attestation/platform_verification_flow.h:132: void
set_testing_prefs(PrefService* testing_prefs) {
On 2013/09/04 12:35:05, Darren Krahn wrote:
> On 2013/09/03 14:31:25, Mattias Nissler wrote:
> > Can we replace these two by just using TestingWebContents and friends?
> 
> Ugh... really?  This is likely possible but I think it greatly simplifies the
> tests with the WebContents dependency removed.  I haven't used
WebContentsTester
> before and maybe it's not as bad as it looks at first glance but it looks too
> heavy for just tracking a URL and pref map.

I guess my main motivation here is to avoid littering the production code with
just-for-test fields. Maybe a TestWebContents is to big of a hammer for that.
Looking at existing code, we have quite a few users of
ChromeRenderViewHostTestHarness, which actually makes creating TestWebContents
quite simple.

Another idea would be to avoid relying on the WebContents in this class
entirely, and pass in the few things you need (i.e. PrefService and URL).

[Darren Krahn, 2013-09-05 11:20:19]

On 2013/09/04 16:01:44, Mattias Nissler wrote:
> I'm still feeling uneasy about the just-for-test member fields, but LGTM given
> that I don't a convincing proposal to improve this (but see my comment).
> 
>
https://chromiumcodereview.appspot.com/23765004/diff/1/chrome/browser/chromeo...
> File chrome/browser/chromeos/attestation/platform_verification_flow.h (right):
> 
>
https://chromiumcodereview.appspot.com/23765004/diff/1/chrome/browser/chromeo...
> chrome/browser/chromeos/attestation/platform_verification_flow.h:132: void
> set_testing_prefs(PrefService* testing_prefs) {
> On 2013/09/04 12:35:05, Darren Krahn wrote:
> > On 2013/09/03 14:31:25, Mattias Nissler wrote:
> > > Can we replace these two by just using TestingWebContents and friends?
> > 
> > Ugh... really?  This is likely possible but I think it greatly simplifies
the
> > tests with the WebContents dependency removed.  I haven't used
> WebContentsTester
> > before and maybe it's not as bad as it looks at first glance but it looks
too
> > heavy for just tracking a URL and pref map.
> 
> I guess my main motivation here is to avoid littering the production code with
> just-for-test fields. Maybe a TestWebContents is to big of a hammer for that.
> Looking at existing code, we have quite a few users of
> ChromeRenderViewHostTestHarness, which actually makes creating TestWebContents
> quite simple.
> 
> Another idea would be to avoid relying on the WebContents in this class
> entirely, and pass in the few things you need (i.e. PrefService and URL).

WebContents needs to stay because it gets passed on to ShowConsentPrompt.

[Darren Krahn, 2013-09-05 11:26:32]

Added OWNERS for the spots where kEnableDRM-related code has been cleaned up.

dmichael: pepper/device_id_fetcher.cc
zelidrag: chromeos/preferences.h

[dmichael (off chromium), 2013-09-05 16:40:10]

pepper lgtm

[zel, 2013-09-05 18:22:51]

LGTM

for chromeos/preferences.h

[commit-bot: I haz the power, 2013-09-05 20:39:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dkrahn@chromium.org/23765004/12001

[commit-bot: I haz the power, 2013-09-06 06:52:00]

Change committed as 221614