🌈 Upstream some code related to PolicyAuditing.

Upstream some code related to PolicyAuditing.

Provides basic mapping from security failure states to an enum for consumption
by auditors.
BUG=607207
TBR=thestig

Committed: https://crrev.com/0c14f2687aaf35d401f532ca65bed43bad603019
Cr-Commit-Position: refs/heads/master@{#390662}

[Yaron, 2016-04-28 15:09:15]

Description was changed from

==========
Upstream some code related to PolicyAuditing.

Provides basic mapping from security failure states to an @IntDef for
consumption
by auditors.
BUG=607207
==========

to

==========
Upstream some code related to PolicyAuditing.

Provides basic mapping from security failure states to an @IntDef for
consumption
by auditors.
BUG=607207
==========

[Yaron, 2016-04-28 15:13:59]

yfriedman@chromium.org changed reviewers:
+ bauerb@chromium.org

[Yaron, 2016-04-28 15:13:59]

mimics the AuditEvent approach that I see was done - lmk what you think

https://codereview.chromium.org/1930963002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/policy/PolicyAuditor.java
(right):

https://codereview.chromium.org/1930963002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/policy/PolicyAuditor.java:60:
public static native int nativeGetCertificateFailure(WebContents webContents);
not the prettiest API but it does the trick..

[Bernhard Bauer, 2016-04-28 16:33:08]

https://codereview.chromium.org/1930963002/diff/1/chrome/browser/android/poli...
File chrome/browser/android/policy/policy_auditor.cc (right):

https://codereview.chromium.org/1930963002/diff/1/chrome/browser/android/poli...
chrome/browser/android/policy/policy_auditor.cc:14: int
GetCertificateFailure(JNIEnv* env,
Just to check: the generated JNI binding declares this as static, right?

https://codereview.chromium.org/1930963002/diff/1/chrome/browser/android/poli...
chrome/browser/android/policy/policy_auditor.cc:20: enum CertificateFailure {
You could put this into a header and generate the Java enum from it with
java_cpp_enum?

https://codereview.chromium.org/1930963002/diff/1/chrome/browser/android/poli...
chrome/browser/android/policy/policy_auditor.cc:57: if (!!(ssl.content_status &
Is the cast to bool necessary if we're using this in an if-statement anyway?

https://codereview.chromium.org/1930963002/diff/1/chrome/browser/android/poli...
chrome/browser/android/policy/policy_auditor.cc:59: return
CERTIFICATE_FAIL_UNSPECIFIED;
I think if the condition is long enough to require multiple lines, it makes
sense to add braces to the body for readability.

https://codereview.chromium.org/1930963002/diff/1/chrome/browser/android/poli...
chrome/browser/android/policy/policy_auditor.cc:63: default:
What other values are there? Could we remove the default case (moving the
NOTREACHED() and return out of the switch), so that the compiler will catch it
if we should forget a value?

[Yaron, 2016-04-28 19:30:49]

https://codereview.chromium.org/1930963002/diff/1/chrome/browser/android/poli...
File chrome/browser/android/policy/policy_auditor.cc (right):

https://codereview.chromium.org/1930963002/diff/1/chrome/browser/android/poli...
chrome/browser/android/policy/policy_auditor.cc:14: int
GetCertificateFailure(JNIEnv* env,
On 2016/04/28 16:33:08, Bernhard Bauer wrote:
> Just to check: the generated JNI binding declares this as static, right?

yep (cause it was static in java):

static jint GetCertificateFailure(JNIEnv* env, const JavaParamRef<jclass>&
    jcaller,
    const JavaParamRef<jobject>& webContents);

https://codereview.chromium.org/1930963002/diff/1/chrome/browser/android/poli...
chrome/browser/android/policy/policy_auditor.cc:20: enum CertificateFailure {
On 2016/04/28 16:33:08, Bernhard Bauer wrote:
> You could put this into a header and generate the Java enum from it with
> java_cpp_enum?

Done.

https://codereview.chromium.org/1930963002/diff/1/chrome/browser/android/poli...
chrome/browser/android/policy/policy_auditor.cc:57: if (!!(ssl.content_status &
On 2016/04/28 16:33:08, Bernhard Bauer wrote:
> Is the cast to bool necessary if we're using this in an if-statement anyway?

Just copied from internal tree

https://codereview.chromium.org/1930963002/diff/1/chrome/browser/android/poli...
chrome/browser/android/policy/policy_auditor.cc:59: return
CERTIFICATE_FAIL_UNSPECIFIED;
On 2016/04/28 16:33:08, Bernhard Bauer wrote:
> I think if the condition is long enough to require multiple lines, it makes
> sense to add braces to the body for readability.

Done.

https://codereview.chromium.org/1930963002/diff/1/chrome/browser/android/poli...
chrome/browser/android/policy/policy_auditor.cc:63: default:
On 2016/04/28 16:33:08, Bernhard Bauer wrote:
> What other values are there? Could we remove the default case (moving the
> NOTREACHED() and return out of the switch), so that the compiler will catch it
> if we should forget a value?

Done.

[Bernhard Bauer, 2016-04-29 08:25:02]

LGTM!

https://codereview.chromium.org/1930963002/diff/60001/chrome/browser/android/...
File chrome/browser/android/policy/policy_auditor.cc (right):

https://codereview.chromium.org/1930963002/diff/60001/chrome/browser/android/...
chrome/browser/android/policy/policy_auditor.cc:60: if (!!(ssl.content_status &
Would you mind removing the !!, just as a small cleanup? Thanks!

[Yaron, 2016-04-29 13:43:44]

Description was changed from

==========
Upstream some code related to PolicyAuditing.

Provides basic mapping from security failure states to an @IntDef for
consumption
by auditors.
BUG=607207
==========

to

==========
Upstream some code related to PolicyAuditing.

Provides basic mapping from security failure states to an @IntDef for
consumption
by auditors.
BUG=607207
TBR=thestig
==========

[Yaron, 2016-04-29 13:43:57]

tbr thestig for trivial chrome.gyp change

https://codereview.chromium.org/1930963002/diff/60001/chrome/browser/android/...
File chrome/browser/android/policy/policy_auditor.cc (right):

https://codereview.chromium.org/1930963002/diff/60001/chrome/browser/android/...
chrome/browser/android/policy/policy_auditor.cc:60: if (!!(ssl.content_status &
On 2016/04/29 08:25:02, Bernhard Bauer wrote:
> Would you mind removing the !!, just as a small cleanup? Thanks!

Done.

[Yaron, 2016-04-29 13:44:06]

The CQ bit was checked by yfriedman@chromium.org

[Yaron, 2016-04-29 13:44:06]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/1930963002/#ps80001
(title: " ")

[commit-bot: I haz the power, 2016-04-29 13:44:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1930963002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1930963002/80001

[commit-bot: I haz the power, 2016-04-29 13:47:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-29 13:47:24]

Try jobs failed on following builders:
  ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[Yaron, 2016-04-29 13:49:26]

Description was changed from

==========
Upstream some code related to PolicyAuditing.

Provides basic mapping from security failure states to an @IntDef for
consumption
by auditors.
BUG=607207
TBR=thestig
==========

to

==========
Upstream some code related to PolicyAuditing.

Provides basic mapping from security failure states to an enum for consumption
by auditors.
BUG=607207
TBR=thestig
==========

[Yaron, 2016-04-29 13:54:10]

The CQ bit was checked by yfriedman@chromium.org

[Yaron, 2016-04-29 13:54:10]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/1930963002/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2016-04-29 13:54:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1930963002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1930963002/100001

[commit-bot: I haz the power, 2016-04-29 16:07:10]

Description was changed from

==========
Upstream some code related to PolicyAuditing.

Provides basic mapping from security failure states to an enum for consumption
by auditors.
BUG=607207
TBR=thestig
==========

to

==========
Upstream some code related to PolicyAuditing.

Provides basic mapping from security failure states to an enum for consumption
by auditors.
BUG=607207
TBR=thestig
==========

[commit-bot: I haz the power, 2016-04-29 16:07:11]

Committed patchset #6 (id:100001)

[mdjones, 2016-04-29 16:55:57]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/1937473002/ by mdjones@chromium.org.

The reason for reverting is: Causes compile failure on
clang-clankium-tot-builder..

[commit-bot: I haz the power, 2016-04-30 17:26:11]

Patchset 6 (id:??) landed as
https://crrev.com/0c14f2687aaf35d401f532ca65bed43bad603019
Cr-Commit-Position: refs/heads/master@{#390662}

[commit-bot: I haz the power, 2016-04-30 18:00:51]

Description was changed from

==========
Upstream some code related to PolicyAuditing.

Provides basic mapping from security failure states to an enum for consumption
by auditors.
BUG=607207
TBR=thestig
==========

to

==========
Upstream some code related to PolicyAuditing.

Provides basic mapping from security failure states to an enum for consumption
by auditors.
BUG=607207
TBR=thestig

Committed: https://crrev.com/0c14f2687aaf35d401f532ca65bed43bad603019
Cr-Commit-Position: refs/heads/master@{#390662}
==========

[Lei Zhang, 2016-05-10 00:50:34]

thestig@chromium.org changed reviewers:
+ thestig@chromium.org

[Lei Zhang, 2016-05-10 00:50:34]

lgtm, but please remember to actually email me.

[Yaron, 2016-05-10 13:08:30]

On 2016/05/10 00:50:34, Lei Zhang wrote:
> lgtm, but please remember to actually email me.

[Yaron, 2016-05-10 13:08:43]

On 2016/05/10 13:08:30, Yaron wrote:
> On 2016/05/10 00:50:34, Lei Zhang wrote:
> > lgtm, but please remember to actually email me.

oops, sorry about that!