localStorage changes aren't reflected between WKWebViews using WKWebViewConfiguration._groupIdentifier

• Component: DOM
• Status: RESOLVED FIXED

[Jake Archibald]

Using Safari 14.1 or later:

1. Tab A: Open https://static-misc-3.glitch.me/localstorage-bug/ in new tab
2. Tab A: Hit 'increment'
3. Tab B: Open https://static-misc-3.glitch.me/localstorage-bug/ in new tab - it displays '1' as expected
4. Tab B: Hit 'increment'

Tab A does not get a 'storage' event, clicking "Re-get value" returns the same value.

It seems like localStorage state is copied at some point, but it doesn't receive any updates after that. However, it's still writing to the underlying data store.

Refreshing the page still returns stale data.

This will result in data loss on websites, as two tabs will be writing to storage, unaware of each others' changes.

Additionally, the 'storage' event is used for cross-tab communication in Safari, since it doesn't support BroadcastChannel, and this bug breaks that. In fact, BroadcastChannel polyfills use the 'storage' event https://github.com/JSmith01/broadcastchannel-polyfill.

[Jake Archibald]

In case anyone needs a workaround, you can switch to indexeddb for storage, and use service workers to communicate the changes between tabs https://static-misc-3.glitch.me/safari-storage-workaround/

[Thomas Steiner]

(Somewhat related: people have been asking for `BroadcastChannel` support in https://bugs.webkit.org/show_bug.cgi?id=161472.)

[Radar WebKit Bug Importer]

<rdar://problem/77496721>

[Jake Archibald]

Here's a failing wpt http://wpt.live/webstorage/storage_local_window_open.window.html

[Luke Warlow]

It's worth noting this appears to only be the case on macOS. iOS seems to behave as expected. Tested with iPad OS 14.5.1 and macOS Big Sur 11.3.1.

[Chris Dumez]

Investigating. Thanks for the bug report.

[Chris Dumez]

Looks like this was caused by a Safari change, not a WebKit change. Keeping this WebKit bug report open for now in case we need a WebKit-side fix.

[Alex Christensen]

Created attachment 427693 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 427693 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=427693&action=review

> Source/WebKit/UIProcess/WebPageGroup.cpp:86
> +        data.pageGroupID = generatePageGroupID();

Why don't we use ObjectIdentifier<> for the page group?

[Alex Christensen]

We should.  This is old code that hasn't been updated to our newer idioms, but I certainly don't want to do that in this patch for the reason stated in the ChangeLog.

[Alex Christensen]

http://trac.webkit.org/r276983

[Darin Adler]

Comment on attachment 427693 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=427693&action=review

> Source/WebKit/UIProcess/WebPageGroup.cpp:81
> +    if (HashMap<String, uint64_t>::isValidKey(identifier)) {

Seems like overkill to use isValidKey here. If null is a special value, meaning we don’t have one, we should check for null. The fact that null also can’t be used as a hash table key is a separate thing that doesn’t seem the same to me.

[Alex Christensen]

It is probably overkill, but I wanted this to be as low risk as possible to merge onto a branch.  I intend to remove all this code soon.

[Darin Adler]

(In reply to Alex Christensen from comment #13)
> It is probably overkill, but I wanted this to be as low risk as possible to
> merge onto a branch.  I intend to remove all this code soon.

I understand what you are saying.

I don’t think that using isValidKey makes this lower risk than an isNull() check.

[Alex Christensen]

I think you're right.  It seems impossible to pass a hash-table-deleted string through our API.

[Thomas Steiner]

This fix seems to have rolled out with iOS/iPadOS 15 beta 1.

[Guido Bouman]

Will this also land in 14.2, if/when that comes? Seems like a big regression that's worth patching before 15. Even worth a 14.1.1 if you ask me.

[Alex Christensen]

We are aware of how important this bug is.  We have no comment on future releases.

[Sam Sneddon [:gsnedders]]

(In reply to Thomas Steiner from comment #16)
> This fix seems to have rolled out with iOS/iPadOS 15 beta 1.

As mentioned above, this never affected iOS/iPadOS, as it was macOS only.

To note, this was fixed in macOS 11.5.