content: defer oauth popup parent reload

commit: 92a8691b7481b2d8f82b0fde51718991ad8eca62
author: brice · 6mo ago · 2026-04-22 16:12 UTC
parent: a814296
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +123 -4

M internal/ui/coordinator/content/oauth.go +92 -1


@@ -386,16 +386,36 @@ func (c *Coordinator) handlePopupOAuthClose(ctx context.Context, popupID port.We
386	386		c.scheduleParentPaneOAuthResume(ctx, state.ParentPaneID, popupID, state.ParentURIAtOpen, state.CallbackURI)
387	387		}
388	388
	389	+	const oauthParentResumeGraceRetries = 5
	390	+
389	391		func (c *Coordinator) scheduleParentPaneOAuthResume(
390	392		ctx context.Context,
391	393		parentPaneID entity.PaneID,
392	394		popupID port.WebViewID,
393	395		parentURIAtOpen string,
394	396		callbackURI string,
	397	+	) {
	398	+	c.scheduleParentPaneOAuthResumeAttempt(
	399	+	ctx,
	400	+	parentPaneID,
	401	+	popupID,
	402	+	parentURIAtOpen,
	403	+	callbackURI,
	404	+	oauthParentResumeGraceRetries,
	405	+	)
	406	+	}
	407	+
	408	+	func (c *Coordinator) scheduleParentPaneOAuthResumeAttempt(
	409	+	ctx context.Context,
	410	+	parentPaneID entity.PaneID,
	411	+	popupID port.WebViewID,
	412	+	parentURIAtOpen string,
	413	+	callbackURI string,
	414	+	remainingGrace int,
395	415		) {
396	416		c.ensurePopupManager().schedulePopupRefresh(parentPaneID, oauthParentRefreshDebounce, func() {
397	417		cb := glib.SourceFunc(func(_ uintptr) bool {
398		-	c.resumeParentPaneAfterOAuth(ctx, parentPaneID, popupID, parentURIAtOpen, callbackURI)
	418	+	c.resumeParentPaneAfterOAuthAttempt(ctx, parentPaneID, popupID, parentURIAtOpen, callbackURI, remainingGrace)
399	419		return false
400	420		})
401	421		glib.IdleAdd(&cb, 0)

@@ -408,6 +428,24 @@ func (c *Coordinator) resumeParentPaneAfterOAuth(
408	428		popupID port.WebViewID,
409	429		parentURIAtOpen string,
410	430		callbackURI string,
	431	+	) {
	432	+	c.resumeParentPaneAfterOAuthAttempt(
	433	+	ctx,
	434	+	parentPaneID,
	435	+	popupID,
	436	+	parentURIAtOpen,
	437	+	callbackURI,
	438	+	oauthParentResumeGraceRetries,
	439	+	)
	440	+	}
	441	+
	442	+	func (c *Coordinator) resumeParentPaneAfterOAuthAttempt(
	443	+	ctx context.Context,
	444	+	parentPaneID entity.PaneID,
	445	+	popupID port.WebViewID,
	446	+	parentURIAtOpen string,
	447	+	callbackURI string,
	448	+	remainingGrace int,
411	449		) {
412	450		log := logging.FromContext(ctx)
413	451		wv := c.getWebViewLocked(parentPaneID)

@@ -431,6 +469,25 @@ func (c *Coordinator) resumeParentPaneAfterOAuth(
431	469		return
432	470		}
433	471
	472	+	if shouldGraceWaitForParentPaneOAuthResume(parentURIAtOpen, parentURI, callbackURI, remainingGrace) {
	473	+	log.Debug().
	474	+	Str("parent_pane_id", string(parentPaneID)).
	475	+	Uint64("popup_id", uint64(popupID)).
	476	+	Int("remaining_grace", remainingGrace).
	477	+	Str("parent_uri_at_open", logging.TruncateURL(parentURIAtOpen, logURLMaxLen)).
	478	+	Str("callback_uri", logging.TruncateURL(callbackURI, logURLMaxLen)).
	479	+	Msg("deferring parent pane oauth resume to allow in-flight same-site navigation to settle")
	480	+	c.scheduleParentPaneOAuthResumeAttempt(
	481	+	ctx,
	482	+	parentPaneID,
	483	+	popupID,
	484	+	parentURIAtOpen,
	485	+	callbackURI,
	486	+	remainingGrace-1,
	487	+	)
	488	+	return
	489	+	}
	490	+
434	491		if err := wv.Reload(ctx); err != nil {
435	492		log.Warn().
436	493		Err(err).

@@ -446,3 +503,37 @@ func (c *Coordinator) resumeParentPaneAfterOAuth(
446	503		Str("callback_uri", logging.TruncateURL(callbackURI, logURLMaxLen)).
447	504		Msg("refreshed parent pane after oauth popup close")
448	505		}
	506	+
	507	+	func shouldGraceWaitForParentPaneOAuthResume(
	508	+	parentURIAtOpen string,
	509	+	currentParentURI string,
	510	+	callbackURI string,
	511	+	remainingGrace int,
	512	+	) bool {
	513	+	if remainingGrace <= 0 {
	514	+	return false
	515	+	}
	516	+	if parentURIAtOpen == "" \|\| currentParentURI == "" \|\| callbackURI == "" {
	517	+	return false
	518	+	}
	519	+	if currentParentURI != parentURIAtOpen {
	520	+	return false
	521	+	}
	522	+	return sameOAuthResumeSite(parentURIAtOpen, callbackURI)
	523	+	}
	524	+
	525	+	func sameOAuthResumeSite(a string, b string) bool {
	526	+	hostA := normalizeOAuthResumeHost(a)
	527	+	hostB := normalizeOAuthResumeHost(b)
	528	+	return hostA != "" && hostA == hostB
	529	+	}
	530	+
	531	+	func normalizeOAuthResumeHost(raw string) string {
	532	+	parsed, err := neturl.Parse(raw)
	533	+	if err != nil {
	534	+	return ""
	535	+	}
	536	+	host := strings.ToLower(strings.TrimSpace(parsed.Hostname()))
	537	+	host = strings.TrimPrefix(host, "www.")
	538	+	return host
	539	+	}


@@ -386,16 +386,36 @@ func (c *Coordinator) handlePopupOAuthClose(ctx context.Context, popupID port.We
386		c.scheduleParentPaneOAuthResume(ctx, state.ParentPaneID, popupID, state.ParentURIAtOpen, state.CallbackURI)	386		c.scheduleParentPaneOAuthResume(ctx, state.ParentPaneID, popupID, state.ParentURIAtOpen, state.CallbackURI)
387		}	387		}
388			388
			389	+	const oauthParentResumeGraceRetries = 5
			390	+
389		func (c *Coordinator) scheduleParentPaneOAuthResume(	391		func (c *Coordinator) scheduleParentPaneOAuthResume(
390		ctx context.Context,	392		ctx context.Context,
391		parentPaneID entity.PaneID,	393		parentPaneID entity.PaneID,
392		popupID port.WebViewID,	394		popupID port.WebViewID,
393		parentURIAtOpen string,	395		parentURIAtOpen string,
394		callbackURI string,	396		callbackURI string,
			397	+	) {
			398	+	c.scheduleParentPaneOAuthResumeAttempt(
			399	+	ctx,
			400	+	parentPaneID,
			401	+	popupID,
			402	+	parentURIAtOpen,
			403	+	callbackURI,
			404	+	oauthParentResumeGraceRetries,
			405	+	)
			406	+	}
			407	+
			408	+	func (c *Coordinator) scheduleParentPaneOAuthResumeAttempt(
			409	+	ctx context.Context,
			410	+	parentPaneID entity.PaneID,
			411	+	popupID port.WebViewID,
			412	+	parentURIAtOpen string,
			413	+	callbackURI string,
			414	+	remainingGrace int,
395		) {	415		) {
396		c.ensurePopupManager().schedulePopupRefresh(parentPaneID, oauthParentRefreshDebounce, func() {	416		c.ensurePopupManager().schedulePopupRefresh(parentPaneID, oauthParentRefreshDebounce, func() {
397		cb := glib.SourceFunc(func(_ uintptr) bool {	417		cb := glib.SourceFunc(func(_ uintptr) bool {
398	-	c.resumeParentPaneAfterOAuth(ctx, parentPaneID, popupID, parentURIAtOpen, callbackURI)	418	+	c.resumeParentPaneAfterOAuthAttempt(ctx, parentPaneID, popupID, parentURIAtOpen, callbackURI, remainingGrace)
399		return false	419		return false
400		})	420		})
401		glib.IdleAdd(&cb, 0)	421		glib.IdleAdd(&cb, 0)

@@ -408,6 +428,24 @@ func (c *Coordinator) resumeParentPaneAfterOAuth(
408		popupID port.WebViewID,	428		popupID port.WebViewID,
409		parentURIAtOpen string,	429		parentURIAtOpen string,
410		callbackURI string,	430		callbackURI string,
			431	+	) {
			432	+	c.resumeParentPaneAfterOAuthAttempt(
			433	+	ctx,
			434	+	parentPaneID,
			435	+	popupID,
			436	+	parentURIAtOpen,
			437	+	callbackURI,
			438	+	oauthParentResumeGraceRetries,
			439	+	)
			440	+	}
			441	+
			442	+	func (c *Coordinator) resumeParentPaneAfterOAuthAttempt(
			443	+	ctx context.Context,
			444	+	parentPaneID entity.PaneID,
			445	+	popupID port.WebViewID,
			446	+	parentURIAtOpen string,
			447	+	callbackURI string,
			448	+	remainingGrace int,
411		) {	449		) {
412		log := logging.FromContext(ctx)	450		log := logging.FromContext(ctx)
413		wv := c.getWebViewLocked(parentPaneID)	451		wv := c.getWebViewLocked(parentPaneID)

@@ -431,6 +469,25 @@ func (c *Coordinator) resumeParentPaneAfterOAuth(
431		return	469		return
432		}	470		}
433			471
			472	+	if shouldGraceWaitForParentPaneOAuthResume(parentURIAtOpen, parentURI, callbackURI, remainingGrace) {
			473	+	log.Debug().
			474	+	Str("parent_pane_id", string(parentPaneID)).
			475	+	Uint64("popup_id", uint64(popupID)).
			476	+	Int("remaining_grace", remainingGrace).
			477	+	Str("parent_uri_at_open", logging.TruncateURL(parentURIAtOpen, logURLMaxLen)).
			478	+	Str("callback_uri", logging.TruncateURL(callbackURI, logURLMaxLen)).
			479	+	Msg("deferring parent pane oauth resume to allow in-flight same-site navigation to settle")
			480	+	c.scheduleParentPaneOAuthResumeAttempt(
			481	+	ctx,
			482	+	parentPaneID,
			483	+	popupID,
			484	+	parentURIAtOpen,
			485	+	callbackURI,
			486	+	remainingGrace-1,
			487	+	)
			488	+	return
			489	+	}
			490	+
434		if err := wv.Reload(ctx); err != nil {	491		if err := wv.Reload(ctx); err != nil {
435		log.Warn().	492		log.Warn().
436		Err(err).	493		Err(err).

@@ -446,3 +503,37 @@ func (c *Coordinator) resumeParentPaneAfterOAuth(
446		Str("callback_uri", logging.TruncateURL(callbackURI, logURLMaxLen)).	503		Str("callback_uri", logging.TruncateURL(callbackURI, logURLMaxLen)).
447		Msg("refreshed parent pane after oauth popup close")	504		Msg("refreshed parent pane after oauth popup close")
448		}	505		}
			506	+
			507	+	func shouldGraceWaitForParentPaneOAuthResume(
			508	+	parentURIAtOpen string,
			509	+	currentParentURI string,
			510	+	callbackURI string,
			511	+	remainingGrace int,
			512	+	) bool {
			513	+	if remainingGrace <= 0 {
			514	+	return false
			515	+	}
			516	+	if parentURIAtOpen == "" \|\| currentParentURI == "" \|\| callbackURI == "" {
			517	+	return false
			518	+	}
			519	+	if currentParentURI != parentURIAtOpen {
			520	+	return false
			521	+	}
			522	+	return sameOAuthResumeSite(parentURIAtOpen, callbackURI)
			523	+	}
			524	+
			525	+	func sameOAuthResumeSite(a string, b string) bool {
			526	+	hostA := normalizeOAuthResumeHost(a)
			527	+	hostB := normalizeOAuthResumeHost(b)
			528	+	return hostA != "" && hostA == hostB
			529	+	}
			530	+
			531	+	func normalizeOAuthResumeHost(raw string) string {
			532	+	parsed, err := neturl.Parse(raw)
			533	+	if err != nil {
			534	+	return ""
			535	+	}
			536	+	host := strings.ToLower(strings.TrimSpace(parsed.Hostname()))
			537	+	host = strings.TrimPrefix(host, "www.")
			538	+	return host
			539	+	}

M internal/ui/coordinator/content/oauth_test.go +31 -3


@@ -115,21 +115,31 @@ func TestHandlePopupOAuthClose_SuccessSchedulesParentResume(t *testing.T) {
115	115		})
116	116		}
117	117
118		-	func TestResumeParentPaneAfterOAuth_UnchangedParentFallsBackToReload(t *testing.T) {
	118	+	func TestResumeParentPaneAfterOAuth_UnchangedParentDefersSameSiteFallbackReload(t *testing.T) {
119	119		parentPaneID := entity.PaneID("parent-pane")
120	120		popupID := port.WebViewID(102)
121	121		parentWV := portmocks.NewMockWebView(t)
122	122		parentWV.EXPECT().IsDestroyed().Return(false).Once()
123	123		parentWV.EXPECT().URI().Return("https://www.notion.so/login").Once()
124		-	parentWV.EXPECT().Reload(mock.Anything).Return(nil).Once()
125	124
126	125		c := &Coordinator{
127	126		webViews: map[entity.PaneID]port.WebView{
128	127		parentPaneID: parentWV,
129	128		},
	129	+	popups: newPopupManager(),
130	130		}
131	131
132	132		c.resumeParentPaneAfterOAuth(context.Background(), parentPaneID, popupID, "https://www.notion.so/login", "https://www.notion.so/googlepopupcallback?code=123")
	133	+
	134	+	c.popups.mu.Lock()
	135	+	refreshTimer := c.popups.popupRefresh[parentPaneID]
	136	+	if refreshTimer != nil {
	137	+	refreshTimer.Stop()
	138	+	delete(c.popups.popupRefresh, parentPaneID)
	139	+	}
	140	+	c.popups.mu.Unlock()
	141	+
	142	+	assert.NotNil(t, refreshTimer, "same-site popup callback should get a grace retry before forcing reload")
133	143		}
134	144
135	145		func TestResumeParentPaneAfterOAuth_ChangedParentSkipsIntervention(t *testing.T) {

@@ -148,7 +158,7 @@ func TestResumeParentPaneAfterOAuth_ChangedParentSkipsIntervention(t *testing.T)
148	158		c.resumeParentPaneAfterOAuth(context.Background(), parentPaneID, popupID, "https://www.notion.so/login", "https://www.notion.so/googlepopupcallback?code=123")
149	159		}
150	160
151		-	func TestResumeParentPaneAfterOAuth_DifferentDomainFallsBackToReload(t *testing.T) {
	161	+	func TestResumeParentPaneAfterOAuthAttempt_GraceExhaustedFallsBackToReload(t *testing.T) {
152	162		parentPaneID := entity.PaneID("parent-pane")
153	163		popupID := port.WebViewID(104)
154	164		parentWV := portmocks.NewMockWebView(t)

@@ -156,6 +166,24 @@ func TestResumeParentPaneAfterOAuth_DifferentDomainFallsBackToReload(t *testing.
156	166		parentWV.EXPECT().URI().Return("https://www.notion.so/login").Once()
157	167		parentWV.EXPECT().Reload(mock.Anything).Return(nil).Once()
158	168
	169	+	c := &Coordinator{
	170	+	webViews: map[entity.PaneID]port.WebView{
	171	+	parentPaneID: parentWV,
	172	+	},
	173	+	popups: newPopupManager(),
	174	+	}
	175	+
	176	+	c.resumeParentPaneAfterOAuthAttempt(context.Background(), parentPaneID, popupID, "https://www.notion.so/login", "https://www.notion.so/googlepopupcallback?code=123", 0)
	177	+	}
	178	+
	179	+	func TestResumeParentPaneAfterOAuth_DifferentDomainFallsBackToReload(t *testing.T) {
	180	+	parentPaneID := entity.PaneID("parent-pane")
	181	+	popupID := port.WebViewID(105)
	182	+	parentWV := portmocks.NewMockWebView(t)
	183	+	parentWV.EXPECT().IsDestroyed().Return(false).Once()
	184	+	parentWV.EXPECT().URI().Return("https://www.notion.so/login").Once()
	185	+	parentWV.EXPECT().Reload(mock.Anything).Return(nil).Once()
	186	+
159	187		c := &Coordinator{
160	188		webViews: map[entity.PaneID]port.WebView{
161	189		parentPaneID: parentWV,

 	})
 }
-func TestResumeParentPaneAfterOAuth_UnchangedParentFallsBackToReload(t *testing.T) {
+func TestResumeParentPaneAfterOAuth_UnchangedParentDefersSameSiteFallbackReload(t *testing.T) {
 	parentPaneID := entity.PaneID("parent-pane")
 	popupID := port.WebViewID(102)
 	parentWV := portmocks.NewMockWebView(t)
 	parentWV.EXPECT().IsDestroyed().Return(false).Once()
 	parentWV.EXPECT().URI().Return("https://www.notion.so/login").Once()
-	parentWV.EXPECT().Reload(mock.Anything).Return(nil).Once()
 	c := &Coordinator{
 		webViews: map[entity.PaneID]port.WebView{
 			parentPaneID: parentWV,
 		},
+		popups: newPopupManager(),
 	}
 	c.resumeParentPaneAfterOAuth(context.Background(), parentPaneID, popupID, "https://www.notion.so/login", "https://www.notion.so/googlepopupcallback?code=123")
+	c.popups.mu.Lock()
+	refreshTimer := c.popups.popupRefresh[parentPaneID]
+	if refreshTimer != nil {
+		refreshTimer.Stop()
+		delete(c.popups.popupRefresh, parentPaneID)
+	}
+	c.popups.mu.Unlock()
+	assert.NotNil(t, refreshTimer, "same-site popup callback should get a grace retry before forcing reload")
 }
 func TestResumeParentPaneAfterOAuth_ChangedParentSkipsIntervention(t *testing.T) {
 	c.resumeParentPaneAfterOAuth(context.Background(), parentPaneID, popupID, "https://www.notion.so/login", "https://www.notion.so/googlepopupcallback?code=123")
 }
-func TestResumeParentPaneAfterOAuth_DifferentDomainFallsBackToReload(t *testing.T) {
+func TestResumeParentPaneAfterOAuthAttempt_GraceExhaustedFallsBackToReload(t *testing.T) {
 	parentPaneID := entity.PaneID("parent-pane")
 	popupID := port.WebViewID(104)
 	parentWV := portmocks.NewMockWebView(t)
 	parentWV.EXPECT().URI().Return("https://www.notion.so/login").Once()
 	parentWV.EXPECT().Reload(mock.Anything).Return(nil).Once()
+	c := &Coordinator{
+		webViews: map[entity.PaneID]port.WebView{
+			parentPaneID: parentWV,
+		},
+		popups: newPopupManager(),
+	}
+	c.resumeParentPaneAfterOAuthAttempt(context.Background(), parentPaneID, popupID, "https://www.notion.so/login", "https://www.notion.so/googlepopupcallback?code=123", 0)
+}
+func TestResumeParentPaneAfterOAuth_DifferentDomainFallsBackToReload(t *testing.T) {
+	parentPaneID := entity.PaneID("parent-pane")
+	popupID := port.WebViewID(105)
+	parentWV := portmocks.NewMockWebView(t)
+	parentWV.EXPECT().IsDestroyed().Return(false).Once()
+	parentWV.EXPECT().URI().Return("https://www.notion.so/login").Once()
+	parentWV.EXPECT().Reload(mock.Anything).Return(nil).Once()
 	c := &Coordinator{
 		webViews: map[entity.PaneID]port.WebView{
 			parentPaneID: parentWV,