Merge pull request #178 from grafana/fix/177-frameAbortedNavigation-nil-pendingDocument

Author: inancgumus

common/frame_manager.go

@@ -137,18 +137,19 @@ func (m *FrameManager) frameAbortedNavigation(frameID cdp.FrameID, errorText, do
 	if documentID != "" && frame.pendingDocument.documentID != documentID {
 		return
 	}
-	frame.pendingDocument = nil
 
 	m.logger.Debugf("FrameManager:frameAbortedNavigation:emit:EventFrameNavigation",
 		"fmid:%d fid:%v err:%s docid:%s fname:%s furl:%s",
 		m.ID(), frameID, errorText, documentID, frame.Name(), frame.URL())
 
-	frame.emit(EventFrameNavigation, &NavigationEvent{
+	ne := &NavigationEvent{
 		url:         frame.URL(),
 		name:        frame.Name(),
 		newDocument: frame.pendingDocument,
 		err:         errors.New(errorText),
-	})
+	}
+	frame.pendingDocument = nil
+	frame.emit(EventFrameNavigation, ne)
 }
 
 func (m *FrameManager) frameAttached(frameID cdp.FrameID, parentFrameID cdp.FrameID) {

common/frame_test.go

@@ -31,7 +31,7 @@ import (
 )
 
 // Test calling Frame.document does not panic with a nil document.
-// See: issue #53 for details.
+// See: Issue #53 for details.
 func TestFrameNilDocument(t *testing.T) {
 	t.Parallel()
 
@@ -80,6 +80,40 @@ func TestFrameNilDocument(t *testing.T) {
 	require.Equal(t, want, got)
 }
 
+// See: Issue #177 for details.
+func TestFrameManagerFrameAbortedNavigationShouldEmitANonNilPendingDocument(t *testing.T) {
+	t.Parallel()
+
+	ctx, log := context.Background(), NewNullLogger()
+
+	// add the frame to frame manager
+	fm := NewFrameManager(ctx, nil, nil, NewTimeoutSettings(nil), log)
+	frame := NewFrame(ctx, fm, nil, cdp.FrameID("42"), log)
+	fm.frames[frame.id] = frame
+
+	// listen for frame navigation events
+	recv := make(chan Event)
+	frame.on(ctx, []string{EventFrameNavigation}, recv)
+
+	// emit the navigation event
+	frame.pendingDocument = &DocumentInfo{
+		documentID: "42",
+	}
+	fm.frameAbortedNavigation(frame.id, "any error", frame.pendingDocument.documentID)
+
+	// receive the emitted event and verify that emitted document
+	// is not nil.
+	e := <-recv
+	require.IsType(t, &NavigationEvent{}, e.data, "event should be a navigation event")
+	ne := e.data.(*NavigationEvent)
+	require.NotNil(t, ne, "event should not be nil")
+	require.NotNil(t, ne.newDocument, "emitted document should not be nil")
+
+	// since the navigation is aborted, the aborting frame should have
+	// a nil pending document.
+	require.Nil(t, frame.pendingDocument)
+}
+
 type executionContextTestStub struct {
 	ExecutionContext
 	evaluateFn func(apiCtx context.Context, opts evaluateOptions, pageFunc goja.Value, args ...goja.Value) (res interface{}, err error)