Make FileReader return ownership of the string content

Make FileReader return ownership of the string content.

Since FileReader reads stuff on separate thread, moving
the content around with std::string& is expensive. Make
FileReader pass the string's ownership via unique_ptr.

This reduces string copy primarily in ExecuteCodeFunction
and web_view declarative content scripts.

BUG=622464
Committed: https://crrev.com/b81e69a88c61106c1471d9585a37787f6624e063
Cr-Commit-Position: refs/heads/master@{#412957}

[lazyboy, 2016-08-11 08:45:33]

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

[commit-bot: I haz the power, 2016-08-11 08:45:44]

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-08-11 08:59:38]

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

[commit-bot: I haz the power, 2016-08-11 08:59:39]

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

[lazyboy, 2016-08-11 17:43:02]

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

[commit-bot: I haz the power, 2016-08-11 17:43:28]

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-08-11 20:07:47]

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

[commit-bot: I haz the power, 2016-08-11 20:07:48]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[lazyboy, 2016-08-12 00:26:31]

Description was changed from

==========
FileReader copy fix

BUG=
==========

to

==========
Make FileReader return ownership of the string content.

Since FileReader reads stuff on separate thread, moving
the content around with std::string& is expensive. Make
FileReader pass the string's ownership via unique_ptr.

This reduces string copy primarily in ExecuteCodeFunction
and web_view declarative content scripts.

BUG=622464
==========

[lazyboy, 2016-08-12 00:27:14]

lazyboy@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[Devlin, 2016-08-12 00:35:11]

lgtm

https://codereview.chromium.org/2231353002/diff/40001/extensions/browser/api/...
File extensions/browser/api/execute_code_function.cc (right):

https://codereview.chromium.org/2231353002/diff/40001/extensions/browser/api/...
extensions/browser/api/execute_code_function.cc:222:
rb.GetRawDataResource(resource_id).as_string())));
Is the compiler smart enough (through rvalue refs) to do only a single copy
here, even though as_string() creates a copy?  Or should we do something like
new std::string(string_piece.data(), string_piece.size())?

https://codereview.chromium.org/2231353002/diff/40001/extensions/browser/file...
File extensions/browser/file_reader.cc (right):

https://codereview.chromium.org/2231353002/diff/40001/extensions/browser/file...
extensions/browser/file_reader.cc:29: std::unique_ptr<std::string> data(new
std::string);
nitty nit: doesn't really matter here, but I slightly prefer explicit
construction (std::string())

https://codereview.chromium.org/2231353002/diff/40001/extensions/browser/gues...
File extensions/browser/guest_view/web_view/web_ui/web_ui_url_fetcher.cc
(right):

https://codereview.chromium.org/2231353002/diff/40001/extensions/browser/gues...
extensions/browser/guest_view/web_view/web_ui/web_ui_url_fetcher.cc:42:
std::unique_ptr<std::string> data(new std::string);
ditto re explicit ction

[lazyboy, 2016-08-12 01:31:21]

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

[lazyboy, 2016-08-12 01:31:22]

+dmazzoni for accessibility_manager.cc

https://codereview.chromium.org/2231353002/diff/40001/extensions/browser/api/...
File extensions/browser/api/execute_code_function.cc (right):

https://codereview.chromium.org/2231353002/diff/40001/extensions/browser/api/...
extensions/browser/api/execute_code_function.cc:222:
rb.GetRawDataResource(resource_id).as_string())));
On 2016/08/12 00:35:11, Devlin (ooo aug12) wrote:
> Is the compiler smart enough (through rvalue refs) to do only a single copy
> here, even though as_string() creates a copy?  
I would *expect*, but not sure, I'm going to explicitly construct it with
data/size from StringPiece as you suggest.
There's no downside to that except someone thinking that it hurts readability.
Or should we do something like
> new std::string(string_piece.data(), string_piece.size())?

https://codereview.chromium.org/2231353002/diff/40001/extensions/browser/file...
File extensions/browser/file_reader.cc (right):

https://codereview.chromium.org/2231353002/diff/40001/extensions/browser/file...
extensions/browser/file_reader.cc:29: std::unique_ptr<std::string> data(new
std::string);
On 2016/08/12 00:35:11, Devlin (ooo aug12) wrote:
> nitty nit: doesn't really matter here, but I slightly prefer explicit
> construction (std::string())

Done.

https://codereview.chromium.org/2231353002/diff/40001/extensions/browser/gues...
File extensions/browser/guest_view/web_view/web_ui/web_ui_url_fetcher.cc
(right):

https://codereview.chromium.org/2231353002/diff/40001/extensions/browser/gues...
extensions/browser/guest_view/web_view/web_ui/web_ui_url_fetcher.cc:42:
std::unique_ptr<std::string> data(new std::string);
On 2016/08/12 00:35:11, Devlin (ooo aug12) wrote:
> ditto re explicit ction

Done.

[lazyboy, 2016-08-12 01:32:05]

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

[commit-bot: I haz the power, 2016-08-12 01:32:21]

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-08-12 02:12:00]

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

[commit-bot: I haz the power, 2016-08-12 02:12:01]

Dry run: This issue passed the CQ dry run.

[lazyboy, 2016-08-16 22:46:00]

@ dmazzoni ping

[dmazzoni, 2016-08-18 17:18:06]

lgtm

[lazyboy, 2016-08-18 17:19:02]

The CQ bit was checked by lazyboy@chromium.org

[lazyboy, 2016-08-18 17:19:02]

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

[commit-bot: I haz the power, 2016-08-18 17:19:25]

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-08-18 19:06:55]

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

[commit-bot: I haz the power, 2016-08-18 19:06:56]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[lazyboy, 2016-08-18 21:02:55]

The CQ bit was checked by lazyboy@chromium.org

[commit-bot: I haz the power, 2016-08-18 21:03:40]

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-08-18 22:35:34]

Description was changed from

==========
Make FileReader return ownership of the string content.

Since FileReader reads stuff on separate thread, moving
the content around with std::string& is expensive. Make
FileReader pass the string's ownership via unique_ptr.

This reduces string copy primarily in ExecuteCodeFunction
and web_view declarative content scripts.

BUG=622464
==========

to

==========
Make FileReader return ownership of the string content.

Since FileReader reads stuff on separate thread, moving
the content around with std::string& is expensive. Make
FileReader pass the string's ownership via unique_ptr.

This reduces string copy primarily in ExecuteCodeFunction
and web_view declarative content scripts.

BUG=622464
==========

[commit-bot: I haz the power, 2016-08-18 22:35:36]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-18 23:24:22]

Description was changed from

==========
Make FileReader return ownership of the string content.

Since FileReader reads stuff on separate thread, moving
the content around with std::string& is expensive. Make
FileReader pass the string's ownership via unique_ptr.

This reduces string copy primarily in ExecuteCodeFunction
and web_view declarative content scripts.

BUG=622464
==========

to

==========
Make FileReader return ownership of the string content.

Since FileReader reads stuff on separate thread, moving
the content around with std::string& is expensive. Make
FileReader pass the string's ownership via unique_ptr.

This reduces string copy primarily in ExecuteCodeFunction
and web_view declarative content scripts.

BUG=622464

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

[commit-bot: I haz the power, 2016-08-18 23:24:24]

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