Changeset 245069 in webkit
- Timestamp:
- May 8, 2019 2:18:48 PM (5 years ago)
- Location:
- trunk/Source/WebKit
- Files:
-
- 6 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebKit/ChangeLog
r245067 r245069 1 2019-05-08 Chris Dumez <cdumez@apple.com> 2 3 Regression: Crash at WebKit: PAL::HysteresisActivity::start 4 https://bugs.webkit.org/show_bug.cgi?id=197666 5 <rdar://problem/50037153> 6 7 Reviewed by Geoffrey Garen. 8 9 We've recently made it so that the WebContent process destroys its WebSQLiteDatabaseTracker when preparing 10 for process suspension and then re-constructs it when resuming. The issue is that the WebSQLiteDatabaseTracker 11 internal implementation was calling callOnMainThread() and capturing |this| to start/stop its HysteresisActivity. 12 As a result, |this| could be dead by the time we're on the main thread and we'd crash. 13 14 To address the issue, we no longer destroy the WebSQLiteDatabaseTracker when preparing to suspend. Instead, we 15 set a 'isSuspended' flag on the WebSQLiteDatabaseTracker so that it stops notifying the WebProcess of changes. 16 17 Also clean up the class a bit so that: 18 1. The constructor takes in a WTF::Function instead of a NetworkProcess / WebProcess reference. This is provides 19 better layering. The WebSQLiteDatabaseTracker should not need to know anything about those objects. 20 2. Use RunLoop::main().dispatch() instead of callOnMainThread() since we're in WebKit2 code. 21 22 * NetworkProcess/NetworkProcess.cpp: 23 (WebKit::NetworkProcess::NetworkProcess): 24 * Shared/WebSQLiteDatabaseTracker.cpp: 25 (WebKit::WebSQLiteDatabaseTracker::WebSQLiteDatabaseTracker): 26 (WebKit::WebSQLiteDatabaseTracker::~WebSQLiteDatabaseTracker): 27 (WebKit::WebSQLiteDatabaseTracker::willBeginFirstTransaction): 28 (WebKit::WebSQLiteDatabaseTracker::didFinishLastTransaction): 29 (WebKit::WebSQLiteDatabaseTracker::hysteresisUpdated): Deleted. 30 * Shared/WebSQLiteDatabaseTracker.h: 31 * WebProcess/WebProcess.cpp: 32 (WebKit::m_nonVisibleProcessCleanupTimer): 33 (WebKit::WebProcess::initializeSQLiteDatabaseTracker): 34 (WebKit::WebProcess::cancelPrepareToSuspend): 35 (WebKit::WebProcess::processDidResume): 36 (WebKit::m_webSQLiteDatabaseTracker): Deleted. 37 * WebProcess/WebProcess.h: 38 1 39 2019-05-08 Tim Horton <timothy_horton@apple.com> 2 40 -
trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp
r245053 r245069 134 134 #endif 135 135 #if PLATFORM(IOS_FAMILY) 136 , m_webSQLiteDatabaseTracker( *this)136 , m_webSQLiteDatabaseTracker([this](bool isHoldingLockedFiles) { parentProcessConnection()->send(Messages::NetworkProcessProxy::SetIsHoldingLockedFiles(isHoldingLockedFiles), 0); }) 137 137 #endif 138 138 { -
trunk/Source/WebKit/Shared/WebSQLiteDatabaseTracker.cpp
r243939 r245069 32 32 #include "WebProcessProxyMessages.h" 33 33 #include <WebCore/SQLiteDatabaseTracker.h> 34 #include <wtf/MainThread.h>35 34 36 35 namespace WebKit { 37 36 using namespace WebCore; 38 37 39 WebSQLiteDatabaseTracker::WebSQLiteDatabaseTracker(WebProcess& process) 40 : m_process(process) 41 , m_processType(AuxiliaryProcess::ProcessType::WebContent) 42 , m_hysteresis([this](PAL::HysteresisState state) { hysteresisUpdated(state); }) 38 WebSQLiteDatabaseTracker::WebSQLiteDatabaseTracker(IsHoldingLockedFilesHandler&& isHoldingLockedFilesHandler) 39 : m_isHoldingLockedFilesHandler(WTFMove(isHoldingLockedFilesHandler)) 40 , m_hysteresis([this](PAL::HysteresisState state) { setIsHoldingLockedFiles(state == PAL::HysteresisState::Started); }) 43 41 { 44 SQLiteDatabaseTracker::setClient(this); 45 } 46 47 WebSQLiteDatabaseTracker::WebSQLiteDatabaseTracker(NetworkProcess& process) 48 : m_process(process) 49 , m_processType(AuxiliaryProcess::ProcessType::Network) 50 , m_hysteresis([this](PAL::HysteresisState state) { hysteresisUpdated(state); }) 51 { 42 ASSERT(RunLoop::isMain()); 52 43 SQLiteDatabaseTracker::setClient(this); 53 44 } … … 59 50 60 51 if (m_hysteresis.state() == PAL::HysteresisState::Started) 61 hysteresisUpdated(PAL::HysteresisState::Stopped); 52 setIsHoldingLockedFiles(false); 53 } 54 55 void WebSQLiteDatabaseTracker::setIsSuspended(bool isSuspended) 56 { 57 ASSERT(RunLoop::isMain()); 58 m_isSuspended = isSuspended; 62 59 } 63 60 64 61 void WebSQLiteDatabaseTracker::willBeginFirstTransaction() 65 62 { 66 callOnMainThread([this] {63 RunLoop::main().dispatch([this] { 67 64 m_hysteresis.start(); 68 65 }); … … 71 68 void WebSQLiteDatabaseTracker::didFinishLastTransaction() 72 69 { 73 callOnMainThread([this] {70 RunLoop::main().dispatch([this] { 74 71 m_hysteresis.stop(); 75 72 }); 76 73 } 77 74 78 void WebSQLiteDatabaseTracker:: hysteresisUpdated(PAL::HysteresisState state)75 void WebSQLiteDatabaseTracker::setIsHoldingLockedFiles(bool isHoldingLockedFiles) 79 76 { 80 switch (m_processType) { 81 case AuxiliaryProcess::ProcessType::WebContent: 82 m_process.parentProcessConnection()->send(Messages::WebProcessProxy::SetIsHoldingLockedFiles(state == PAL::HysteresisState::Started), 0); 83 break; 84 case AuxiliaryProcess::ProcessType::Network: 85 m_process.parentProcessConnection()->send(Messages::NetworkProcessProxy::SetIsHoldingLockedFiles(state == PAL::HysteresisState::Started), 0); 86 break; 87 default: 88 ASSERT_NOT_REACHED(); 89 } 77 ASSERT(RunLoop::isMain()); 78 79 if (m_isSuspended) 80 return; 81 82 m_isHoldingLockedFilesHandler(isHoldingLockedFiles); 90 83 } 91 84 -
trunk/Source/WebKit/Shared/WebSQLiteDatabaseTracker.h
r243939 r245069 32 32 namespace WebKit { 33 33 34 class NetworkProcess; 35 class WebProcess; 36 37 class WebSQLiteDatabaseTracker : public WebCore::SQLiteDatabaseTrackerClient { 34 class WebSQLiteDatabaseTracker final : public WebCore::SQLiteDatabaseTrackerClient { 38 35 WTF_MAKE_NONCOPYABLE(WebSQLiteDatabaseTracker) 39 36 public: 40 explicit WebSQLiteDatabaseTracker(WebProcess&);41 explicit WebSQLiteDatabaseTracker( NetworkProcess&);37 using IsHoldingLockedFilesHandler = Function<void(bool)>; 38 explicit WebSQLiteDatabaseTracker(IsHoldingLockedFilesHandler&&); 42 39 43 40 ~WebSQLiteDatabaseTracker(); 44 41 45 // WebCore::SQLiteDatabaseTrackerClient 46 void willBeginFirstTransaction() override; 47 void didFinishLastTransaction() override; 42 void setIsSuspended(bool); 48 43 49 44 private: 50 void hysteresisUpdated(PAL::HysteresisState);45 void setIsHoldingLockedFiles(bool); 51 46 52 AuxiliaryProcess& m_process; 53 AuxiliaryProcess::ProcessType m_processType; 47 // WebCore::SQLiteDatabaseTrackerClient. 48 void willBeginFirstTransaction() final; 49 void didFinishLastTransaction() final; 50 51 IsHoldingLockedFilesHandler m_isHoldingLockedFilesHandler; 54 52 PAL::HysteresisActivity m_hysteresis; 53 bool m_isSuspended { false }; 55 54 }; 56 55 -
trunk/Source/WebKit/WebProcess/WebProcess.cpp
r244979 r245069 189 189 , m_nonVisibleProcessCleanupTimer(*this, &WebProcess::nonVisibleProcessCleanupTimerFired) 190 190 #if PLATFORM(IOS_FAMILY) 191 , m_webSQLiteDatabaseTracker( std::make_unique<WebSQLiteDatabaseTracker>(*this))191 , m_webSQLiteDatabaseTracker([this](bool isHoldingLockedFiles) { parentProcessConnection()->send(Messages::WebProcessProxy::SetIsHoldingLockedFiles(isHoldingLockedFiles), 0); }) 192 192 #endif 193 193 { … … 1469 1469 1470 1470 #if PLATFORM(IOS_FAMILY) 1471 m_webSQLiteDatabaseTracker = nullptr;1471 m_webSQLiteDatabaseTracker.setIsSuspended(true); 1472 1472 SQLiteDatabase::setIsDatabaseOpeningForbidden(true); 1473 1473 if (DatabaseTracker::isInitialized()) … … 1520 1520 1521 1521 #if PLATFORM(IOS_FAMILY) 1522 m_webSQLiteDatabaseTracker = std::make_unique<WebSQLiteDatabaseTracker>(*this);1522 m_webSQLiteDatabaseTracker.setIsSuspended(false); 1523 1523 SQLiteDatabase::setIsDatabaseOpeningForbidden(false); 1524 1524 accessibilityProcessSuspendedNotification(false); … … 1596 1596 1597 1597 #if PLATFORM(IOS_FAMILY) 1598 m_webSQLiteDatabaseTracker = std::make_unique<WebSQLiteDatabaseTracker>(*this);1598 m_webSQLiteDatabaseTracker.setIsSuspended(false); 1599 1599 SQLiteDatabase::setIsDatabaseOpeningForbidden(false); 1600 1600 accessibilityProcessSuspendedNotification(false); -
trunk/Source/WebKit/WebProcess/WebProcess.h
r244091 r245069 38 38 #include "WebInspectorInterruptDispatcher.h" 39 39 #include "WebProcessCreationParameters.h" 40 #include "WebSQLiteDatabaseTracker.h" 40 41 #include <WebCore/ActivityState.h> 41 42 #include <WebCore/RegistrableDomain.h> … … 113 114 struct WebPageGroupData; 114 115 struct WebPreferencesStore; 115 class WebSQLiteDatabaseTracker;116 116 struct WebsiteData; 117 117 struct WebsiteDataStoreParameters; … … 513 513 514 514 #if PLATFORM(IOS_FAMILY) 515 std::unique_ptr<WebSQLiteDatabaseTracker>m_webSQLiteDatabaseTracker;515 WebSQLiteDatabaseTracker m_webSQLiteDatabaseTracker; 516 516 ProcessTaskStateObserver m_taskStateObserver { *this }; 517 517 #endif
Note: See TracChangeset
for help on using the changeset viewer.