Title: [289962] branches/safari-613-branch/Source/WebKit
- Revision
- 289962
- Author
- repst...@apple.com
- Date
- 2022-02-16 14:10:35 -0800 (Wed, 16 Feb 2022)
Log Message
Cherry-pick r289875. rdar://problem/88358696
Web Inspector: [Cocoa] Reentrancy in WebKit::WebInspectorUIProxy::open
https://bugs.webkit.org/show_bug.cgi?id=236672
Reviewed by Devin Rousso.
Speculative fix for non-reproducible reentrancy. Because `WebInspectorUIProxy::open` calls
`WebInspectorUIProxy::platformBringToFront`, which under some conditions can call `WebInspectorUIProxy::open`,
there was an opportunity for recurssion. This appears to happen when the window of the inspector view does not
match the window of the inspected web view, which in general should not be possible for a newly opened
inspector. My suspicion is that the web view is not actually attached to a window at the time the inspector is
being opened. This patch adds a fail-safe that will detach the inspector view into its own window when these
conditions are met while we are in middle of opening the inspector, and also adds logging to indicate if the
inspected web view was actually in a window. This should both prevent the crash from the re-entry as well as
provide more context when the issue does occur.
* UIProcess/Inspector/WebInspectorUIProxy.cpp:
(WebKit::WebInspectorUIProxy::open):
* UIProcess/Inspector/WebInspectorUIProxy.h:
* UIProcess/Inspector/mac/WebInspectorUIProxyMac.mm:
(WebKit::WebInspectorUIProxy::platformBringToFront):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@289875 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-613-branch/Source/WebKit/ChangeLog (289961 => 289962)
--- branches/safari-613-branch/Source/WebKit/ChangeLog 2022-02-16 21:59:47 UTC (rev 289961)
+++ branches/safari-613-branch/Source/WebKit/ChangeLog 2022-02-16 22:10:35 UTC (rev 289962)
@@ -1,3 +1,54 @@
+2022-02-16 Russell Epstein <repst...@apple.com>
+
+ Cherry-pick r289875. rdar://problem/88358696
+
+ Web Inspector: [Cocoa] Reentrancy in WebKit::WebInspectorUIProxy::open
+ https://bugs.webkit.org/show_bug.cgi?id=236672
+
+ Reviewed by Devin Rousso.
+
+ Speculative fix for non-reproducible reentrancy. Because `WebInspectorUIProxy::open` calls
+ `WebInspectorUIProxy::platformBringToFront`, which under some conditions can call `WebInspectorUIProxy::open`,
+ there was an opportunity for recurssion. This appears to happen when the window of the inspector view does not
+ match the window of the inspected web view, which in general should not be possible for a newly opened
+ inspector. My suspicion is that the web view is not actually attached to a window at the time the inspector is
+ being opened. This patch adds a fail-safe that will detach the inspector view into its own window when these
+ conditions are met while we are in middle of opening the inspector, and also adds logging to indicate if the
+ inspected web view was actually in a window. This should both prevent the crash from the re-entry as well as
+ provide more context when the issue does occur.
+
+ * UIProcess/Inspector/WebInspectorUIProxy.cpp:
+ (WebKit::WebInspectorUIProxy::open):
+ * UIProcess/Inspector/WebInspectorUIProxy.h:
+ * UIProcess/Inspector/mac/WebInspectorUIProxyMac.mm:
+ (WebKit::WebInspectorUIProxy::platformBringToFront):
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@289875 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2022-02-15 Patrick Angle <pan...@apple.com>
+
+ Web Inspector: [Cocoa] Reentrancy in WebKit::WebInspectorUIProxy::open
+ https://bugs.webkit.org/show_bug.cgi?id=236672
+
+ Reviewed by Devin Rousso.
+
+ Speculative fix for non-reproducible reentrancy. Because `WebInspectorUIProxy::open` calls
+ `WebInspectorUIProxy::platformBringToFront`, which under some conditions can call `WebInspectorUIProxy::open`,
+ there was an opportunity for recurssion. This appears to happen when the window of the inspector view does not
+ match the window of the inspected web view, which in general should not be possible for a newly opened
+ inspector. My suspicion is that the web view is not actually attached to a window at the time the inspector is
+ being opened. This patch adds a fail-safe that will detach the inspector view into its own window when these
+ conditions are met while we are in middle of opening the inspector, and also adds logging to indicate if the
+ inspected web view was actually in a window. This should both prevent the crash from the re-entry as well as
+ provide more context when the issue does occur.
+
+ * UIProcess/Inspector/WebInspectorUIProxy.cpp:
+ (WebKit::WebInspectorUIProxy::open):
+ * UIProcess/Inspector/WebInspectorUIProxy.h:
+ * UIProcess/Inspector/mac/WebInspectorUIProxyMac.mm:
+ (WebKit::WebInspectorUIProxy::platformBringToFront):
+
2022-02-15 Russell Epstein <repst...@apple.com>
Revert r289340. rdar://problem/88629773
Modified: branches/safari-613-branch/Source/WebKit/UIProcess/Inspector/WebInspectorUIProxy.cpp (289961 => 289962)
--- branches/safari-613-branch/Source/WebKit/UIProcess/Inspector/WebInspectorUIProxy.cpp 2022-02-16 21:59:47 UTC (rev 289961)
+++ branches/safari-613-branch/Source/WebKit/UIProcess/Inspector/WebInspectorUIProxy.cpp 2022-02-16 22:10:35 UTC (rev 289962)
@@ -500,9 +500,7 @@
if (!m_inspectorPage)
return;
-#if PLATFORM(GTK)
SetForScope<bool> isOpening(m_isOpening, true);
-#endif
m_isVisible = true;
m_inspectorPage->send(Messages::WebInspectorUI::SetIsVisible(m_isVisible));
Modified: branches/safari-613-branch/Source/WebKit/UIProcess/Inspector/WebInspectorUIProxy.h (289961 => 289962)
--- branches/safari-613-branch/Source/WebKit/UIProcess/Inspector/WebInspectorUIProxy.h 2022-02-16 21:59:47 UTC (rev 289961)
+++ branches/safari-613-branch/Source/WebKit/UIProcess/Inspector/WebInspectorUIProxy.h 2022-02-16 22:10:35 UTC (rev 289962)
@@ -305,6 +305,7 @@
bool m_elementSelectionActive { false };
bool m_ignoreElementSelectionChange { false };
bool m_isActiveFrontend { false };
+ bool m_isOpening { false };
bool m_closing { false };
AttachmentSide m_attachmentSide {AttachmentSide::Bottom};
@@ -325,7 +326,6 @@
GtkWidget* m_inspectorWindow { nullptr };
GtkWidget* m_headerBar { nullptr };
String m_inspectedURLString;
- bool m_isOpening { false };
#elif PLATFORM(WIN)
HWND m_inspectedViewWindow { nullptr };
HWND m_inspectedViewParentWindow { nullptr };
Modified: branches/safari-613-branch/Source/WebKit/UIProcess/Inspector/mac/WebInspectorUIProxyMac.mm (289961 => 289962)
--- branches/safari-613-branch/Source/WebKit/UIProcess/Inspector/mac/WebInspectorUIProxyMac.mm 2022-02-16 21:59:47 UTC (rev 289961)
+++ branches/safari-613-branch/Source/WebKit/UIProcess/Inspector/mac/WebInspectorUIProxyMac.mm 2022-02-16 22:10:35 UTC (rev 289962)
@@ -32,6 +32,7 @@
#import "APIInspectorConfiguration.h"
#import "APIUIClient.h"
#import "GlobalFindInPageState.h"
+#import "Logging.h"
#import "WKInspectorPrivateMac.h"
#import "WKInspectorViewController.h"
#import "WKObject.h"
@@ -397,6 +398,16 @@
// then we need to reopen the Inspector to get it attached to the right window.
// This can happen when dragging tabs to another window in Safari.
if (m_isAttached && [m_inspectorViewController webView].window != inspectedPage()->platformWindow()) {
+ if (m_isOpening) {
+ // <rdar://88358696> If we are currently opening an attached inspector, the windows should have already
+ // matched, and calling back to `open` isn't going to correct this. As a fail-safe to prevent reentrancy,
+ // fall back to detaching the inspector when there is a mismatch in the web view's window and the
+ // inspector's window.
+ RELEASE_LOG(Inspector, "WebInspectorUIProxy::platformBringToFront - Inspected and inspector windows did not match while opening inspector. Falling back to detached inspector. Inspected page had window: %s", inspectedPage()->platformWindow() ? "YES" : "NO");
+ detach();
+ return;
+ }
+
open();
return;
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
