From b8efcf06917c82d407b2c1b56c4f9a1423633546 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Mon, 8 Apr 2019 12:56:16 +0200 Subject: [PATCH] support using BrowserOption in NewContext While at it, remove the error return from BrowserOption, to make it consistent with all the other option func types. We don't have a good test for this feature yet, but at least we check that it doesn't crash or error via an example. Fixes #292. --- allocate.go | 6 +++--- browser.go | 27 +++++++++------------------ chromedp.go | 38 +++++++++++++++++++++++++++++++++----- example_test.go | 4 +++- 4 files changed, 48 insertions(+), 27 deletions(-) diff --git a/allocate.go b/allocate.go index 66bf79f..b99f78b 100644 --- a/allocate.go +++ b/allocate.go @@ -20,7 +20,7 @@ type Allocator interface { // Allocate creates a new browser. It can be cancelled via the provided // context, at which point all the resources used by the browser (such // as temporary directories) will be freed. - Allocate(context.Context) (*Browser, error) + Allocate(context.Context, ...BrowserOption) (*Browser, error) // TODO: Wait should probably return an error, which can then be // retrieved by the user if just calling cancel(). @@ -73,7 +73,7 @@ type ExecAllocator struct { } // Allocate satisfies the Allocator interface. -func (p *ExecAllocator) Allocate(ctx context.Context) (*Browser, error) { +func (p *ExecAllocator) Allocate(ctx context.Context, opts ...BrowserOption) (*Browser, error) { c := FromContext(ctx) if c == nil { return nil, ErrInvalidContext @@ -152,7 +152,7 @@ func (p *ExecAllocator) Allocate(ctx context.Context) (*Browser, error) { } stderr.Close() - browser, err := NewBrowser(ctx, wsURL) + browser, err := NewBrowser(ctx, wsURL, opts...) if err != nil { return nil, err } diff --git a/browser.go b/browser.go index 304713e..add65ae 100644 --- a/browser.go +++ b/browser.go @@ -78,9 +78,7 @@ func NewBrowser(ctx context.Context, urlstr string, opts ...BrowserOption) (*Bro // apply options for _, o := range opts { - if err := o(b); err != nil { - return nil, err - } + o(b) } // ensure errf is set @@ -322,29 +320,22 @@ func (b *Browser) run(ctx context.Context) { } // BrowserOption is a browser option. -type BrowserOption func(*Browser) error +type BrowserOption func(*Browser) -// WithLogf is a browser option to specify a func to receive general logging. -func WithLogf(f func(string, ...interface{})) BrowserOption { - return func(b *Browser) error { - b.logf = f - return nil - } +// WithBrowserLogf is a browser option to specify a func to receive general logging. +func WithBrowserLogf(f func(string, ...interface{})) BrowserOption { + return func(b *Browser) { b.logf = f } } -// WithErrorf is a browser option to specify a func to receive error logging. -func WithErrorf(f func(string, ...interface{})) BrowserOption { - return func(b *Browser) error { - b.errf = f - return nil - } +// WithBrowserErrorf is a browser option to specify a func to receive error logging. +func WithBrowserErrorf(f func(string, ...interface{})) BrowserOption { + return func(b *Browser) { b.errf = f } } // WithConsolef is a browser option to specify a func to receive chrome log events. // // Note: NOT YET IMPLEMENTED. func WithConsolef(f func(string, ...interface{})) BrowserOption { - return func(b *Browser) error { - return nil + return func(b *Browser) { } } diff --git a/chromedp.go b/chromedp.go index 15412f9..d96ed73 100644 --- a/chromedp.go +++ b/chromedp.go @@ -36,6 +36,11 @@ type Context struct { // have its own unique Target pointing to a separate browser tab (page). Target *Target + // browserOpts holds the browser options passed to NewContext via + // WithBrowserOption, so that they can later be used when allocating a + // browser in Run. + browserOpts []BrowserOption + // cancel simply cancels the context that was used to start Browser. // This is useful to stop all activity and avoid deadlocks if we detect // that the browser was closed or happened to crash. Note that this @@ -56,12 +61,14 @@ type Context struct { func NewContext(parent context.Context, opts ...ContextOption) (context.Context, context.CancelFunc) { ctx, cancel := context.WithCancel(parent) - c := &Context{cancel: cancel} + c := &Context{cancel: cancel, first: true} if pc := FromContext(parent); pc != nil { c.Allocator = pc.Allocator c.Browser = pc.Browser // don't inherit SessionID, so that NewContext can be used to // create a new tab on the same browser. + + c.first = c.Browser == nil } for _, o := range opts { @@ -127,12 +134,11 @@ func Run(ctx context.Context, actions ...Action) error { return ErrInvalidContext } if c.Browser == nil { - browser, err := c.Allocator.Allocate(ctx) + browser, err := c.Allocator.Allocate(ctx, c.browserOpts...) if err != nil { return err } c.Browser = browser - c.first = true } if c.Target == nil { if err := c.newSession(ctx); err != nil { @@ -197,8 +203,31 @@ func (c *Context) newSession(ctx context.Context) error { return nil } +// ContextOption is a context option. type ContextOption func(*Context) +// WithLogf is a shortcut for WithBrowserOption(WithBrowserLogf(f)). +func WithLogf(f func(string, ...interface{})) ContextOption { + return WithBrowserOption(WithBrowserLogf(f)) +} + +// WithErrorf is a shortcut for WithBrowserOption(WithBrowserErrorf(f)). +func WithErrorf(f func(string, ...interface{})) ContextOption { + return WithBrowserOption(WithBrowserErrorf(f)) +} + +// WithBrowserOption allows passing a number of browser options to the allocator +// when allocating a new browser. As such, this context option can only be used +// when NewContext is allocating a new browser. +func WithBrowserOption(opts ...BrowserOption) ContextOption { + return func(c *Context) { + if !c.first { + panic("WithBrowserOption can only be used when allocating a new browser") + } + c.browserOpts = append(c.browserOpts, opts...) + } +} + // Targets lists all the targets in the browser attached to the given context. func Targets(ctx context.Context) ([]*target.Info, error) { // Don't rely on Run, as that needs to be able to call Targets, and we @@ -209,12 +238,11 @@ func Targets(ctx context.Context) ([]*target.Info, error) { return nil, ErrInvalidContext } if c.Browser == nil { - browser, err := c.Allocator.Allocate(ctx) + browser, err := c.Allocator.Allocate(ctx, c.browserOpts...) if err != nil { return nil, err } c.Browser = browser - c.first = true } return target.GetTargets().Do(ctx, c.Browser) } diff --git a/example_test.go b/example_test.go index 64be637..01489ba 100644 --- a/example_test.go +++ b/example_test.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "io/ioutil" + "log" "os" "path/filepath" @@ -48,7 +49,8 @@ func ExampleExecAllocator() { allocCtx, cancel := chromedp.NewExecAllocator(context.Background(), opts...) defer cancel() - taskCtx, cancel := chromedp.NewContext(allocCtx) + // also set up a custom logger + taskCtx, cancel := chromedp.NewContext(allocCtx, chromedp.WithLogf(log.Printf)) defer cancel() // ensure that the browser process is started