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

Fix potential nil exception when getting frame #1152

Merged
merged 3 commits into from
Jan 11, 2024
Merged

Fix potential nil exception when getting frame #1152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d15ae71
Fix potential nil exception when getting frame
ankur22 Jan 10, 2024
13b9e19
Apply suggestions from code review
ankur22 Jan 11, 2024
5bb8c60
Refactor found to ok
ankur22 Jan 11, 2024
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
22 changes: 16 additions & 6 deletions common/element_handle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,13 +440,13 @@ func (h *ElementHandle) ownerFrame(apiCtx context.Context) *Frame {
if frameId == "" {
return nil
}
frame := h.frame.page.frameManager.getFrameByID(frameId)
if frame != nil {
frame, ok := h.frame.page.frameManager.getFrameByID(frameId)
if ok {
return frame
}
for _, page := range h.frame.page.browserCtx.browser.pages {
frame = page.frameManager.getFrameByID(frameId)
if frame != nil {
frame, ok = page.frameManager.getFrameByID(frameId)
if ok {
return frame
}
}
Expand Down Expand Up @@ -766,7 +766,12 @@ func (h *ElementHandle) ContentFrame() (*Frame, error) {
return nil, fmt.Errorf("element is not an iframe")
}

return h.frame.manager.getFrameByID(node.FrameID), nil
frame, ok := h.frame.manager.getFrameByID(node.FrameID)
if !ok {
return nil, fmt.Errorf("frame not found for id %s", node.FrameID)
}

return frame, nil
}

func (h *ElementHandle) Dblclick(opts goja.Value) {
Expand Down Expand Up @@ -1003,7 +1008,12 @@ func (h *ElementHandle) OwnerFrame() (*Frame, error) {
return nil, fmt.Errorf("no frame found for node: %w", err)
}

return h.frame.manager.getFrameByID(node.FrameID), nil
frame, ok := h.frame.manager.getFrameByID(node.FrameID)
if !ok {
return nil, fmt.Errorf("no frame found for id %s", node.FrameID)
}

return frame, nil
}

func (h *ElementHandle) Press(key string, opts goja.Value) {
Expand Down
4 changes: 2 additions & 2 deletions common/frame.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,8 @@ func (f *Frame) onLoadingStopped() {
}

func (f *Frame) position() (*Position, error) {
frame := f.manager.getFrameByID(cdp.FrameID(f.page.targetID))
if frame == nil {
frame, ok := f.manager.getFrameByID(cdp.FrameID(f.page.targetID))
if !ok {
return nil, fmt.Errorf("could not find frame with id %s", f.page.targetID)
}
if frame == f.page.frameManager.MainFrame() {
Expand Down
25 changes: 15 additions & 10 deletions common/frame_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,8 @@ func (m *FrameManager) frameAttached(frameID cdp.FrameID, parentFrameID cdp.Fram
func (m *FrameManager) frameDetached(frameID cdp.FrameID, reason cdppage.FrameDetachedReason) {
m.logger.Debugf("FrameManager:frameDetached", "fmid:%d fid:%v", m.ID(), frameID)

frame := m.getFrameByID(frameID)
if frame == nil {
frame, ok := m.getFrameByID(frameID)
if !ok {
m.logger.Debugf("FrameManager:frameDetached:return",
"fmid:%d fid:%v cannot find frame",
m.ID(), frameID)
Expand All @@ -190,8 +190,8 @@ func (m *FrameManager) frameLifecycleEvent(frameID cdp.FrameID, event LifecycleE
"fmid:%d fid:%v event:%s",
m.ID(), frameID, lifecycleEventToString[event])

frame := m.getFrameByID(frameID)
if frame != nil {
frame, ok := m.getFrameByID(frameID)
if ok {
frame.onLifecycleEvent(event)
}
}
Expand All @@ -200,8 +200,8 @@ func (m *FrameManager) frameLoadingStarted(frameID cdp.FrameID) {
m.logger.Debugf("FrameManager:frameLoadingStarted",
"fmid:%d fid:%v", m.ID(), frameID)

frame := m.getFrameByID(frameID)
if frame != nil {
frame, ok := m.getFrameByID(frameID)
if ok {
frame.onLoadingStarted()
}
}
Expand All @@ -210,8 +210,8 @@ func (m *FrameManager) frameLoadingStopped(frameID cdp.FrameID) {
m.logger.Debugf("FrameManager:frameLoadingStopped",
"fmid:%d fid:%v", m.ID(), frameID)

frame := m.getFrameByID(frameID)
if frame != nil {
frame, ok := m.getFrameByID(frameID)
if ok {
frame.onLoadingStopped()
}
}
Expand Down Expand Up @@ -390,10 +390,15 @@ func (m *FrameManager) frameRequestedNavigation(frameID cdp.FrameID, url string,
return nil
}

func (m *FrameManager) getFrameByID(id cdp.FrameID) *Frame {
// getFrameByID finds a frame with id. If found, it returns the frame and true,
// otherwise, it returns nil and false.
func (m *FrameManager) getFrameByID(id cdp.FrameID) (*Frame, bool) {
m.framesMu.RLock()
defer m.framesMu.RUnlock()
return m.frames[id]

frame, ok := m.frames[id]

return frame, ok
inancgumus marked this conversation as resolved.
Show resolved Hide resolved
}

func (m *FrameManager) removeChildFramesRecursively(frame *Frame) {
Expand Down
23 changes: 13 additions & 10 deletions common/frame_session.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,10 @@ func (fs *FrameSession) initIsolatedWorld(name string) error {
if fs.isMainFrame() {
frames = fs.manager.Frames()
} else {
frames = []*Frame{fs.manager.getFrameByID(cdp.FrameID(fs.targetID))}
frame, ok := fs.manager.getFrameByID(cdp.FrameID(fs.targetID))
if ok {
frames = []*Frame{frame}
}
}
for _, frame := range frames {
// A frame could have been removed before we execute this, so don't wait around for a reply.
Expand Down Expand Up @@ -659,8 +662,8 @@ func (fs *FrameSession) onExecutionContextCreated(event *cdpruntime.EventExecuti
k6ext.Panic(fs.ctx, "unmarshaling executionContextCreated event JSON: %w", err)
}
var world executionWorld
frame := fs.manager.getFrameByID(i.FrameID)
if frame != nil {
frame, ok := fs.manager.getFrameByID(i.FrameID)
if ok {
if i.IsDefault {
world = mainWorld
} else if event.Context.Name == utilityWorldName && !frame.hasContext(utilityWorld) {
Expand Down Expand Up @@ -760,12 +763,12 @@ func (fs *FrameSession) onFrameNavigated(frame *cdp.Frame, initial bool) {
)

var (
spanID = fs.mainFrameSpan.SpanContext().SpanID().String()
newFrame = fs.manager.getFrameByID(frame.ID)
spanID = fs.mainFrameSpan.SpanContext().SpanID().String()
newFrame, ok = fs.manager.getFrameByID(frame.ID)
)

// Only set the k6SpanId reference if it's a new frame.
if newFrame == nil {
if !ok {
return
}

Expand Down Expand Up @@ -851,8 +854,8 @@ func (fs *FrameSession) onPageLifecycle(event *cdppage.EventLifecycleEvent) {
"sid:%v tid:%v fid:%v event:%s eventTime:%q",
fs.session.ID(), fs.targetID, event.FrameID, event.Name, event.Timestamp.Time())

frame := fs.manager.getFrameByID(event.FrameID)
if frame == nil {
_, ok := fs.manager.getFrameByID(event.FrameID)
if !ok {
return
}

Expand Down Expand Up @@ -953,8 +956,8 @@ func (fs *FrameSession) onAttachedToTarget(event *target.EventAttachedToTarget)

// attachIFrameToTarget attaches an IFrame target to a given session.
func (fs *FrameSession) attachIFrameToTarget(ti *target.Info, sid target.SessionID) error {
fr := fs.manager.getFrameByID(cdp.FrameID(ti.TargetID))
if fr == nil {
fr, ok := fs.manager.getFrameByID(cdp.FrameID(ti.TargetID))
if !ok {
// IFrame should be attached to fs.page with EventFrameAttached
// event before.
fs.logger.Debugf("FrameSession:attachIFrameToTarget:return",
Expand Down
5 changes: 3 additions & 2 deletions common/network_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,10 +458,11 @@ func (m *NetworkManager) onRequest(event *network.EventRequestWillBeSent, interc
}

var frame *Frame = nil
var ok bool
if event.FrameID != "" {
frame = m.frameManager.getFrameByID(event.FrameID)
frame, ok = m.frameManager.getFrameByID(event.FrameID)
}
if frame == nil {
if !ok {
m.logger.Debugf("NetworkManager:onRequest", "url:%s method:%s type:%s fid:%s frame is nil",
event.Request.URL, event.Request.Method, event.Initiator.Type, event.FrameID)
}
Expand Down