...
Created attachment 371874 [details] Patch
Created attachment 371879 [details] Patch
Created attachment 371968 [details] Patch
Comment on attachment 371968 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371968&action=review r=me > Source/WebKit/Shared/SessionState.h:116 > + bool isDestructed { false }; > + ~FrameState() { isDestructed = true; } This probably deserves a comment that it's only here to help debug an undiagnosed bug, with a Bugzilla or Radar link.
Created attachment 371990 [details] Patch for landing
Comment on attachment 371990 [details] Patch for landing Clearing flags on attachment: 371990 Committed r246382: <https://trac.webkit.org/changeset/246382>
All reviewed patches have been landed. Closing bug.
<rdar://problem/51686085>
Comment on attachment 371990 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=371990&action=review > Source/WebKit/UIProcess/API/C/WKPage.cpp:485 > + auto data = encodeLegacySessionState(sessionState); > if (shouldReturnData) > - return toAPI(encodeLegacySessionState(sessionState).leakRef()); > + return toAPI(data.leakRef()); This isn’t adding an assertion. This is refactoring that causes us to do additional work when shouldReturnData is false. Nothing in the change log makes it clear why this is a desirable change.
Comment on attachment 371990 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=371990&action=review >> Source/WebKit/UIProcess/API/C/WKPage.cpp:485 >> + return toAPI(data.leakRef()); > > This isn’t adding an assertion. This is refactoring that causes us to do additional work when shouldReturnData is false. Nothing in the change log makes it clear why this is a desirable change. Right, add some explaining here for reference. Previously the crash traces tell us: WKPageCopySessionState returns WKSessionStateRef => Some pointer casts by user => WKSessionStateCopyData(WKSessionStateRef) -> encodeLegacySessionState(sessionState) [Crash] Now: WKPageCopySessionState [try encodeLegacySessionState(sessionState)] returns WKSessionStateRef => pointer casts => WKSessionStateCopyData(WKSessionStateRef) -> encodeLegacySessionState(sessionState) If we start to see crashes in WKPageCopySessionState after this change, we can confirm sessionState is bad before the casts.