Removed RefPtr::Release.

Removed RefPtr::Release.

Removed RefPtr::Release method and remaining uses of it, including:
- Removed calls to RefPtr::Release in return statements that return a
  WebThreadSafeData object. Also added an initialising constructor for
  WebThreadSafeData that takes a RefPtr&& so that this return is valid.
- Added move wraps for returned variables that are not automatically
  moved

BUG=494719
Review-Url: https://codereview.chromium.org/2949103006
Cr-Commit-Position: refs/heads/master@{#482532}
Committed: https://chromium.googlesource.com/chromium/src/+/dd68099cbb43f639901b84e0b38ad76509bacb6a

[Bugs Nash, 2017-06-23 05:28:15]

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

[commit-bot: I haz the power, 2017-06-23 05:28:30]

Dry run: CQ is trying da patch.

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

[Bugs Nash, 2017-06-23 05:29:52]

bugsnash@chromium.org changed reviewers:
+ haraken@chromium.org, yutak@chromium.org

[Bugs Nash, 2017-06-23 05:39:57]

Note that this patch is also blocked on removing use of RefPtr::Release in unit
tests.

[Bugs Nash, 2017-06-23 05:39:59]

The CQ bit was unchecked by bugsnash@chromium.org

[haraken, 2017-06-23 05:59:50]

LGTM

https://codereview.chromium.org/2949103006/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/fetch/BlobBytesConsumer.cpp (right):

https://codereview.chromium.org/2949103006/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/fetch/BlobBytesConsumer.cpp:130: return
std::move(blob_data_handle_);

If we forget adding std::move, what will happen?

[Bugs Nash, 2017-06-23 06:09:49]

https://codereview.chromium.org/2949103006/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/fetch/BlobBytesConsumer.cpp (right):

https://codereview.chromium.org/2949103006/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/fetch/BlobBytesConsumer.cpp:130: return
std::move(blob_data_handle_);
On 2017/06/23 at 05:59:50, haraken wrote:
> If we forget adding std::move, what will happen?

ref churn - it will copy the RefPtr and the member will not be nullified (once
the return type changes to RefPtr). While the return type is PassRefPtr the
result is essentially the same but not through a simple copy.

[Yuta Kitamura, 2017-06-23 06:21:32]

LGTM.

My LGTM will stand as long as you only apply the similar
fixes (i.e. .Release() -> std::move) to other places to
fix builds.

[Bugs Nash, 2017-06-26 00:11:42]

rebased onto unit test changes

[Bugs Nash, 2017-06-26 00:11:51]

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

[commit-bot: I haz the power, 2017-06-26 00:11:58]

Dry run: CQ is trying da patch.

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

[Bugs Nash, 2017-06-26 00:23:25]

The CQ bit was checked by bugsnash@chromium.org

[Bugs Nash, 2017-06-26 00:23:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, yutak@chromium.org
Link to the patchset: https://codereview.chromium.org/2949103006/#ps20001
(title: "rebased onto unit test changes")

[commit-bot: I haz the power, 2017-06-26 00:23:28]

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-06-26 00:48:35]

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

[commit-bot: I haz the power, 2017-06-26 00:48:36]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Bugs Nash, 2017-06-27 00:59:50]

Removed Release from mac specific code

[Bugs Nash, 2017-06-27 01:00:35]

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

[commit-bot: I haz the power, 2017-06-27 01:01:00]

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, 2017-06-27 02:50:38]

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

[commit-bot: I haz the power, 2017-06-27 02:50:38]

Dry run: This issue passed the CQ dry run.

[Bugs Nash, 2017-06-27 03:33:38]

The CQ bit was checked by bugsnash@chromium.org

[Bugs Nash, 2017-06-27 03:33:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, yutak@chromium.org
Link to the patchset: https://codereview.chromium.org/2949103006/#ps40001
(title: "Removed Release from mac specific code")

[commit-bot: I haz the power, 2017-06-27 03:33:49]

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-06-27 03:38:09]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1498534418573250,
"parent_rev": "2f7ff6f11ae536cc187f246e147fb03c21304713", "commit_rev":
"dd68099cbb43f639901b84e0b38ad76509bacb6a"}

[commit-bot: I haz the power, 2017-06-27 03:38:20]

Description was changed from

==========
Removed RefPtr::Release.

Removed RefPtr::Release method and remaining uses of it, including:
- Removed calls to RefPtr::Release in return statements that return a
  WebThreadSafeData object. Also added an initialising constructor for
  WebThreadSafeData that takes a RefPtr&& so that this return is valid.
- Added move wraps for returned variables that are not automatically
  moved

BUG=494719
==========

to

==========
Removed RefPtr::Release.

Removed RefPtr::Release method and remaining uses of it, including:
- Removed calls to RefPtr::Release in return statements that return a
  WebThreadSafeData object. Also added an initialising constructor for
  WebThreadSafeData that takes a RefPtr&& so that this return is valid.
- Added move wraps for returned variables that are not automatically
  moved

BUG=494719

Review-Url: https://codereview.chromium.org/2949103006
Cr-Commit-Position: refs/heads/master@{#482532}
Committed:
https://chromium.googlesource.com/chromium/src/+/dd68099cbb43f639901b84e0b38a...
==========

[commit-bot: I haz the power, 2017-06-27 03:38:21]

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

[dcheng1, 2017-06-27 03:43:42]

dcheng@google.com changed reviewers:
+ dcheng@google.com

[dcheng1, 2017-06-27 03:43:43]

https://codereview.chromium.org/2949103006/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/fetch/BlobBytesConsumer.cpp (right):

https://codereview.chromium.org/2949103006/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/fetch/BlobBytesConsumer.cpp:130: return
std::move(blob_data_handle_);
On 2017/06/23 06:09:49, Bugs Nash wrote:
> On 2017/06/23 at 05:59:50, haraken wrote:
> > If we forget adding std::move, what will happen?
> 
> ref churn - it will copy the RefPtr and the member will not be nullified (once
> the return type changes to RefPtr). While the return type is PassRefPtr the
> result is essentially the same but not through a simple copy.

It's worth noting that forgetting this could potentially be a functional bug: if
a function is called TakeBuffer() and it forgets to std::move() the buffer_
field, then the buffer won't have its ownership transferred out.