Skip to content
Permalink
Browse files
Web Inspector: reproducible debug ASSERT when inspecting the inspecto…
…r (WK2)

https://bugs.webkit.org/show_bug.cgi?id=152080

Reviewed by Timothy Hatcher.

We hit an assert underneath ChildProcessProxy::addMessageReceiver when opening Inspector[2]
because we try to add WebInpectorProxy as a message receiver twice for the same process.

On investigating, I found several interrelated issues that caused this state of affairs:

 - WebInspectorProxy adds message receivers for inspector page's WebProcess and the
inspected page's WebProcess. When inspecting the inspector, we mistakenly add a receiver
again because the inspector is now the inspected page.
 - We mixed up process ids when adding message receivers.
 - invalidate() is re-entrant, causing us to try and double-remove the message receiver.

Fix this by removing add/remove of message recievers for the inspector page when then
inspected page is itself an inspector page. In that case, the receivers are managed by
the inspector page's WebInspectorProxy instance.

* UIProcess/WebInspectorProxy.cpp:
(WebKit::WebInspectorProxy::invalidate): Re-arrange to guard against useless reentrancy.
(WebKit::WebInspectorProxy::didRelaunchInspectorPageProcess):
(WebKit::WebInspectorProxy::eagerlyCreateInspectorPage):
(WebKit::WebInspectorProxy::didClose):


Canonical link: https://commits.webkit.org/170782@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@194544 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
burg committed Jan 4, 2016
1 parent 8903e55 commit 618a1ea1de1be0164f8cddd0192943d2b7510835
Showing with 42 additions and 6 deletions.
  1. +28 −0 Source/WebKit2/ChangeLog
  2. +14 −6 Source/WebKit2/UIProcess/WebInspectorProxy.cpp
@@ -1,3 +1,31 @@
2016-01-04 Brian Burg <bburg@apple.com>

Web Inspector: reproducible debug ASSERT when inspecting the inspector (WK2)
https://bugs.webkit.org/show_bug.cgi?id=152080

Reviewed by Timothy Hatcher.

We hit an assert underneath ChildProcessProxy::addMessageReceiver when opening Inspector[2]
because we try to add WebInpectorProxy as a message receiver twice for the same process.

On investigating, I found several interrelated issues that caused this state of affairs:

- WebInspectorProxy adds message receivers for inspector page's WebProcess and the
inspected page's WebProcess. When inspecting the inspector, we mistakenly add a receiver
again because the inspector is now the inspected page.
- We mixed up process ids when adding message receivers.
- invalidate() is re-entrant, causing us to try and double-remove the message receiver.

Fix this by removing add/remove of message recievers for the inspector page when then
inspected page is itself an inspector page. In that case, the receivers are managed by
the inspector page's WebInspectorProxy instance.

* UIProcess/WebInspectorProxy.cpp:
(WebKit::WebInspectorProxy::invalidate): Re-arrange to guard against useless reentrancy.
(WebKit::WebInspectorProxy::didRelaunchInspectorPageProcess):
(WebKit::WebInspectorProxy::eagerlyCreateInspectorPage):
(WebKit::WebInspectorProxy::didClose):

2016-01-04 Alex Christensen <achristensen@webkit.org>

Build fix after r194536 when using NetworkSession.
@@ -108,13 +108,17 @@ void WebInspectorProxy::invalidate()
WebInspectorServer::singleton().unregisterPage(m_remoteInspectionPageId);
#endif

m_inspectedPage->process().removeMessageReceiver(Messages::WebInspectorProxy::messageReceiverName(), m_inspectedPage->pageID());
// We can be called reentrantly through platformInvalidate(), in which case nothing needs to be done.
if (!m_inspectedPage)
return;

didClose();
platformInvalidate();
m_inspectedPage->process().removeMessageReceiver(Messages::WebInspectorProxy::messageReceiverName(), m_inspectedPage->pageID());

pageLevelMap().remove(m_inspectedPage);
m_inspectedPage = nullptr;

didClose();
platformInvalidate();
}

// Public APIs
@@ -180,7 +184,8 @@ void WebInspectorProxy::close()

void WebInspectorProxy::didRelaunchInspectorPageProcess()
{
m_inspectorPage->process().addMessageReceiver(Messages::WebInspectorProxy::messageReceiverName(), m_inspectedPage->pageID(), *this);
if (inspectionLevel() == 1)
m_inspectorPage->process().addMessageReceiver(Messages::WebInspectorProxy::messageReceiverName(), m_inspectedPage->pageID(), *this);
m_inspectorPage->process().assumeReadAccessToBaseURL(WebInspectorProxy::inspectorBaseURL());

// When didRelaunchInspectorPageProcess is called we can assume it is during a load request.
@@ -510,7 +515,8 @@ void WebInspectorProxy::eagerlyCreateInspectorPage()
WKPageSetPageLoaderClient(toAPI(m_inspectorPage), &loaderClient.base);
WKPageSetPageContextMenuClient(toAPI(m_inspectorPage), &contextMenuClient.base);

m_inspectorPage->process().addMessageReceiver(Messages::WebInspectorProxy::messageReceiverName(), m_inspectedPage->pageID(), *this);
if (inspectionLevel() == 1)
m_inspectorPage->process().addMessageReceiver(Messages::WebInspectorProxy::messageReceiverName(), m_inspectedPage->pageID(), *this);
m_inspectorPage->process().assumeReadAccessToBaseURL(WebInspectorProxy::inspectorBaseURL());
}

@@ -572,7 +578,9 @@ void WebInspectorProxy::didClose()
if (!m_inspectorPage)
return;

m_inspectorPage->process().removeMessageReceiver(Messages::WebInspectorProxy::messageReceiverName(), m_inspectedPage->pageID());
if (inspectionLevel() == 1)
m_inspectorPage->process().removeMessageReceiver(Messages::WebInspectorProxy::messageReceiverName(), m_inspectorPage->pageID());

m_inspectorPage = nullptr;

m_isVisible = false;

0 comments on commit 618a1ea

Please sign in to comment.