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.

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

Async JSHandle prepare #1365

Merged
merged 3 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion browser/js_handle_mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func mapJSHandle(vu moduleVU, jsh common.JSHandleAPI) mapping {
return rt.ToValue(m).ToObject(rt)
},
"dispose": jsh.Dispose,
"evaluate": func(pageFunc goja.Value, gargs ...goja.Value) any {
"evaluate": func(pageFunc goja.Value, gargs ...goja.Value) (any, error) {
args := make([]any, 0, len(gargs))
for _, a := range gargs {
args = append(args, exportArg(a))
Expand Down
59 changes: 41 additions & 18 deletions common/element_handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ func (h *ElementHandle) boundingBox() (*Rect, error) {
}

func (h *ElementHandle) checkHitTargetAt(apiCtx context.Context, point Position) (bool, error) {
frame := h.ownerFrame(apiCtx)
frame, err := h.ownerFrame(apiCtx)
if err != nil {
return false, fmt.Errorf("checking hit target at %v: %w", point, err)
}
if frame != nil && frame.parentFrame != nil {
el, err := h.frame.FrameElement()
if err != nil {
Expand Down Expand Up @@ -424,22 +427,26 @@ func (h *ElementHandle) offsetPosition(apiCtx context.Context, offset *Position)
}, nil
}

func (h *ElementHandle) ownerFrame(apiCtx context.Context) *Frame {
frameId := h.frame.page.getOwnerFrame(apiCtx, h)
if frameId == "" {
return nil
func (h *ElementHandle) ownerFrame(apiCtx context.Context) (*Frame, error) {
frameID, err := h.frame.page.getOwnerFrame(apiCtx, h)
if err != nil {
return nil, err
}
if frameID == "" {
return nil, nil //nolint:nilnil
}
frame, ok := h.frame.page.frameManager.getFrameByID(frameId)
frame, ok := h.frame.page.frameManager.getFrameByID(frameID)
if ok {
return frame
return frame, nil
}
for _, page := range h.frame.page.browserCtx.browser.pages {
frame, ok = page.frameManager.getFrameByID(frameId)
frame, ok = page.frameManager.getFrameByID(frameID)
if ok {
return frame
return frame, nil
}
}
return nil

return nil, nil //nolint:nilnil
}

func (h *ElementHandle) scrollRectIntoViewIfNeeded(apiCtx context.Context, rect *dom.Rect) error {
Expand Down Expand Up @@ -1011,7 +1018,7 @@ func (h *ElementHandle) IsVisible() (bool, error) {
}

// OwnerFrame returns the frame containing this element.
func (h *ElementHandle) OwnerFrame() (*Frame, error) {
func (h *ElementHandle) OwnerFrame() (_ *Frame, rerr error) {
fn := `
(node, injected) => {
return injected.getDocumentElement(node);
Expand All @@ -1033,7 +1040,13 @@ func (h *ElementHandle) OwnerFrame() (*Frame, error) {
if !ok {
return nil, fmt.Errorf("unexpected result type while getting document element: %T", res)
}
defer documentHandle.Dispose()
defer func() {
if err := documentHandle.Dispose(); err != nil {
err = fmt.Errorf("disposing document element: %w", err)
rerr = errors.Join(err, rerr)
}
}()

inancgumus marked this conversation as resolved.
Show resolved Hide resolved
if documentHandle.remoteObject.ObjectID == "" {
return nil, err
}
Expand Down Expand Up @@ -1079,7 +1092,7 @@ func (h *ElementHandle) Press(key string, opts goja.Value) error {

// Query runs "element.querySelector" within the page. If no element matches the selector,
// the return value resolves to "null".
func (h *ElementHandle) Query(selector string, strict bool) (*ElementHandle, error) {
func (h *ElementHandle) Query(selector string, strict bool) (_ *ElementHandle, rerr error) {
parsedSelector, err := NewSelector(selector)
if err != nil {
return nil, fmt.Errorf("parsing selector %q: %w", selector, err)
Expand All @@ -1106,7 +1119,12 @@ func (h *ElementHandle) Query(selector string, strict bool) (*ElementHandle, err
}
element := handle.AsElement()
if element == nil {
handle.Dispose()
defer func() {
if err := handle.Dispose(); err != nil {
err = fmt.Errorf("disposing element handle: %w", err)
rerr = errors.Join(err, rerr)
}
}()
return nil, fmt.Errorf("querying selector %q", selector)
}

Expand All @@ -1124,7 +1142,7 @@ func (h *ElementHandle) QueryAll(selector string) ([]*ElementHandle, error) {
return handles, nil
}

func (h *ElementHandle) queryAll(selector string, eval evalFunc) ([]*ElementHandle, error) {
func (h *ElementHandle) queryAll(selector string, eval evalFunc) (_ []*ElementHandle, rerr error) {
parsedSelector, err := NewSelector(selector)
if err != nil {
return nil, fmt.Errorf("parsing selector %q: %w", selector, err)
Expand All @@ -1147,7 +1165,12 @@ func (h *ElementHandle) queryAll(selector string, eval evalFunc) ([]*ElementHand
if !ok {
return nil, fmt.Errorf("getting element handle for selector %q: %w", selector, ErrJSHandleInvalid)
}
defer handles.Dispose()
defer func() {
if err := handles.Dispose(); err != nil {
err = fmt.Errorf("disposing element handles: %w", err)
rerr = errors.Join(err, rerr)
}
}()

props, err := handles.GetProperties()
if err != nil {
Expand All @@ -1159,8 +1182,8 @@ func (h *ElementHandle) queryAll(selector string, eval evalFunc) ([]*ElementHand
for _, prop := range props {
if el := prop.AsElement(); el != nil {
els = append(els, el)
} else {
prop.Dispose()
} else if err := prop.Dispose(); err != nil {
return nil, fmt.Errorf("disposing property while querying all selectors %q: %w", selector, err)
}
}

Expand Down
3 changes: 2 additions & 1 deletion common/element_handle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,9 @@ func (s *jsHandleStub) AsElement() *ElementHandle {
return s.asElementFn()
}

func (s *jsHandleStub) Dispose() {
func (s *jsHandleStub) Dispose() error {
s.disposeCalls++
return nil
}

func (s *jsHandleStub) GetProperties() (map[string]JSHandleAPI, error) {
Expand Down
23 changes: 17 additions & 6 deletions common/frame.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,20 +228,26 @@ func (f *Frame) clearLifecycle() {
f.inflightRequestsMu.Unlock()
}

func (f *Frame) detach() {
func (f *Frame) detach() error {
f.log.Debugf("Frame:detach", "fid:%s furl:%q", f.ID(), f.URL())

f.setDetached(true)
if f.parentFrame != nil {
f.parentFrame.removeChildFrame(f)
}
f.parentFrame = nil

// detach() is called by the same frame Goroutine that manages execution
// context switches. so this should be safe.
// we don't need to protect the following with executionContextMu.
if f.documentHandle != nil {
f.documentHandle.Dispose()
if f.documentHandle == nil {
return nil
}
if err := f.documentHandle.Dispose(); err != nil {
return fmt.Errorf("disposing document handle while detaching frame: %w", err)
}

return nil
}

func (f *Frame) defaultTimeout() time.Duration {
Expand Down Expand Up @@ -516,7 +522,7 @@ func (f *Frame) waitForSelectorRetry(
return nil, err
}

func (f *Frame) waitForSelector(selector string, opts *FrameWaitForSelectorOptions) (*ElementHandle, error) {
func (f *Frame) waitForSelector(selector string, opts *FrameWaitForSelectorOptions) (_ *ElementHandle, rerr error) {
f.log.Debugf("Frame:waitForSelector", "fid:%s furl:%q sel:%q", f.ID(), f.URL(), selector)

document, err := f.document()
Expand All @@ -543,9 +549,14 @@ func (f *Frame) waitForSelector(selector string, opts *FrameWaitForSelectorOptio
// an element should belong to the current execution context.
// otherwise, we should adopt it to this execution context.
if ec != handle.execCtx {
defer handle.Dispose()
defer func() {
if err := handle.Dispose(); err != nil {
err = fmt.Errorf("disposing element handle: %w", err)
rerr = errors.Join(err, rerr)
}
}()
if handle, err = ec.adoptElementHandle(handle); err != nil {
return nil, fmt.Errorf("adopting element handle while waiting for selector %q: %w", selector, err)
return nil, fmt.Errorf("waiting for selector %q: adopting element handle: %w", selector, err)
}
}

Expand Down
36 changes: 24 additions & 12 deletions common/frame_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,15 +162,15 @@ func (m *FrameManager) frameAttached(frameID cdp.FrameID, parentFrameID cdp.Fram
}
}

func (m *FrameManager) frameDetached(frameID cdp.FrameID, reason cdppage.FrameDetachedReason) {
func (m *FrameManager) frameDetached(frameID cdp.FrameID, reason cdppage.FrameDetachedReason) error {
m.logger.Debugf("FrameManager:frameDetached", "fmid:%d fid:%v", m.ID(), frameID)

frame, ok := m.getFrameByID(frameID)
if !ok {
m.logger.Debugf("FrameManager:frameDetached:return",
"fmid:%d fid:%v cannot find frame",
m.ID(), frameID)
return
return nil
}

// This helps prevent an iframe and its child frames from being removed
Expand All @@ -187,7 +187,7 @@ func (m *FrameManager) frameDetached(frameID cdp.FrameID, reason cdppage.FrameDe
m.logger.Debugf("FrameManager:frameDetached:notSameSession:return",
"fmid:%d fid:%v event session and frame session do not match",
m.ID(), frameID)
return
return nil
}
}

Expand All @@ -196,11 +196,10 @@ func (m *FrameManager) frameDetached(frameID cdp.FrameID, reason cdppage.FrameDe
// frame, we want to keep the current frame which is
// still referenced by the (incoming) remote frame, but
// remove all its child frames.
m.removeChildFramesRecursively(frame)
return
return m.removeChildFramesRecursively(frame)
}

m.removeFramesRecursively(frame)
return m.removeFramesRecursively(frame)
}

func (m *FrameManager) frameLifecycleEvent(frameID cdp.FrameID, event LifecycleEvent) {
Expand Down Expand Up @@ -257,7 +256,10 @@ func (m *FrameManager) frameNavigated(frameID cdp.FrameID, parentFrameID cdp.Fra
if frame != nil {
m.framesMu.Unlock()
for _, child := range frame.ChildFrames() {
m.removeFramesRecursively(child)
if err := m.removeFramesRecursively(child); err != nil {
m.framesMu.Lock()
inancgumus marked this conversation as resolved.
Show resolved Hide resolved
return fmt.Errorf("removing child frames recursively: %w", err)
}
}
m.framesMu.Lock()
}
Expand Down Expand Up @@ -419,22 +421,30 @@ func (m *FrameManager) getFrameByID(id cdp.FrameID) (*Frame, bool) {
return frame, ok
}

func (m *FrameManager) removeChildFramesRecursively(frame *Frame) {
func (m *FrameManager) removeChildFramesRecursively(frame *Frame) error {
for _, child := range frame.ChildFrames() {
m.removeFramesRecursively(child)
if err := m.removeFramesRecursively(child); err != nil {
return fmt.Errorf("removing child frames recursively: %w", err)
}
}

return nil
}

func (m *FrameManager) removeFramesRecursively(frame *Frame) {
func (m *FrameManager) removeFramesRecursively(frame *Frame) error {
for _, child := range frame.ChildFrames() {
m.logger.Debugf("FrameManager:removeFramesRecursively",
"fmid:%d cfid:%v pfid:%v cfname:%s cfurl:%s",
m.ID(), child.ID(), frame.ID(), child.Name(), child.URL())

m.removeFramesRecursively(child)
if err := m.removeFramesRecursively(child); err != nil {
return fmt.Errorf("removing frames recursively: %w", err)
}
}

frame.detach()
if err := frame.detach(); err != nil {
return fmt.Errorf("removing frames recursively: detaching frame: %w", err)
}

m.framesMu.Lock()
m.logger.Debugf("FrameManager:removeFramesRecursively:delParentFrame",
Expand All @@ -451,6 +461,8 @@ func (m *FrameManager) removeFramesRecursively(frame *Frame) {

m.page.emit(EventPageFrameDetached, frame)
}

return nil
}

func (m *FrameManager) requestFailed(req *Request, canceled bool) {
Expand Down
4 changes: 3 additions & 1 deletion common/frame_session.go
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,9 @@ func (fs *FrameSession) onFrameDetached(frameID cdp.FrameID, reason cdppage.Fram
"sid:%v tid:%v fid:%v reason:%s",
fs.session.ID(), fs.targetID, frameID, reason)

fs.manager.frameDetached(frameID, reason)
if err := fs.manager.frameDetached(frameID, reason); err != nil {
k6ext.Panic(fs.ctx, "handling frameDetached event: %w", err)
}
}

func (fs *FrameSession) onFrameNavigated(frame *cdp.Frame, initial bool) {
Expand Down
Loading
Loading