New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 696112 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

Session Storage cache is never cleared

Project Member Reported by ssid@chromium.org, Feb 25 2017

Issue description

DOMStorageNamespace is expected to clear the caches for both local storage and session storage after committing to the disk / database. But, the session storage database caches are never cleared.

This causes upto 10MiB of cache usage just with browsing google.com and google.co.uk on Android.
 

Comment 1 by ssid@chromium.org, Feb 25 2017

I think the issue is DOMStorageNamespace::PurgeMemory checks for directory_.empty() and returns. But for session storage it is always empty since the backing for session storage is on leveldb database.

Comment 2 by ssid@chromium.org, Feb 25 2017

Cc: mariakho...@chromium.org dskiba@chromium.org
Components: Blink>Storage>DOMStorage
Labels: Hotlist-EM Performance-Memory
Labels: -Hotlist-EM LowMemory
Status: Assigned (was: Untriaged)
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c72ce2a231f22abab37a62e41f15a79545ba4cce

commit c72ce2a231f22abab37a62e41f15a79545ba4cce
Author: ssid <ssid@chromium.org>
Date: Tue Feb 28 00:07:23 2017

Purge session storage caches on memory pressure

The session storage caches are never cleared because the namespace
assumes that backing is only file based. The session storage is backed
by leveldb database.
This change would not accidently delete storage since
DOMStorageArea::PurgeMemory also checks if backing is present and commit
tasks are done before purging.

BUG= 696112 

Review-Url: https://codereview.chromium.org/2717843002
Cr-Commit-Position: refs/heads/master@{#453403}

[modify] https://crrev.com/c72ce2a231f22abab37a62e41f15a79545ba4cce/content/browser/dom_storage/dom_storage_namespace.cc

Comment 5 by ssid@chromium.org, Mar 24 2017

Status: Fixed (was: Assigned)
After this fix the main process crashes on DCHECK here:
https://cs.chromium.org/chromium/src/content/browser/dom_storage/dom_storage_area.cc?rcl=56cc3c1bee04c0bd207d33c27e35863b7ed19783&l=312

Reproducing steps:
1) Run browser. Open several tabs
2) Simulate high memory usage
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2f67c627788bb0e03c9028d80edc11786051474f

commit 2f67c627788bb0e03c9028d80edc11786051474f
Author: Siddhartha <ssid@chromium.org>
Date: Fri Jul 14 05:01:22 2017

Support purging of session storage area

Discussion at crrev.com/10983063. The comments at
crrev.com/10983063#msg16 says that the only issue with supporting
PurgeMemory for session storage is it would set is_initial_import_done_
to false and this could potentially cause an import if GetItem is
called. So, the fix should just be not setting import_done_ to false if
map is empty.
For the case when a SetItem is called before Purge, PurgeMemory would
be noop since it has uncommitted changes.

BUG= 696112 

Change-Id: I1a57c49dea41e030f5770b9d74742984b920f1d1
Reviewed-on: https://chromium-review.googlesource.com/560722
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Michael Nordman <michaeln@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486680}
[modify] https://crrev.com/2f67c627788bb0e03c9028d80edc11786051474f/content/browser/dom_storage/dom_storage_area.cc

Sign in to add a comment