Bug 27072 – Split StorageNamespace and StorageArea into an interface and implementation

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jeremy Orlow]

I need to split StorageNamespace and StorageArea into an interface and implementation.  In a later patch, I'll implement a proxy interface that'll run inside the Chromium renderer process.

This is a continuation of other refactoring work: https://bugs.webkit.org/show_bug.cgi?id=25376

[Jeremy Orlow]

Created attachment 32435 [details]
v1

I need to split StorageNamespace and StorageArea into an interface and implementation.  In a later patch, I'll implement a proxy interface that'll run inside the Chromium renderer process.

This is a continuation of other refactoring work: https://bugs.webkit.org/show_bug.cgi?id=25376

[Darin Fisher (:fishd, Google)]

Comment on attachment 32435 [details]
v1

> Index: WebCore/storage/StorageArea.cpp

> +#if PLATFORM(CHROMIUM)
> +#else
> +    return StorageAreaImpl::create(storageType, origin, syncManager);
>  #endif

I think this should be #if !PLATFORM(CHROMIUM) to avoid the #else, or
perhaps you should add a similar FIXME comment there as you do below:


> Index: WebCore/storage/StorageNamespace.cpp

> +#if PLATFORM(CHROMIUM)
> +    // FIXME: Implement.
> +#else
> +    return StorageNamespaceImpl::localStorageNamespace(path);
> +#endif

Otherwise, looks good to me.  R=me

[Jeremy Orlow]

Created attachment 32470 [details]
v2

The main difference is that I took out the Chromium ifdef's and replaced them with an #error for Chromium.

[Darin Fisher (:fishd, Google)]

Landed as:  http://trac.webkit.org/changeset/45643

[Darin Fisher (:fishd, Google)]

I had to revert that change due to build errors.  r45644 was the rollback.

[Jeremy Orlow]

Created attachment 32486 [details]
Fix build in release mode

I'm not sure why this showed up in release mode and not debug (probably optimizations?), but there was a build error in the last patch.  To fix it, I defined 2 virtual functions in StorageArea and made StorageAreaSync use a StorageArea* rather than a StroageAreaImpl* which removed some complex dependencies.

[Darin Fisher (:fishd, Google)]

> I'm not sure why this showed up in release mode and not debug (probably
> optimizations?), but there was a build error in the last patch.  To fix it, I
> defined 2 virtual functions in StorageArea and made StorageAreaSync use a
> StorageArea* rather than a StroageAreaImpl* which removed some complex
> dependencies.

I think it would be better to make StorageAreaSync talk directly to the implementation of StorageArea since StorageAreaSync is in effect part of the implementation of StorageArea.  This can be done as a follow-on patch as far as I am concerned.

[Darin Fisher (:fishd, Google)]

Landed (take 2) as:  http://trac.webkit.org/changeset/45651