Skip to content

Commit

Permalink
increase the websocket write buffer size to 1MiB
Browse files Browse the repository at this point in the history
See the comment on wsWriteBufferSize for context. This somewhat fixes
the regression in dc09186, as we went from a 25MiB message size limit
to a 4KiB one. 1MiB seems like a reasonable middle ground for the time
being, keeping the memory usage low.

Fixes #395.
  • Loading branch information
mvdan committed Jun 26, 2019
1 parent b9d197d commit bd26d4c
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 2 deletions.
14 changes: 14 additions & 0 deletions chromedp_test.go
Expand Up @@ -516,3 +516,17 @@ func TestListenCancel(t *testing.T) {
t.Fatalf("want %d target events; got %d", want, targetCount)
}
}

func TestLargeOutboundMessages(t *testing.T) {
t.Parallel()

ctx, cancel := testAllocate(t, "")
defer cancel()

// ~50KiB of JS should fit just fine in our current buffer of 1MiB.
expr := fmt.Sprintf("//%s\n", strings.Repeat("x", 50 << 10))
res := new([]byte)
if err := Run(ctx, Evaluate(expr, res)); err != nil {
t.Fatal(err)
}
}
18 changes: 16 additions & 2 deletions conn.go
Expand Up @@ -40,6 +40,19 @@ type Conn struct {
dbgf func(string, ...interface{})
}

// Chrome doesn't support fragmentation of incoming websocket messages. To
// compensate this, they support single-fragment messages of up to 100MiB.
//
// If our write buffer size is too small, large messages will get fragmented,
// and Chrome will silently crash. And if it's too large, chromedp will require
// more memory for all users.
//
// For now, make this a middle ground. 1MiB is large enough for practically any
// outgoing message, but small enough to not take too much meomry.
//
// See https://github.com/ChromeDevTools/devtools-protocol/issues/175.
const wsWriteBufferSize = 1 << 20

// DialContext dials the specified websocket URL using gobwas/ws.
func DialContext(ctx context.Context, urlstr string, opts ...DialOption) (*Conn, error) {
// connect
Expand All @@ -53,8 +66,9 @@ func DialContext(ctx context.Context, urlstr string, opts ...DialOption) (*Conn,

// apply opts
c := &Conn{
conn: conn,
writer: *wsutil.NewWriterBufferSize(conn, ws.StateClientSide, ws.OpText, 4096),
conn: conn,
writer: *wsutil.NewWriterBufferSize(conn,
ws.StateClientSide, ws.OpText, wsWriteBufferSize),
}
for _, o := range opts {
o(c)
Expand Down

0 comments on commit bd26d4c

Please sign in to comment.