Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(147)

Issue 733703002: Make sure message ports that are transferred to a serviceworker end up in the right process. (Closed)

Created:
6 years, 1 month ago by Marijn Kruisselbrink
Modified:
6 years, 1 month ago
Reviewers:
michaeln, falken, jsbell
CC:
chromium-reviews, michaeln, jsbell+serviceworker_chromium.org, tzik, serviceworker-reviews, jam, kinuko+serviceworker, nhiroki, darin-cc_chromium.org, horo+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@service-worker-message-port-transfer
Project:
chromium
Visibility:
Public.

Description

Make sure message ports that are transferred to a serviceworker end up in the right process. This still has one remaining issue in that there are some situation where MessagePortService in the browser process might unqueue these transferred message ports before the service worker has been started. That will be fixed in a separate CL. https://codereview.chromium.org/730543005/ has a test for this fix. BUG=432678 Committed: https://crrev.com/aa5ac2e2e069f7a4fb5b8a81bf2ae23c3be609c6 Cr-Commit-Position: refs/heads/master@{#305074}

Patch Set 1 #

Patch Set 2 : fix test compile #

Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -12 lines) Patch
M content/browser/service_worker/embedded_worker_instance.h View 2 chunks +2 lines, -0 lines 0 comments Download
M content/browser/service_worker/embedded_worker_instance.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M content/browser/service_worker/embedded_worker_registry.h View 4 chunks +10 lines, -1 line 0 comments Download
M content/browser/service_worker/embedded_worker_registry.cc View 2 chunks +11 lines, -1 line 0 comments Download
M content/browser/service_worker/embedded_worker_test_helper.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download
M content/browser/service_worker/service_worker_dispatcher_host.cc View 2 chunks +3 lines, -8 lines 0 comments Download
M content/browser/service_worker/service_worker_version.h View 1 chunk +5 lines, -0 lines 0 comments Download
M content/browser/service_worker/service_worker_version.cc View 2 chunks +25 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (4 generated)
Marijn Kruisselbrink
6 years, 1 month ago (2014-11-17 23:29:30 UTC) #2
jsbell
lgtm but will need OWNERS review Thanks for finding/fixing!
6 years, 1 month ago (2014-11-18 00:29:06 UTC) #3
Marijn Kruisselbrink
+michaeln for OWNERS review
6 years, 1 month ago (2014-11-18 01:08:42 UTC) #5
jsbell
michaeln is on vacation. Maybe falken can review?
6 years, 1 month ago (2014-11-19 18:53:57 UTC) #7
falken
lgtm. Thanks for this fix!
6 years, 1 month ago (2014-11-20 02:12:38 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/733703002/20001
6 years, 1 month ago (2014-11-20 19:31:01 UTC) #10
commit-bot: I haz the power
Committed patchset #2 (id:20001)
6 years, 1 month ago (2014-11-20 20:24:45 UTC) #11
commit-bot: I haz the power
6 years, 1 month ago (2014-11-20 20:26:00 UTC) #12
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/aa5ac2e2e069f7a4fb5b8a81bf2ae23c3be609c6
Cr-Commit-Position: refs/heads/master@{#305074}

Powered by Google App Engine
This is Rietveld 408576698