simplify the allocator API

Exposing NewAllocator and AllocatorOption was unnecessary, and it made
the API more complex to use and understand.

Instead, have users call NewExecAllocator directly. This removes some
code, and simplifies the examples and tests.
This commit is contained in:
Daniel Martí 2019-04-08 12:10:59 +02:00
parent 687cf6d766
commit d0484ed1c5
5 changed files with 27 additions and 36 deletions

View File

@ -31,30 +31,9 @@ type Allocator interface {
Wait() Wait()
} }
// NewAllocator creates a new allocator context, suitable for use with // setupExecAllocator is similar to NewExecAllocator, but it allows NewContext
// NewContext or Run. // to create the allocator without the unnecessary context layer.
func NewAllocator(parent context.Context, opts ...AllocatorOption) (context.Context, context.CancelFunc) { func setupExecAllocator(opts ...ExecAllocatorOption) *ExecAllocator {
ctx, cancel := context.WithCancel(parent)
c := &Context{}
for _, o := range opts {
o(&c.Allocator)
}
ctx = context.WithValue(ctx, contextKey{}, c)
cancelWait := func() {
cancel()
c.Allocator.Wait()
}
return ctx, cancelWait
}
// AllocatorOption is a allocator option.
type AllocatorOption func(*Allocator)
// WithExecAllocator returns an AllocatorOption which sets up an ExecAllocator.
func WithExecAllocator(opts ...ExecAllocatorOption) func(*Allocator) {
return func(p *Allocator) {
ep := &ExecAllocator{ ep := &ExecAllocator{
initFlags: make(map[string]interface{}), initFlags: make(map[string]interface{}),
} }
@ -64,8 +43,21 @@ func WithExecAllocator(opts ...ExecAllocatorOption) func(*Allocator) {
if ep.execPath == "" { if ep.execPath == "" {
ep.execPath = findExecPath() ep.execPath = findExecPath()
} }
*p = ep return ep
} }
// NewExecAllocator creates a new context set up with an ExecAllocator, suitable
// for use with NewContext or Run.
func NewExecAllocator(parent context.Context, opts ...ExecAllocatorOption) (context.Context, context.CancelFunc) {
ctx, cancel := context.WithCancel(parent)
c := &Context{Allocator: setupExecAllocator(opts...)}
ctx = context.WithValue(ctx, contextKey{}, c)
cancelWait := func() {
cancel()
c.Allocator.Wait()
}
return ctx, cancelWait
} }
// ExecAllocatorOption is a exec allocator option. // ExecAllocatorOption is a exec allocator option.

View File

@ -9,7 +9,7 @@ import (
func TestExecAllocator(t *testing.T) { func TestExecAllocator(t *testing.T) {
t.Parallel() t.Parallel()
poolCtx, poolCancel := NewAllocator(context.Background(), WithExecAllocator(allocOpts...)) poolCtx, poolCancel := NewExecAllocator(context.Background(), allocOpts...)
defer poolCancel() defer poolCancel()
// TODO: test that multiple child contexts are run in different // TODO: test that multiple child contexts are run in different
@ -41,7 +41,7 @@ func TestExecAllocator(t *testing.T) {
func TestExecAllocatorCancelParent(t *testing.T) { func TestExecAllocatorCancelParent(t *testing.T) {
t.Parallel() t.Parallel()
poolCtx, poolCancel := NewAllocator(context.Background(), WithExecAllocator(allocOpts...)) poolCtx, poolCancel := NewExecAllocator(context.Background(), allocOpts...)
defer poolCancel() defer poolCancel()
// TODO: test that multiple child contexts are run in different // TODO: test that multiple child contexts are run in different

View File

@ -53,7 +53,7 @@ func TestMain(m *testing.M) {
allocOpts = append(allocOpts, NoSandbox) allocOpts = append(allocOpts, NoSandbox)
} }
allocCtx, cancel := NewAllocator(context.Background(), WithExecAllocator(allocOpts...)) allocCtx, cancel := NewExecAllocator(context.Background(), allocOpts...)
// start the browser // start the browser
browserCtx, _ = NewContext(allocCtx) browserCtx, _ = NewContext(allocCtx)

View File

@ -62,11 +62,11 @@ func NewContext(parent context.Context, opts ...ContextOption) (context.Context,
o(c) o(c)
} }
if c.Allocator == nil { if c.Allocator == nil {
WithExecAllocator( c.Allocator = setupExecAllocator(
NoFirstRun, NoFirstRun,
NoDefaultBrowserCheck, NoDefaultBrowserCheck,
Headless, Headless,
)(&c.Allocator) )
} }
ctx = context.WithValue(ctx, contextKey{}, c) ctx = context.WithValue(ctx, contextKey{}, c)
@ -113,8 +113,8 @@ func FromContext(ctx context.Context) *Context {
} }
// Run runs an action against the provided context. The provided context must // Run runs an action against the provided context. The provided context must
// contain a valid Allocator; typically, that will be created via NewContext or // contain a valid Allocator; typically, that will be created via NewContext, or
// NewAllocator. // via one of the allocator constructors like NewExecAllocator.
func Run(ctx context.Context, actions ...Action) error { func Run(ctx context.Context, actions ...Action) error {
c := FromContext(ctx) c := FromContext(ctx)
if c == nil || c.Allocator == nil { if c == nil || c.Allocator == nil {

View File

@ -45,8 +45,7 @@ func ExampleExecAllocator() {
chromedp.UserDataDir(dir), chromedp.UserDataDir(dir),
} }
allocCtx, cancel := chromedp.NewAllocator(context.Background(), allocCtx, cancel := chromedp.NewExecAllocator(context.Background(), opts...)
chromedp.WithExecAllocator(opts...))
defer cancel() defer cancel()
taskCtx, cancel := chromedp.NewContext(allocCtx) taskCtx, cancel := chromedp.NewContext(allocCtx)