Allow to clear session and local storage when not backed on disk

Allow to clear session and local storage when not backed on disk

This patch allow GetLocalStorageUsage and GetSessionStorageUsage
to return non empty UsageInfo so that one can call DeleteLocalStorage
and DeleteSessionStorage.

Which both internally call DOMStorageContextImpl::NotifyAreaCleared to
send DOMStorageMsg_Event IPC from browser process to renderer process.
On receiver side DomStorageDispatcher calls
blink::WebStorageEventDispatcher::dispatch to clear the cache from the
LocalFrame.

BUG=604745
R=michaeln@chromium.org, msramek@chromium.org

Committed: https://crrev.com/8d07be600b13008d0fcbb1608eef835eadd171a0
Cr-Commit-Position: refs/heads/master@{#391886}

[Julien Isorce Samsung, 2016-04-19 15:21:59]

Description was changed from

==========
Allow to clear session and local storage when not backed on disk

This patch allow GetLocalStorageUsage and GetSessionStorageUsage
to return non empty UsageInfo so that one can call DeleteLocalStorage
and DeleteSessionStorage.

Which both internally call DOMStorageContextImpl::NotifyAreaCleared to
send DOMStorageMsg_Event IPC from browser process to renderer process.
On receiver side DomStorageDispatcher calls
blink::WebStorageEventDispatcher::dispatch to clear the cache from the
LocalFrame.

BUG=

R=
==========

to

==========
Allow to clear session and local storage when not backed on disk

This patch allow GetLocalStorageUsage and GetSessionStorageUsage
to return non empty UsageInfo so that one can call DeleteLocalStorage
and DeleteSessionStorage.

Which both internally call DOMStorageContextImpl::NotifyAreaCleared to
send DOMStorageMsg_Event IPC from browser process to renderer process.
On receiver side DomStorageDispatcher calls
blink::WebStorageEventDispatcher::dispatch to clear the cache from the
LocalFrame.

BUG=604745

R=
==========

[Julien Isorce Samsung, 2016-05-03 08:56:36]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-03 08:57:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1900793003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1900793003/1

[commit-bot: I haz the power, 2016-05-03 10:34:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-03 10:34:47]

Dry run: This issue passed the CQ dry run.

[msramek, 2016-05-04 18:53:47]

msramek@chromium.org changed reviewers:
+ michaeln@chromium.org, msramek@chromium.org

[msramek, 2016-05-04 18:53:48]

As per our discussion in crbug.com/604745, and discussion in the email where you
confirmed that this only affects in-memory data in the regular mode browser
context, LGTM.

However, only on a philosophical level :) I am not familiar with the codebase,
so let me add +Michael for an owner's review. Michael, please refer to the
linked bug for context.

Cheers,
Martin

[michaeln, 2016-05-05 00:38:24]

i have a couple small requests, otherwise this look ok by me

https://codereview.chromium.org/1900793003/diff/1/content/browser/dom_storage...
File content/browser/dom_storage/dom_storage_context_impl.cc (right):

https://codereview.chromium.org/1900793003/diff/1/content/browser/dom_storage...
content/browser/dom_storage/dom_storage_context_impl.cc:102: for
(std::vector<GURL>::const_iterator origin_it = origins.begin();
please use a range based for loop for this one

for (const GURL& origin : origins) {
}

https://codereview.chromium.org/1900793003/diff/1/content/browser/dom_storage...
content/browser/dom_storage/dom_storage_context_impl.cc:132: namespace_it !=
namespaces_.end(); ++namespace_it) {
might be nice to use range based loops here too

https://codereview.chromium.org/1900793003/diff/1/content/browser/dom_storage...
File content/browser/dom_storage/dom_storage_namespace.cc (right):

https://codereview.chromium.org/1900793003/diff/1/content/browser/dom_storage...
content/browser/dom_storage/dom_storage_namespace.cc:42: for
(AreaMap::const_iterator it = areas_.begin(); it != areas_.end(); ++it)
range based for loop please

for (const auto& entry : areas_)
  origins->push_back(entry.first);

https://codereview.chromium.org/1900793003/diff/1/content/browser/dom_storage...
File content/browser/dom_storage/dom_storage_namespace.h (right):

https://codereview.chromium.org/1900793003/diff/1/content/browser/dom_storage...
content/browser/dom_storage/dom_storage_namespace.h:55: void
ReadOrigins(std::vector<GURL>* origins) const;
Can you rename to GetOriginsWithAreas()?

The more generic method name is a little misleading, "read" often means from
disk and the method sounds like it will return all origins in the namespace
including those without loaded areas. But neither of those are true.

Also please move this method down to the bottom of the public method list (and
move the method body in the .cc file down too)

[Julien Isorce Samsung, 2016-05-05 11:05:04]

Thx Martin for the comments and thx Michael for the review.
I addressed remarks and uploaded new "Patch Set 2". Please have a look.

https://codereview.chromium.org/1900793003/diff/1/content/browser/dom_storage...
File content/browser/dom_storage/dom_storage_context_impl.cc (right):

https://codereview.chromium.org/1900793003/diff/1/content/browser/dom_storage...
content/browser/dom_storage/dom_storage_context_impl.cc:102: for
(std::vector<GURL>::const_iterator origin_it = origins.begin();
On 2016/05/05 00:38:23, michaeln wrote:
> please use a range based for loop for this one
> 
> for (const GURL& origin : origins) {
> }

Done.

https://codereview.chromium.org/1900793003/diff/1/content/browser/dom_storage...
content/browser/dom_storage/dom_storage_context_impl.cc:132: namespace_it !=
namespaces_.end(); ++namespace_it) {
On 2016/05/05 00:38:23, michaeln wrote:
> might be nice to use range based loops here too

Done and I used "auto" like in DOMStorageContextImpl::Flush(). Alternatively I
could use "StorageNamespaceMap::value_type" or verbose "std::pair<int64_t,
scoped_refptr<DOMStorageNamespace>>"

https://codereview.chromium.org/1900793003/diff/1/content/browser/dom_storage...
File content/browser/dom_storage/dom_storage_namespace.cc (right):

https://codereview.chromium.org/1900793003/diff/1/content/browser/dom_storage...
content/browser/dom_storage/dom_storage_namespace.cc:42: for
(AreaMap::const_iterator it = areas_.begin(); it != areas_.end(); ++it)
On 2016/05/05 00:38:23, michaeln wrote:
> range based for loop please
> 
> for (const auto& entry : areas_)
>   origins->push_back(entry.first);

Done.

https://codereview.chromium.org/1900793003/diff/1/content/browser/dom_storage...
File content/browser/dom_storage/dom_storage_namespace.h (right):

https://codereview.chromium.org/1900793003/diff/1/content/browser/dom_storage...
content/browser/dom_storage/dom_storage_namespace.h:55: void
ReadOrigins(std::vector<GURL>* origins) const;
On 2016/05/05 00:38:23, michaeln wrote:
> Can you rename to GetOriginsWithAreas()?
> 
> The more generic method name is a little misleading, "read" often means from
> disk and the method sounds like it will return all origins in the namespace
> including those without loaded areas. But neither of those are true.
> 
> Also please move this method down to the bottom of the public method list (and
> move the method body in the .cc file down too)

Done.

[Julien Isorce Samsung, 2016-05-05 11:05:55]

Description was changed from

==========
Allow to clear session and local storage when not backed on disk

This patch allow GetLocalStorageUsage and GetSessionStorageUsage
to return non empty UsageInfo so that one can call DeleteLocalStorage
and DeleteSessionStorage.

Which both internally call DOMStorageContextImpl::NotifyAreaCleared to
send DOMStorageMsg_Event IPC from browser process to renderer process.
On receiver side DomStorageDispatcher calls
blink::WebStorageEventDispatcher::dispatch to clear the cache from the
LocalFrame.

BUG=604745

R=
==========

to

==========
Allow to clear session and local storage when not backed on disk

This patch allow GetLocalStorageUsage and GetSessionStorageUsage
to return non empty UsageInfo so that one can call DeleteLocalStorage
and DeleteSessionStorage.

Which both internally call DOMStorageContextImpl::NotifyAreaCleared to
send DOMStorageMsg_Event IPC from browser process to renderer process.
On receiver side DomStorageDispatcher calls
blink::WebStorageEventDispatcher::dispatch to clear the cache from the
LocalFrame.

BUG=604745

R=michaeln@chromium.org, msramek@chromium.org
==========

[Julien Isorce Samsung, 2016-05-05 11:06:22]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-05 11:06:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1900793003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1900793003/20001

[commit-bot: I haz the power, 2016-05-05 11:08:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-05 11:08:33]

Dry run: Try jobs failed on following builders:
  ios-device on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Julien Isorce Samsung, 2016-05-05 12:10:44]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-05 12:10:56]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1900793003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1900793003/40001

[commit-bot: I haz the power, 2016-05-05 13:00:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-05 13:00:03]

Dry run: This issue passed the CQ dry run.

[michaeln, 2016-05-05 19:40:53]

lgtm, thnx

[Julien Isorce Samsung, 2016-05-05 20:01:11]

The CQ bit was checked by j.isorce@samsung.com

[Julien Isorce Samsung, 2016-05-05 20:01:12]

The patchset sent to the CQ was uploaded after l-g-t-m from msramek@chromium.org
Link to the patchset: https://codereview.chromium.org/1900793003/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2016-05-05 20:01:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1900793003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1900793003/40001

[commit-bot: I haz the power, 2016-05-05 20:06:43]

Description was changed from

==========
Allow to clear session and local storage when not backed on disk

This patch allow GetLocalStorageUsage and GetSessionStorageUsage
to return non empty UsageInfo so that one can call DeleteLocalStorage
and DeleteSessionStorage.

Which both internally call DOMStorageContextImpl::NotifyAreaCleared to
send DOMStorageMsg_Event IPC from browser process to renderer process.
On receiver side DomStorageDispatcher calls
blink::WebStorageEventDispatcher::dispatch to clear the cache from the
LocalFrame.

BUG=604745

R=michaeln@chromium.org, msramek@chromium.org
==========

to

==========
Allow to clear session and local storage when not backed on disk

This patch allow GetLocalStorageUsage and GetSessionStorageUsage
to return non empty UsageInfo so that one can call DeleteLocalStorage
and DeleteSessionStorage.

Which both internally call DOMStorageContextImpl::NotifyAreaCleared to
send DOMStorageMsg_Event IPC from browser process to renderer process.
On receiver side DomStorageDispatcher calls
blink::WebStorageEventDispatcher::dispatch to clear the cache from the
LocalFrame.

BUG=604745

R=michaeln@chromium.org, msramek@chromium.org
==========

[commit-bot: I haz the power, 2016-05-05 20:06:44]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-05 20:08:54]

Description was changed from

==========
Allow to clear session and local storage when not backed on disk

This patch allow GetLocalStorageUsage and GetSessionStorageUsage
to return non empty UsageInfo so that one can call DeleteLocalStorage
and DeleteSessionStorage.

Which both internally call DOMStorageContextImpl::NotifyAreaCleared to
send DOMStorageMsg_Event IPC from browser process to renderer process.
On receiver side DomStorageDispatcher calls
blink::WebStorageEventDispatcher::dispatch to clear the cache from the
LocalFrame.

BUG=604745

R=michaeln@chromium.org, msramek@chromium.org
==========

to

==========
Allow to clear session and local storage when not backed on disk

This patch allow GetLocalStorageUsage and GetSessionStorageUsage
to return non empty UsageInfo so that one can call DeleteLocalStorage
and DeleteSessionStorage.

Which both internally call DOMStorageContextImpl::NotifyAreaCleared to
send DOMStorageMsg_Event IPC from browser process to renderer process.
On receiver side DomStorageDispatcher calls
blink::WebStorageEventDispatcher::dispatch to clear the cache from the
LocalFrame.

BUG=604745

R=michaeln@chromium.org, msramek@chromium.org

Committed: https://crrev.com/8d07be600b13008d0fcbb1608eef835eadd171a0
Cr-Commit-Position: refs/heads/master@{#391886}
==========

[commit-bot: I haz the power, 2016-05-05 20:08:57]

Patchset 3 (id:??) landed as
https://crrev.com/8d07be600b13008d0fcbb1608eef835eadd171a0
Cr-Commit-Position: refs/heads/master@{#391886}