Modified: trunk/Source/WebCore/ChangeLog (118253 => 118254)
--- trunk/Source/WebCore/ChangeLog 2012-05-23 21:59:44 UTC (rev 118253)
+++ trunk/Source/WebCore/ChangeLog 2012-05-23 22:04:51 UTC (rev 118254)
@@ -1,3 +1,23 @@
+2012-05-23 Michael Nordman <[email protected]>
+
+ [chromium] DomStorage events handling needs TLC (3)
+ https://bugs.webkit.org/show_bug.cgi?id=87031
+ https://code.google.com/p/chromium/issues/detail?id=128482
+ Create WebCore::Storage instances as a side effect of attaching storage event handlers.
+ This allows storage events in chromium to be propagated with less IPC chatter.
+
+ Worth noting that in non-chromium ports, creation of the localStorage instance can have
+ an additional side effect of scheduling a background task to read the area's values
+ from disk, which given the interest in events is probably a beneficial side effect.
+
+ Reviewed by Adam Barth.
+
+ No new tests. Existing tests cover this.
+
+ * page/DOMWindow.cpp:
+ (WebCore::didAddStorageEventListener):
+ (WebCore::DOMWindow::addEventListener):
+
2012-05-23 Michael Saboff <[email protected]>
Crash in fast/files/read tests during Garbage Collection
Modified: trunk/Source/WebCore/page/DOMWindow.cpp (118253 => 118254)
--- trunk/Source/WebCore/page/DOMWindow.cpp 2012-05-23 21:59:44 UTC (rev 118253)
+++ trunk/Source/WebCore/page/DOMWindow.cpp 2012-05-23 22:04:51 UTC (rev 118254)
@@ -1546,6 +1546,17 @@
}
#endif
+static void didAddStorageEventListener(DOMWindow* window)
+{
+ // Creating these WebCore::Storage objects informs the system that we'd like to receive
+ // notifications about storage events that might be triggered in other processes. Rather
+ // than subscribe to these notifications explicitly, we subscribe to them implicitly to
+ // simplify the work done by the system.
+ ExceptionCode unused;
+ window->localStorage(unused);
+ window->sessionStorage(unused);
+}
+
bool DOMWindow::addEventListener(const AtomicString& eventType, PassRefPtr<EventListener> listener, bool useCapture)
{
if (!EventTarget::addEventListener(eventType, listener, useCapture))
@@ -1557,6 +1568,8 @@
document->didAddWheelEventHandler();
else if (eventNames().isTouchEventType(eventType))
document->didAddTouchEventHandler();
+ else if (eventType == eventNames().storageEvent)
+ didAddStorageEventListener(this);
}
if (eventType == eventNames().unloadEvent)
Modified: trunk/Source/WebKit/chromium/ChangeLog (118253 => 118254)
--- trunk/Source/WebKit/chromium/ChangeLog 2012-05-23 21:59:44 UTC (rev 118253)
+++ trunk/Source/WebKit/chromium/ChangeLog 2012-05-23 22:04:51 UTC (rev 118254)
@@ -1,3 +1,19 @@
+2012-05-23 Michael Nordman <[email protected]>
+
+ [chromium] DomStorage events handling needs TLC (3)
+ https://bugs.webkit.org/show_bug.cgi?id=87031
+ https://code.google.com/p/chromium/issues/detail?id=128482
+ Only queue storage events for Documents which have allocated
+ a WebCore::Storage instance since pages that have attached onstorage
+ event handlers must have a non-null Storage instance.
+
+ Reviewed by Adam Barth.
+
+ * src/StorageAreaProxy.cpp:
+ (WebCore::StorageAreaProxy::dispatchLocalStorageEvent):
+ (WebCore::StorageAreaProxy::dispatchSessionStorageEvent):
+ (WebCore::StorageAreaProxy::isEventSource):
+
2012-05-23 Sheriff Bot <[email protected]>
Unreviewed, rolling out r118218.
Modified: trunk/Source/WebKit/chromium/src/StorageAreaProxy.cpp (118253 => 118254)
--- trunk/Source/WebKit/chromium/src/StorageAreaProxy.cpp 2012-05-23 21:59:44 UTC (rev 118253)
+++ trunk/Source/WebKit/chromium/src/StorageAreaProxy.cpp 2012-05-23 22:04:51 UTC (rev 118254)
@@ -125,13 +125,9 @@
const HashSet<Page*>& pages = pageGroup->pages();
for (HashSet<Page*>::const_iterator it = pages.begin(); it != pages.end(); ++it) {
for (Frame* frame = (*it)->mainFrame(); frame; frame = frame->tree()->traverseNext()) {
- if (frame->document()->securityOrigin()->equal(securityOrigin) && !isEventSource(frame->domWindow()->optionalLocalStorage(), sourceAreaInstance)) {
- // FIXME: maybe only raise if the window has an onstorage listener attached to avoid creating the Storage instance.
- ExceptionCode ec = 0;
- Storage* storage = frame->domWindow()->localStorage(ec);
- if (!ec)
- frame->document()->enqueueWindowEvent(StorageEvent::create(eventNames().storageEvent, key, oldValue, newValue, pageURL, storage));
- }
+ Storage* storage = frame->domWindow()->optionalLocalStorage();
+ if (storage && frame->document()->securityOrigin()->equal(securityOrigin) && !isEventSource(storage, sourceAreaInstance))
+ frame->document()->enqueueWindowEvent(StorageEvent::create(eventNames().storageEvent, key, oldValue, newValue, pageURL, storage));
}
}
}
@@ -157,20 +153,15 @@
return;
for (Frame* frame = page->mainFrame(); frame; frame = frame->tree()->traverseNext()) {
- if (frame->document()->securityOrigin()->equal(securityOrigin) && !isEventSource(frame->domWindow()->optionalSessionStorage(), sourceAreaInstance)) {
- // FIXME: maybe only raise if the window has an onstorage listener attached to avoid creating the Storage instance.
- ExceptionCode ec = 0;
- Storage* storage = frame->domWindow()->sessionStorage(ec);
- if (!ec)
- frame->document()->enqueueWindowEvent(StorageEvent::create(eventNames().storageEvent, key, oldValue, newValue, pageURL, storage));
- }
+ Storage* storage = frame->domWindow()->optionalSessionStorage();
+ if (storage && frame->document()->securityOrigin()->equal(securityOrigin) && !isEventSource(storage, sourceAreaInstance))
+ frame->document()->enqueueWindowEvent(StorageEvent::create(eventNames().storageEvent, key, oldValue, newValue, pageURL, storage));
}
}
bool StorageAreaProxy::isEventSource(Storage* storage, WebKit::WebStorageArea* sourceAreaInstance)
{
- if (!storage)
- return false;
+ ASSERT(storage);
StorageAreaProxy* areaProxy = static_cast<StorageAreaProxy*>(storage->area());
return areaProxy->m_storageArea == sourceAreaInstance;
}
