[ABANDONED] Use new Websocket SendBlob IPC from renderer.

[ABANDONED] Use new Websocket SendBlob IPC from renderer.

Remove BlobLoader code from DocumentWebSocketChannel and replace it with
code to use the new SendBlob IPC for browser-side Blob sending.

See design document:
https://docs.google.com/document/d/1CDiXB9pBumhFVVfmIn1CRI6v6byxyqWu2urEE9xp714/edit

BUG=571656
TEST=webkit_unit_tests, layout tests

[Adam Rice, 2016-01-12 20:33:43]

Description was changed from

==========
[WIP] Use new Websocket SendBlob IPC from renderer.

Remove BlobLoader code from DocumentWebSocketChannel and replace it with
code to use the new SendBlob IPC for browser-side Blob sending.

See design document:
https://docs.google.com/document/d/1CDiXB9pBumhFVVfmIn1CRI6v6byxyqWu2urEE9xp7...

BUG=571656
TEST=webkit_unit_tests, layout tests
==========

to

==========
[WIP] Use new Websocket SendBlob IPC from renderer.

Remove BlobLoader code from DocumentWebSocketChannel and replace it with
code to use the new SendBlob IPC for browser-side Blob sending.

See design document:
https://docs.google.com/document/d/1CDiXB9pBumhFVVfmIn1CRI6v6byxyqWu2urEE9xp7...

BUG=571656
TEST=webkit_unit_tests, layout tests
==========

[Adam Rice, 2016-01-14 22:56:16]

Patchset #3 (id:40001) has been deleted

[Adam Rice, 2016-01-14 22:59:15]

Description was changed from

==========
[WIP] Use new Websocket SendBlob IPC from renderer.

Remove BlobLoader code from DocumentWebSocketChannel and replace it with
code to use the new SendBlob IPC for browser-side Blob sending.

See design document:
https://docs.google.com/document/d/1CDiXB9pBumhFVVfmIn1CRI6v6byxyqWu2urEE9xp7...

BUG=571656
TEST=webkit_unit_tests, layout tests
==========

to

==========
Use new Websocket SendBlob IPC from renderer.

Remove BlobLoader code from DocumentWebSocketChannel and replace it with
code to use the new SendBlob IPC for browser-side Blob sending.

See design document:
https://docs.google.com/document/d/1CDiXB9pBumhFVVfmIn1CRI6v6byxyqWu2urEE9xp7...

BUG=571656
TEST=webkit_unit_tests, layout tests
==========

[Adam Rice, 2016-02-04 15:27:35]

ricea@chromium.org changed reviewers:
+ mpcomplete@chromium.org

[Adam Rice, 2016-02-04 15:27:36]

Hello mpcomplete.

I have to change the interface to blink::WebSocketHandle in order to implement
browser-side Blob sending. Unfortunately, this breaks
html_viewer::WebSocketHandleImpl.

For the time being I have stubbed out the new sendBlob() method to get it to
compile. It's possible to implement it within the renderer, but it's moderately
complex and I don't know how to test html_viewer.

How would you like to proceed? Is it okay to postpone the implementation and fix
it later? Would you prefer me to try to add an untested implementation?

By the way, I have another CL with a similar issue coming down the pipeline:
https://codereview.chromium.org/1666893003/

Thanks,
ricea

[mpcomplete, 2016-02-04 18:21:22]

mpcomplete@google.com changed reviewers:
+ mpcomplete@google.com

[mpcomplete, 2016-02-04 18:21:22]

I'm afraid I'm not the right person to review this. I'm not familiar with this
code, and don't work on Chrome anymore.

[dglazkov, 2016-02-04 18:23:06]

dglazkov@chromium.org changed reviewers:
+ sky@chromium.org
- mpcomplete@chromium.org, mpcomplete@google.com

[dglazkov, 2016-02-04 18:23:24]

+sky

Scott, could you peek at this?

[sky, 2016-02-04 22:22:40]

html_viewer LGTM (we're going to nuke that directory soon)

[Adam Rice, 2016-02-05 07:20:37]

Patchset #11 (id:220001) has been deleted

[Adam Rice, 2016-02-08 00:43:09]

ricea@chromium.org changed reviewers:
+ tyoshino@chromium.org

[Adam Rice, 2016-02-08 00:43:09]

+tyoshino for websockets OWNERS.

This will need rebasing after https://codereview.chromium.org/1568523002/
(content::WebSocketBlobSender) has landed, but otherwise it is ready for review.

[tyoshino (SeeGerritForStatus), 2016-02-09 14:29:10]

https://codereview.chromium.org/1574213003/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp
(right):

https://codereview.chromium.org/1574213003/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp:476:
m_client->didConsumeBufferedAmount(blobUsedQuota);
IIUC, it's possible that this would process flow control replenishments for the
message sent before the blob. Is that true?

I don't think we have to be so strict about that, but wanna understand what's
strictly implemented and what's not.

[Adam Rice, 2016-02-10 01:37:55]

https://codereview.chromium.org/1574213003/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp
(right):

https://codereview.chromium.org/1574213003/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp:476:
m_client->didConsumeBufferedAmount(blobUsedQuota);
On 2016/02/09 14:29:10, tyoshino wrote:
> IIUC, it's possible that this would process flow control replenishments for
the
> message sent before the blob. Is that true?

Correct. For example, if the user sends an 8KB Text message then immediately an
8KB Blob, bufferedAmount will go from 16KB to 8KB when the Text message is sent
to the browser process, and then from 8KB to 0KB when the flow control is
replenished.

This is obviously not quite right, but because we normally consider a message
"sent" when we send it to the browser, I don't consider it a big problem. I
don't think there's benefit to developers in spending more resources tracking it
when we still have no guarantee that the data has been transmitted by the OS
kernel.

> I don't think we have to be so strict about that, but wanna understand what's
> strictly implemented and what's not.

How should I make this clearer? Add a comment? Or maybe add a test (the
behaviour is not quite right, but it is WAI)?

[tyoshino (SeeGerritForStatus), 2016-02-10 06:48:46]

https://codereview.chromium.org/1574213003/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp
(right):

https://codereview.chromium.org/1574213003/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp:476:
m_client->didConsumeBufferedAmount(blobUsedQuota);
On 2016/02/10 at 01:37:55, Adam Rice wrote:
> On 2016/02/09 14:29:10, tyoshino wrote:
> > IIUC, it's possible that this would process flow control replenishments for
the
> > message sent before the blob. Is that true?
> 
> Correct. For example, if the user sends an 8KB Text message then immediately
an 8KB Blob, bufferedAmount will go from 16KB to 8KB when the Text message is
sent to the browser process, and then from 8KB to 0KB when the flow control is
replenished.
> 
> This is obviously not quite right, but because we normally consider a message
"sent" when we send it to the browser, I don't consider it a big problem. I
don't think there's benefit to developers in spending more resources tracking it
when we still have no guarantee that the data has been transmitted by the OS
kernel.
> 

Right. bufferedAmount doesn't tell the actual memory pressure to you reliably.

> > I don't think we have to be so strict about that, but wanna understand
what's
> > strictly implemented and what's not.
> 
> How should I make this clearer? Add a comment? Or maybe add a test (the
behaviour is not quite right, but it is WAI)?

Adding a comment is enough. Please.

[Adam Rice, 2016-02-10 12:58:27]

https://codereview.chromium.org/1574213003/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp
(right):

https://codereview.chromium.org/1574213003/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp:476:
m_client->didConsumeBufferedAmount(blobUsedQuota);
On 2016/02/10 06:48:46, tyoshino wrote:
> Adding a comment is enough. Please.

Comment added. I tried to keep it short to match Blink style. PTAL.

[sky, 2016-02-10 17:29:22]

sky@chromium.org changed reviewers:
- sky@chromium.org

[sky, 2016-02-10 17:29:41]

I nuked components/html_viewer the other day, so you don't need as a reviewer
anymore. I've removed myself.

[Adam Rice, 2016-02-12 02:26:35]

tyoshino: I will take out the components/html_viewer parts when I rebase. Please
let me know when is a good time to rebase.

[Adam Rice, 2016-09-12 05:57:37]

Description was changed from

==========
Use new Websocket SendBlob IPC from renderer.

Remove BlobLoader code from DocumentWebSocketChannel and replace it with
code to use the new SendBlob IPC for browser-side Blob sending.

See design document:
https://docs.google.com/document/d/1CDiXB9pBumhFVVfmIn1CRI6v6byxyqWu2urEE9xp7...

BUG=571656
TEST=webkit_unit_tests, layout tests
==========

to

==========
[ABANDONED] Use new Websocket SendBlob IPC from renderer.

Remove BlobLoader code from DocumentWebSocketChannel and replace it with
code to use the new SendBlob IPC for browser-side Blob sending.

See design document:
https://docs.google.com/document/d/1CDiXB9pBumhFVVfmIn1CRI6v6byxyqWu2urEE9xp7...

BUG=571656
TEST=webkit_unit_tests, layout tests
==========