WebKit Bugzilla
Attachment 340887 Details for
Bug 185832
: Regression(AsyncPolicyDelegates): Box.app login Window is blank
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-185832-20180521141355.patch (text/plain), 13.25 KB, created by
Chris Dumez
on 2018-05-21 14:13:56 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Chris Dumez
Created:
2018-05-21 14:13:56 PDT
Size:
13.25 KB
patch
obsolete
>Subversion Revision: 232025 >diff --git a/Source/WebKitLegacy/mac/ChangeLog b/Source/WebKitLegacy/mac/ChangeLog >index c3d63f3b5808f6d02be3be48019cfc76df7f7650..8bdfbac27fe5fb7e3e299610a679ddecd10c16b4 100644 >--- a/Source/WebKitLegacy/mac/ChangeLog >+++ b/Source/WebKitLegacy/mac/ChangeLog >@@ -1,3 +1,32 @@ >+2018-05-21 Chris Dumez <cdumez@apple.com> >+ >+ Regression(AsyncPolicyDelegates): Box.app login Window is blank >+ https://bugs.webkit.org/show_bug.cgi?id=185832 >+ <rdar://problem/40307871> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ We used to have a bug where where we would fail to wait for the policy decision for >+ the navigation response from the client and the load would keep going, racing with >+ the client's policy decision. If the client did not respond in time, the behavior >+ would be the same as "Use" policy action. >+ >+ Box.app fails to make any policy decision in its decidePolicyForMIMEType delegate >+ but the load happened to proceed anyway due to our bug. Now that we've fixed the >+ WebKit bug, however, the load would hang because the completion handler for the >+ decidePolicyForNavigationResponse would never get called. >+ >+ To work around the issue, I made the policy listener weak on the WebFrameLoaderClient >+ instead of retaining it. If the policy listener object gets destroyed because getting >+ resolved, we now use "Use" policy action in its dealloc function to maintain previous >+ behavior. >+ >+ * WebCoreSupport/WebFrameLoaderClient.h: >+ * WebCoreSupport/WebFrameLoaderClient.mm: >+ (WebFrameLoaderClient::cancelPolicyCheck): >+ (WebFrameLoaderClient::setUpPolicyListener): >+ (-[WebFramePolicyListener dealloc]): >+ > 2018-05-21 Jer Noble <jer.noble@apple.com> > > Complete fix for enabling modern EME by default >diff --git a/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.h b/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.h >index a2c7932f68269976c2933694c3eedc6029652dd1..799b0e6ab950b03bdeadeeff2fdd3c67be55f4fc 100644 >--- a/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.h >+++ b/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.h >@@ -260,5 +260,5 @@ private: > > RetainPtr<WebFrame> m_webFrame; > >- RetainPtr<WebFramePolicyListener> m_policyListener; >+ __weak WebFramePolicyListener *m_policyListener; > }; >diff --git a/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm b/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm >index 1a411348a8eb6c77d87edafcdc49ed1859b5f616..5e465d7af5326a6c2460fdf89051d29608ff7d7f 100644 >--- a/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm >+++ b/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm >@@ -914,8 +914,11 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const Navigat > > void WebFrameLoaderClient::cancelPolicyCheck() > { >+ if (!m_policyListener) >+ return; >+ > [m_policyListener invalidate]; >- m_policyListener = nullptr; >+ m_policyListener = nil; > } > > void WebFrameLoaderClient::dispatchUnableToImplementPolicy(const ResourceError& error) >@@ -1524,14 +1527,17 @@ RetainPtr<WebFramePolicyListener> WebFrameLoaderClient::setUpPolicyListener(Fram > // FIXME: <rdar://5634381> We need to support multiple active policy listeners. > [m_policyListener invalidate]; > >+ RetainPtr<WebFramePolicyListener> policyListener; > #if HAVE(APP_LINKS) > if (appLinkURL) >- m_policyListener = adoptNS([[WebFramePolicyListener alloc] initWithFrame:core(m_webFrame.get()) policyFunction:WTFMove(function) appLinkURL:appLinkURL]); >+ policyListener = adoptNS([[WebFramePolicyListener alloc] initWithFrame:core(m_webFrame.get()) policyFunction:WTFMove(function) appLinkURL:appLinkURL]); > else > #endif >- m_policyListener = adoptNS([[WebFramePolicyListener alloc] initWithFrame:core(m_webFrame.get()) policyFunction:WTFMove(function)]); >+ policyListener = adoptNS([[WebFramePolicyListener alloc] initWithFrame:core(m_webFrame.get()) policyFunction:WTFMove(function)]); >+ >+ m_policyListener = policyListener.get(); > >- return m_policyListener; >+ return policyListener; > } > > String WebFrameLoaderClient::userAgent(const URL& url) >@@ -2426,6 +2432,14 @@ - (void)dealloc > if (WebCoreObjCScheduleDeallocateOnMainThread([WebFramePolicyListener class], self)) > return; > >+ // If the app did not respond before the listener is destroyed, then we let the load >+ // proceed with policy "Use". >+ _frame = nullptr; >+ if (auto policyFunction = std::exchange(_policyFunction, nullptr)) { >+ RELEASE_LOG_ERROR(Loading, "Client application failed to make a policy decision via WebPolicyDecisionListener, letting the load proceed"); >+ policyFunction(PolicyAction::Use); >+ } >+ > [super dealloc]; > } > >diff --git a/Tools/ChangeLog b/Tools/ChangeLog >index db55e3c883da710b03e8cd9a1f22b97987cc671c..29cefe9fe95fea9afcab540a7a2a93e8a2ddfebe 100644 >--- a/Tools/ChangeLog >+++ b/Tools/ChangeLog >@@ -1,3 +1,20 @@ >+2018-05-21 Chris Dumez <cdumez@apple.com> >+ >+ Regression(AsyncPolicyDelegates): Box.app login Window is blank >+ https://bugs.webkit.org/show_bug.cgi?id=185832 >+ <rdar://problem/40307871> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Add API test coverage. >+ >+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj: >+ * TestWebKitAPI/Tests/mac/NoPolicyDelegateResponse.mm: Added. >+ (-[NoPolicyDelegateDecisionDelegate webView:decidePolicyForNavigationAction:request:frame:decisionListener:]): >+ (-[NoPolicyDelegateDecisionDelegate webView:decidePolicyForMIMEType:request:frame:decisionListener:]): >+ (-[NoPolicyDelegateDecisionDelegate webView:didFinishLoadForFrame:]): >+ (TestWebKitAPI::TEST): >+ > 2018-05-21 Jer Noble <jer.noble@apple.com> > > Complete fix for enabling modern EME by default >diff --git a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj >index 0372673c68009200fbb179812d8dcc90a870df38..8f32e4c5830f5746e2dac910888093e4943936bf 100644 >--- a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj >+++ b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj >@@ -565,6 +565,7 @@ > 83BAEE8D1EF4625500DDE894 /* PluginLoadClientPolicies.mm in Sources */ = {isa = PBXBuildFile; fileRef = 83BAEE8C1EF4625500DDE894 /* PluginLoadClientPolicies.mm */; }; > 83DB79691EF63B3C00BFA5E5 /* Function.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 83DB79671EF63B3C00BFA5E5 /* Function.cpp */; }; > 83DE134D1EF1C50800C1B355 /* ResponsivenessTimer.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 83DE134C1EF1C4FE00C1B355 /* ResponsivenessTimer.cpp */; }; >+ 83F22C6420B355F80034277E /* NoPolicyDelegateResponse.mm in Sources */ = {isa = PBXBuildFile; fileRef = 83F22C6320B355EB0034277E /* NoPolicyDelegateResponse.mm */; }; > 8C10AF98206467920018FD90 /* localstorage-empty-string-value.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 8C10AF97206467830018FD90 /* localstorage-empty-string-value.html */; }; > 8C10AF99206467A90018FD90 /* LocalStoragePersistence.mm in Sources */ = {isa = PBXBuildFile; fileRef = 8C10AF96206467770018FD90 /* LocalStoragePersistence.mm */; }; > 8E4A85371E1D1AB200F53B0F /* GridPosition.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 8E4A85361E1D1AA100F53B0F /* GridPosition.cpp */; }; >@@ -1604,6 +1605,7 @@ > 83BAEE8C1EF4625500DDE894 /* PluginLoadClientPolicies.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = PluginLoadClientPolicies.mm; sourceTree = "<group>"; }; > 83DB79671EF63B3C00BFA5E5 /* Function.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = Function.cpp; sourceTree = "<group>"; }; > 83DE134C1EF1C4FE00C1B355 /* ResponsivenessTimer.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ResponsivenessTimer.cpp; sourceTree = "<group>"; }; >+ 83F22C6320B355EB0034277E /* NoPolicyDelegateResponse.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = NoPolicyDelegateResponse.mm; sourceTree = "<group>"; }; > 86BD19971A2DB05B006DCF0A /* RefCounter.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = RefCounter.cpp; sourceTree = "<group>"; }; > 8A2C750D16CED9550024F352 /* ResizeWindowAfterCrash.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ResizeWindowAfterCrash.cpp; sourceTree = "<group>"; }; > 8A3AF93A16C9ED2700D248C1 /* ReloadPageAfterCrash.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ReloadPageAfterCrash.cpp; sourceTree = "<group>"; }; >@@ -3041,6 +3043,7 @@ > 5C0BF88C1DD5957400B00328 /* MemoryPressureHandler.mm */, > 7A99D9931AD4A29D00373141 /* MenuTypesForMouseEvents.mm */, > E19DB9781B32137C00DB38D4 /* NavigatorLanguage.mm */, >+ 83F22C6320B355EB0034277E /* NoPolicyDelegateResponse.mm */, > A57A34EF16AF677200C2501F /* PageVisibilityStateWithWindowChanges.mm */, > 00BC16851680FE810065F1E5 /* PublicSuffix.mm */, > 37C784DE197C8F2E0010A496 /* RenderedImageFromDOMNode.mm */, >@@ -3830,6 +3833,7 @@ > 37B47E301D64E7CA005F4EFF /* WKObject.mm in Sources */, > 7C89D2AC1A69B80D003A5FDE /* WKPageConfiguration.cpp in Sources */, > 52D673EE1AFB127300FA19FE /* WKPageCopySessionStateWithFiltering.cpp in Sources */, >+ 83F22C6420B355F80034277E /* NoPolicyDelegateResponse.mm in Sources */, > 7CCE7F1F1A411AE600447C4C /* WKPageGetScaleFactorNotZero.cpp in Sources */, > 7CCE7F201A411AE600447C4C /* WKPageIsPlayingAudio.cpp in Sources */, > A14AAB631E78D7DE00C1ADC2 /* WKPDFView.mm in Sources */, >diff --git a/Tools/TestWebKitAPI/Tests/mac/NoPolicyDelegateResponse.mm b/Tools/TestWebKitAPI/Tests/mac/NoPolicyDelegateResponse.mm >new file mode 100644 >index 0000000000000000000000000000000000000000..cbc3ac6339b81fc88a3144b51a2b91135d991190 >--- /dev/null >+++ b/Tools/TestWebKitAPI/Tests/mac/NoPolicyDelegateResponse.mm >@@ -0,0 +1,77 @@ >+/* >+ * Copyright (C) 2018 Apple Inc. All rights reserved. >+ * >+ * Redistribution and use in source and binary forms, with or without >+ * modification, are permitted provided that the following conditions >+ * are met: >+ * 1. Redistributions of source code must retain the above copyright >+ * notice, this list of conditions and the following disclaimer. >+ * 2. Redistributions in binary form must reproduce the above copyright >+ * notice, this list of conditions and the following disclaimer in the >+ * documentation and/or other materials provided with the distribution. >+ * >+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' >+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, >+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR >+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS >+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR >+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF >+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS >+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN >+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) >+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF >+ * THE POSSIBILITY OF SUCH DAMAGE. >+ */ >+ >+#import "config.h" >+#import "PlatformUtilities.h" >+#import <WebKit/WebViewPrivate.h> >+#import <wtf/RetainPtr.h> >+ >+@interface NoPolicyDelegateDecisionDelegate : NSObject <WebPolicyDelegate, WebFrameLoadDelegate> { >+} >+@end >+ >+static bool didFinishLoad; >+static bool didNavigationActionCheck; >+static bool didNavigationResponseCheck; >+ >+@implementation NoPolicyDelegateDecisionDelegate >+ >+- (void)webView:(WebView *)webView decidePolicyForNavigationAction:(NSDictionary *)actionInformation request:(NSURLRequest *)request frame:(WebFrame *)frame decisionListener:(id<WebPolicyDecisionListener>)listener >+{ >+ // Implements decidePolicyForNavigationAction but fails to call the decision listener. >+ didNavigationActionCheck = YES; >+} >+ >+- (void)webView:(WebView *)webView decidePolicyForMIMEType:(NSString *)type request:(NSURLRequest *)request frame:(WebFrame *)frame decisionListener:(id<WebPolicyDecisionListener>)listener >+{ >+ // Implements decidePolicyForMIMEType but fails to call the decision listener. >+ didNavigationResponseCheck = YES; >+} >+ >+- (void)webView:(WebView *)sender didFinishLoadForFrame:(WebFrame *)frame >+{ >+ didFinishLoad = true; >+} >+ >+@end >+ >+namespace TestWebKitAPI { >+ >+TEST(WebKitLegacy, NoPolicyDelegateDecision) >+{ >+ auto webView = adoptNS([[WebView alloc] initWithFrame:NSZeroRect frameName:nil groupName:nil]); >+ auto delegate = adoptNS([NoPolicyDelegateDecisionDelegate new]); >+ >+ webView.get().frameLoadDelegate = delegate.get(); >+ webView.get().policyDelegate = delegate.get(); >+ [[webView.get() mainFrame] loadRequest:[NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"verboseMarkup" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]]]; >+ >+ Util::run(&didFinishLoad); >+ >+ EXPECT_TRUE(didNavigationActionCheck); >+ EXPECT_TRUE(didNavigationResponseCheck); >+} >+ >+} // namespace TestWebKitAPI
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 185832
:
340874
|
340876
|
340885
|
340887
|
340888
|
340901
|
340909
|
340916
|
340920
|
340921