Diff
Modified: trunk/Source/WebKit2/ChangeLog (219153 => 219154)
--- trunk/Source/WebKit2/ChangeLog 2017-07-05 20:32:41 UTC (rev 219153)
+++ trunk/Source/WebKit2/ChangeLog 2017-07-05 20:41:04 UTC (rev 219154)
@@ -1,3 +1,43 @@
+2017-07-05 Chris Dumez <cdu...@apple.com>
+
+ Regression(r218821): Bad cast to WebKit::DiagnosticLoggingClient in WKWebView's _setDiagnosticLoggingDelegate
+ https://bugs.webkit.org/show_bug.cgi?id=174163
+ <rdar://problem/33067518>
+
+ Reviewed by Alex Christensen.
+
+ After r218821, WebPageProxy::setDiagnosticLoggingClient() would no longer set the client in
+ private session. This is an issue because there is code in WKWebView which expects the
+ DiagnosticLoggingClient to be set and of a specific type. This would lead to bad casts in
+ WKWebView's _diagnosticLoggingDelegate / _setDiagnosticLoggingDelegate.
+
+ To avoid the issue, we now unconditionally update the client in
+ WebPageProxy::setDiagnosticLoggingClient(). Instead, we implement the logic for disabling
+ logging in private session in the logging methods themselves, via the
+ WebPageProxy::effectiveDiagnosticLoggingClient() helper instead.
+
+ I also added null checks in WKWebView's _diagnosticLoggingDelegate /
+ _setDiagnosticLoggingDelegate. Even though the WKWebView sets itself as the diagnostic
+ logging client, WebPageProxy::diagnosticLoggingClient() can still return null after
+ calling WebPageProxy::close(). Previously, we would have done a bad cast if the WKWebView's
+ client would have used this API after calling 'close' as well.
+
+ * UIProcess/API/APIDiagnosticLoggingClient.h:
+ * UIProcess/API/Cocoa/WKWebView.mm:
+ (-[WKWebView _diagnosticLoggingDelegate]):
+ (-[WKWebView _setDiagnosticLoggingDelegate:]):
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::WebPageProxy):
+ (WebKit::WebPageProxy::setDiagnosticLoggingClient):
+ (WebKit::WebPageProxy::close):
+ (WebKit::WebPageProxy::effectiveDiagnosticLoggingClient):
+ (WebKit::WebPageProxy::logDiagnosticMessage):
+ (WebKit::WebPageProxy::logDiagnosticMessageWithResult):
+ (WebKit::WebPageProxy::logDiagnosticMessageWithValue):
+ (WebKit::WebPageProxy::logDiagnosticMessageWithEnhancedPrivacy):
+ * UIProcess/WebPageProxy.h:
+ (WebKit::WebPageProxy::diagnosticLoggingClient):
+
2017-07-05 Daniel Bates <daba...@apple.com>
Do not pass API::FrameInfo for source frame or clear out page of target frame on
Modified: trunk/Source/WebKit2/UIProcess/API/APIDiagnosticLoggingClient.h (219153 => 219154)
--- trunk/Source/WebKit2/UIProcess/API/APIDiagnosticLoggingClient.h 2017-07-05 20:32:41 UTC (rev 219153)
+++ trunk/Source/WebKit2/UIProcess/API/APIDiagnosticLoggingClient.h 2017-07-05 20:41:04 UTC (rev 219154)
@@ -39,10 +39,10 @@
public:
virtual ~DiagnosticLoggingClient() { }
- virtual void logDiagnosticMessage(WebKit::WebPageProxy*, const WTF::String& message, const WTF::String& description) { UNUSED_PARAM(message); UNUSED_PARAM(description); }
- virtual void logDiagnosticMessageWithResult(WebKit::WebPageProxy*, const WTF::String& message, const WTF::String& description, WebCore::DiagnosticLoggingResultType) { UNUSED_PARAM(message); UNUSED_PARAM(description); }
- virtual void logDiagnosticMessageWithValue(WebKit::WebPageProxy*, const WTF::String& message, const WTF::String& description, const WTF::String& value) { UNUSED_PARAM(message); UNUSED_PARAM(description); UNUSED_PARAM(value); }
- virtual void logDiagnosticMessageWithEnhancedPrivacy(WebKit::WebPageProxy*, const WTF::String& message, const WTF::String& description) { UNUSED_PARAM(message); UNUSED_PARAM(description); }
+ virtual void logDiagnosticMessage(WebKit::WebPageProxy*, const WTF::String& message, const WTF::String& description) = 0;
+ virtual void logDiagnosticMessageWithResult(WebKit::WebPageProxy*, const WTF::String& message, const WTF::String& description, WebCore::DiagnosticLoggingResultType) = 0;
+ virtual void logDiagnosticMessageWithValue(WebKit::WebPageProxy*, const WTF::String& message, const WTF::String& description, const WTF::String& value) = 0;
+ virtual void logDiagnosticMessageWithEnhancedPrivacy(WebKit::WebPageProxy*, const WTF::String& message, const WTF::String& description) = 0;
};
} // namespace API
Modified: trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm (219153 => 219154)
--- trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm 2017-07-05 20:32:41 UTC (rev 219153)
+++ trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm 2017-07-05 20:41:04 UTC (rev 219154)
@@ -4204,12 +4204,20 @@
- (id <_WKDiagnosticLoggingDelegate>)_diagnosticLoggingDelegate
{
- return [static_cast<WebKit::DiagnosticLoggingClient&>(_page->diagnosticLoggingClient()).delegate().leakRef() autorelease];
+ auto* diagnosticLoggingClient = _page->diagnosticLoggingClient();
+ if (!diagnosticLoggingClient)
+ return nil;
+
+ return [static_cast<WebKit::DiagnosticLoggingClient&>(*diagnosticLoggingClient).delegate().leakRef() autorelease];
}
- (void)_setDiagnosticLoggingDelegate:(id<_WKDiagnosticLoggingDelegate>)diagnosticLoggingDelegate
{
- static_cast<WebKit::DiagnosticLoggingClient&>(_page->diagnosticLoggingClient()).setDelegate(diagnosticLoggingDelegate);
+ auto* diagnosticLoggingClient = _page->diagnosticLoggingClient();
+ if (!diagnosticLoggingClient)
+ return;
+
+ static_cast<WebKit::DiagnosticLoggingClient&>(*diagnosticLoggingClient).setDelegate(diagnosticLoggingDelegate);
}
- (id <_WKFindDelegate>)_findDelegate
Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp (219153 => 219154)
--- trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2017-07-05 20:32:41 UTC (rev 219153)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2017-07-05 20:41:04 UTC (rev 219154)
@@ -332,7 +332,6 @@
, m_uiClient(std::make_unique<API::UIClient>())
, m_findClient(std::make_unique<API::FindClient>())
, m_findMatchesClient(std::make_unique<API::FindMatchesClient>())
- , m_diagnosticLoggingClient(std::make_unique<API::DiagnosticLoggingClient>())
#if ENABLE(CONTEXT_MENUS)
, m_contextMenuClient(std::make_unique<API::ContextMenuClient>())
#endif
@@ -563,12 +562,6 @@
void WebPageProxy::setDiagnosticLoggingClient(std::unique_ptr<API::DiagnosticLoggingClient>&& diagnosticLoggingClient)
{
- // Diagnostic logging is disabled for ephemeral sessions for privacy reasons.
- if (sessionID().isEphemeral() || !diagnosticLoggingClient) {
- m_diagnosticLoggingClient = std::make_unique<API::DiagnosticLoggingClient>();
- return;
- }
-
m_diagnosticLoggingClient = WTFMove(diagnosticLoggingClient);
}
@@ -766,7 +759,7 @@
m_uiClient = std::make_unique<API::UIClient>();
m_findClient = std::make_unique<API::FindClient>();
m_findMatchesClient = std::make_unique<API::FindMatchesClient>();
- m_diagnosticLoggingClient = std::make_unique<API::DiagnosticLoggingClient>();
+ m_diagnosticLoggingClient = nullptr;
#if ENABLE(CONTEXT_MENUS)
m_contextMenuClient = std::make_unique<API::ContextMenuClient>();
#endif
@@ -5142,36 +5135,49 @@
}
#endif
+inline API::DiagnosticLoggingClient* WebPageProxy::effectiveDiagnosticLoggingClient(ShouldSample shouldSample)
+{
+ // Diagnostic logging is disabled for ephemeral sessions for privacy reasons.
+ if (sessionID().isEphemeral())
+ return nullptr;
+
+ return DiagnosticLoggingClient::shouldLogAfterSampling(shouldSample) ? diagnosticLoggingClient() : nullptr;
+}
+
void WebPageProxy::logDiagnosticMessage(const String& message, const String& description, WebCore::ShouldSample shouldSample)
{
- if (!DiagnosticLoggingClient::shouldLogAfterSampling(shouldSample))
+ auto* effectiveClient = effectiveDiagnosticLoggingClient(shouldSample);
+ if (!effectiveClient)
return;
- diagnosticLoggingClient().logDiagnosticMessage(this, message, description);
+ effectiveClient->logDiagnosticMessage(this, message, description);
}
void WebPageProxy::logDiagnosticMessageWithResult(const String& message, const String& description, uint32_t result, WebCore::ShouldSample shouldSample)
{
- if (!DiagnosticLoggingClient::shouldLogAfterSampling(shouldSample))
+ auto* effectiveClient = effectiveDiagnosticLoggingClient(shouldSample);
+ if (!effectiveClient)
return;
- diagnosticLoggingClient().logDiagnosticMessageWithResult(this, message, description, static_cast<WebCore::DiagnosticLoggingResultType>(result));
+ effectiveClient->logDiagnosticMessageWithResult(this, message, description, static_cast<WebCore::DiagnosticLoggingResultType>(result));
}
void WebPageProxy::logDiagnosticMessageWithValue(const String& message, const String& description, double value, unsigned significantFigures, ShouldSample shouldSample)
{
- if (!DiagnosticLoggingClient::shouldLogAfterSampling(shouldSample))
+ auto* effectiveClient = effectiveDiagnosticLoggingClient(shouldSample);
+ if (!effectiveClient)
return;
- diagnosticLoggingClient().logDiagnosticMessageWithValue(this, message, description, String::number(value, significantFigures));
+ effectiveClient->logDiagnosticMessageWithValue(this, message, description, String::number(value, significantFigures));
}
void WebPageProxy::logDiagnosticMessageWithEnhancedPrivacy(const String& message, const String& description, ShouldSample shouldSample)
{
- if (!DiagnosticLoggingClient::shouldLogAfterSampling(shouldSample))
+ auto* effectiveClient = effectiveDiagnosticLoggingClient(shouldSample);
+ if (!effectiveClient)
return;
- diagnosticLoggingClient().logDiagnosticMessageWithEnhancedPrivacy(this, message, description);
+ effectiveClient->logDiagnosticMessageWithEnhancedPrivacy(this, message, description);
}
void WebPageProxy::logScrollingEvent(uint32_t eventType, MonotonicTime timestamp, uint64_t data)
Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.h (219153 => 219154)
--- trunk/Source/WebKit2/UIProcess/WebPageProxy.h 2017-07-05 20:32:41 UTC (rev 219153)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.h 2017-07-05 20:41:04 UTC (rev 219154)
@@ -359,7 +359,7 @@
void setFindClient(std::unique_ptr<API::FindClient>&&);
API::FindMatchesClient& findMatchesClient() { return *m_findMatchesClient; }
void setFindMatchesClient(std::unique_ptr<API::FindMatchesClient>&&);
- API::DiagnosticLoggingClient& diagnosticLoggingClient() { return *m_diagnosticLoggingClient; }
+ API::DiagnosticLoggingClient* diagnosticLoggingClient() { return m_diagnosticLoggingClient.get(); }
void setDiagnosticLoggingClient(std::unique_ptr<API::DiagnosticLoggingClient>&&);
void setFormClient(std::unique_ptr<API::FormClient>&&);
void setNavigationClient(std::unique_ptr<API::NavigationClient>&&);
@@ -1587,6 +1587,7 @@
#endif
uint64_t generateNavigationID();
+ API::DiagnosticLoggingClient* effectiveDiagnosticLoggingClient(WebCore::ShouldSample);
WebPreferencesStore preferencesStore() const;
Modified: trunk/Tools/ChangeLog (219153 => 219154)
--- trunk/Tools/ChangeLog 2017-07-05 20:32:41 UTC (rev 219153)
+++ trunk/Tools/ChangeLog 2017-07-05 20:41:04 UTC (rev 219154)
@@ -1,3 +1,17 @@
+2017-07-05 Chris Dumez <cdu...@apple.com>
+
+ Regression(r218821): Bad cast to WebKit::DiagnosticLoggingClient in WKWebView's _setDiagnosticLoggingDelegate
+ https://bugs.webkit.org/show_bug.cgi?id=174163
+ <rdar://problem/33067518>
+
+ Reviewed by Alex Christensen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewDiagnosticLogging.mm: Added.
+ (TEST):
+
2017-07-05 Wenson Hsieh <wenson_hs...@apple.com>
When dragging a selection, clearing the selection in dragstart should not crash the web process
Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (219153 => 219154)
--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2017-07-05 20:32:41 UTC (rev 219153)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2017-07-05 20:41:04 UTC (rev 219154)
@@ -141,6 +141,7 @@
46C519E61D3563FD00DAA51A /* LocalStorageNullEntries.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 46C519E21D35629600DAA51A /* LocalStorageNullEntries.html */; };
46C519E71D3563FD00DAA51A /* LocalStorageNullEntries.localstorage in Copy Resources */ = {isa = PBXBuildFile; fileRef = 46C519E31D35629600DAA51A /* LocalStorageNullEntries.localstorage */; };
46C519E81D3563FD00DAA51A /* LocalStorageNullEntries.localstorage-shm in Copy Resources */ = {isa = PBXBuildFile; fileRef = 46C519E41D35629600DAA51A /* LocalStorageNullEntries.localstorage-shm */; };
+ 46E66A901F0D75590026D83C /* WKWebViewDiagnosticLogging.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46E66A8F1F0D75590026D83C /* WKWebViewDiagnosticLogging.mm */; };
46E816F81E79E29C00375ADC /* RestoreStateAfterTermination.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46E816F71E79E29100375ADC /* RestoreStateAfterTermination.mm */; };
4BFDFFA71314776C0061F24B /* HitTestResultNodeHandle_Bundle.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4BFDFFA61314776C0061F24B /* HitTestResultNodeHandle_Bundle.cpp */; };
510477721D298DDD009747EB /* IDBDeleteRecovery.sqlite3 in Copy Resources */ = {isa = PBXBuildFile; fileRef = 5104776F1D298D85009747EB /* IDBDeleteRecovery.sqlite3 */; };
@@ -1158,6 +1159,7 @@
46C519E21D35629600DAA51A /* LocalStorageNullEntries.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = LocalStorageNullEntries.html; sourceTree = "<group>"; };
46C519E31D35629600DAA51A /* LocalStorageNullEntries.localstorage */ = {isa = PBXFileReference; lastKnownFileType = file; path = LocalStorageNullEntries.localstorage; sourceTree = "<group>"; };
46C519E41D35629600DAA51A /* LocalStorageNullEntries.localstorage-shm */ = {isa = PBXFileReference; lastKnownFileType = file; path = "LocalStorageNullEntries.localstorage-shm"; sourceTree = "<group>"; };
+ 46E66A8F1F0D75590026D83C /* WKWebViewDiagnosticLogging.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKWebViewDiagnosticLogging.mm; sourceTree = "<group>"; };
46E816F71E79E29100375ADC /* RestoreStateAfterTermination.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = RestoreStateAfterTermination.mm; sourceTree = "<group>"; };
4A410F4B19AF7BD6002EBAB5 /* UserMedia.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = UserMedia.cpp; sourceTree = "<group>"; };
4A410F4D19AF7BEF002EBAB5 /* getUserMedia.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = getUserMedia.html; sourceTree = "<group>"; };
@@ -1910,6 +1912,7 @@
D3BE5E341E4CE85E00FD563A /* WKWebViewGetContents.mm */,
93F56DA81E5F9181003EDE84 /* WKWebViewSnapshot.mm */,
9984FACA1CFFAEEE008D198C /* WKWebViewTextInput.mm */,
+ 46E66A8F1F0D75590026D83C /* WKWebViewDiagnosticLogging.mm */,
);
name = "WebKit2 Cocoa";
path = WebKit2Cocoa;
@@ -3206,6 +3209,7 @@
A14FC5901B8AE36F00D107EB /* TestProtocol.mm in Sources */,
7CCE7EAE1A411A3400447C4C /* TestsController.cpp in Sources */,
2EFF06D41D8AEDBB0004BB30 /* TestWKWebView.mm in Sources */,
+ 46E66A901F0D75590026D83C /* WKWebViewDiagnosticLogging.mm in Sources */,
CE3524F91B1441C40028A7C5 /* TextFieldDidBeginAndEndEditing.cpp in Sources */,
7CCE7EDD1A411A9200447C4C /* TimeRanges.cpp in Sources */,
0F4FFA9E1ED3AA8500F7111F /* SnapshotViaRenderInContext.mm in Sources */,
Added: trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewDiagnosticLogging.mm (0 => 219154)
--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewDiagnosticLogging.mm (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewDiagnosticLogging.mm 2017-07-05 20:41:04 UTC (rev 219154)
@@ -0,0 +1,74 @@
+/*
+ * Copyright (C) 2016 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"
+
+#if WK_API_ENABLED
+
+#import "PlatformUtilities.h"
+#import "Test.h"
+#import "TestNavigationDelegate.h"
+#import <WebKit/WKWebViewPrivate.h>
+#import <WebKit/_WKDiagnosticLoggingDelegate.h>
+#import <wtf/RetainPtr.h>
+
+@interface TestLoggingDelegate : NSObject <_WKDiagnosticLoggingDelegate>
+@end
+
+@implementation TestLoggingDelegate
+@end
+
+TEST(WKWebView, PrivateSessionDiagnosticLoggingDelegate)
+{
+ auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ configuration.get().websiteDataStore = [WKWebsiteDataStore nonPersistentDataStore];
+ auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+
+ auto testLoggingDelegate = adoptNS([[TestLoggingDelegate alloc] init]);
+ webView.get()._diagnosticLoggingDelegate = testLoggingDelegate.get();
+
+ EXPECT_EQ(testLoggingDelegate.get(), webView.get()._diagnosticLoggingDelegate);
+
+ webView.get()._diagnosticLoggingDelegate = nil;
+ EXPECT_EQ(nil, webView.get()._diagnosticLoggingDelegate);
+}
+
+TEST(WKWebView, DiagnosticLoggingDelegateAfterClose)
+{
+ auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+
+ auto testLoggingDelegate = adoptNS([[TestLoggingDelegate alloc] init]);
+ webView.get()._diagnosticLoggingDelegate = testLoggingDelegate.get();
+
+ EXPECT_EQ(testLoggingDelegate.get(), webView.get()._diagnosticLoggingDelegate);
+
+ [webView.get() _close];
+ EXPECT_EQ(nil, webView.get()._diagnosticLoggingDelegate);
+
+ webView.get()._diagnosticLoggingDelegate = testLoggingDelegate.get();
+ EXPECT_EQ(nil, webView.get()._diagnosticLoggingDelegate);
+}
+
+#endif
