Title: [202464] trunk/Source/WebKit2
Revision
202464
Author
[email protected]
Date
2016-06-24 17:36:47 -0700 (Fri, 24 Jun 2016)

Log Message

REGRESSION(r201171): CRASH at WebKit::WebInspectorProxy::open() + 31 when running inspector layout tests
https://bugs.webkit.org/show_bug.cgi?id=159070
<rdar://problem/26768628>

Reviewed by Joseph Pecoraro.

We have been seeing a few crashes underneath WebInspectorProxy::bringToFront() on the bots.
Previously, this code didn't use m_inspectorPage so there was nothing to null-dereference.

However, it doesn't make sense that we would hit the null dereference here:

 - The only caller of bringToFront() on the WebProcess side is InspectorController::show().
   It only tries to bring to front if there is already a local frontend connection, which
   shouldn't be the case if m_inspectorPage is null.

 - It's guarded by m_underTest, which should have been set to true in createInspectorPage().

These clues lead me to believe that we may be improperly tearing down the inspector between tests.
For example, it seems possible that a local frontend connection is not being torn down, so
InspectorController never asks to create a inspector page when the next test calls showWebInspector.

Since this crash is not easy to reproduce, we don't have much to go on. For now, this patch
adds an early return in the case where m_inspectorPage is null when calling open().

* UIProcess/WebInspectorProxy.cpp:
(WebKit::WebInspectorProxy::open):

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (202463 => 202464)


--- trunk/Source/WebKit2/ChangeLog	2016-06-25 00:03:44 UTC (rev 202463)
+++ trunk/Source/WebKit2/ChangeLog	2016-06-25 00:36:47 UTC (rev 202464)
@@ -1,3 +1,32 @@
+2016-06-24  Brian Burg  <[email protected]>
+
+        REGRESSION(r201171): CRASH at WebKit::WebInspectorProxy::open() + 31 when running inspector layout tests
+        https://bugs.webkit.org/show_bug.cgi?id=159070
+        <rdar://problem/26768628>
+
+        Reviewed by Joseph Pecoraro.
+
+        We have been seeing a few crashes underneath WebInspectorProxy::bringToFront() on the bots.
+        Previously, this code didn't use m_inspectorPage so there was nothing to null-dereference.
+
+        However, it doesn't make sense that we would hit the null dereference here:
+
+         - The only caller of bringToFront() on the WebProcess side is InspectorController::show().
+           It only tries to bring to front if there is already a local frontend connection, which
+           shouldn't be the case if m_inspectorPage is null.
+
+         - It's guarded by m_underTest, which should have been set to true in createInspectorPage().
+
+        These clues lead me to believe that we may be improperly tearing down the inspector between tests.
+        For example, it seems possible that a local frontend connection is not being torn down, so
+        InspectorController never asks to create a inspector page when the next test calls showWebInspector.
+
+        Since this crash is not easy to reproduce, we don't have much to go on. For now, this patch
+        adds an early return in the case where m_inspectorPage is null when calling open().
+
+        * UIProcess/WebInspectorProxy.cpp:
+        (WebKit::WebInspectorProxy::open):
+
 2016-06-24  Jer Noble  <[email protected]>
 
         Vimeo.com videos do not get playback controls

Modified: trunk/Source/WebKit2/UIProcess/WebInspectorProxy.cpp (202463 => 202464)


--- trunk/Source/WebKit2/UIProcess/WebInspectorProxy.cpp	2016-06-25 00:03:44 UTC (rev 202463)
+++ trunk/Source/WebKit2/UIProcess/WebInspectorProxy.cpp	2016-06-25 00:36:47 UTC (rev 202464)
@@ -580,6 +580,9 @@
     if (m_underTest)
         return;
 
+    if (!m_inspectorPage)
+        return;
+
     m_isVisible = true;
     m_inspectorPage->process().send(Messages::WebInspectorUI::SetIsVisible(m_isVisible), m_inspectorPage->pageID());
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to