First half of http://codereview.chromium.org/274014/show

This fixes storage events in single process mode, fixes a bug due to the glue/webkitclient_impl not being updated when I introduced quota support, introduces a params struct for storage events, and is general cleanup.  Submitting this first since the change to add the url param made things bigger than I liked.

TBR=darin
TEST=none
BUG=25427

Review URL: http://codereview.chromium.org/348071

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@30945 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 45 insertions, 50 deletions

diff --git a/chrome/browser/in_process_webkit/browser_webkitclient_impl.cc b/chrome/browser/in_process_webkit/browser_webkitclient_impl.cc
index ec893fb..e4105f6 100644
--- a/chrome/browser/in_process_webkit/browser_webkitclient_impl.cc
+++ b/chrome/browser/in_process_webkit/browser_webkitclient_impl.cc
@@ -95,7 +95,7 @@ WebKit::WebData BrowserWebKitClientImpl::loadResource(const char* name) {
 
 WebKit::WebStorageNamespace*
 BrowserWebKitClientImpl::createLocalStorageNamespace(
-    const WebKit::WebString& path) {
+    const WebKit::WebString& path, unsigned quota) {
   // The "WebStorage" interface is used for renderer WebKit -> browser WebKit
   // communication only.  "WebStorageClient" will be used for browser WebKit ->
   // renderer WebKit.  So this will never be implemented.
diff --git a/chrome/browser/in_process_webkit/browser_webkitclient_impl.h b/chrome/browser/in_process_webkit/browser_webkitclient_impl.h
index ff79350..b562fd8 100644
--- a/chrome/browser/in_process_webkit/browser_webkitclient_impl.h
+++ b/chrome/browser/in_process_webkit/browser_webkitclient_impl.h
@@ -31,7 +31,7 @@ class BrowserWebKitClientImpl : public webkit_glue::WebKitClientImpl {
   virtual void getPluginList(bool refresh, WebKit::WebPluginListBuilder*);
   virtual WebKit::WebData loadResource(const char* name);
   virtual WebKit::WebStorageNamespace* createLocalStorageNamespace(
-      const WebKit::WebString& path);
+      const WebKit::WebString& path, unsigned quota);
   virtual WebKit::WebStorageNamespace* createSessionStorageNamespace();
   virtual void dispatchStorageEvent(const WebKit::WebString& key,
       const WebKit::WebString& oldValue, const WebKit::WebString& newValue,
diff --git a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc
index 9380cd3..63ff96e 100644
--- a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc
+++ b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc
@@ -13,21 +13,22 @@
 #include "chrome/browser/renderer_host/browser_render_process_host.h"
 #include "chrome/common/render_messages.h"
 
-DOMStorageDispatcherHost* DOMStorageDispatcherHost::current_ = NULL;
+DOMStorageDispatcherHost* DOMStorageDispatcherHost::storage_event_host_ = NULL;
 
 DOMStorageDispatcherHost::
-AutoSetCurrentDispatcherHost::AutoSetCurrentDispatcherHost(
+ScopedStorageEventContext::ScopedStorageEventContext(
     DOMStorageDispatcherHost* dispatcher_host) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::WEBKIT));
-  DCHECK(!current_);
-  current_ = dispatcher_host;
+  DCHECK(!storage_event_host_);
+  storage_event_host_ = dispatcher_host;
+  DCHECK(storage_event_host_);
 }
 
 DOMStorageDispatcherHost::
-AutoSetCurrentDispatcherHost::~AutoSetCurrentDispatcherHost() {
+ScopedStorageEventContext::~ScopedStorageEventContext() {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::WEBKIT));
-  DCHECK(current_);
-  current_ = NULL;
+  DCHECK(storage_event_host_);
+  storage_event_host_ = NULL;
 }
 
 DOMStorageDispatcherHost::DOMStorageDispatcherHost(
@@ -76,16 +77,24 @@ void DOMStorageDispatcherHost::Shutdown() {
 }
 
 /* static */
-void DOMStorageDispatcherHost::DispatchStorageEvent(const string16& key,
+void DOMStorageDispatcherHost::DispatchStorageEvent(const NullableString16& key,
     const NullableString16& old_value, const NullableString16& new_value,
     const string16& origin, bool is_local_storage) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::WEBKIT));
-  DCHECK(current_);
-  ChromeThread::PostTask(
-      ChromeThread::IO, FROM_HERE,
-      NewRunnableMethod(
-          current_, &DOMStorageDispatcherHost::OnStorageEvent, key, old_value,
-          new_value, origin, is_local_storage));
+  DCHECK(is_local_storage);  // Only LocalStorage is implemented right now.
+  DCHECK(storage_event_host_);
+  ViewMsg_DOMStorageEvent_Params params;
+  params.key_ = key;
+  params.old_value_ = old_value;
+  params.new_value_ = new_value;
+  params.origin_ = origin;
+  params.storage_type_ = is_local_storage ? DOM_STORAGE_LOCAL
+                                          : DOM_STORAGE_SESSION;
+  // The storage_event_host_ is the DOMStorageDispatcherHost that is up in the
+  // current call stack since it caused the storage event to fire.
+  ChromeThread::PostTask(ChromeThread::IO, FROM_HERE,
+      NewRunnableMethod(storage_event_host_,
+          &DOMStorageDispatcherHost::OnStorageEvent, params));
 }
 
 bool DOMStorageDispatcherHost::OnMessageReceived(const IPC::Message& message,
@@ -282,14 +291,14 @@ void DOMStorageDispatcherHost::OnSetItem(int64 storage_area_id,
     return;
   }
 
-  AutoSetCurrentDispatcherHost auto_set(this);
+  ScopedStorageEventContext scope(this);
   storage_area->SetItem(key, value, &quota_exception);
   ViewHostMsg_DOMStorageSetItem::WriteReplyParams(reply_msg, quota_exception);
   Send(reply_msg);
 }
 
-void DOMStorageDispatcherHost::OnRemoveItem(int64 storage_area_id,
-                                            const string16& key) {
+void DOMStorageDispatcherHost::OnRemoveItem(
+    int64 storage_area_id, const string16& key) {
   if (ChromeThread::CurrentlyOn(ChromeThread::IO)) {
     PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this,
         &DOMStorageDispatcherHost::OnRemoveItem, storage_area_id, key));
@@ -304,7 +313,7 @@ void DOMStorageDispatcherHost::OnRemoveItem(int64 storage_area_id,
     return;
   }
 
-  AutoSetCurrentDispatcherHost auto_set(this);
+  ScopedStorageEventContext scope(this);
   storage_area->RemoveItem(key);
 }
 
@@ -323,23 +332,18 @@ void DOMStorageDispatcherHost::OnClear(int64 storage_area_id) {
     return;
   }
 
-  AutoSetCurrentDispatcherHost auto_set(this);
+  ScopedStorageEventContext scope(this);
   storage_area->Clear();
 }
 
-void DOMStorageDispatcherHost::OnStorageEvent(const string16& key,
-    const NullableString16& old_value, const NullableString16& new_value,
-    const string16& origin, bool is_local_storage) {
+void DOMStorageDispatcherHost::OnStorageEvent(
+    const ViewMsg_DOMStorageEvent_Params& params) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
-  DCHECK(is_local_storage);  // Only LocalStorage is implemented right now.
-  DOMStorageType dom_storage_type = is_local_storage ? DOM_STORAGE_LOCAL
-                                                     : DOM_STORAGE_SESSION;
   const DOMStorageContext::DispatcherHostSet* set =
       Context()->GetDispatcherHostSet();
   DOMStorageContext::DispatcherHostSet::const_iterator cur = set->begin();
   while (cur != set->end()) {
-    (*cur)->Send(new ViewMsg_DOMStorageEvent(key, old_value, new_value, origin,
-                                             dom_storage_type));
+    (*cur)->Send(new ViewMsg_DOMStorageEvent(params));
     ++cur;
   }
 }
diff --git a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h
index 94a7c00..28e3fc8 100644
--- a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h
+++ b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h
@@ -16,6 +16,7 @@
 class DOMStorageContext;
 class Task;
 class WebKitThread;
+struct ViewMsg_DOMStorageEvent_Params;
 
 // This class handles the logistics of DOM Storage within the browser process.
 // It mostly ferries information between IPCs and the WebKit implementations,
@@ -42,7 +43,7 @@ class DOMStorageDispatcherHost :
   void Send(IPC::Message* message);
 
   // Only call on the WebKit thread.
-  static void DispatchStorageEvent(const string16& key,
+  static void DispatchStorageEvent(const NullableString16& key,
       const NullableString16& old_value, const NullableString16& new_value,
       const string16& origin, bool is_local_storage);
 
@@ -60,14 +61,12 @@ class DOMStorageDispatcherHost :
   void OnGetItem(int64 storage_area_id, const string16& key,
                  IPC::Message* reply_msg);
   void OnSetItem(int64 storage_area_id, const string16& key,
-                 const string16& value, IPC::Message* reply_msg);
+      const string16& value, IPC::Message* reply_msg);
   void OnRemoveItem(int64 storage_area_id, const string16& key);
   void OnClear(int64 storage_area_id);
 
   // Only call on the IO thread.
-  void OnStorageEvent(const string16& key, const NullableString16& old_value,
-      const NullableString16& new_value, const string16& origin,
-      bool is_local_storage);
+  void OnStorageEvent(const ViewMsg_DOMStorageEvent_Params& params);
 
   // A shortcut for accessing our context.
   DOMStorageContext* Context() {
@@ -79,14 +78,14 @@ class DOMStorageDispatcherHost :
       const tracked_objects::Location& from_here, Task* task);
 
   // Use whenever there's a chance OnStorageEvent will be called.
-  class AutoSetCurrentDispatcherHost {
+  class ScopedStorageEventContext {
    public:
-    AutoSetCurrentDispatcherHost(DOMStorageDispatcherHost* dispatcher_host);
-    ~AutoSetCurrentDispatcherHost();
+    ScopedStorageEventContext(DOMStorageDispatcherHost* dispatcher_host);
+    ~ScopedStorageEventContext();
   };
 
   // Only access on the WebKit thread!  Used for storage events.
-  static DOMStorageDispatcherHost* current_;
+  static DOMStorageDispatcherHost* storage_event_host_;
 
   // Data shared between renderer processes with the same profile.
   scoped_refptr<WebKitContext> webkit_context_;
diff --git a/chrome/browser/in_process_webkit/dom_storage_uitest.cc b/chrome/browser/in_process_webkit/dom_storage_uitest.cc
index f90fc9e..2ea3f5d 100644
--- a/chrome/browser/in_process_webkit/dom_storage_uitest.cc
+++ b/chrome/browser/in_process_webkit/dom_storage_uitest.cc
@@ -83,6 +83,7 @@ class DOMStorageTest : public UILayoutTest {
     ASSERT_TRUE(tab.get());
 
     GURL url = GetTestUrl(L"layout_tests", L"clear_dom_storage.html");
+    tab->SetCookie(url, "");
     ASSERT_TRUE(tab->NavigateToURL(url));
 
     WaitUntilCookieNonEmpty(tab.get(), url, "cleared", kTestIntervalMs,
@@ -101,24 +102,15 @@ class DOMStorageTest : public UILayoutTest {
   FilePath test_dir_;
 };
 
-// http://code.google.com/p/chromium/issues/detail?id=24145
-#if defined(OS_WIN)
-#define MAYBE_DOMStorageLayoutTests FLAKY_DOMStorageLayoutTests
-#define MAYBE_SessionStorageLayoutTests FLAKY_SessionStorageLayoutTests
-#else
-#define MAYBE_DOMStorageLayoutTests DOMStorageLayoutTests
-#define MAYBE_SessionStorageLayoutTests SessionStorageLayoutTests
-#endif  // defined(OS_WIN)
 
-
-TEST_F(DOMStorageTest, MAYBE_DOMStorageLayoutTests) {
+TEST_F(DOMStorageTest, DOMStorageLayoutTests) {
   InitializeForLayoutTest(test_dir_, FilePath(), false);
   AddResources();
   RunTests(kTopLevelFiles);
 }
 
 
-TEST_F(DOMStorageTest, FLAKY_LocalStorageLayoutTests) {
+TEST_F(DOMStorageTest, LocalStorageLayoutTests) {
   InitializeForLayoutTest(test_dir_, FilePath().AppendASCII("localstorage"),
                           false);
   AddResources();
@@ -127,7 +119,7 @@ TEST_F(DOMStorageTest, FLAKY_LocalStorageLayoutTests) {
   RunTests(kLocalStorageFiles);
 }
 
-TEST_F(DOMStorageTest, MAYBE_SessionStorageLayoutTests) {
+TEST_F(DOMStorageTest, SessionStorageLayoutTests) {
   InitializeForLayoutTest(test_dir_, FilePath().AppendASCII("sessionstorage"),
                           false);
   AddResources();