Factor getCertificateChain out of ConnectionPopupInfo

Factor getCertificateChain out of ConnectionPopupInfo

This patch refactors the GetCertificateChain logic out of
connection_info_popup_android.cc so we can share it between
ConnectionInfoPopup and PaymentRequestImpl.

BUG=587995
Committed: https://crrev.com/013458048b4ef7d09cc2fddded677d19c4e20c2d
Cr-Commit-Position: refs/heads/master@{#439237}

[please use gerrit instead, 2016-12-09 17:16:01]

This looks great. Good job!

https://codereview.chromium.org/2567673002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/pageinfo/ConnectionInfoPopup.java
(right):

https://codereview.chromium.org/2567673002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/pageinfo/ConnectionInfoPopup.java:231:
public static byte[][] getCertificateChain(WebContents webContents) {
It would be best to place this in a separate class that's not related to the
"connection info popup".

[rwlbuis, 2016-12-09 18:42:03]

Patchset #2 (id:20001) has been deleted

[rwlbuis, 2016-12-09 19:36:32]

Description was changed from

==========
[WIP] Send site certificate chain to android payment app

BUG=587995
==========

to

==========
[WIP] Obtain site certificate chain to send to android payment app

Obtain site certificate chain to send to android payment app as specced here:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE/

This patch refactor the GetCertificateChain logic out of
connection_info_popup_android.cc so we can share it between
ConnectionInfoPopup and PaymentRequestImpl.

BUG=587995
==========

[rwlbuis, 2016-12-09 20:21:54]

Description was changed from

==========
[WIP] Obtain site certificate chain to send to android payment app

Obtain site certificate chain to send to android payment app as specced here:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE/

This patch refactor the GetCertificateChain logic out of
connection_info_popup_android.cc so we can share it between
ConnectionInfoPopup and PaymentRequestImpl.

BUG=587995
==========

to

==========
[WIP] Obtain site certificate chain to send to android payment app

Obtain site certificate chain to send to android payment app as specced here:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE/

This patch refactors the GetCertificateChain logic out of
connection_info_popup_android.cc so we can share it between
ConnectionInfoPopup and PaymentRequestImpl.

BUG=587995
==========

[rwlbuis, 2016-12-09 20:25:35]

On 2016/12/09 17:16:01, rouslan wrote:
> This looks great. Good job!
> 
>
https://codereview.chromium.org/2567673002/diff/1/chrome/android/java/src/org...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/pageinfo/ConnectionInfoPopup.java
> (right):
> 
>
https://codereview.chromium.org/2567673002/diff/1/chrome/android/java/src/org...
>
chrome/android/java/src/org/chromium/chrome/browser/pageinfo/ConnectionInfoPopup.java:231:
> public static byte[][] getCertificateChain(WebContents webContents) {
> It would be best to place this in a separate class that's not related to the
> "connection info popup".

Done.

[rwlbuis, 2016-12-09 20:43:18]

Patchset #3 (id:60001) has been deleted

[rwlbuis, 2016-12-09 20:51:50]

Description was changed from

==========
[WIP] Obtain site certificate chain to send to android payment app

Obtain site certificate chain to send to android payment app as specced here:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE/

This patch refactors the GetCertificateChain logic out of
connection_info_popup_android.cc so we can share it between
ConnectionInfoPopup and PaymentRequestImpl.

BUG=587995
==========

to

==========
Factor getCertificateChain out of ConnectionPopupInfo

This patch refactors the GetCertificateChain logic out of
connection_info_popup_android.cc so we can share it between
ConnectionInfoPopup and PaymentRequestImpl.

BUG=587995
==========

[rwlbuis, 2016-12-09 21:41:37]

rob.buis@samsung.com changed reviewers:
+ avi@chromium.org, eseckler@chromium.org

[rwlbuis, 2016-12-09 21:41:38]

PTAL, this refactor will be useful for PaymentRequest.

[Avi (use Gerrit), 2016-12-09 23:59:10]

Please specify what code you want each reviewer to look at.

Why am I reviewer, though? I don't own any of this code.

https://codereview.chromium.org/2567673002/diff/80001/chrome/browser/ui/andro...
File chrome/browser/ui/android/page_info/certificate_chain_helper.cc (right):

https://codereview.chromium.org/2567673002/diff/80001/chrome/browser/ui/andro...
chrome/browser/ui/android/page_info/certificate_chain_helper.cc:46: ++it) {
for (const auto& handle : cert_handles) ?

At least auto.

[rwlbuis, 2016-12-16 13:22:04]

On 2016/12/09 23:59:10, Avi wrote:
> Please specify what code you want each reviewer to look at.
> 
> Why am I reviewer, though? I don't own any of this code.

I did a quick git log in page_info/ and your name came up :) But I'll try to
find a reviewer for most if not all this code.

[rwlbuis, 2016-12-16 13:22:42]

rob.buis@samsung.com changed reviewers:
+ tedchoc@chromium.org

[rwlbuis, 2016-12-16 13:22:42]

@tedchoc PTAL at all parts you feel comfortable reviewing.

[Avi (use Gerrit), 2016-12-16 15:54:16]

On 2016/12/16 13:22:04, rwlbuis wrote:
> On 2016/12/09 23:59:10, Avi wrote:
> > Please specify what code you want each reviewer to look at.
> > 
> > Why am I reviewer, though? I don't own any of this code.
> 
> I did a quick git log in page_info/ and your name came up :) But I'll try to
> find a reviewer for most if not all this code.

Even if my name comes up, if I'm not in the OWNERS file then my LG wouldn't
help.

I'm not an Android person, though, so it's unlikely I could help. Ted is a great
guy for Android and a good choice.

[Ted C, 2016-12-16 17:21:16]

lgtm % avi's suggestion

https://codereview.chromium.org/2567673002/diff/80001/chrome/browser/ui/andro...
File chrome/browser/ui/android/page_info/certificate_chain_helper.cc (right):

https://codereview.chromium.org/2567673002/diff/80001/chrome/browser/ui/andro...
chrome/browser/ui/android/page_info/certificate_chain_helper.cc:1: // Copyright
2015 The Chromium Authors. All rights reserved.
2016

[rwlbuis, 2016-12-16 20:06:34]

The CQ bit was checked by rob.buis@samsung.com

[rwlbuis, 2016-12-16 20:06:35]

The patchset sent to the CQ was uploaded after l-g-t-m from tedchoc@chromium.org
Link to the patchset: https://codereview.chromium.org/2567673002/#ps100001
(title: "Address review comments")

[commit-bot: I haz the power, 2016-12-16 20:07:18]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-16 23:59:33]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1481918794691470,
"parent_rev": "6118ac4ff121754873ab87eb2639bc06d58bcf88", "commit_rev":
"92466f91c7a7ae129e19b1e24ed85bb8855f5204"}

[commit-bot: I haz the power, 2016-12-17 00:00:04]

Description was changed from

==========
Factor getCertificateChain out of ConnectionPopupInfo

This patch refactors the GetCertificateChain logic out of
connection_info_popup_android.cc so we can share it between
ConnectionInfoPopup and PaymentRequestImpl.

BUG=587995
==========

to

==========
Factor getCertificateChain out of ConnectionPopupInfo

This patch refactors the GetCertificateChain logic out of
connection_info_popup_android.cc so we can share it between
ConnectionInfoPopup and PaymentRequestImpl.

BUG=587995

Review-Url: https://codereview.chromium.org/2567673002
==========

[commit-bot: I haz the power, 2016-12-17 00:00:05]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2016-12-17 00:03:00]

Description was changed from

==========
Factor getCertificateChain out of ConnectionPopupInfo

This patch refactors the GetCertificateChain logic out of
connection_info_popup_android.cc so we can share it between
ConnectionInfoPopup and PaymentRequestImpl.

BUG=587995

Review-Url: https://codereview.chromium.org/2567673002
==========

to

==========
Factor getCertificateChain out of ConnectionPopupInfo

This patch refactors the GetCertificateChain logic out of
connection_info_popup_android.cc so we can share it between
ConnectionInfoPopup and PaymentRequestImpl.

BUG=587995

Committed: https://crrev.com/013458048b4ef7d09cc2fddded677d19c4e20c2d
Cr-Commit-Position: refs/heads/master@{#439237}
==========

[commit-bot: I haz the power, 2016-12-17 00:03:01]

Patchset 4 (id:??) landed as
https://crrev.com/013458048b4ef7d09cc2fddded677d19c4e20c2d
Cr-Commit-Position: refs/heads/master@{#439237}