Bug 139519 – Get rid of the storage strategy

• Component: New Bugs
• Status: RESOLVED FIXED

[Anders Carlsson]

Get rid of the storage strategy

[Anders Carlsson]

Created attachment 243085 [details]
Patch

[Anders Carlsson]

Created attachment 243092 [details]
Patch

[Csaba Osztrogonác]

Comment on attachment 243092 [details]
Patch

nit: Please remove the duplicated changelog entries.

[Antti Koivisto]

Comment on attachment 243092 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=243092&action=review

> Source/WebCore/ChangeLog:31
> +        * storage/StorageStrategy.cpp: Removed.
> +        * storage/StorageStrategy.h: Removed.

Nice!

> Source/WebCore/ChangeLog:36
> +
> [phone]  Anders Carlsson  <[e-mail]>
> +
> +        Get rid of the storage strategy
> +        https://bugs.webkit.org/show_bug.cgi?id=139519

Duplicated ChangeLog entry

> Source/WebKit2/ChangeLog:18
> +        * WebProcess/WebCoreSupport/WebPlatformStrategies.h:
> +
> [phone]  Anders Carlsson  <[e-mail]>
> +
> +        Get rid of the storage strategy
> +        https://bugs.webkit.org/show_bug.cgi?id=139519

Here too.

[Anders Carlsson]

Committed r177151: <http://trac.webkit.org/changeset/177151>

[Joseph Pecoraro]

Comment on attachment 243092 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=243092&action=review

> Source/WebCore/page/PageGroup.cpp:170
>  StorageNamespace* PageGroup::localStorage()
>  {
> -    if (!m_localStorage)
> -        m_localStorage = StorageNamespace::localStorageNamespace(this);
> -
>      return m_localStorage.get();
>  }

The inspector used this interface, and always expected a Storage object, and is now crashing.

It doesn't look like m_localStorage is ever assigned now, so this is always null.

Is this being phased out, or is there something else we should use in Inspector land?

[Joseph Pecoraro]

Comment on attachment 243092 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=243092&action=review

> Source/WebCore/page/DOMWindow.cpp:864
> +    if (document->securityOrigin()->canAccessLocalStorage(document->topOrigin()))
> +        storageArea = page->storageNamespaceProvider().localStorageNamespace().storageArea(document->securityOrigin());
> +    else
> +        storageArea = page->storageNamespaceProvider().transientLocalStorageNamespace(*document->topOrigin()).storageArea(document->securityOrigin());

Sounds like we should switch to using storageNamespaceProvider.