Change NetLog::ParametersCallback to return a scoped_ptr.

Change NetLog::ParametersCallback to return a scoped_ptr.

BUG=479898, 485606
TBR=benjhayden@chromium.org,sclittle@chromium.org

Committed: https://crrev.com/5e5529d1a3a3fd5b987d2ea5df89b88f4ca4948a
Cr-Commit-Position: refs/heads/master@{#330982}

[Evan Stade, 2015-05-21 03:08:51]

estade@chromium.org changed reviewers:
+ eroman@chromium.org

[Evan Stade, 2015-05-21 03:08:53]

https://codereview.chromium.org/1149763005/diff/1/net/dns/dns_config_service.cc
File net/dns/dns_config_service.cc (right):

https://codereview.chromium.org/1149763005/diff/1/net/dns/dns_config_service....
net/dns/dns_config_service.cc:142: scoped_ptr<base::DictionaryValue> dict(new
base::DictionaryValue());
changes like this one are collateral damage of using sed. Ideally,
Foo::ToValue() will one day return a scoped_ptr, but that's outside the scope of
this CL. In the meantime, making this change is not harmful, and even helps a
little because "dict.release();" is a marker for bad code.

[eroman, 2015-05-21 03:19:39]

Looks like this is also BUG=485606

[eroman, 2015-05-21 03:37:43]

LGTM. Awesome, thanks for doing this!

Was this an automated refactor, or did you have to do it manually?

https://codereview.chromium.org/1149763005/diff/20001/components/data_reducti...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_event_creator.cc
(right):

https://codereview.chromium.org/1149763005/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_event_creator.cc:39:
// the Data Reduction Proxy. Ownership of the base::Value is passed to the
nit: delete the ownership comment, which is now redundant

https://codereview.chromium.org/1149763005/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_event_creator.cc:64:
// the Data Reduction Proxy. Ownership of the base::Value is passed to the
Same here

https://codereview.chromium.org/1149763005/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_event_creator.cc:74:
// the Data Reduction Proxy. Ownership of the base::Value is passed to the
Ditto

https://codereview.chromium.org/1149763005/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_event_creator.cc:96:
// the Data Reduction Proxy. Ownership of the base::Value is passed to the
Same

https://codereview.chromium.org/1149763005/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_event_creator.cc:118:
// completing the Data Reduction Proxy secure proxy check. Ownership of the
And here

https://codereview.chromium.org/1149763005/diff/20001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_event_creator.cc:133:
// completing the Data Reduction Proxy configuration request. Ownership of the
And here

https://codereview.chromium.org/1149763005/diff/20001/extensions/browser/api/...
File extensions/browser/api/web_request/web_request_api_helpers.cc (right):

https://codereview.chromium.org/1149763005/diff/20001/extensions/browser/api/...
extensions/browser/api/web_request/web_request_api_helpers.cc:220: // Caller
takes ownership of returned value.
Delete this

https://codereview.chromium.org/1149763005/diff/20001/net/cert/multi_threaded...
File net/cert/multi_threaded_cert_verifier.cc (right):

https://codereview.chromium.org/1149763005/diff/20001/net/cert/multi_threaded...
net/cert/multi_threaded_cert_verifier.cc:109: results->Set("public_key_hashes",
hashes.Pass());
oh is this new?

https://codereview.chromium.org/1149763005/diff/20001/net/quic/quic_http_util...
File net/quic/quic_http_utils.cc (right):

https://codereview.chromium.org/1149763005/diff/20001/net/quic/quic_http_util...
net/quic/quic_http_utils.cc:29: SpdyHeaderBlockNetLogCallback(headers,
capture_mode).release()));
This is weird. Is it needed because of the static_cast ?

https://codereview.chromium.org/1149763005/diff/20001/net/quic/quic_http_utils.h
File net/quic/quic_http_utils.h (right):

https://codereview.chromium.org/1149763005/diff/20001/net/quic/quic_http_util...
net/quic/quic_http_utils.h:22: // Converts a SpdyHeaderBlock and priority into
NetLog event parameters.  Caller
Delete the comment on ownership

https://codereview.chromium.org/1149763005/diff/20001/net/spdy/spdy_header_bl...
File net/spdy/spdy_header_block.h (right):

https://codereview.chromium.org/1149763005/diff/20001/net/spdy/spdy_header_bl...
net/spdy/spdy_header_block.h:20: // Converts a SpdyHeaderBlock into NetLog event
parameters.  Caller takes
Delete the comment on ownership

[Evan Stade, 2015-05-21 15:30:33]

On 2015/05/21 03:37:43, eroman (sick slow) wrote:
> LGTM. Awesome, thanks for doing this!
> 
> Was this an automated refactor, or did you have to do it manually?

combination of sed and manual. Luckily the format of the code is generally
formulaic (down to variable names and so forth), which helped.

I'll update the ownership comments before committing.

[Evan Stade, 2015-05-21 15:30:42]

https://codereview.chromium.org/1149763005/diff/20001/net/cert/multi_threaded...
File net/cert/multi_threaded_cert_verifier.cc (right):

https://codereview.chromium.org/1149763005/diff/20001/net/cert/multi_threaded...
net/cert/multi_threaded_cert_verifier.cc:109: results->Set("public_key_hashes",
hashes.Pass());
On 2015/05/21 03:37:42, eroman (sick slow) wrote:
> oh is this new?

it didn't always exist but I don't think it's particularly new at this point

https://codereview.chromium.org/1149763005/diff/20001/net/quic/quic_http_util...
File net/quic/quic_http_utils.cc (right):

https://codereview.chromium.org/1149763005/diff/20001/net/quic/quic_http_util...
net/quic/quic_http_utils.cc:29: SpdyHeaderBlockNetLogCallback(headers,
capture_mode).release()));
On 2015/05/21 03:37:42, eroman (sick slow) wrote:
> This is weird. Is it needed because of the static_cast ?

yea, because of the cast. I don't know of a better way to do it. There's a
GetAsDictionary function but that doesn't play well with scoped_ptr either.

[Evan Stade, 2015-05-21 18:12:15]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2015-05-21 18:12:16]

The patchset sent to the CQ was uploaded after l-g-t-m from eroman@chromium.org
Link to the patchset: https://codereview.chromium.org/1149763005/#ps40001
(title: "comments on ownership removed")

[commit-bot: I haz the power, 2015-05-21 18:13:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1149763005/40001

[commit-bot: I haz the power, 2015-05-21 18:25:05]

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

[commit-bot: I haz the power, 2015-05-21 18:25:06]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Evan Stade, 2015-05-21 18:44:29]

estade@chromium.org changed reviewers:
+ benjhayden@chromium.org, sclittle@chromium.org

[Evan Stade, 2015-05-21 18:44:30]

because this is a mechanical change, I will TBR:
+benjhayden for content/browser/download
+sclittle for components/data_reduction_proxy

[Evan Stade, 2015-05-21 18:44:52]

The CQ bit was checked by estade@chromium.org

[sclittle, 2015-05-21 18:46:29]

components/data_reduction_proxy LGTM

[commit-bot: I haz the power, 2015-05-21 18:46:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1149763005/40001

[commit-bot: I haz the power, 2015-05-21 20:59:22]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-05-21 21:00:16]

Patchset 3 (id:??) landed as
https://crrev.com/5e5529d1a3a3fd5b987d2ea5df89b88f4ca4948a
Cr-Commit-Position: refs/heads/master@{#330982}