Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(874)

Issue 155845: DOM Storage: Add browser-process IPC code + tweak the WebKit Thread. (Closed)

Created:
11 years, 5 months ago by jorlow
Modified:
9 years, 6 months ago
CC:
chromium-reviews_googlegroups.com, darin (slow to review), brettw, jam, Ben Goodger (Google)
Visibility:
Public.

Description

One part of many for enabling DOM Storage. Add browser-process IPC code + tweak the WebKit Thread. Note that this code can't possibly be called/run yet. WebKitThread now has a PostIOThreadTask methods that can safely post back to the IO thread. This should be used for all WebKit thread -> the rest of the world communication. There are many TODOs in this code that should be resolved before this is shipped without a --enable-local-storage flag. BUG=4360 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=21346

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Total comments: 21

Patch Set 5 : '' #

Total comments: 2

Patch Set 6 : '' #

Patch Set 7 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+536 lines, -22 lines) Patch
M chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h View 1 2 3 4 5 6 4 chunks +67 lines, -3 lines 0 comments Download
M chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc View 1 2 3 4 5 6 2 chunks +343 lines, -13 lines 0 comments Download
M chrome/browser/in_process_webkit/webkit_thread.h View 2 3 4 5 6 2 chunks +14 lines, -0 lines 0 comments Download
M chrome/browser/in_process_webkit/webkit_thread.cc View 2 3 4 5 6 3 chunks +33 lines, -5 lines 0 comments Download
M chrome/browser/renderer_host/resource_dispatcher_host.cc View 2 3 4 5 6 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/renderer_host/resource_message_filter.cc View 1 2 3 4 5 6 2 chunks +5 lines, -1 line 0 comments Download
M chrome/common/render_messages_internal.h View 1 2 3 4 5 6 1 chunk +71 lines, -0 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
jorlow
I believe this is ready for review. One more easy one after this. :-)
11 years, 5 months ago (2009-07-22 04:40:58 UTC) #1
darin (slow to review)
LGTM http://codereview.chromium.org/155845/diff/1041/1042 File chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc (right): http://codereview.chromium.org/155845/diff/1041/1042#newcode61 Line 61: // Delete all WebStorageNamespace instances we hold. ...
11 years, 5 months ago (2009-07-22 16:43:08 UTC) #2
jorlow
Darin, one question: http://codereview.chromium.org/155845/diff/1041/1046 File chrome/browser/in_process_webkit/webkit_thread.cc (right): http://codereview.chromium.org/155845/diff/1041/1046#newcode37 Line 37: // TODO(jorlow): Make this safe! ...
11 years, 5 months ago (2009-07-22 17:02:22 UTC) #3
darin (slow to review)
http://codereview.chromium.org/155845/diff/1041/1046 File chrome/browser/in_process_webkit/webkit_thread.cc (right): http://codereview.chromium.org/155845/diff/1041/1046#newcode37 Line 37: // TODO(jorlow): Make this safe! This code assumes ...
11 years, 5 months ago (2009-07-22 17:09:52 UTC) #4
jorlow
The WebKit thread shuts down after the IO thread and there are some good reasons ...
11 years, 5 months ago (2009-07-22 17:15:35 UTC) #5
darin (slow to review)
Yeah... let's chat about this today. -Darin On Wed, Jul 22, 2009 at 10:14 AM, ...
11 years, 5 months ago (2009-07-22 17:23:37 UTC) #6
jam
lgtm with these comments http://codereview.chromium.org/155845/diff/1041/1042 File chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc (right): http://codereview.chromium.org/155845/diff/1041/1042#newcode129 Line 129: &DOMStorageDispatcherHost::OnNamespaceId, nit: the spacing ...
11 years, 5 months ago (2009-07-22 17:52:37 UTC) #7
jorlow
Jam, there's a few questions below. (Darin, you might have an opinion as well.) http://codereview.chromium.org/155845/diff/1041/1042 ...
11 years, 5 months ago (2009-07-22 18:09:57 UTC) #8
jorlow
Nm. I changed all the style stuff you talked about, John. http://codereview.chromium.org/155845/diff/1041/1042 File chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc (right): ...
11 years, 5 months ago (2009-07-22 18:32:16 UTC) #9
jorlow
This passes on the try bots and I looked very carefully at the 4->5 diff, ...
11 years, 5 months ago (2009-07-22 20:30:24 UTC) #10
darin (slow to review)
http://codereview.chromium.org/155845/diff/1059/1064 File chrome/browser/in_process_webkit/webkit_thread.cc (right): http://codereview.chromium.org/155845/diff/1059/1064#newcode41 Line 41: delete task; minor nit: avoid deleting the task ...
11 years, 5 months ago (2009-07-22 21:46:18 UTC) #11
jorlow
Good thinking. Changed and re-uploaded. http://codereview.chromium.org/155845/diff/1059/1064 File chrome/browser/in_process_webkit/webkit_thread.cc (right): http://codereview.chromium.org/155845/diff/1059/1064#newcode41 Line 41: delete task; On ...
11 years, 5 months ago (2009-07-22 21:52:28 UTC) #12
darin (slow to review)
11 years, 5 months ago (2009-07-22 21:53:37 UTC) #13
LGTM

Powered by Google App Engine
This is Rietveld 408576698