clean up various pieces of the API

First, collapse Browser.Start with NewBrowser. There's no reason to
split them up.

Second, unexport Browser.userDataDir, since it's only needed for a test.
It's also a bad precedent, as only the ExecAllocator will control the
user data directory.

Third, export Context.Browser, since we were already exporting
Context.Allocator.

Finally, remove the Executor interface, a duplicate of cdp.Executor.
This commit is contained in:
Daniel Martí 2019-03-21 15:44:28 +00:00
parent a93c63124f
commit 32d4bae280
5 changed files with 19 additions and 27 deletions

View File

@ -142,8 +142,7 @@ func (p *ExecAllocator) Allocate(ctx context.Context) (*Browser, error) {
if err != nil { if err != nil {
return nil, err return nil, err
} }
browser.UserDataDir = dataDir browser.userDataDir = dataDir
browser.Start(ctx)
return browser, nil return browser, nil
} }

View File

@ -30,7 +30,7 @@ func TestExecAllocator(t *testing.T) {
t.Fatalf("wanted %q, got %q", want, got) t.Fatalf("wanted %q, got %q", want, got)
} }
tempDir := FromContext(taskCtx).browser.UserDataDir tempDir := FromContext(taskCtx).Browser.userDataDir
pool := FromContext(taskCtx).Allocator pool := FromContext(taskCtx).Allocator
cancel() cancel()

View File

@ -23,7 +23,7 @@ import (
// the browser process runner, WebSocket clients, associated targets, and // the browser process runner, WebSocket clients, associated targets, and
// network, page, and DOM events. // network, page, and DOM events.
type Browser struct { type Browser struct {
UserDataDir string userDataDir string
pages map[target.SessionID]*Target pages map[target.SessionID]*Target
@ -56,7 +56,12 @@ func NewBrowser(ctx context.Context, urlstr string, opts ...BrowserOption) (*Bro
b := &Browser{ b := &Browser{
conn: conn, conn: conn,
pages: make(map[target.SessionID]*Target, 1024), pages: make(map[target.SessionID]*Target, 1024),
cmdQueue: make(chan cmdJob),
qres: make(chan *cdproto.Message),
logf: log.Printf, logf: log.Printf,
} }
@ -72,6 +77,7 @@ func NewBrowser(ctx context.Context, urlstr string, opts ...BrowserOption) (*Bro
b.errf = func(s string, v ...interface{}) { b.logf("ERROR: "+s, v...) } b.errf = func(s string, v ...interface{}) { b.logf("ERROR: "+s, v...) }
} }
go b.run(ctx)
return b, nil return b, nil
} }
@ -154,13 +160,6 @@ func (b *Browser) Execute(ctx context.Context, method string, params json.Marsha
return nil return nil
} }
func (b *Browser) Start(ctx context.Context) {
b.cmdQueue = make(chan cmdJob)
b.qres = make(chan *cdproto.Message)
go b.run(ctx)
}
func (b *Browser) run(ctx context.Context) { func (b *Browser) run(ctx context.Context) {
defer b.conn.Close() defer b.conn.Close()

View File

@ -44,7 +44,7 @@ func (c *Conn) Write(msg *cdproto.Message) error {
return c.WriteJSON(msg) return c.WriteJSON(msg)
} }
// Dial dials the specified websocket URL using gorilla/websocket. // DialContext dials the specified websocket URL using gorilla/websocket.
func DialContext(ctx context.Context, urlstr string) (*Conn, error) { func DialContext(ctx context.Context, urlstr string) (*Conn, error) {
d := &websocket.Dialer{ d := &websocket.Dialer{
ReadBufferSize: DefaultReadBufferSize, ReadBufferSize: DefaultReadBufferSize,

View File

@ -2,7 +2,6 @@ package chromedp
import ( import (
"context" "context"
"encoding/json"
"fmt" "fmt"
"github.com/chromedp/cdproto/css" "github.com/chromedp/cdproto/css"
@ -14,16 +13,11 @@ import (
"github.com/chromedp/cdproto/target" "github.com/chromedp/cdproto/target"
) )
// Executor
type Executor interface {
Execute(context.Context, string, json.Marshaler, json.Unmarshaler) error
}
// Context is attached to any context.Context which is valid for use with Run. // Context is attached to any context.Context which is valid for use with Run.
type Context struct { type Context struct {
Allocator Allocator Allocator Allocator
browser *Browser Browser *Browser
sessionID target.SessionID sessionID target.SessionID
} }
@ -68,35 +62,35 @@ func Run(ctx context.Context, action Action) error {
if c == nil || c.Allocator == nil { if c == nil || c.Allocator == nil {
return ErrInvalidContext return ErrInvalidContext
} }
if c.browser == nil { if c.Browser == nil {
browser, err := c.Allocator.Allocate(ctx) browser, err := c.Allocator.Allocate(ctx)
if err != nil { if err != nil {
return err return err
} }
c.browser = browser c.Browser = browser
} }
if c.sessionID == "" { if c.sessionID == "" {
if err := c.newSession(ctx); err != nil { if err := c.newSession(ctx); err != nil {
return err return err
} }
} }
return action.Do(ctx, c.browser.executorForTarget(ctx, c.sessionID)) return action.Do(ctx, c.Browser.executorForTarget(ctx, c.sessionID))
} }
func (c *Context) newSession(ctx context.Context) error { func (c *Context) newSession(ctx context.Context) error {
create := target.CreateTarget("about:blank") create := target.CreateTarget("about:blank")
targetID, err := create.Do(ctx, c.browser) targetID, err := create.Do(ctx, c.Browser)
if err != nil { if err != nil {
return err return err
} }
attach := target.AttachToTarget(targetID) attach := target.AttachToTarget(targetID)
sessionID, err := attach.Do(ctx, c.browser) sessionID, err := attach.Do(ctx, c.Browser)
if err != nil { if err != nil {
return err return err
} }
target := c.browser.executorForTarget(ctx, sessionID) target := c.Browser.executorForTarget(ctx, sessionID)
// enable domains // enable domains
for _, enable := range []Action{ for _, enable := range []Action{