Net-related fixups for scoped_refptr conversion operator cleanup.

Net-related fixups for scoped_refptr conversion operator cleanup.

BUG=110610
TBR=pkasting@chromium.org

Committed: https://crrev.com/39030edb9763a85a699c52539a9fe4181d03bd57
Cr-Commit-Position: refs/heads/master@{#292600}

[dcheng, 2014-08-28 09:22:07]

dcheng@chromium.org changed reviewers:
+ mmenke@chromium.org

[dcheng, 2014-08-28 09:22:07]

Manual fixups for cases the tool skips.

I noticed that there were a lot of instances of refcounted objects being passed
as raw pointers that are immediately saved into a scoped_refptr; I've converted
those functions to take a const scoped_refptr<T>& instead to signal that the
callee will take a ref to them.

[mmenke, 2014-08-28 13:47:33]

mmenke@chromium.org changed reviewers:
+ rsleevi@chromium.org

[mmenke, 2014-08-28 13:47:33]

[+rsleevi]

Other than the changes to ssl_client_auth_requestor_mock.h, this LGTM.  Changes
to that file may have some effect on object lifetimes in some tests, and I'm not
familiar with the class, or the tests that use it.

Ryan:  Mind taking a look at that file?

https://codereview.chromium.org/511163002/diff/1/chrome/browser/ui/views/ssl_...
File chrome/browser/ui/views/ssl_client_certificate_selector.cc (right):

https://codereview.chromium.org/511163002/diff/1/chrome/browser/ui/views/ssl_...
chrome/browser/ui/views/ssl_client_certificate_selector.cc:89: model_(new
CertificateSelectorTableModel(cert_request_info.get())),
We aren't holding onto a pointer to cert_request_info either in
SSLClientCertificateSelector or CertificateSelectorTableModel...  If it's worth
modifying SSLClientCertificateSelector's constructor in this case, we should
modify CertificateSelectorTableModel's constructor, too, no?

[dcheng, 2014-08-28 16:23:15]

https://codereview.chromium.org/511163002/diff/1/chrome/browser/ui/views/ssl_...
File chrome/browser/ui/views/ssl_client_certificate_selector.cc (right):

https://codereview.chromium.org/511163002/diff/1/chrome/browser/ui/views/ssl_...
chrome/browser/ui/views/ssl_client_certificate_selector.cc:89: model_(new
CertificateSelectorTableModel(cert_request_info.get())),
On 2014/08/28 at 13:47:32, mmenke wrote:
> We aren't holding onto a pointer to cert_request_info either in
SSLClientCertificateSelector or CertificateSelectorTableModel...  If it's worth
modifying SSLClientCertificateSelector's constructor in this case, we should
modify CertificateSelectorTableModel's constructor, too, no?

This class doesn't, but its base class (SSLClientAuthObserver) refs it.
CertificateSelectorTableModel doesn't ref it at all, as far as I can tell.

[Ryan Sleevi, 2014-08-29 09:04:04]

lgtm

[dcheng, 2014-08-29 09:04:25]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2014-08-29 09:04:49]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dcheng@chromium.org/511163002/1

[Ryan Sleevi, 2014-08-29 09:07:29]

https://codereview.chromium.org/511163002/diff/1/net/http/transport_security_...
File net/http/transport_security_persister.cc (right):

https://codereview.chromium.org/511163002/diff/1/net/http/transport_security_...
net/http/transport_security_persister.cc:100: const
scoped_refptr<base::SequencedTaskRunner>& background_runner,
Boo for the inconsistency in parameter names from the .h

bonus points to fix the two to be in sync. Either direction (file or background)
works for me, just don't forget any comments ;)

[dcheng, 2014-08-29 09:10:07]

The CQ bit was unchecked by dcheng@chromium.org

[dcheng, 2014-08-29 09:14:23]

dcheng@chromium.org changed reviewers:
+ pkasting@chromium.org

[dcheng, 2014-08-29 09:15:29]

https://codereview.chromium.org/511163002/diff/1/net/http/transport_security_...
File net/http/transport_security_persister.cc (right):

https://codereview.chromium.org/511163002/diff/1/net/http/transport_security_...
net/http/transport_security_persister.cc:100: const
scoped_refptr<base::SequencedTaskRunner>& background_runner,
On 2014/08/29 09:07:29, Ryan Sleevi wrote:
> Boo for the inconsistency in parameter names from the .h
> 
> bonus points to fix the two to be in sync. Either direction (file or
background)
> works for me, just don't forget any comments ;)

Done. Fixed the header file, since I think it's nice if the argument name and
field name match, and fixing the header was less lines changed. =)

[dcheng, 2014-08-29 09:15:36]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2014-08-29 09:16:04]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dcheng@chromium.org/511163002/20001

[commit-bot: I haz the power, 2014-08-29 10:14:13]

Committed patchset #2 (id:20001) as aa3dd9e5a2d3215f9c756ddcbb11268569cbe473

[mlamouri (slow - plz ping), 2014-08-29 11:20:39]

A revert of this CL (patchset #2) has been created in
https://codereview.chromium.org/517873003/ by mlamouri@chromium.org.

The reason for reverting is: It seems to be breaking GPU Mac Builder (Blink):
http://build.chromium.org/p/chromium.webkit/builders/GPU%20Mac%20Builder/buil....

[commit-bot: I haz the power, 2014-09-10 03:07:30]

Patchset 2 (id:??) landed as
https://crrev.com/39030edb9763a85a699c52539a9fe4181d03bd57
Cr-Commit-Position: refs/heads/master@{#292600}