Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2604733003:
SharedWorker: Clean up SharedWorkerHost (Closed)
DescriptionSharedWorker: Clean up SharedWorkerHost
This is just a cleanup and does not change behavior.
BUG=n/a
Committed: https://crrev.com/af12323be5a834ab027b7298d746386a21f2f274
Cr-Commit-Position: refs/heads/master@{#440828}
Patch Set 1 #Patch Set 2 : fix test crashes #
Total comments: 2
Patch Set 3 : revert std::unique_ptr use #Messages
Total messages: 20 (14 generated)
The CQ bit was checked by nhiroki@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...
nhiroki@chromium.org changed reviewers: + horo@chromium.org, kinuko@chromium.org
PTAL, thanks.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
The CQ bit was checked by nhiroki@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.
lgtm, but one comment (not super motivated to make this cleanup change for traditional IPC code) https://codereview.chromium.org/2604733003/diff/20001/content/browser/shared_... File content/browser/shared_worker/shared_worker_host.h (right): https://codereview.chromium.org/2604733003/diff/20001/content/browser/shared_... content/browser/shared_worker/shared_worker_host.h:40: bool Send(std::unique_ptr<IPC::Message> message); This generally follows IPC::Sender pattern, it's fine but just passing through ptr is more common
Thank you for your comment. https://codereview.chromium.org/2604733003/diff/20001/content/browser/shared_... File content/browser/shared_worker/shared_worker_host.h (right): https://codereview.chromium.org/2604733003/diff/20001/content/browser/shared_... content/browser/shared_worker/shared_worker_host.h:40: bool Send(std::unique_ptr<IPC::Message> message); On 2016/12/27 15:38:29, kinuko wrote: > This generally follows IPC::Sender pattern, it's fine but just passing through > ptr is more common I reverted this change because I plan to mojofy shared worker anytime soon and this pattern will be gone regardless of this change.
The CQ bit was checked by nhiroki@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org Link to the patchset: https://codereview.chromium.org/2604733003/#ps40001 (title: "revert std::unique_ptr use")
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": 40001, "attempt_start_ts": 1482891648720490,
"parent_rev": "eee183aba1ca02ea93556b7cdc2c7b3976a37a72", "commit_rev":
"06395d6061224f690f197938c488be37120310a9"}
Message was sent while issue was closed.
Description was changed from ========== SharedWorker: Clean up SharedWorkerHost This is just a cleanup and does not change behavior. BUG=n/a ========== to ========== SharedWorker: Clean up SharedWorkerHost This is just a cleanup and does not change behavior. BUG=n/a Review-Url: https://codereview.chromium.org/2604733003 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== SharedWorker: Clean up SharedWorkerHost This is just a cleanup and does not change behavior. BUG=n/a Review-Url: https://codereview.chromium.org/2604733003 ========== to ========== SharedWorker: Clean up SharedWorkerHost This is just a cleanup and does not change behavior. BUG=n/a Committed: https://crrev.com/af12323be5a834ab027b7298d746386a21f2f274 Cr-Commit-Position: refs/heads/master@{#440828} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/af12323be5a834ab027b7298d746386a21f2f274 Cr-Commit-Position: refs/heads/master@{#440828} |
