Check m_isClosed in WebPageProxy::platformWindow.
<rdar://problem/52323585>
Created attachment 374031 [details] Patch
Comment on attachment 374031 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374031&action=review > Source/WebKit/UIProcess/mac/WebPageProxyMac.mm:640 > + if (m_isClosed) I would instead update this check in SSO code: if (!m_page || m_viewController) { uiCallback(NO, adoptNS([[NSError alloc] initWithDomain:SOErrorDomain code:kSOErrorAuthorizationPresentationFailed userInfo:nil]).get()); return; } -> if (!m_page || m_page->isClosed() || m_viewController) { uiCallback(NO, adoptNS([[NSError alloc] initWithDomain:SOErrorDomain code:kSOErrorAuthorizationPresentationFailed userInfo:nil]).get()); return; }
(In reply to Chris Dumez from comment #3) > Comment on attachment 374031 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=374031&action=review > > > Source/WebKit/UIProcess/mac/WebPageProxyMac.mm:640 > > + if (m_isClosed) > > I would instead update this check in SSO code: > if (!m_page || m_viewController) { > uiCallback(NO, adoptNS([[NSError alloc] initWithDomain:SOErrorDomain > code:kSOErrorAuthorizationPresentationFailed userInfo:nil]).get()); > return; > } > > -> > > if (!m_page || m_page->isClosed() || m_viewController) { > uiCallback(NO, adoptNS([[NSError alloc] initWithDomain:SOErrorDomain > code:kSOErrorAuthorizationPresentationFailed userInfo:nil]).get()); > return; > } Also, do we have API testing for SSO? If so, it'd be good to have a test that starts an SSO request then calls _close on the webView right away.
Comment on attachment 374031 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374031&action=review Thanks Chris for reviewing the patch. >>> Source/WebKit/UIProcess/mac/WebPageProxyMac.mm:640 >>> + if (m_isClosed) >> >> I would instead update this check in SSO code: >> if (!m_page || m_viewController) { >> uiCallback(NO, adoptNS([[NSError alloc] initWithDomain:SOErrorDomain code:kSOErrorAuthorizationPresentationFailed userInfo:nil]).get()); >> return; >> } >> >> -> >> >> if (!m_page || m_page->isClosed() || m_viewController) { >> uiCallback(NO, adoptNS([[NSError alloc] initWithDomain:SOErrorDomain code:kSOErrorAuthorizationPresentationFailed userInfo:nil]).get()); >> return; >> } > > Also, do we have API testing for SSO? If so, it'd be good to have a test that starts an SSO request then calls _close on the webView right away. Sure thing. However, it still feels weird that caller of m_page->platformWindow() needs to check m_page->isClosed(). SSO tests are all API tests which should be able to reproduce what you suggest. Let me try.
Created attachment 374045 [details] Patch
Comment on attachment 374045 [details] Patch r=me
(In reply to Chris Dumez from comment #7) > Comment on attachment 374045 [details] > Patch > > r=me Thanks, Chris.
Comment on attachment 374045 [details] Patch Clearing flags on attachment: 374045 Committed r247410: <https://trac.webkit.org/changeset/247410>
All reviewed patches have been landed. Closing bug.