replace deprecated version of SetWithoutPathExpansion

replace deprecated version of SetWithoutPathExpansion

BUG=650082
Committed: https://crrev.com/ac5ff4c30081cf17e1c8b19bccf9615f7840343c
Cr-Commit-Position: refs/heads/master@{#421877}

[dhna, 2016-09-25 08:57:48]

corona10@gmail.com changed reviewers:
+ jinho.bang@samsung.com

[dhna, 2016-09-25 12:02:25]

Patchset #1 (id:1) has been deleted

[dhna, 2016-09-25 13:11:46]

Patchset #5 (id:100001) has been deleted

[dhna, 2016-09-25 13:12:02]

Patchset #4 (id:80001) has been deleted

[dhna, 2016-09-25 13:12:16]

Patchset #3 (id:60001) has been deleted

[dhna, 2016-09-25 13:12:46]

Patchset #2 (id:40001) has been deleted

[dhna, 2016-09-25 13:12:53]

Patchset #1 (id:20001) has been deleted

[dhna, 2016-09-25 13:17:03]

The CQ bit was checked by corona10@gmail.com

[dhna, 2016-09-25 13:17:04]

The CQ bit was unchecked by corona10@gmail.com

[zino, 2016-09-25 13:30:51]

The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-25 13:30:57]

Dry run: 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-09-25 13:38:56]

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

[commit-bot: I haz the power, 2016-09-25 13:38:57]

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

[zino, 2016-09-25 16:15:02]

The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-25 16:15:07]

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

[dhna, 2016-09-25 16:22:06]

Description was changed from

==========
[WIP]replace deprecated version of SetWithoutPathExpansion
==========

to

==========
replace deprecated version of SetWithoutPathExpansion
BUG=650082
==========

[dhna, 2016-09-25 17:14:00]

Description was changed from

==========
replace deprecated version of SetWithoutPathExpansion
BUG=650082
==========

to

==========
replace deprecated version of SetWithoutPathExpansion

BUG=650082
==========

[zino, 2016-09-25 17:20:11]

The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-25 17:20:21]

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

[dhna, 2016-09-25 17:57:38]

corona10@gmail.com changed reviewers:
+ lazyboy@chromium.org

[commit-bot: I haz the power, 2016-09-25 18:33:23]

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

[commit-bot: I haz the power, 2016-09-25 18:33:24]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[dhna, 2016-09-25 23:38:57]

corona10@gmail.com changed reviewers:
- lazyboy@chromium.org

[dhna, 2016-09-26 00:49:47]

corona10@gmail.com changed reviewers:
+ lazyboy@chromium.org

[dhna, 2016-09-26 00:50:41]

lazyboy@ PTAL

[dhna, 2016-09-27 01:01:11]

corona10@gmail.com changed reviewers:
+ estade@chromium.org, rdevlin.cronin@chromium.org
- lazyboy@chromium.org

[dhna, 2016-09-27 01:04:42]

corona10@gmail.com changed reviewers:
+ asargent@chromium.org
- rdevlin.cronin@chromium.org

[dhna, 2016-09-27 01:04:42]

eastade@
asargent@ 
PTAL

[dhna, 2016-09-27 04:35:28]

corona10@gmail.com changed reviewers:
+ finnur@chromium.org
- asargent@chromium.org, estade@chromium.org

[Finnur, 2016-09-27 09:25:35]

finnur@chromium.org changed reviewers:
+ asargent@chromium.org
- finnur@chromium.org

[dhna, 2016-09-27 18:51:28]

corona10@gmail.com changed reviewers:
+ finnur@chromium.org
- asargent@chromium.org

[dhna, 2016-09-27 18:51:29]

finnur@ PTAL

[asargent_no_longer_on_chrome, 2016-09-27 23:15:47]

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

[asargent_no_longer_on_chrome, 2016-09-27 23:15:48]

Seems fine overall with one small change requested. 

Also, please don't keep switching reviewers around every N hours. Pick one and
if you don't get a response in ~1 business day, feel free to ping reviewer via
individual email.

https://codereview.chromium.org/2370633002/diff/210001/extensions/browser/app...
File extensions/browser/app_window/app_window_geometry_cache_unittest.cc
(right):

https://codereview.chromium.org/2370633002/diff/210001/extensions/browser/app...
extensions/browser/app_window/app_window_geometry_cache_unittest.cc:124:
std::unique_ptr<base::DictionaryValue> value(new base::DictionaryValue);
let's use base::MakeUnique for lines 123 and 124 here

[dhna, 2016-09-27 23:25:12]

On 2016/09/27 23:15:48, Antony Sargent wrote:
> Seems fine overall with one small change requested. 
> 
> Also, please don't keep switching reviewers around every N hours. Pick one and
> if you don't get a response in ~1 business day, feel free to ping reviewer via
> individual email.
> 
>
https://codereview.chromium.org/2370633002/diff/210001/extensions/browser/app...
> File extensions/browser/app_window/app_window_geometry_cache_unittest.cc
> (right):
> 
>
https://codereview.chromium.org/2370633002/diff/210001/extensions/browser/app...
> extensions/browser/app_window/app_window_geometry_cache_unittest.cc:124:
> std::unique_ptr<base::DictionaryValue> value(new base::DictionaryValue);
> let's use base::MakeUnique for lines 123 and 124 here

Oh thanks. I will fix it.
and thanks for the advice.
 I will not change reviewer frequently. :-)

[simonhong, 2016-09-28 00:24:22]

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

[commit-bot: I haz the power, 2016-09-28 00:24:41]

Dry run: 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-09-28 01:09:10]

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

[commit-bot: I haz the power, 2016-09-28 01:09:11]

Dry run: This issue passed the CQ dry run.

[dhna, 2016-09-28 01:10:38]

On 2016/09/27 23:25:12, dhna wrote:
> On 2016/09/27 23:15:48, Antony Sargent wrote:
> > Seems fine overall with one small change requested. 
> > 
> > Also, please don't keep switching reviewers around every N hours. Pick one
and
> > if you don't get a response in ~1 business day, feel free to ping reviewer
via
> > individual email.
> > 
> >
>
https://codereview.chromium.org/2370633002/diff/210001/extensions/browser/app...
> > File extensions/browser/app_window/app_window_geometry_cache_unittest.cc
> > (right):
> > 
> >
>
https://codereview.chromium.org/2370633002/diff/210001/extensions/browser/app...
> > extensions/browser/app_window/app_window_geometry_cache_unittest.cc:124:
> > std::unique_ptr<base::DictionaryValue> value(new base::DictionaryValue);
> > let's use base::MakeUnique for lines 123 and 124 here
> 
> Oh thanks. I will fix it.
> and thanks for the advice.
>  I will not change reviewer frequently. :-)

Antony Sargent@ Ready to get review

[Finnur, 2016-09-28 10:03:19]

This change is a bit too mechanical to need multiple reviewers and I would not
be adding much to what Antony has to say. Nothing in this CL am I more of a
specialist in than Antony (or the only owner), so I trust him fully to review
it. 

In the future, I'd advice you to list what you want each reviewer to do. Someone
should be the primary reviewer and you also need owners approval. Often that
comes from the same person, but if you get someone who's not an owner of the
code to review then (after getting the green light) add someone from the OWNERS
list and ask for an "OWNERS check", which in that case would be more or less
just a "check that the reviewer chosen was appropriate, make sure the green
light has been given by that person and perhaps do an additional review of the
CL, if needed".

[Finnur, 2016-09-28 10:03:38]

finnur@chromium.org changed reviewers:
- finnur@chromium.org

[dhna, 2016-09-28 10:32:29]

On 2016/09/28 10:03:19, Finnur wrote:
> This change is a bit too mechanical to need multiple reviewers and I would not
> be adding much to what Antony has to say. Nothing in this CL am I more of a
> specialist in than Antony (or the only owner), so I trust him fully to review
> it. 
> 
> In the future, I'd advice you to list what you want each reviewer to do.
Someone
> should be the primary reviewer and you also need owners approval. Often that
> comes from the same person, but if you get someone who's not an owner of the
> code to review then (after getting the green light) add someone from the
OWNERS
> list and ask for an "OWNERS check", which in that case would be more or less
> just a "check that the reviewer chosen was appropriate, make sure the green
> light has been given by that person and perhaps do an additional review of the
> CL, if needed".

Finnur@ Thank you for your advice :-)

[dhna, 2016-09-29 06:41:13]

On 2016/09/28 10:32:29, dhna wrote:
> On 2016/09/28 10:03:19, Finnur wrote:
> > This change is a bit too mechanical to need multiple reviewers and I would
not
> > be adding much to what Antony has to say. Nothing in this CL am I more of a
> > specialist in than Antony (or the only owner), so I trust him fully to
review
> > it. 
> > 
> > In the future, I'd advice you to list what you want each reviewer to do.
> Someone
> > should be the primary reviewer and you also need owners approval. Often that
> > comes from the same person, but if you get someone who's not an owner of the
> > code to review then (after getting the green light) add someone from the
> OWNERS
> > list and ask for an "OWNERS check", which in that case would be more or less
> > just a "check that the reviewer chosen was appropriate, make sure the green
> > light has been given by that person and perhaps do an additional review of
the
> > CL, if needed".
> 
> Finnur@ Thank you for your advice :-)

Antony Sargent@ PTAL

[asargent_no_longer_on_chrome, 2016-09-29 16:58:25]

https://codereview.chromium.org/2370633002/diff/250001/extensions/browser/app...
File extensions/browser/app_window/app_window_geometry_cache_unittest.cc
(right):

https://codereview.chromium.org/2370633002/diff/250001/extensions/browser/app...
extensions/browser/app_window/app_window_geometry_cache_unittest.cc:124:
std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue);
Please use MakeUnique on this line as well

https://codereview.chromium.org/2370633002/diff/250001/extensions/browser/app...
extensions/browser/app_window/app_window_geometry_cache_unittest.cc:126: value =
base::MakeUnique<base::DictionaryValue>();
No need to use two separate statements, you should be able to just use one:

std::unique_ptr<base::DictionaryValue> value = 
    base::MakeUnique<base::DictionaryValue>();

[dhna, 2016-09-29 17:29:09]

On 2016/09/29 16:58:25, Antony Sargent wrote:
>
https://codereview.chromium.org/2370633002/diff/250001/extensions/browser/app...
> File extensions/browser/app_window/app_window_geometry_cache_unittest.cc
> (right):
> 
>
https://codereview.chromium.org/2370633002/diff/250001/extensions/browser/app...
> extensions/browser/app_window/app_window_geometry_cache_unittest.cc:124:
> std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue);
> Please use MakeUnique on this line as well
> 
>
https://codereview.chromium.org/2370633002/diff/250001/extensions/browser/app...
> extensions/browser/app_window/app_window_geometry_cache_unittest.cc:126: value
=
> base::MakeUnique<base::DictionaryValue>();
> No need to use two separate statements, you should be able to just use one:
> 
> std::unique_ptr<base::DictionaryValue> value = 
>     base::MakeUnique<base::DictionaryValue>();

Antony Sargent@ 
Thank you for review :-)
I fixed it as your comment.

[asargent_no_longer_on_chrome, 2016-09-29 17:37:17]

lgtm

[dhna, 2016-09-29 17:39:33]

The CQ bit was checked by corona10@gmail.com

[commit-bot: I haz the power, 2016-09-29 17:39:50]

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

[dhna, 2016-09-29 17:41:15]

On 2016/09/29 17:37:17, Antony Sargent wrote:
> lgtm

Antony Sargent@ 
Thank you :-)

[commit-bot: I haz the power, 2016-09-29 18:34:32]

Description was changed from

==========
replace deprecated version of SetWithoutPathExpansion

BUG=650082
==========

to

==========
replace deprecated version of SetWithoutPathExpansion

BUG=650082
==========

[commit-bot: I haz the power, 2016-09-29 18:34:34]

Committed patchset #9 (id:270001)

[commit-bot: I haz the power, 2016-09-29 18:38:57]

Description was changed from

==========
replace deprecated version of SetWithoutPathExpansion

BUG=650082
==========

to

==========
replace deprecated version of SetWithoutPathExpansion

BUG=650082

Committed: https://crrev.com/ac5ff4c30081cf17e1c8b19bccf9615f7840343c
Cr-Commit-Position: refs/heads/master@{#421877}
==========

[commit-bot: I haz the power, 2016-09-29 18:38:59]

Patchset 9 (id:??) landed as
https://crrev.com/ac5ff4c30081cf17e1c8b19bccf9615f7840343c
Cr-Commit-Position: refs/heads/master@{#421877}