DomStorageCachedArea + DomStorageProxy interface and unittests. These classes aren't used yet.

DomStorageCachedArea + DomStorageProxy interface and unittests. These classes aren't used yet.

BUG=94382
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=138921

[michaeln, 2012-05-24 01:49:56]

Here's the first chunk of the larger CL. Josh, since eric's out till next
tuesday, can you take a look?

[michaeln, 2012-05-24 18:49:34]

@tony, should we be putting new unittests in content_tests? i can move the
existing dom_storage unittests if thats the plan, i'd like to keep all of the
dom_storage related unittests together in the same test harness.

[tony, 2012-05-24 18:59:42]

On 2012/05/24 18:49:34, michaeln wrote:
> @tony, should we be putting new unittests in content_tests? i can move the
> existing dom_storage unittests if thats the plan, i'd like to keep all of the
> dom_storage related unittests together in the same test harness.

Yes, in the long run, we want to move all the tests from test_shell_tests into
either content_unittests or upstream to webkit_unit_tests.

Feel free to add this new test to test_shell_tests for now.  I'll try and move
them together later when I get a chance.  If you want to move them before or
after this change, that would also work.

[jsbell, 2012-05-24 20:20:22]

lgtm, with just nits and a question.

https://chromiumcodereview.appspot.com/10450009/diff/9002/webkit/dom_storage/...
File webkit/dom_storage/dom_storage_cached_area.h (right):

https://chromiumcodereview.appspot.com/10450009/diff/9002/webkit/dom_storage/...
webkit/dom_storage/dom_storage_cached_area.h:46: // Primes the cache, loading
all values for the area.
Can you add a comment near the top of the class indicating that this is a
complete cache, not a sparse cache?

https://chromiumcodereview.appspot.com/10450009/diff/9002/webkit/dom_storage/...
webkit/dom_storage/dom_storage_cached_area.h:53: // Resets the class back to its
newly constructed state.
Nit: s/class/object/

https://chromiumcodereview.appspot.com/10450009/diff/9002/webkit/dom_storage/...
File webkit/dom_storage/dom_storage_cached_area_unittest.cc (right):

https://chromiumcodereview.appspot.com/10450009/diff/9002/webkit/dom_storage/...
webkit/dom_storage/dom_storage_cached_area_unittest.cc:16: // An mock
implementation of the DomStorageProxy interface.
Nit: s/An/A/

https://chromiumcodereview.appspot.com/10450009/diff/9002/webkit/dom_storage/...
webkit/dom_storage/dom_storage_cached_area_unittest.cc:299: // completion
callback should have been cancelled.
I'm not seeing how the first is cancelled (either in the code or test). Can you
explain?

[michaeln, 2012-05-24 20:43:11]

Thnx for looking... also renamed Proxy::AsyncOperationCallback to just
CompletionCallback

https://chromiumcodereview.appspot.com/10450009/diff/9002/webkit/dom_storage/...
File webkit/dom_storage/dom_storage_cached_area.h (right):

https://chromiumcodereview.appspot.com/10450009/diff/9002/webkit/dom_storage/...
webkit/dom_storage/dom_storage_cached_area.h:46: // Primes the cache, loading
all values for the area.
On 2012/05/24 20:20:22, jsbell wrote:
> Can you add a comment near the top of the class indicating that this is a
> complete cache, not a sparse cache?

Done.

// Unlike the other classes in the dom_storage library, this one is intended
// for use in renderer processes. It maintains a complete cach of the an
// origin's Map of key/value pairs for fast access. The cache is primed on
// first access and changes are written to the backend thru the |proxy|.
// Mutations originating in other processes are applied to the cache via
// the ApplyMutation method.

https://chromiumcodereview.appspot.com/10450009/diff/9002/webkit/dom_storage/...
webkit/dom_storage/dom_storage_cached_area.h:53: // Resets the class back to its
newly constructed state.
On 2012/05/24 20:20:22, jsbell wrote:
> Nit: s/class/object/

Done.

https://chromiumcodereview.appspot.com/10450009/diff/9002/webkit/dom_storage/...
File webkit/dom_storage/dom_storage_cached_area_unittest.cc (right):

https://chromiumcodereview.appspot.com/10450009/diff/9002/webkit/dom_storage/...
webkit/dom_storage/dom_storage_cached_area_unittest.cc:16: // An mock
implementation of the DomStorageProxy interface.
On 2012/05/24 20:20:22, jsbell wrote:
> Nit: s/An/A/

Done.

https://chromiumcodereview.appspot.com/10450009/diff/9002/webkit/dom_storage/...
webkit/dom_storage/dom_storage_cached_area_unittest.cc:299: // completion
callback should have been cancelled.
On 2012/05/24 20:20:22, jsbell wrote:
> I'm not seeing how the first is cancelled (either in the code or test). Can
you
> explain?

DomStorageCachedArea::Reset() calls weak_factory_.InvalidateWeakPtrs(). Each
completion callback holds weakptr back to the cachedArea. After those weakptrs
have been invalidated, running the callback will be a noop, it won't call into
the cachedArea method it was bound to.

The test verifies the first callback is ignored by testing
IsIgnoringAllMutations after having invoked the first callback. If
OnClearComplete had run, the ignore flag would have been reset to false and that
expectation would fail.