Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

auto grows the buffer to workaround the lack of fragmentation support #782

Closed
wants to merge 1 commit into from
Closed

auto grows the buffer to workaround the lack of fragmentation support #782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions chromedp_test.go
View file
Open in desktop
Expand Up @@ -586,8 +586,8 @@ func TestLargeOutboundMessages(t *testing.T) {
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))
// ~5MiB of JS to test the grow feature of github.com/gobwas/ws.
expr := fmt.Sprintf("//%s\n", strings.Repeat("x", 5<<20))
res := new([]byte)
if err := Run(ctx, Evaluate(expr, res)); err != nil {
t.Fatal(err)
Expand Down
31 changes: 17 additions & 14 deletions conn.go
View file
Open in desktop
Expand Up @@ -40,19 +40,6 @@ 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 @@ -67,8 +54,13 @@ func DialContext(ctx context.Context, urlstr string, opts ...DialOption) (*Conn,
// apply opts
c := &Conn{
conn: conn,
// The default buffer size is 4KiB, which should be enough.
// But github.com/gobwas/ws has a bug right row
// (see https://github.com/gobwas/ws/pull/134),
// so we will pass 65544 to make it reserve 10 or 14 bytes for header.
// Once the issue is fixed, we can pass 0 to use the default size (4KiB).
writer: *wsutil.NewWriterBufferSize(conn,
ws.StateClientSide, ws.OpText, wsWriteBufferSize),
ws.StateClientSide, ws.OpText, 65544),
}
for _, o := range opts {
o(c)
Expand Down Expand Up @@ -112,6 +104,17 @@ func (c *Conn) Read(_ context.Context, msg *cdproto.Message) error {
// Write writes a message.
func (c *Conn) Write(_ context.Context, msg *cdproto.Message) error {
c.writer.Reset(c.conn, ws.StateClientSide, ws.OpText)
// Chrome doesn't support fragmentation of incoming websocket messages. To
// compensate this, they support single-fragment messages of up to 100MiB.
//
// See https://github.com/ChromeDevTools/devtools-protocol/issues/175.
//
// And according to https://bugs.chromium.org/p/chromium/issues/detail?id=1069431,
// it seems like that fragmentation won't be supported very soon.
// Luckily, now github.com/gobwas/ws will grow the buffer if needed.
// The func name DisableFlush is a little misleading,
// but it do make it grow the buffer if needed.
c.writer.DisableFlush()

// Reuse the easyjson writer.
c.encoder = jwriter.Writer{}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Expand Up @@ -6,7 +6,7 @@ require (
github.com/chromedp/cdproto v0.0.0-20210323015217-0942afbea50e
github.com/gobwas/httphead v0.1.0 // indirect
github.com/gobwas/pool v0.2.1 // indirect
github.com/gobwas/ws v1.0.4
github.com/gobwas/ws v1.1.0-rc.5
github.com/mailru/easyjson v0.7.7
golang.org/x/sys v0.0.0-20210324051608-47abb6519492 // indirect
)
3 changes: 3 additions & 0 deletions go.sum
View file
Open in desktop
Expand Up @@ -8,9 +8,12 @@ github.com/gobwas/pool v0.2.1 h1:xfeeEhW7pwmX8nuLVlqbzVc7udMDrwetjEv+TZIz1og=
github.com/gobwas/pool v0.2.1/go.mod h1:q8bcK0KcYlCgd9e7WYLm9LpyS+YeLd8JVDW6WezmKEw=
github.com/gobwas/ws v1.0.4 h1:5eXU1CZhpQdq5kXbKb+sECH5Ia5KiO6CYzIzdlVx6Bs=
github.com/gobwas/ws v1.0.4/go.mod h1:szmBTxLgaFppYjEmNtny/v3w89xOydFnnZMcgRRu/EM=
github.com/gobwas/ws v1.1.0-rc.5 h1:QOAag7FoBaBYYHRqzqkhhd8fq5RTubvI4v3Ft/gDVVQ=
github.com/gobwas/ws v1.1.0-rc.5/go.mod h1:nzvNcVha5eUziGrbxFCo6qFIojQHjJV5cLYIbezhfL0=
github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=
github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0=
github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
golang.org/x/sys v0.0.0-20201207223542-d4d67f95c62d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210324051608-47abb6519492 h1:Paq34FxTluEPvVyayQqMPgHm+vTOrIifmcYxFBx9TLg=
golang.org/x/sys v0.0.0-20210324051608-47abb6519492/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=