Changeset 100041 in webkit

Timestamp:

Nov 11, 2011, 3:54:14 PM (15 years ago)

Author:

adamk@chromium.org

Message:

Remove no-op StorageNamespace::unlock method
​https://bugs.webkit.org/show_bug.cgi?id=72181

Reviewed by Darin Adler.

Source/WebCore:

The method was meant to support the localStorage mutex,
but the approach of using a mutex for localStorage has never been
implemented (and almost certainly won't be). Even if it were implemented,
it's not being called at the right time, due to bugs in the V8 bindings'
use of V8Proxy (see ​http://webkit.org/b/72063 for details).

If, in the future, some replacement for a mutex is used to synchronize
localStorage access, it can easily be re-added to the (hopefully
fixed-by-then) replacement for V8Proxy::didLeaveScriptContext.

• bindings/v8/V8Proxy.cpp:

(WebCore::V8Proxy::didLeaveScriptContext): Stop calling unlock().

• page/Navigator.cpp:

(WebCore::Navigator::getStorageUpdates): Remove implementation (a call to unlock), but leave code as it's exposed to the platform.

• page/Navigator.idl: Added a FIXME to remove getStorageUpdates from the platform.
• storage/StorageNamespace.h:
• storage/StorageNamespaceImpl.cpp:
• storage/StorageNamespaceImpl.h:

Source/WebKit/chromium:

• src/StorageNamespaceProxy.cpp:
• src/StorageNamespaceProxy.h:

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/bindings/v8/V8Proxy.cpp (modified) (3 diffs)
• WebCore/page/Navigator.cpp (modified) (1 diff)
• WebCore/page/Navigator.idl (modified) (1 diff)
• WebCore/storage/StorageNamespace.h (modified) (1 diff)
• WebCore/storage/StorageNamespaceImpl.cpp (modified) (1 diff)
• WebCore/storage/StorageNamespaceImpl.h (modified) (1 diff)
• WebKit/chromium/ChangeLog (modified) (1 diff)
• WebKit/chromium/src/StorageNamespaceProxy.cpp (modified) (1 diff)
• WebKit/chromium/src/StorageNamespaceProxy.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-11-11  Adam Klein  <adamk@chromium.org>
+
+        Remove no-op StorageNamespace::unlock method
+        https://bugs.webkit.org/show_bug.cgi?id=72181
+
+        Reviewed by Darin Adler.
+
+        The method was meant to support the localStorage mutex,
+        but the approach of using a mutex for localStorage has never been
+        implemented (and almost certainly won't be). Even if it were implemented,
+        it's not being called at the right time, due to bugs in the V8 bindings'
+        use of V8Proxy (see http://webkit.org/b/72063 for details).
+
+        If, in the future, some replacement for a mutex is used to synchronize
+        localStorage access, it can easily be re-added to the (hopefully
+        fixed-by-then) replacement for V8Proxy::didLeaveScriptContext.
+
+        * bindings/v8/V8Proxy.cpp:
+        (WebCore::V8Proxy::didLeaveScriptContext): Stop calling unlock().
+        * page/Navigator.cpp:
+        (WebCore::Navigator::getStorageUpdates): Remove implementation (a call to unlock), but leave code as it's exposed to the platform.
+        * page/Navigator.idl: Added a FIXME to remove getStorageUpdates from the platform.
+        * storage/StorageNamespace.h:
+        * storage/StorageNamespaceImpl.cpp:
+        * storage/StorageNamespaceImpl.h:
+
 2011-11-11  Alexis Menard  <alexis.menard@openbossa.org>
 

trunk/Source/WebCore/bindings/v8/V8Proxy.cpp

@@ -46 +46 @@
 #include "InspectorInstrumentation.h"
 #include "Page.h"
-#include "PageGroup.h"
 #include "PlatformSupport.h"
 #include "ScriptSourceCode.h"
 #include "SecurityOrigin.h"
 #include "Settings.h"
-#include "StorageNamespace.h"
 #include "V8Binding.h"
 #include "V8BindingState.h"
@@ -590 +588 @@
 void V8Proxy::didLeaveScriptContext()
 {
-    Page* page = m_frame->page();
-    if (!page)
-        return;
-    // If we've just left a top level script context and local storage has been
-    // instantiated, we must ensure that any storage locks have been freed.
-    // Per http://dev.w3.org/html5/spec/Overview.html#storage-mutex
     if (m_recursion)
         return;
+
 #if ENABLE(INDEXED_DATABASE)
     // If we've just left a script context and indexed database has been
@@ -604 +597 @@
     IDBPendingTransactionMonitor::abortPendingTransactions();
 #endif // ENABLE(INDEXED_DATABASE)
-    if (page->group().hasLocalStorage())
-        page->group().localStorage()->unlock();
 
 #if ENABLE(MUTATION_OBSERVERS)

trunk/Source/WebCore/page/Navigator.cpp

@@ -173 +173 @@
 void Navigator::getStorageUpdates()
 {
-    if (!m_frame)
-        return;
-
-    Page* page = m_frame->page();
-    if (!page)
-        return;
-
-    StorageNamespace* localStorage = page->group().localStorage();
-    if (localStorage)
-        localStorage->unlock();
+    // FIXME: Remove this method or rename to yieldForStorageUpdates.
 }
 

trunk/Source/WebCore/page/Navigator.idl

@@ -45 +45 @@
 #endif
 
-        void getStorageUpdates();
+        void getStorageUpdates(); // FIXME: Remove this method or rename to yieldForStorageUpdates.
 
 #if defined(ENABLE_REGISTER_PROTOCOL_HANDLER) && ENABLE_REGISTER_PROTOCOL_HANDLER

trunk/Source/WebCore/storage/StorageNamespace.h

@@ -48 +48 @@
     virtual PassRefPtr<StorageNamespace> copy() = 0;
     virtual void close() = 0;
-    virtual void unlock() = 0;
     virtual void clearOriginForDeletion(SecurityOrigin*) = 0;
     virtual void clearAllOriginsForDeletion() = 0;

trunk/Source/WebCore/storage/StorageNamespaceImpl.cpp

@@ -140 +140 @@
 }
 
-void StorageNamespaceImpl::unlock()
-{
-    // Because there's a single event loop per-process, this is a no-op.
-}
-
 void StorageNamespaceImpl::clearOriginForDeletion(SecurityOrigin* origin)
 {

trunk/Source/WebCore/storage/StorageNamespaceImpl.h

@@ -48 +48 @@
         virtual PassRefPtr<StorageNamespace> copy();
         virtual void close();
-        virtual void unlock();
 
         // Not removing the origin's StorageArea from m_storageAreaMap because

trunk/Source/WebKit/chromium/ChangeLog

@@ +1 @@
+2011-11-11  Adam Klein  <adamk@chromium.org>
+
+        Remove no-op StorageNamespace::unlock method
+        https://bugs.webkit.org/show_bug.cgi?id=72181
+
+        Reviewed by Darin Adler.
+
+        * src/StorageNamespaceProxy.cpp:
+        * src/StorageNamespaceProxy.h:
+
 2011-11-11  Stephen Chenney  <schenney@chromium.org>
 

trunk/Source/WebKit/chromium/src/StorageNamespaceProxy.cpp

@@ -86 +86 @@
 }
 
-void StorageNamespaceProxy::unlock()
-{
-    // FIXME: Implement.
-}
-
 void StorageNamespaceProxy::clearOriginForDeletion(SecurityOrigin* origin)
 {

trunk/Source/WebKit/chromium/src/StorageNamespaceProxy.h

@@ -41 +41 @@
     virtual PassRefPtr<StorageNamespace> copy();
     virtual void close();
-    virtual void unlock();
     
     virtual void clearOriginForDeletion(SecurityOrigin*);