Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
REGRESSION(r253231): Debug assertion failures under ~WebDeviceOrienta…
…tionUpdateProvider(Proxy)

https://bugs.webkit.org/show_bug.cgi?id=204977
rdar://problem/57724251

Reviewed by Per Arne Vollan.

In the UI process, have the WebPageProxy swap out its WebDeviceOrientationUpdateProviderProxy whenever
it connects to a new process, instead of creating one on construction and holding onto it for the rest
of the WebPageProxy lifetime. The …UpdateProviderProxy assumes that its page will have the same ID
at the time of registering as a message receiver and later unregistering, but the page ID could change
if the WebPageProxy swaps web processes. Using a new instance per web page ID ensures that the
updater is always able to successfully remove itself as a message receiver when deallocating.

In the Web process, ~WebDeviceOrientationUpdateProvider() should remove itself as a message receiver
specifically for its page ID, rather than as a global message receiver.

No new tests -- existing tests (at least the ProcessSwap API tests) revealed the regression.

* UIProcess/WebPageProxy.cpp:
(WebKit::m_tryCloseTimeoutTimer):
(WebKit::WebPageProxy::didAttachToRunningProcess):
(WebKit::WebPageProxy::resetState):
(WebKit::m_webDeviceOrientationUpdateProviderProxy): Deleted.
* UIProcess/WebPageProxy.h:
* UIProcess/ios/WebDeviceOrientationUpdateProviderProxy.h:
* WebProcess/WebCoreSupport/WebDeviceOrientationUpdateProvider.cpp:
(WebKit::WebDeviceOrientationUpdateProvider::WebDeviceOrientationUpdateProvider):
(WebKit::WebDeviceOrientationUpdateProvider::~WebDeviceOrientationUpdateProvider):
* WebProcess/WebCoreSupport/WebDeviceOrientationUpdateProvider.h:


Canonical link: https://commits.webkit.org/218203@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@253256 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
davidquesada committed Dec 7, 2019
1 parent eca7d34 commit 769f047
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 10 deletions.
32 changes: 32 additions & 0 deletions Source/WebKit/ChangeLog
@@ -1,3 +1,35 @@
2019-12-07 David Quesada <david_quesada@apple.com>

REGRESSION(r253231): Debug assertion failures under ~WebDeviceOrientationUpdateProvider(Proxy)
https://bugs.webkit.org/show_bug.cgi?id=204977
rdar://problem/57724251

Reviewed by Per Arne Vollan.

In the UI process, have the WebPageProxy swap out its WebDeviceOrientationUpdateProviderProxy whenever
it connects to a new process, instead of creating one on construction and holding onto it for the rest
of the WebPageProxy lifetime. The …UpdateProviderProxy assumes that its page will have the same ID
at the time of registering as a message receiver and later unregistering, but the page ID could change
if the WebPageProxy swaps web processes. Using a new instance per web page ID ensures that the
updater is always able to successfully remove itself as a message receiver when deallocating.

In the Web process, ~WebDeviceOrientationUpdateProvider() should remove itself as a message receiver
specifically for its page ID, rather than as a global message receiver.

No new tests -- existing tests (at least the ProcessSwap API tests) revealed the regression.

* UIProcess/WebPageProxy.cpp:
(WebKit::m_tryCloseTimeoutTimer):
(WebKit::WebPageProxy::didAttachToRunningProcess):
(WebKit::WebPageProxy::resetState):
(WebKit::m_webDeviceOrientationUpdateProviderProxy): Deleted.
* UIProcess/WebPageProxy.h:
* UIProcess/ios/WebDeviceOrientationUpdateProviderProxy.h:
* WebProcess/WebCoreSupport/WebDeviceOrientationUpdateProvider.cpp:
(WebKit::WebDeviceOrientationUpdateProvider::WebDeviceOrientationUpdateProvider):
(WebKit::WebDeviceOrientationUpdateProvider::~WebDeviceOrientationUpdateProvider):
* WebProcess/WebCoreSupport/WebDeviceOrientationUpdateProvider.h:

2019-12-07 Tim Horton <timothy_horton@apple.com>

Implement encoding for DrawImage and DrawRoundedRect display list items
Expand Down
16 changes: 13 additions & 3 deletions Source/WebKit/UIProcess/WebPageProxy.cpp
Expand Up @@ -256,6 +256,10 @@
#include <d3d11_1.h>
#endif

#if PLATFORM(IOS_FAMILY) && ENABLE(DEVICE_ORIENTATION)
#include "WebDeviceOrientationUpdateProviderProxy.h"
#endif

// This controls what strategy we use for mouse wheel coalescing.
#define MERGE_WHEEL_EVENTS 1

Expand Down Expand Up @@ -451,9 +455,6 @@ WebPageProxy::WebPageProxy(PageClient& pageClient, WebProcessProxy& process, Ref
#endif
, m_resetRecentCrashCountTimer(RunLoop::main(), this, &WebPageProxy::resetRecentCrashCount)
, m_tryCloseTimeoutTimer(RunLoop::main(), this, &WebPageProxy::tryCloseTimedOut)
#if PLATFORM(IOS_FAMILY) && ENABLE(DEVICE_ORIENTATION)
, m_webDeviceOrientationUpdateProviderProxy(*this)
#endif
{
RELEASE_LOG_IF_ALLOWED(Loading, "constructor:");

Expand Down Expand Up @@ -909,6 +910,11 @@ void WebPageProxy::didAttachToRunningProcess()
ASSERT(!m_editableImageController);
m_editableImageController = makeUnique<EditableImageController>(*this);
#endif

#if PLATFORM(IOS_FAMILY) && ENABLE(DEVICE_ORIENTATION)
ASSERT(!m_webDeviceOrientationUpdateProviderProxy);
m_webDeviceOrientationUpdateProviderProxy = makeUnique<WebDeviceOrientationUpdateProviderProxy>(*this);
#endif
}

RefPtr<API::Navigation> WebPageProxy::launchProcessForReload()
Expand Down Expand Up @@ -7250,6 +7256,10 @@ void WebPageProxy::resetState(ResetStateReason resetStateReason)
#if HAVE(PENCILKIT)
m_editableImageController = nullptr;
#endif

#if PLATFORM(IOS_FAMILY) && ENABLE(DEVICE_ORIENTATION)
m_webDeviceOrientationUpdateProviderProxy = nullptr;
#endif

CallbackBase::Error error { };
switch (resetStateReason) {
Expand Down
7 changes: 2 additions & 5 deletions Source/WebKit/UIProcess/WebPageProxy.h
Expand Up @@ -149,10 +149,6 @@ OBJC_CLASS _WKRemoteObjectRegistry;
#include "SOAuthorizationLoadPolicy.h"
#endif

#if PLATFORM(IOS_FAMILY) && ENABLE(DEVICE_ORIENTATION)
#include "WebDeviceOrientationUpdateProviderProxy.h"
#endif

#if ENABLE(MEDIA_SESSION)
namespace WebCore {
class MediaSessionMetadata;
Expand Down Expand Up @@ -301,6 +297,7 @@ class WebProcessProxy;
class WebUserContentControllerProxy;
class WebWheelEvent;
class WebsiteDataStore;
class WebDeviceOrientationUpdateProviderProxy;
class WebViewDidMoveToWindowObserver;

struct AttributedString;
Expand Down Expand Up @@ -2638,7 +2635,7 @@ class WebPageProxy : public API::ObjectImpl<API::Object::Type::Page>
String m_overriddenMediaType;

#if PLATFORM(IOS_FAMILY) && ENABLE(DEVICE_ORIENTATION)
WebDeviceOrientationUpdateProviderProxy m_webDeviceOrientationUpdateProviderProxy;
std::unique_ptr<WebDeviceOrientationUpdateProviderProxy> m_webDeviceOrientationUpdateProviderProxy;
#endif
};

Expand Down
Expand Up @@ -35,6 +35,7 @@ namespace WebKit {
class WebPageProxy;

class WebDeviceOrientationUpdateProviderProxy : public WebCore::MotionManagerClient, private IPC::MessageReceiver {
WTF_MAKE_FAST_ALLOCATED;
public:
WebDeviceOrientationUpdateProviderProxy(WebPageProxy&);
~WebDeviceOrientationUpdateProviderProxy();
Expand All @@ -46,7 +47,7 @@ class WebDeviceOrientationUpdateProviderProxy : public WebCore::MotionManagerCli
// WebCore::WebCoreMotionManagerClient
void orientationChanged(double, double, double, double, double) final;

// IPC::MessageReceiver.
// IPC::MessageReceiver
void didReceiveMessage(IPC::Connection&, IPC::Decoder&) override;

WebPageProxy& m_page;
Expand Down
Expand Up @@ -39,13 +39,14 @@ namespace WebKit {

WebDeviceOrientationUpdateProvider::WebDeviceOrientationUpdateProvider(WebPage& page)
: m_page(makeWeakPtr(page))
, m_pageIdentifier(page.identifier())
{
WebProcess::singleton().addMessageReceiver(Messages::WebDeviceOrientationUpdateProvider::messageReceiverName(), page.identifier(), *this);
}

WebDeviceOrientationUpdateProvider::~WebDeviceOrientationUpdateProvider()
{
WebProcess::singleton().removeMessageReceiver(Messages::WebDeviceOrientationUpdateProvider::messageReceiverName());
WebProcess::singleton().removeMessageReceiver(Messages::WebDeviceOrientationUpdateProvider::messageReceiverName(), m_pageIdentifier);
}

void WebDeviceOrientationUpdateProvider::startUpdating(WebCore::MotionManagerClient& client)
Expand Down
Expand Up @@ -54,6 +54,7 @@ class WebDeviceOrientationUpdateProvider final : public WebCore::DeviceOrientati
void didReceiveMessage(IPC::Connection&, IPC::Decoder&) override;

WeakPtr<WebPage> m_page;
WebCore::PageIdentifier m_pageIdentifier;
HashSet<WebCore::MotionManagerClient*> m_clients;
};

Expand Down

0 comments on commit 769f047

Please sign in to comment.