193204 2019-01-07 12:21:18 -0800 Web Inspector: Remote inspector can crash if attempting to navigate inspector page 2019-01-07 14:13:38 -0800 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck webkit-unassigned commit-queue hi inspector-bugzilla-changes joepeck webkit-bug-importer oldest_to_newest 1492335 0 joepeck 2019-01-07 12:21:18 -0800 Remote inspector can crash if attempting to navigate inspector page Steps to Reproduce: 1. Open a remote inspector 2. Inspect that inspector 3. In inspector² js> location.href = "http://apple.com" => Inspector crash Crash: ... Exception Type: EXC_BAD_ACCESS (SIGSEGV) Exception Codes: KERN_INVALID_ADDRESS at 0x00000000000005c2 Exception Note: EXC_CORPSE_NOTIFY Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.WebKit 0x000000010e4f754b WebKit::WebPageProxy::loadRequest(WebCore::ResourceRequest&&, WebCore::ShouldOpenExternalURLsPolicy, API::Object*) + 37 (WebPageProxy.cpp:1068) 1 com.apple.WebKit 0x000000010e572e4c -[WKInspectorViewController webView:decidePolicyForNavigationAction:decisionHandler:] + 281 (utility:898) 2 com.apple.WebKit 0x000000010e494549 WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction(WebKit::WebPageProxy&, WTF::Ref<API::NavigationAction, WTF::DumbPtrTraits<API::NavigationAction> >&&, WTF::Ref<WebKit::WebFramePolicyListenerProxy, WTF::DumbPtrTraits<WebKit::WebFramePolicyListenerProxy> >&&, API::Object*) + 647 ... 1492336 1 joepeck 2019-01-07 12:21:28 -0800 <rdar://problem/45550428> 1492337 2 358512 joepeck 2019-01-07 12:22:20 -0800 Created attachment 358512 [PATCH] Proposed Fix 1492373 3 358512 hi 2019-01-07 13:45:26 -0800 Comment on attachment 358512 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=358512&action=review rs=me, never a bad idea to add a nullptr check 😁 > Source/WebKit/UIProcess/mac/RemoteWebInspectorProxyMac.mm:88 > + m_inspectorView = adoptNS([[WKInspectorViewController alloc] initWithInspectedPage:nullptr]); Just for my knowledge, is it preferred to use `nil` or `nullptr` in ObjC code? Is `nullptr` preferred here because the value expected by `initWithInspectedPage` is a `WebPageProxy`, which is a C++ object, or am I just overthinking things? 1492376 4 358512 joepeck 2019-01-07 13:47:45 -0800 Comment on attachment 358512 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=358512&action=review >> Source/WebKit/UIProcess/mac/RemoteWebInspectorProxyMac.mm:88 >> + m_inspectorView = adoptNS([[WKInspectorViewController alloc] initWithInspectedPage:nullptr]); > > Just for my knowledge, is it preferred to use `nil` or `nullptr` in ObjC code? > Is `nullptr` preferred here because the value expected by `initWithInspectedPage` is a `WebPageProxy`, which is a C++ object, or am I just overthinking things? Using nil/nullptr and friends just like where to place the "*", the type is what matters: 0 - never NULL - C type nullptr - C++ type nil - ObjC type Nil - ObjC Class type I think that covers all the various cases. 1492395 5 358512 commit-queue 2019-01-07 14:13:37 -0800 Comment on attachment 358512 [PATCH] Proposed Fix Clearing flags on attachment: 358512 Committed r239697: <https://trac.webkit.org/changeset/239697> 1492396 6 commit-queue 2019-01-07 14:13:38 -0800 All reviewed patches have been landed. Closing bug. 358512 2019-01-07 12:22:20 -0800 2019-01-07 14:13:37 -0800 [PATCH] Proposed Fix proposed-fix-1.patch text/plain 2420 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nIGIvU291cmNlL1dlYktpdC9DaGFu Z2VMb2cKaW5kZXggNzA3Yjc4MWEwYjIuLjc3OGE2ZjZkNWY0IDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViS2l0L0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViS2l0L0NoYW5nZUxvZwpAQCAtMSwzICsx LDE2IEBACisyMDE5LTAxLTA3ICBKb3NlcGggUGVjb3Jhcm8gIDxwZWNvcmFyb0BhcHBsZS5jb20+ CisKKyAgICAgICAgV2ViIEluc3BlY3RvcjogUmVtb3RlIGluc3BlY3RvciBjYW4gY3Jhc2ggaWYg YXR0ZW1wdGluZyB0byBuYXZpZ2F0ZSBpbnNwZWN0b3IgcGFnZQorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTkzMjA0CisgICAgICAgIDxyZGFyOi8vcHJv YmxlbS80NTU1MDQyOD4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICAqIFVJUHJvY2Vzcy9tYWMvUmVtb3RlV2ViSW5zcGVjdG9yUHJveHlNYWMubW06Cisg ICAgICAgIChXZWJLaXQ6OlJlbW90ZVdlYkluc3BlY3RvclByb3h5OjpwbGF0Zm9ybUNyZWF0ZUZy b250ZW5kUGFnZUFuZFdpbmRvdyk6CisgICAgICAgICogVUlQcm9jZXNzL21hYy9XS0luc3BlY3Rv clZpZXdDb250cm9sbGVyLm1tOgorICAgICAgICAoLVtXS0luc3BlY3RvclZpZXdDb250cm9sbGVy IHdlYlZpZXc6ZGVjaWRlUG9saWN5Rm9yTmF2aWdhdGlvbkFjdGlvbjpkZWNpc2lvbkhhbmRsZXI6 XSk6CisKIDIwMTktMDEtMDQgIEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4K IAogICAgICAgICBbQ29jb2FdIEFkZCBTUEkgdG8gY2hlY2sgaWYgYSBXS1dlYlZpZXcgaGFzIGFu IGluc3BlY3RvciBmcm9udGVuZApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9VSVByb2Nlc3Mv bWFjL1JlbW90ZVdlYkluc3BlY3RvclByb3h5TWFjLm1tIGIvU291cmNlL1dlYktpdC9VSVByb2Nl c3MvbWFjL1JlbW90ZVdlYkluc3BlY3RvclByb3h5TWFjLm1tCmluZGV4IGNhZWUyNmJkYzIyLi5l YmQ2YTM5MTY0MyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvbWFjL1JlbW90 ZVdlYkluc3BlY3RvclByb3h5TWFjLm1tCisrKyBiL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL21h Yy9SZW1vdGVXZWJJbnNwZWN0b3JQcm94eU1hYy5tbQpAQCAtODUsNyArODUsNyBAQCBXZWJQYWdl UHJveHkqIFJlbW90ZVdlYkluc3BlY3RvclByb3h5OjpwbGF0Zm9ybUNyZWF0ZUZyb250ZW5kUGFn ZUFuZFdpbmRvdygpCiB7CiAgICAgbV9vYmpDQWRhcHRlciA9IGFkb3B0TlMoW1tXS1JlbW90ZVdl Ykluc3BlY3RvclByb3h5T2JqQ0FkYXB0ZXIgYWxsb2NdIGluaXRXaXRoUmVtb3RlV2ViSW5zcGVj dG9yUHJveHk6dGhpc10pOwogCi0gICAgbV9pbnNwZWN0b3JWaWV3ID0gYWRvcHROUyhbW1dLSW5z cGVjdG9yVmlld0NvbnRyb2xsZXIgYWxsb2NdIGluaXRXaXRoSW5zcGVjdGVkUGFnZTpuaWxdKTsK KyAgICBtX2luc3BlY3RvclZpZXcgPSBhZG9wdE5TKFtbV0tJbnNwZWN0b3JWaWV3Q29udHJvbGxl ciBhbGxvY10gaW5pdFdpdGhJbnNwZWN0ZWRQYWdlOm51bGxwdHJdKTsKICAgICBbbV9pbnNwZWN0 b3JWaWV3LmdldCgpIHNldERlbGVnYXRlOm1fb2JqQ0FkYXB0ZXIuZ2V0KCldOwogCiAgICAgbV93 aW5kb3cgPSBXZWJJbnNwZWN0b3JQcm94eTo6Y3JlYXRlRnJvbnRlbmRXaW5kb3coTlNaZXJvUmVj dCk7CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9tYWMvV0tJbnNwZWN0b3JW aWV3Q29udHJvbGxlci5tbSBiL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL21hYy9XS0luc3BlY3Rv clZpZXdDb250cm9sbGVyLm1tCmluZGV4IGM1MzFkZGY0OGQwLi42Y2I2YzJhZjc3ZCAxMDA2NDQK LS0tIGEvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvbWFjL1dLSW5zcGVjdG9yVmlld0NvbnRyb2xs ZXIubW0KKysrIGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvbWFjL1dLSW5zcGVjdG9yVmlld0Nv bnRyb2xsZXIubW0KQEAgLTIyNiw3ICsyMjYsOCBAQCAtICh2b2lkKXdlYlZpZXc6KFdLV2ViVmll dyAqKXdlYlZpZXcgZGVjaWRlUG9saWN5Rm9yTmF2aWdhdGlvbkFjdGlvbjooV0tOYXZpZ2F0aQog ICAgIGRlY2lzaW9uSGFuZGxlcihXS05hdmlnYXRpb25BY3Rpb25Qb2xpY3lDYW5jZWwpOwogICAg IAogICAgIC8vIEFuZCBpbnN0ZWFkIGxvYWQgaXQgaW4gdGhlIGluc3BlY3RlZCBwYWdlLgotICAg IF9pbnNwZWN0ZWRQYWdlLT5sb2FkUmVxdWVzdChuYXZpZ2F0aW9uQWN0aW9uLnJlcXVlc3QpOwor ICAgIGlmIChfaW5zcGVjdGVkUGFnZSkKKyAgICAgICAgX2luc3BlY3RlZFBhZ2UtPmxvYWRSZXF1 ZXN0KG5hdmlnYXRpb25BY3Rpb24ucmVxdWVzdCk7CiB9CiAKIC8vIE1BUks6IFdLSW5zcGVjdG9y V0tXZWJWaWV3RGVsZWdhdGUgbWV0aG9kcwo=