Skip to content

Commit 22d7518

Browse files
chaxusclaude
andcommitted
fix(sw): reload on a controller change only when the build really changed
The previous commit reloaded on any controller change, and three E2E shards said no. A controller changing is routine: the vendored editor registers a worker of its own into this scope, ours is re-installed and left waiting on ordinary loads, and the browser activates it by itself at the next navigation -- same build, same caches, nothing behind it. Measured at 80ms into a plain reload of a fresh profile, which is why sw-vendor-cache-first died with "Execution context was destroyed" in the middle of its probe. So the condition is two things, and both are load-bearing: - the new controller is a build this browser has not been running, judged the way healStaleController judges it -- by the runtime cache, named after the vendor tree's content. The cache names are snapshotted at boot: read them when the swap happens and the incoming worker has already created its own, and asking the outgoing worker instead is asking something a swap may already have terminated; - and there are no unsaved edits. Not "is a document open", which is what left a torn page unrepaired, and not "did this tab ask for the swap", which misses the cases that actually happen. Reverse-verified in both directions, locally, which the earlier attempts could not manage: pinning isNewBuild false turns sw-silent-update red, pinning it true turns sw-vendor-cache-first red. With the real value, sw-silent-update, sw-vendor-cache-first, sw-warm, autosave-recovery and font-cache pass -- eight cases. Tried and dropped: having sw.js skip its install-time takeover when a core cache under this build's name already exists, so that a same-build re-install stops swapping the controller mid-load. Its unit case reverse-verified, but the local suite is green with and without it, and it has a real cost -- a rollback to a build this browser has run would no longer be delivered, which is the font revert's own scenario. Unproven benefit, delicate path, out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent c242b2f commit 22d7518

6 files changed

Lines changed: 126 additions & 56 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,9 @@ notes. Entries describe what users experience, not internal refactors.
102102
- **An update could leave the editor blank.** When a new version took over
103103
while the page was still starting up, the request for the editor itself was
104104
cancelled mid-flight and nothing asked for it again -- an empty white page
105-
that reloading was the only way out of. The page now reloads itself whenever
106-
the version serving it changes, which is what repairs it -- unless you have
107-
unsaved edits, which nothing is allowed to reload over.
105+
that reloading was the only way out of. The page now reloads itself when the
106+
version serving it really has changed, which is what repairs it -- unless you
107+
have unsaved edits, which nothing is allowed to reload over.
108108
- The four home-screen buttons ("View/Edit Document", "New Word"...) no longer
109109
flash behind the spinner while a document named in the URL is loading.
110110
- Chinese, Japanese and Korean text in an exported PDF came out blank. The

CLAUDE.md

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -535,16 +535,18 @@ docs/explorations/2026-08-19-ci-e2e-sharding.md。
535535
`DEPLOY_COUPLED`(由 `hosting-contract.test.ts` / `sw-routing.test.ts` 钉住)。
536536
`open-local.js` / `landing-prefetch.js` 同理,2026-08-20 起补齐——它们从路由
537537
拆分起一直漏在 SWR 上,改这两个文件的部署,落地页会一直跑旧的那份。
538-
**controller 一换就 reload,唯一的否决是未保存改动**(2026-08-23):激活新 worker
539-
会终止旧 worker,它手上 in-flight 的 fetch 全部失败——其中就有编辑器 iframe 自己的
540-
文档请求,没人重试,标签页永远白屏。拒绝刷新撤不回那次交接,只会把人留在白屏上。
541-
**别再把"有没有文档打开"或"是不是本页请求的交接"加回判据**:换人的有三个来源,
542-
只有一个跟本页面有关(`sw.js` 自己 install 里 skipWaiting、别的标签页经落地页提升、
543-
**浏览器在旧 worker 的客户端全没了时自动激活**——一次 reload 正好安排了这件事,
544-
实测交接落在 reload 之后 50ms、比它杀掉的请求早 170ms)。另一半是
545-
`documentIsExpected``?new=` / `?file=` / `?src=` / `?open=` / `?saved=` / `?embed=`):
546-
正在开文档的页面**不要提升**,那种等待交给静默自愈,它会先 `isUnseenBuild()`
547-
确认真是新构建。见
538+
**controllerchange 的 reload 判据只有两条:是不是新构建 + 有没有未保存改动**
539+
(2026-08-23)。激活新 worker 会终止旧 worker,它手上 in-flight 的 fetch 全部失败——
540+
其中就有编辑器 iframe 自己的文档请求,没人重试,标签页永远白屏;拒绝刷新撤不回那次
541+
交接。所以**别把"有没有文档打开"加回判据**。但也**别只按"换了就刷"**:controller
542+
变更是常态(厂商 worker 的来回切换让我们的 sw.js 在普通加载里就被重装并 waiting,
543+
浏览器下一次导航自己激活它),实测一次普通 reload 之后 80ms 就换人,无差别刷新会把
544+
`sw-vendor-cache-first` 打成 `Execution context was destroyed`。新构建的判据是
545+
`isUnseenBuild`**缓存名快照必须在启动时取**(交接时再读,新 worker 已经建好自己的
546+
那个了)。另有 `documentIsExpected``?new=` / `?file=` / `?src=` / `?open=` /
547+
`?saved=` / `?embed=`):正在开文档的页面**不要提升****别去改 `sw.js` 的 install
548+
让它对同一构建不接管**——试过,本地证不出必要性,却会让"回滚到本机跑过的构建"不再
549+
自动投递。见
548550
docs/explorations/2026-08-23-promotion-without-reload-blank-editor.md。
549551
落地页那侧的提升要覆盖三种到达方式:已经 `waiting``installing` 中途、
550552
以及 `updatefound` 时已经 `installed``statechange` 只报此后的迁移,

docs/explorations/2026-08-23-promotion-without-reload-blank-editor.md

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,23 @@ CLAUDE.md 里记着这条路径上相反方向的那个 bug("打开流程排
112112
所以"这一页有没有请求过这次交接"是个错的判据,它恰好漏掉了真正会发生的那几种。
113113
`promotedFromThisTab``onPromoted` 这套管线一起删掉了。
114114

115-
**真正的判据只有一条:有没有未保存的改动。** 交接已经发生,拒绝刷新撤不回它,
116-
只会把人留在白屏上;而真正不能被刷新盖掉的东西就是未保存的工作。两者不会撞车——
117-
被撕成两半的页面根本没加载完,里面没有任何未保存的东西。
115+
**判据有两条,缺一条都会立刻出事。**
116+
117+
第一条:**必须是另一个构建**。"controller 换了"是常态——厂商 worker 的来回切换让我们的
118+
sw.js 在普通加载里就被重装并留在 waiting,浏览器会在下一次导航时自己把它激活,
119+
同一个构建、同一批缓存、没有任何事情发生。只按"换了就刷"做,等于每隔一次页面浏览就刷一次
120+
(实测:一次普通 reload 之后 80 毫秒就换了人,`sw-vendor-cache-first` 当场
121+
`Execution context was destroyed`)。判据用的还是 `isUnseenBuild` 那一条:
122+
运行时缓存按 vendor 内容哈希命名,**在启动时**拍一张缓存名快照(不是等交接发生时再读,
123+
那时新 worker 已经建好自己的那个了;也不去问旧 worker,它可能已经被终止)。
124+
125+
第二条:**不能盖掉未保存的工作**。除此之外没有别的拒绝理由。原先那条"有文档打开就不刷"
126+
听着谨慎,其实不是:交接已经发生,拒绝刷新撤不回它,只会把人留在白屏上;
127+
而被撕成两半的页面根本没加载完,里面没有任何未保存的东西——两者不会撞车。
128+
129+
**谁请求的交接不是判据**,这是第一版栽的地方:能换人的有三个,只有一个跟本页面有关
130+
`sw.js` 自己在 install 里 skipWaiting、别的标签页经落地页提升、
131+
浏览器在旧 worker 的客户端全没了时自动激活)。
118132

119133
`documentIsExpected` 留着,它另有其用:不要往一个正在开文档的页面里塞提升(嵌入模式
120134
理由更硬,宿主随时可能推文档进来)。
@@ -133,6 +147,21 @@ CLAUDE.md 里记着这条路径上相反方向的那个 bug("打开流程排
133147
`documentIsExpected` 三条分别变红。E2E `autosave-recovery`(三条)+
134148
`sw-silent-update` + `sw-warm` 本地全绿。
135149

136-
**诚实说一句**:这个白屏在本地从来没复现过——三版实现本地都是绿的,红只发生在 CI。
137-
所以 E2E 这一侧算不上反向验证,判据是上面那条时间轴,以及"无论谁换了 controller
138-
都会修"这个更强的不变式。
150+
E2E 这一侧两个方向都验过,**都在本地**(前几版复现不了,是因为它们碰巧没触发;
151+
把判据写死才逼出来):
152+
153+
- `isNewBuild` 恒为 `false`(等于旧的"不刷")→ `sw-silent-update` 变红;
154+
- 恒为 `true`(等于"换了就刷")→ `sw-vendor-cache-first` 变红
155+
`Execution context was destroyed`)。
156+
157+
两条都恢复正常值时,`sw-silent-update` / `sw-vendor-cache-first` / `sw-warm` ×2 /
158+
`autosave-recovery` ×3 / `font-cache` 共 8 条全绿。
159+
160+
## 试过又撤掉的:让 sw.js 别为同一个构建接管
161+
162+
中途改过 `public/sw.js`:install 时先问 `caches.has(CORE_CACHE)`,已经装过这个构建
163+
就不 `skipWaiting()`——理由是同一构建的接管什么也不换,却会终止旧 worker、打断它手上
164+
的请求。单测反向验证是过的,但**本地整套 SW 用例在有没有它的情况下都全绿**,证不出必要性;
165+
而它有一处真实代价:**回滚到这台浏览器跑过的构建**(vendor 未变那种)会因为核心缓存
166+
同名而不再自动接管,投递就哑了——那正是字体那次 revert 踩过的场景。证不出收益、
167+
却动了投递路径,所以撤掉。

index.ts

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
documentIsExpected,
33
healStaleController,
4+
isUnseenBuild,
45
onWaitingWorker,
56
shouldReloadOnControllerChange,
67
wireServiceWorkerUpdates,
@@ -240,18 +241,30 @@ if ('serviceWorker' in navigator) {
240241
// into the middle of its own load. The URL already knows.
241242
const hasOpenDocument = () => Boolean(getDocmentObj().fileName) || documentIsExpected(window.location.search);
242243

244+
// The runtime caches as they were before anything could have changed them.
245+
// Read at boot rather than when a swap happens: by then the incoming worker
246+
// has created its own, and the question is which builds this browser was
247+
// running BEFORE. Taken from the cache rather than by asking the outgoing
248+
// worker, which a swap may already have terminated.
249+
const cachesAtBoot = typeof caches === 'undefined' ? Promise.resolve([]) : caches.keys();
250+
const bootCacheNames = { keys: () => cachesAtBoot };
251+
243252
navigator.serviceWorker.addEventListener('controllerchange', () => {
244-
if (
245-
!shouldReloadOnControllerChange({
246-
hadController,
247-
alreadyReloading: reloadingForUpdate,
248-
hasUnsavedChanges: hasUnsavedChanges(),
249-
})
250-
) {
251-
return;
252-
}
253-
reloadingForUpdate = true;
254-
window.location.reload();
253+
void (async () => {
254+
const isNewBuild = await isUnseenBuild(navigator.serviceWorker.controller, bootCacheNames);
255+
if (
256+
!shouldReloadOnControllerChange({
257+
hadController,
258+
alreadyReloading: reloadingForUpdate,
259+
isNewBuild,
260+
hasUnsavedChanges: hasUnsavedChanges(),
261+
})
262+
) {
263+
return;
264+
}
265+
reloadingForUpdate = true;
266+
window.location.reload();
267+
})();
255268
});
256269

257270
// The script we register, absolute: the vendored editor registers one of its

lib/sw-update.ts

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -146,35 +146,42 @@ export function onWaitingWorker(
146146
/**
147147
* Whether a controllerchange should reload the page.
148148
*
149-
* The condition used to be "not while a document is open", and the reason it
150-
* was wrong is worth keeping: activating a worker terminates the outgoing one,
151-
* and every request it still had in flight fails. On the editor route the one
152-
* in flight is the vendored iframe's own document, which nothing retries -- so
153-
* refusing the reload leaves a blank editor that only a manual reload escapes.
154-
* By the time the event arrives the page has already been torn in half.
155-
* Refusing does not undo the swap.
149+
* Two conditions, and the history of getting them wrong is the reason both are
150+
* spelled out here.
156151
*
157-
* It is also not this page's business WHO swapped. Three different things do
158-
* it and only one of them is us: sw.js calls skipWaiting() itself when
159-
* activating would not discard vendor assets, the landing page promotes from
160-
* another tab, and the browser activates a waiting worker on its own once the
161-
* clients the old one controlled are gone -- which a reload arranges. Gating
162-
* the repair on "this tab asked for it" therefore missed the cases that
163-
* actually happen (measured: the swap landed 50ms into a reload, 170ms before
164-
* the iframe request it killed).
152+
* **It has to be a different build.** A controller changing is routine: the
153+
* vendored editor registers a worker of its own into this scope from inside
154+
* its iframe, so ours is re-installed and left waiting on ordinary loads, and
155+
* the browser activates it by itself at the next navigation -- same build,
156+
* same caches, nothing to tell anyone. Reloading on every swap is a reload on
157+
* every second page view (measured: 80ms into the load, on a plain reload of
158+
* a fresh profile). The evidence for "different" is the runtime cache, named
159+
* after the vendor tree's content: see isUnseenBuild.
165160
*
166-
* What genuinely must not be reloaded over is unsaved work, and that is the
167-
* condition. A torn page has nothing unsaved in it -- nothing finished
168-
* loading -- so the two never collide.
161+
* **And it must not throw away unsaved work.** Nothing else is a reason to
162+
* refuse. It used to be "not while a document is open", which sounds careful
163+
* and is not: activating a worker terminates the outgoing one and every
164+
* request it still had in flight fails -- on this route the vendored iframe's
165+
* own document, which nothing retries. By the time this event arrives the page
166+
* may already be torn in half, and refusing does not undo the swap; it only
167+
* leaves the reader on a blank editor with no way out but a manual reload.
168+
*
169+
* Who asked for the swap is deliberately not a condition. Three things do it
170+
* and only one is us: sw.js promotes itself on install, another tab promotes
171+
* through the landing page, and the browser activates a waiting worker on its
172+
* own.
169173
*/
170174
export function shouldReloadOnControllerChange(state: {
171175
hadController: boolean;
172176
alreadyReloading: boolean;
177+
/** The new controller is a build this browser has not been running. */
178+
isNewBuild: boolean;
173179
hasUnsavedChanges?: boolean;
174180
}): boolean {
175181
// No controller at startup means this is the first install, not an update:
176-
// nothing was being served by anyone, so nothing was torn.
182+
// nobody was serving this page, so nothing was interrupted.
177183
if (!state.hadController || state.alreadyReloading) return false;
184+
if (!state.isNewBuild) return false;
178185
return !state.hasUnsavedChanges;
179186
}
180187

test/unit/sw-update.test.ts

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,14 +122,25 @@ describe('documentIsExpected', () => {
122122
});
123123

124124
describe('shouldReloadOnControllerChange', () => {
125+
const update = { hadController: true, alreadyReloading: false, isNewBuild: true };
125126
it('reloads once on an update', () => {
126-
expect(shouldReloadOnControllerChange({ hadController: true, alreadyReloading: false })).toBe(true);
127+
expect(shouldReloadOnControllerChange(update)).toBe(true);
127128
});
128129
it('does not reload on a first install, or twice', () => {
129130
// No controller at startup means nobody was serving this page, so nothing
130131
// was interrupted -- and reloading the first visit would be a stutter.
131-
expect(shouldReloadOnControllerChange({ hadController: false, alreadyReloading: false })).toBe(false);
132-
expect(shouldReloadOnControllerChange({ hadController: true, alreadyReloading: true })).toBe(false);
132+
expect(shouldReloadOnControllerChange({ ...update, hadController: false })).toBe(false);
133+
expect(shouldReloadOnControllerChange({ ...update, alreadyReloading: true })).toBe(false);
134+
});
135+
136+
/**
137+
* The routine case, and the one that made an earlier attempt reload every
138+
* second page view: the vendored editor keeps our worker re-installing, the
139+
* browser activates it at the next navigation, and the controller changes
140+
* with nothing behind it. Measured at 80ms into a plain reload.
141+
*/
142+
it('ignores a swap between two instances of the same build', () => {
143+
expect(shouldReloadOnControllerChange({ ...update, isNewBuild: false })).toBe(false);
133144
});
134145
});
135146

@@ -750,15 +761,23 @@ describe('shouldReloadOnControllerChange after a swap this page did not ask for'
750761
// `hasOpenDocument` was the old condition, and passing it is the point:
751762
// this exact input used to return false, which is how a reader ended up
752763
// on a blank page with no way out but a manual reload.
753-
const state = { hadController: true, alreadyReloading: false, hasOpenDocument: true } as Parameters<
754-
typeof shouldReloadOnControllerChange
755-
>[0];
764+
const state = {
765+
hadController: true,
766+
alreadyReloading: false,
767+
isNewBuild: true,
768+
hasOpenDocument: true,
769+
} as Parameters<typeof shouldReloadOnControllerChange>[0];
756770
expect(shouldReloadOnControllerChange(state)).toBe(true);
757771
});
758772

759773
it('never reloads over unsaved edits', () => {
760774
expect(
761-
shouldReloadOnControllerChange({ hadController: true, alreadyReloading: false, hasUnsavedChanges: true }),
775+
shouldReloadOnControllerChange({
776+
hadController: true,
777+
alreadyReloading: false,
778+
isNewBuild: true,
779+
hasUnsavedChanges: true,
780+
}),
762781
).toBe(false);
763782
});
764783
});

0 commit comments

Comments
 (0)