Title: [145426] trunk/Source/WebKit2
Revision
145426
Author
mark....@apple.com
Date
2013-03-11 16:36:07 -0700 (Mon, 11 Mar 2013)

Log Message

Fix some WebDatabaseManagerProxy methods to only send to one WebProcsess
instead of broadcasting to all WebProcesses.
https://bugs.webkit.org/show_bug.cgi?id=112074.

Reviewed by Alexey Proskuryakov.

The reason for this is because the intent of these messages is to act on
the tracker database that is shared between all WebProcesses. It is
redundant and inefficient for multiple WebProcesses to service the same
request/message. And because of multi-process contention on accessing
the tracker database, the results returned to the UIProcess may also be
erroneous.

For example, if getDatabaseOrigins() is broadcasted to all WebProcesses,
they will contend to open the tracker database at the same time. If one
of these processes fails because the database is already in use, then
it may return with an empty list when it should not be empty.

With this fix, only one WebProcess gets the message and will perform the
requested query/action on behalf of all WebProcesses.

* UIProcess/WebContext.h:
(WebKit::WebContext::sendToOneProcess):
* UIProcess/WebDatabaseManagerProxy.cpp:
(WebKit::WebDatabaseManagerProxy::getDatabasesByOrigin):
(WebKit::WebDatabaseManagerProxy::getDatabaseOrigins):
(WebKit::WebDatabaseManagerProxy::deleteDatabaseWithNameForOrigin):
(WebKit::WebDatabaseManagerProxy::deleteDatabasesForOrigin):
(WebKit::WebDatabaseManagerProxy::deleteAllDatabases):
(WebKit::WebDatabaseManagerProxy::setQuotaForOrigin):

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (145425 => 145426)


--- trunk/Source/WebKit2/ChangeLog	2013-03-11 23:25:41 UTC (rev 145425)
+++ trunk/Source/WebKit2/ChangeLog	2013-03-11 23:36:07 UTC (rev 145426)
@@ -1,3 +1,36 @@
+2013-03-11  Mark Lam  <mark....@apple.com>
+
+        Fix some WebDatabaseManagerProxy methods to only send to one WebProcsess
+        instead of broadcasting to all WebProcesses.
+        https://bugs.webkit.org/show_bug.cgi?id=112074.
+
+        Reviewed by Alexey Proskuryakov.
+
+        The reason for this is because the intent of these messages is to act on
+        the tracker database that is shared between all WebProcesses. It is
+        redundant and inefficient for multiple WebProcesses to service the same
+        request/message. And because of multi-process contention on accessing
+        the tracker database, the results returned to the UIProcess may also be
+        erroneous.
+
+        For example, if getDatabaseOrigins() is broadcasted to all WebProcesses,
+        they will contend to open the tracker database at the same time. If one
+        of these processes fails because the database is already in use, then
+        it may return with an empty list when it should not be empty.
+
+        With this fix, only one WebProcess gets the message and will perform the
+        requested query/action on behalf of all WebProcesses.
+
+        * UIProcess/WebContext.h:
+        (WebKit::WebContext::sendToOneProcess):
+        * UIProcess/WebDatabaseManagerProxy.cpp:
+        (WebKit::WebDatabaseManagerProxy::getDatabasesByOrigin):
+        (WebKit::WebDatabaseManagerProxy::getDatabaseOrigins):
+        (WebKit::WebDatabaseManagerProxy::deleteDatabaseWithNameForOrigin):
+        (WebKit::WebDatabaseManagerProxy::deleteDatabasesForOrigin):
+        (WebKit::WebDatabaseManagerProxy::deleteAllDatabases):
+        (WebKit::WebDatabaseManagerProxy::setQuotaForOrigin):
+
 2013-03-11  Jeffrey Pfau  <jp...@apple.com>
 
         List cache partitions as units instead of as their contents

Modified: trunk/Source/WebKit2/UIProcess/WebContext.h (145425 => 145426)


--- trunk/Source/WebKit2/UIProcess/WebContext.h	2013-03-11 23:25:41 UTC (rev 145425)
+++ trunk/Source/WebKit2/UIProcess/WebContext.h	2013-03-11 23:36:07 UTC (rev 145426)
@@ -133,6 +133,7 @@
 
     template<typename U> void sendToAllProcesses(const U& message);
     template<typename U> void sendToAllProcessesRelaunchingThemIfNecessary(const U& message);
+    template<typename U> void sendToOneProcess(const U& message);
 
     // Sends the message to WebProcess or NetworkProcess as approporiate for current process model.
     template<typename U> void sendToNetworkingProcess(const U& message);
@@ -536,6 +537,30 @@
     sendToAllProcesses(message);
 }
 
+template<typename U> inline void WebContext::sendToOneProcess(const U& message)
+{
+    if (m_processModel == ProcessModelSharedSecondaryProcess)
+        ensureSharedWebProcess();
+
+    bool messageSent = false;
+    size_t processCount = m_processes.size();
+    for (size_t i = 0; i < processCount; ++i) {
+        WebProcessProxy* process = m_processes[i].get();
+        if (process->canSendMessage()) {
+            process->send(message, 0);
+            messageSent = true;
+            break;
+        }
+    }
+
+    if (!messageSent && m_processModel == ProcessModelMultipleSecondaryProcesses) {
+        warmInitialProcess();
+        RefPtr<WebProcessProxy> process = m_processes.last();
+        if (process->canSendMessage())
+            process->send(message, 0);
+    }
+}
+
 } // namespace WebKit
 
 #endif // WebContext_h

Modified: trunk/Source/WebKit2/UIProcess/WebDatabaseManagerProxy.cpp (145425 => 145426)


--- trunk/Source/WebKit2/UIProcess/WebDatabaseManagerProxy.cpp	2013-03-11 23:25:41 UTC (rev 145425)
+++ trunk/Source/WebKit2/UIProcess/WebDatabaseManagerProxy.cpp	2013-03-11 23:36:07 UTC (rev 145426)
@@ -145,8 +145,7 @@
     uint64_t callbackID = callback->callbackID();
     m_arrayCallbacks.set(callbackID, callback.release());
 
-    // FIXME (Multi-WebProcess): <rdar://problem/12239765> Databases shouldn't be stored in the web process.
-    context()->sendToAllProcessesRelaunchingThemIfNecessary(Messages::WebDatabaseManager::GetDatabasesByOrigin(callbackID));
+    context()->sendToOneProcess(Messages::WebDatabaseManager::GetDatabasesByOrigin(callbackID));
 }
 
 void WebDatabaseManagerProxy::didGetDatabasesByOrigin(const Vector<OriginAndDatabases>& originAndDatabasesVector, uint64_t callbackID)
@@ -198,8 +197,7 @@
     uint64_t callbackID = callback->callbackID();
     m_arrayCallbacks.set(callbackID, callback.release());
 
-    // FIXME (Multi-WebProcess): <rdar://problem/12239765> Databases shouldn't be stored in the web process.
-    context()->sendToAllProcessesRelaunchingThemIfNecessary(Messages::WebDatabaseManager::GetDatabaseOrigins(callbackID));
+    context()->sendToOneProcess(Messages::WebDatabaseManager::GetDatabaseOrigins(callbackID));
 }
 
 void WebDatabaseManagerProxy::didGetDatabaseOrigins(const Vector<String>& originIdentifiers, uint64_t callbackID)
@@ -221,26 +219,22 @@
 
 void WebDatabaseManagerProxy::deleteDatabaseWithNameForOrigin(const String& databaseIdentifier, WebSecurityOrigin* origin)
 {
-    // FIXME (Multi-WebProcess): <rdar://problem/7855696> Databases shouldn't be stored in the web process.
-    context()->sendToAllProcessesRelaunchingThemIfNecessary(Messages::WebDatabaseManager::DeleteDatabaseWithNameForOrigin(databaseIdentifier, origin->databaseIdentifier()));
+    context()->sendToOneProcess(Messages::WebDatabaseManager::DeleteDatabaseWithNameForOrigin(databaseIdentifier, origin->databaseIdentifier()));
 }
 
 void WebDatabaseManagerProxy::deleteDatabasesForOrigin(WebSecurityOrigin* origin)
 {
-    // FIXME (Multi-WebProcess): <rdar://problem/7855696> Databases shouldn't be stored in the web process.
-    context()->sendToAllProcessesRelaunchingThemIfNecessary(Messages::WebDatabaseManager::DeleteDatabasesForOrigin(origin->databaseIdentifier()));
+    context()->sendToOneProcess(Messages::WebDatabaseManager::DeleteDatabasesForOrigin(origin->databaseIdentifier()));
 }
 
 void WebDatabaseManagerProxy::deleteAllDatabases()
 {
-    // FIXME (Multi-WebProcess): <rdar://problem/7855696> Databases shouldn't be stored in the web process.
-    context()->sendToAllProcessesRelaunchingThemIfNecessary(Messages::WebDatabaseManager::DeleteAllDatabases());
+    context()->sendToOneProcess(Messages::WebDatabaseManager::DeleteAllDatabases());
 }
 
 void WebDatabaseManagerProxy::setQuotaForOrigin(WebSecurityOrigin* origin, uint64_t quota)
 {
-    // FIXME (Multi-WebProcess): <rdar://problem/7855696> Databases shouldn't be stored in the web process.
-    context()->sendToAllProcessesRelaunchingThemIfNecessary(Messages::WebDatabaseManager::SetQuotaForOrigin(origin->databaseIdentifier(), quota));
+    context()->sendToOneProcess(Messages::WebDatabaseManager::SetQuotaForOrigin(origin->databaseIdentifier(), quota));
 }
 
 void WebDatabaseManagerProxy::didModifyOrigin(const String& originIdentifier)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to