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

Issue 2612043004: SharedWorker: Simplify message queueing mechanism in WebSharedWorkerProxy (Closed)

Created:
3 years, 11 months ago by nhiroki
Modified:
3 years, 11 months ago
Reviewers:
kinuko, horo
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, kinuko+watch, jam, darin-cc_chromium.org, blink-worker-reviews_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

SharedWorker: Simplify message queueing mechanism in WebSharedWorkerProxy This is just a cleanup and should not change behavior. WebSharedWorkerProxy has a message queueing mechanism to delay sending messages until a shared worker is created. This mechanism can handle any message types and any number of messages, but this would be an overkill. This is because (1) a connection request is the only message type that uses this mechanism, and (2) the connection request is issued only one time. In addition, the current implementation checks |created_| flag on Send() but this is always false when it's initially called from connect() because a worker creation request is sent immediately before connect() call. This CL makes WebSharedWorkerProxy hold only one pending request and removes the unnecessary |created_| check for cleanup. BUG=612308 Review-Url: https://codereview.chromium.org/2612043004 Cr-Commit-Position: refs/heads/master@{#442527} Committed: https://chromium.googlesource.com/chromium/src/+/e1333fa774f8efbe7833ee19618a120c1135bc99

Patch Set 1 #

Total comments: 2

Patch Set 2 : add DCHECK #

Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -50 lines) Patch
M content/renderer/websharedworker_proxy.h View 3 chunks +1 line, -12 lines 0 comments Download
M content/renderer/websharedworker_proxy.cc View 1 4 chunks +12 lines, -38 lines 0 comments Download

Messages

Total messages: 28 (20 generated)
nhiroki
PTAL. This cleanup is helpful for my mojofication CL: https://codereview.chromium.org/2600113003/
3 years, 11 months ago (2017-01-06 03:15:48 UTC) #8
horo
lgtm with a nit https://codereview.chromium.org/2612043004/diff/20001/content/renderer/websharedworker_proxy.cc File content/renderer/websharedworker_proxy.cc (right): https://codereview.chromium.org/2612043004/diff/20001/content/renderer/websharedworker_proxy.cc#newcode31 content/renderer/websharedworker_proxy.cc:31: ConnectListener* listener) { DCHECK_EQ(MSG_ROUTING_NONE, message_port_id_);
3 years, 11 months ago (2017-01-06 06:59:45 UTC) #10
nhiroki
Thank you. https://codereview.chromium.org/2612043004/diff/20001/content/renderer/websharedworker_proxy.cc File content/renderer/websharedworker_proxy.cc (right): https://codereview.chromium.org/2612043004/diff/20001/content/renderer/websharedworker_proxy.cc#newcode31 content/renderer/websharedworker_proxy.cc:31: ConnectListener* listener) { On 2017/01/06 06:59:45, horo ...
3 years, 11 months ago (2017-01-06 07:20:47 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2612043004/40001
3 years, 11 months ago (2017-01-07 02:24:25 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/336983)
3 years, 11 months ago (2017-01-07 02:31:52 UTC) #20
kinuko
lgtm
3 years, 11 months ago (2017-01-10 05:19:48 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2612043004/40001
3 years, 11 months ago (2017-01-10 05:26:28 UTC) #25
commit-bot: I haz the power
3 years, 11 months ago (2017-01-10 07:55:23 UTC) #28
Message was sent while issue was closed.
Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/e1333fa774f8efbe7833ee19618a...

Powered by Google App Engine
This is Rietveld 408576698