Simplify StorageArea getter functions
authorandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 18 May 2013 22:11:11 +0000 (22:11 +0000)
committerandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 18 May 2013 22:11:11 +0000 (22:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=116399

Reviewed by Sam Weinig.

Source/WebCore:

Move the security and private browsing checks from StorageArea to Storage so we can share
more code between WebKit1 and WebKit2.

* inspector/InspectorDOMStorageAgent.cpp:
(WebCore::InspectorDOMStorageAgent::getDOMStorageItems):
Update to new StorageArea interface. The security checks are unnecessary here anyway because
InspectorDOMStorageAgent::findStorageArea will only return a StorageArea that can be accessed
by the frame's security origin.

* storage/Storage.cpp:
(WebCore::Storage::key):
(WebCore::Storage::getItem):
(WebCore::Storage::contains):
* storage/StorageArea.h:
(StorageArea):
* storage/StorageAreaImpl.cpp:
(WebCore::StorageAreaImpl::key):
(WebCore::StorageAreaImpl::item):
(WebCore::StorageAreaImpl::contains):
* storage/StorageAreaImpl.h:
(StorageAreaImpl):

Source/WebKit2:

Remove security and private browsing checks.

* WebProcess/Storage/StorageAreaImpl.cpp:
(WebKit::StorageAreaImpl::key):
(WebKit::StorageAreaImpl::item):
(WebKit::StorageAreaImpl::contains):
* WebProcess/Storage/StorageAreaImpl.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@150342 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/inspector/InspectorDOMStorageAgent.cpp
Source/WebCore/storage/Storage.cpp
Source/WebCore/storage/StorageArea.h
Source/WebCore/storage/StorageAreaImpl.cpp
Source/WebCore/storage/StorageAreaImpl.h
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/Storage/StorageAreaImpl.cpp
Source/WebKit2/WebProcess/Storage/StorageAreaImpl.h

index 1431e94..40ed2f4 100644 (file)
@@ -1,3 +1,32 @@
+2013-05-18  Anders Carlsson  <andersca@apple.com>
+
+        Simplify StorageArea getter functions
+        https://bugs.webkit.org/show_bug.cgi?id=116399
+
+        Reviewed by Sam Weinig.
+
+        Move the security and private browsing checks from StorageArea to Storage so we can share
+        more code between WebKit1 and WebKit2.
+
+        * inspector/InspectorDOMStorageAgent.cpp:
+        (WebCore::InspectorDOMStorageAgent::getDOMStorageItems):
+        Update to new StorageArea interface. The security checks are unnecessary here anyway because
+        InspectorDOMStorageAgent::findStorageArea will only return a StorageArea that can be accessed
+        by the frame's security origin.
+        
+        * storage/Storage.cpp:
+        (WebCore::Storage::key):
+        (WebCore::Storage::getItem):
+        (WebCore::Storage::contains):
+        * storage/StorageArea.h:
+        (StorageArea):
+        * storage/StorageAreaImpl.cpp:
+        (WebCore::StorageAreaImpl::key):
+        (WebCore::StorageAreaImpl::item):
+        (WebCore::StorageAreaImpl::contains):
+        * storage/StorageAreaImpl.h:
+        (StorageAreaImpl):
+
 2013-05-17  Simon Fraser  <simon.fraser@apple.com>
 
         Garbage on page background while http://canberraballoons.com.au is loading
index 76d905a..5e704ba 100644 (file)
@@ -60,20 +60,6 @@ namespace DOMStorageAgentState {
 static const char domStorageAgentEnabled[] = "domStorageAgentEnabled";
 };
 
-static bool hadException(ExceptionCode ec, ErrorString* errorString)
-{
-    switch (ec) {
-    case 0:
-        return false;
-    case SECURITY_ERR:
-        *errorString = "Security error";
-        return true;
-    default:
-        *errorString = "Unknown DOM storage error";
-        return true;
-    }
-}
-
 InspectorDOMStorageAgent::InspectorDOMStorageAgent(InstrumentingAgents* instrumentingAgents, InspectorPageAgent* pageAgent, InspectorCompositeState* state)
     : InspectorBaseAgent<InspectorDOMStorageAgent>("DOMStorage", instrumentingAgents, state)
     , m_pageAgent(pageAgent)
@@ -126,19 +112,16 @@ void InspectorDOMStorageAgent::getDOMStorageItems(ErrorString* errorString, cons
 
     RefPtr<TypeBuilder::Array<TypeBuilder::Array<String> > > storageItems = TypeBuilder::Array<TypeBuilder::Array<String> >::create();
 
-    ExceptionCode ec = 0;
     for (unsigned i = 0; i < storageArea->length(); ++i) {
-        String name(storageArea->key(i, ec, frame));
-        if (hadException(ec, errorString))
-            return;
-        String value(storageArea->getItem(name, ec, frame));
-        if (hadException(ec, errorString))
-            return;
+        String key = storageArea->key(i);
+        String value = storageArea->item(key);
+
         RefPtr<TypeBuilder::Array<String> > entry = TypeBuilder::Array<String>::create();
-        entry->addItem(name);
+        entry->addItem(key);
         entry->addItem(value);
-        storageItems->addItem(entry);
+        storageItems->addItem(entry.release());
     }
+
     items = storageItems.release();
 }
 
index 2520fa6..6ccd4ea 100644 (file)
@@ -74,12 +74,28 @@ unsigned Storage::length(ExceptionCode& ec) const
 
 String Storage::key(unsigned index, ExceptionCode& ec) const
 {
-    return m_storageArea->key(index, ec, m_frame);
+    if (!m_storageArea->canAccessStorage(m_frame)) {
+        ec = SECURITY_ERR;
+        return String();
+    }
+
+    if (isDisabledByPrivateBrowsing())
+        return String();
+
+    return m_storageArea->key(index);
 }
 
 String Storage::getItem(const String& key, ExceptionCode& ec) const
 {
-    return m_storageArea->getItem(key, ec, m_frame);
+    if (!m_storageArea->canAccessStorage(m_frame)) {
+        ec = SECURITY_ERR;
+        return String();
+    }
+
+    if (isDisabledByPrivateBrowsing())
+        return String();
+
+    return m_storageArea->item(key);
 }
 
 void Storage::setItem(const String& key, const String& value, ExceptionCode& ec)
@@ -99,7 +115,15 @@ void Storage::clear(ExceptionCode& ec)
 
 bool Storage::contains(const String& key, ExceptionCode& ec) const
 {
-    return m_storageArea->contains(key, ec, m_frame);
+    if (!m_storageArea->canAccessStorage(m_frame)) {
+        ec = SECURITY_ERR;
+        return false;
+    }
+
+    if (isDisabledByPrivateBrowsing())
+        return false;
+
+    return m_storageArea->contains(key);
 }
 
 bool Storage::isDisabledByPrivateBrowsing() const
index e399cfc..65732c8 100644 (file)
@@ -45,12 +45,12 @@ public:
     // The HTML5 DOM Storage API
     // FIXME: We should pass Document instead of Frame. Also, that parameter should go first.
     virtual unsigned length() = 0;
-    virtual String key(unsigned index, ExceptionCode&, Frame* sourceFrame) = 0;
-    virtual String getItem(const String& key, ExceptionCode&, Frame* sourceFrame) = 0;
+    virtual String key(unsigned index) = 0;
+    virtual String item(const String& key) = 0;
     virtual void setItem(const String& key, const String& value, ExceptionCode&, Frame* sourceFrame) = 0;
     virtual void removeItem(const String& key, ExceptionCode&, Frame* sourceFrame) = 0;
     virtual void clear(ExceptionCode&, Frame* sourceFrame) = 0;
-    virtual bool contains(const String& key, ExceptionCode&, Frame* sourceFrame) = 0;
+    virtual bool contains(const String& key) = 0;
 
     virtual bool canAccessStorage(Frame*) = 0;
     virtual StorageType storageType() const = 0;
index 22d1cb2..9a536a9 100644 (file)
@@ -131,32 +131,16 @@ unsigned StorageAreaImpl::length()
     return m_storageMap->length();
 }
 
-String StorageAreaImpl::key(unsigned index, ExceptionCode& ec, Frame* frame)
+String StorageAreaImpl::key(unsigned index)
 {
-    ec = 0;
-    if (!canAccessStorage(frame)) {
-        ec = SECURITY_ERR;
-        return String();
-    }
-    if (disabledByPrivateBrowsingInFrame(frame))
-        return String();
-
     ASSERT(!m_isShutdown);
     blockUntilImportComplete();
 
     return m_storageMap->key(index);
 }
 
-String StorageAreaImpl::getItem(const String& key, ExceptionCode& ec, Frame* frame)
+String StorageAreaImpl::item(const String& key)
 {
-    ec = 0;
-    if (!canAccessStorage(frame)) {
-        ec = SECURITY_ERR;
-        return String();
-    }
-    if (disabledByPrivateBrowsingInFrame(frame))
-        return String();
-
     ASSERT(!m_isShutdown);
     blockUntilImportComplete();
 
@@ -252,16 +236,8 @@ void StorageAreaImpl::clear(ExceptionCode& ec, Frame* frame)
     dispatchStorageEvent(String(), String(), String(), frame);
 }
 
-bool StorageAreaImpl::contains(const String& key, ExceptionCode& ec, Frame* frame)
+bool StorageAreaImpl::contains(const String& key)
 {
-    ec = 0;
-    if (!canAccessStorage(frame)) {
-        ec = SECURITY_ERR;
-        return false;
-    }
-    if (disabledByPrivateBrowsingInFrame(frame))
-        return false;
-
     ASSERT(!m_isShutdown);
     blockUntilImportComplete();
 
index e3bc97b..5447f7c 100644 (file)
@@ -45,12 +45,12 @@ public:
     virtual ~StorageAreaImpl();
 
     virtual unsigned length() OVERRIDE;
-    virtual String key(unsigned index, ExceptionCode&, Frame* sourceFrame) OVERRIDE;
-    virtual String getItem(const String& key, ExceptionCode&, Frame* sourceFrame) OVERRIDE;
+    virtual String key(unsigned index) OVERRIDE;
+    virtual String item(const String& key) OVERRIDE;
     virtual void setItem(const String& key, const String& value, ExceptionCode&, Frame* sourceFrame) OVERRIDE;
     virtual void removeItem(const String& key, ExceptionCode&, Frame* sourceFrame) OVERRIDE;
     virtual void clear(ExceptionCode&, Frame* sourceFrame) OVERRIDE;
-    virtual bool contains(const String& key, ExceptionCode&, Frame* sourceFrame) OVERRIDE;
+    virtual bool contains(const String& key) OVERRIDE;
 
     virtual bool canAccessStorage(Frame* sourceFrame) OVERRIDE;
     virtual StorageType storageType() const OVERRIDE;
index a5c86ad..8fa4ec0 100644 (file)
@@ -1,3 +1,18 @@
+2013-05-18  Anders Carlsson  <andersca@apple.com>
+
+        Simplify StorageArea getter functions
+        https://bugs.webkit.org/show_bug.cgi?id=116399
+
+        Reviewed by Sam Weinig.
+
+        Remove security and private browsing checks.
+
+        * WebProcess/Storage/StorageAreaImpl.cpp:
+        (WebKit::StorageAreaImpl::key):
+        (WebKit::StorageAreaImpl::item):
+        (WebKit::StorageAreaImpl::contains):
+        * WebProcess/Storage/StorageAreaImpl.h:
+
 2013-05-18  Patrick Gansterer  <paroga@webkit.org>
 
         [CMake] Replace *_LIBRARY_NAME with *_OUTPUT_NAME
index 7ce6b5c..d70b57f 100644 (file)
@@ -64,31 +64,13 @@ unsigned StorageAreaImpl::length()
     return m_storageAreaMap->length();
 }
 
-String StorageAreaImpl::key(unsigned index, ExceptionCode& ec, Frame* sourceFrame)
+String StorageAreaImpl::key(unsigned index)
 {
-    ec = 0;
-    if (!canAccessStorage(sourceFrame)) {
-        ec = SECURITY_ERR;
-        return String();
-    }
-
-    if (disabledByPrivateBrowsingInFrame(sourceFrame))
-        return String();
-
     return m_storageAreaMap->key(index);
 }
 
-String StorageAreaImpl::getItem(const String& key, ExceptionCode& ec, Frame* sourceFrame)
+String StorageAreaImpl::item(const String& key)
 {
-    ec = 0;
-    if (!canAccessStorage(sourceFrame)) {
-        ec = SECURITY_ERR;
-        return String();
-    }
-
-    if (disabledByPrivateBrowsingInFrame(sourceFrame))
-        return String();
-
     return m_storageAreaMap->item(key);
 }
 
@@ -142,16 +124,8 @@ void StorageAreaImpl::clear(ExceptionCode& ec, Frame* sourceFrame)
     m_storageAreaMap->clear(sourceFrame, this);
 }
 
-bool StorageAreaImpl::contains(const String& key, ExceptionCode& ec, Frame* sourceFrame)
+bool StorageAreaImpl::contains(const String& key)
 {
-    ec = 0;
-    if (!canAccessStorage(sourceFrame)) {
-        ec = SECURITY_ERR;
-        return false;
-    }
-    if (disabledByPrivateBrowsingInFrame(sourceFrame))
-        return false;
-
     return m_storageAreaMap->contains(key);
 }
 
index c1db507..9ad8423 100644 (file)
@@ -47,12 +47,12 @@ private:
 
     // WebCore::StorageArea.
     virtual unsigned length() OVERRIDE;
-    virtual String key(unsigned index, WebCore::ExceptionCode&, WebCore::Frame* sourceFrame) OVERRIDE;
-    virtual String getItem(const String& key, WebCore::ExceptionCode&, WebCore::Frame* sourceFrame) OVERRIDE;
+    virtual String key(unsigned index) OVERRIDE;
+    virtual String item(const String& key) OVERRIDE;
     virtual void setItem(const String& key, const String& value, WebCore::ExceptionCode&, WebCore::Frame* sourceFrame) OVERRIDE;
     virtual void removeItem(const String& key, WebCore::ExceptionCode&, WebCore::Frame* sourceFrame) OVERRIDE;
     virtual void clear(WebCore::ExceptionCode&, WebCore::Frame* sourceFrame) OVERRIDE;
-    virtual bool contains(const String& key, WebCore::ExceptionCode&, WebCore::Frame* sourceFrame) OVERRIDE;
+    virtual bool contains(const String& key) OVERRIDE;
     virtual bool canAccessStorage(WebCore::Frame*) OVERRIDE;
     virtual WebCore::StorageType storageType() const OVERRIDE;
     virtual size_t memoryBytesUsedByCache() OVERRIDE;