|
|
Created:
3 years, 9 months ago by Noel Gordon Modified:
3 years, 9 months ago CC:
chromium-reviews, yamaguchi+watch_chromium.org, oka+watch_chromium.org, rginda+watch_chromium.org, oshima+watch_chromium.org, fukino+watch_chromium.org, davemoore+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionConvert utility process zip creator IPC to mojo: part 2
Follow on from [1], the comment added there about |this| during
the mojo callback is wrong. Checking this->hasOneRef() after we
reset() the mojo client shows there's always one reference left
in all cases (mojo error callback, mojo success callback).
Ditch the comment. The reset and client callback can be done in
any order. Invoke the client callback with a one-liner.
Covered by existing tests: ZipFileCreatorTest.SomeFilesZip, per
https://codereview.chromium.org/2703463005
[1] https://codereview.chromium.org/2705613003
TBR=jochen@chromium.org
BUG=597124
Review-Url: https://codereview.chromium.org/2730583002
Cr-Commit-Position: refs/heads/master@{#454181}
Committed: https://chromium.googlesource.com/chromium/src/+/e9cc49145670e9a034505aeda4f9ee7bb0a7b3c6
Patch Set 1 #
Messages
Total messages: 20 (13 generated)
The CQ bit was checked by noel@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Convert utility process zip creator IPC to mojo: part 2 BUG=597124 ========== to ========== Convert utility process zip creator IPC to mojo: part 2 Follow on from [1], the comment added there about |this| during the mojo callback is wrong. Checking this->hasOneRef() after we reset() the mojo client shows there's always one reference left in all cases (mojo error callback, mojo success callback). Ditch the comment. The reset and client callback can be done in any order. Invoke the client callback with a one-liner. Covered by existing tests: ZipFileCreatorTest.SomeFilesZip, per https://codereview.chromium.org/2703463005 [1] https://codereview.chromium.org/2705613003 BUG=597124 ==========
noel@chromium.org changed reviewers: + dcheng@chromium.org, jochen@chromium.org, sammc@chromium.org, tibell@chromium.org
PTAL.
lgtm
lgtm
lgtm
TBR=jochen I'm sure jochen will enjoy telling us that mojo is well designed for this instance.
The CQ bit was checked by noel@chromium.org
Description was changed from ========== Convert utility process zip creator IPC to mojo: part 2 Follow on from [1], the comment added there about |this| during the mojo callback is wrong. Checking this->hasOneRef() after we reset() the mojo client shows there's always one reference left in all cases (mojo error callback, mojo success callback). Ditch the comment. The reset and client callback can be done in any order. Invoke the client callback with a one-liner. Covered by existing tests: ZipFileCreatorTest.SomeFilesZip, per https://codereview.chromium.org/2703463005 [1] https://codereview.chromium.org/2705613003 BUG=597124 ========== to ========== Convert utility process zip creator IPC to mojo: part 2 Follow on from [1], the comment added there about |this| during the mojo callback is wrong. Checking this->hasOneRef() after we reset() the mojo client shows there's always one reference left in all cases (mojo error callback, mojo success callback). Ditch the comment. The reset and client callback can be done in any order. Invoke the client callback with a one-liner. Covered by existing tests: ZipFileCreatorTest.SomeFilesZip, per https://codereview.chromium.org/2703463005 [1] https://codereview.chromium.org/2705613003 TBR=jochen BUG=597124 ==========
Description was changed from ========== Convert utility process zip creator IPC to mojo: part 2 Follow on from [1], the comment added there about |this| during the mojo callback is wrong. Checking this->hasOneRef() after we reset() the mojo client shows there's always one reference left in all cases (mojo error callback, mojo success callback). Ditch the comment. The reset and client callback can be done in any order. Invoke the client callback with a one-liner. Covered by existing tests: ZipFileCreatorTest.SomeFilesZip, per https://codereview.chromium.org/2703463005 [1] https://codereview.chromium.org/2705613003 TBR=jochen BUG=597124 ========== to ========== Convert utility process zip creator IPC to mojo: part 2 Follow on from [1], the comment added there about |this| during the mojo callback is wrong. Checking this->hasOneRef() after we reset() the mojo client shows there's always one reference left in all cases (mojo error callback, mojo success callback). Ditch the comment. The reset and client callback can be done in any order. Invoke the client callback with a one-liner. Covered by existing tests: ZipFileCreatorTest.SomeFilesZip, per https://codereview.chromium.org/2703463005 [1] https://codereview.chromium.org/2705613003 TBR=jochen@chromium.org BUG=597124 ==========
The CQ bit was unchecked by noel@chromium.org
The CQ bit was checked by noel@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 1, "attempt_start_ts": 1488429231232820, "parent_rev": "32a45a277461ac61b4556e773cb3a16d4e213fa9", "commit_rev": "e9cc49145670e9a034505aeda4f9ee7bb0a7b3c6"}
Message was sent while issue was closed.
Description was changed from ========== Convert utility process zip creator IPC to mojo: part 2 Follow on from [1], the comment added there about |this| during the mojo callback is wrong. Checking this->hasOneRef() after we reset() the mojo client shows there's always one reference left in all cases (mojo error callback, mojo success callback). Ditch the comment. The reset and client callback can be done in any order. Invoke the client callback with a one-liner. Covered by existing tests: ZipFileCreatorTest.SomeFilesZip, per https://codereview.chromium.org/2703463005 [1] https://codereview.chromium.org/2705613003 TBR=jochen@chromium.org BUG=597124 ========== to ========== Convert utility process zip creator IPC to mojo: part 2 Follow on from [1], the comment added there about |this| during the mojo callback is wrong. Checking this->hasOneRef() after we reset() the mojo client shows there's always one reference left in all cases (mojo error callback, mojo success callback). Ditch the comment. The reset and client callback can be done in any order. Invoke the client callback with a one-liner. Covered by existing tests: ZipFileCreatorTest.SomeFilesZip, per https://codereview.chromium.org/2703463005 [1] https://codereview.chromium.org/2705613003 TBR=jochen@chromium.org BUG=597124 Review-Url: https://codereview.chromium.org/2730583002 Cr-Commit-Position: refs/heads/master@{#454181} Committed: https://chromium.googlesource.com/chromium/src/+/e9cc49145670e9a034505aeda4f9... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/e9cc49145670e9a034505aeda4f9... |