Transfer DictionaryValue ownership in SandboxedUnpacker::OnUnpackSuccess()

Transfer DictionaryValue ownership in SandboxedUnpacker::OnUnpackSuccess()

This would avoid DeepCopy() in clients that care about retaining the
DictionaryValue's ownership.

BUG=699528
Test=None

Review-Url: https://codereview.chromium.org/2788563002
Cr-Commit-Position: refs/heads/master@{#460835}
Committed: https://chromium.googlesource.com/chromium/src/+/6cff6b18a5bc0741499ac4b1ba45ffe9c4ddccdd

[lazyboy, 2017-03-30 06:01:41]

The CQ bit was checked by lazyboy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 06:02:12]

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

[lazyboy, 2017-03-30 06:09:08]

The CQ bit was checked by lazyboy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 06:09:21]

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

[lazyboy, 2017-03-30 06:10:30]

lazyboy@chromium.org changed reviewers:
+ noel@chromium.org

[lazyboy, 2017-03-30 06:10:30]

Hi Noel, mind reviewing this CL?

[Noel Gordon, 2017-03-30 06:27:03]

On 2017/03/30 06:10:30, lazyboy wrote:
> Hi Noel, mind reviewing this CL?

Don't mind at all, and thanks for fixing.

[Noel Gordon, 2017-03-30 06:35:41]

All the base::DictionaryValue* original_manifest ->
std::unique_ptr<base::DictionaryValue> original_manifest changes look correct to
me, thanks for fixing.

LGTM - with a minor nit.

(I'm not an extensions reviewer btw, so you will need their approval to submit,
of course).

https://codereview.chromium.org/2788563002/diff/20001/extensions/browser/sand...
File extensions/browser/sandboxed_unpacker.cc (right):

https://codereview.chromium.org/2788563002/diff/20001/extensions/browser/sand...
extensions/browser/sandboxed_unpacker.cc:687: // Client takes ownership of
temporary directory and extension.
Maybe this comment needs updating too?  Perhaps:

// Client takes ownership of temporary directory, manifest, and extension.

or similar.

[commit-bot: I haz the power, 2017-03-30 06:50:24]

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

[commit-bot: I haz the power, 2017-03-30 06:50:25]

Dry run: This issue passed the CQ dry run.

[lazyboy, 2017-03-30 07:01:50]

lazyboy@chromium.org changed reviewers:
+ xiyuan@chromium.org

[lazyboy, 2017-03-30 07:01:51]

Thanks, will add other reviewer(s), but wanted to run by you quick.

+Xiyuan for chrome/browser/chromeos/*

https://codereview.chromium.org/2788563002/diff/20001/extensions/browser/sand...
File extensions/browser/sandboxed_unpacker.cc (right):

https://codereview.chromium.org/2788563002/diff/20001/extensions/browser/sand...
extensions/browser/sandboxed_unpacker.cc:687: // Client takes ownership of
temporary directory and extension.
On 2017/03/30 06:35:41, noel gordon wrote:
> Maybe this comment needs updating too?  Perhaps:
> 
> // Client takes ownership of temporary directory, manifest, and extension.
> 
> or similar.
> 

Done.

[xiyuan, 2017-03-30 16:04:12]

c/b/chromeos/* lgtm

[lazyboy, 2017-03-30 17:45:46]

The CQ bit was checked by lazyboy@chromium.org

[lazyboy, 2017-03-30 17:45:47]

The patchset sent to the CQ was uploaded after l-g-t-m from noel@chromium.org
Link to the patchset: https://codereview.chromium.org/2788563002/#ps40001
(title: "address comments")

[commit-bot: I haz the power, 2017-03-30 17:46:23]

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, 2017-03-30 18:43:55]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490895946611090,
"parent_rev": "d22af31b5bf873a9ad1d7fcd59210df08ea33a89", "commit_rev":
"6cff6b18a5bc0741499ac4b1ba45ffe9c4ddccdd"}

[commit-bot: I haz the power, 2017-03-30 18:44:46]

Description was changed from

==========
Transfer DictionaryValue ownership in SandboxedUnpacker::OnUnpackSuccess()

This would avoid DeepCopy() in clients that care about retaining the
DictionaryValue's ownership.

BUG=699528
Test=None
==========

to

==========
Transfer DictionaryValue ownership in SandboxedUnpacker::OnUnpackSuccess()

This would avoid DeepCopy() in clients that care about retaining the
DictionaryValue's ownership.

BUG=699528
Test=None

Review-Url: https://codereview.chromium.org/2788563002
Cr-Commit-Position: refs/heads/master@{#460835}
Committed:
https://chromium.googlesource.com/chromium/src/+/6cff6b18a5bc0741499ac4b1ba45...
==========

[commit-bot: I haz the power, 2017-03-30 18:44:47]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/6cff6b18a5bc0741499ac4b1ba45...