[WebSocket] Task creation should be separated from task posting.

[WebSocket] Task creation should be separated from task posting.

Having a complex argument as a waitForMethodCompletion may keep alive
temporary objects which must be killed before posting a task to another
thread.

BUG=390174
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=177359

[tyoshino (SeeGerritForStatus), 2014-07-01 13:23:17]

lgtm

[hiroshige, 2014-07-01 14:13:41]

lgtm.

I added comments for more possible code changes, but I'm not so sure.

https://codereview.chromium.org/368453003/diff/1/Source/modules/websockets/Wo...
File Source/modules/websockets/WorkerThreadableWebSocketChannel.cpp (left):

https://codereview.chromium.org/368453003/diff/1/Source/modules/websockets/Wo...
Source/modules/websockets/WorkerThreadableWebSocketChannel.cpp:525:
m_loaderProxy.postTaskToLoader(CallClosureTask::create(bind(&Peer::close,
m_peer, code, reason.isolatedCopy())));
Should we modify this line as well?
The temporary objects (if not omitted by compiler optimization) are not
destroyed strictly before submitting the task.
(The probability of error is much lower than waitForMethodCompletion though)

https://codereview.chromium.org/368453003/diff/1/Source/modules/websockets/Wo...
Source/modules/websockets/WorkerThreadableWebSocketChannel.cpp:533:
m_loaderProxy.postTaskToLoader(CallClosureTask::create(bind(&Peer::fail, m_peer,
reason.isolatedCopy(), level, sourceURL.isolatedCopy(), lineNumber)));
same as above.

[yhirano, 2014-07-02 01:35:23]

https://codereview.chromium.org/368453003/diff/1/Source/modules/websockets/Wo...
File Source/modules/websockets/WorkerThreadableWebSocketChannel.cpp (left):

https://codereview.chromium.org/368453003/diff/1/Source/modules/websockets/Wo...
Source/modules/websockets/WorkerThreadableWebSocketChannel.cpp:525:
m_loaderProxy.postTaskToLoader(CallClosureTask::create(bind(&Peer::close,
m_peer, code, reason.isolatedCopy())));
On 2014/07/01 14:13:41, hiroshige wrote:
> Should we modify this line as well?
> The temporary objects (if not omitted by compiler optimization) are not
> destroyed strictly before submitting the task.
> (The probability of error is much lower than waitForMethodCompletion though)

Thanks, done.

https://codereview.chromium.org/368453003/diff/1/Source/modules/websockets/Wo...
Source/modules/websockets/WorkerThreadableWebSocketChannel.cpp:533:
m_loaderProxy.postTaskToLoader(CallClosureTask::create(bind(&Peer::fail, m_peer,
reason.isolatedCopy(), level, sourceURL.isolatedCopy(), lineNumber)));
On 2014/07/01 14:13:41, hiroshige wrote:
> same as above.

Done.

[yhirano, 2014-07-02 01:35:26]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2014-07-02 01:35:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/yhirano@chromium.org/368453003/60001

[commit-bot: I haz the power, 2014-07-02 02:40:33]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/14955)

[commit-bot: I haz the power, 2014-07-02 08:08:08]

Change committed as 177359