is there a higher level bug that's motivating this sort of change, i'm wondering what specifically the goals are

This is targeted at reducing memory use on 512MB Android devices. Chrome runs poorly on memory constrained phones today and we are looking to reduce the browser process footprint.

Of course for this specific bug the problem isn't that chrome runs poorly on memory constrained devices, but that websites that use lots of session storage run poorly on memory constrained devices. And the solution to that is to fix the website... There might be some room for improvements on the chrome site, but ultimately there isn't much we can do to guard against websites using lots of memory, or using APIs in a way that force chrome to use lots of memory.

Or in other words, I'd say we've achieved whatever our goal is with low memory devices, if all memory usages can be directly attributed to something the website did/something that is under full control of the website. And I'd argue that DOMStorage memory usage largely falls into that category.

Whether we keep a cache of session storage values in memory or have it persisted on disk seems like a Chrome browser implementation detail, rather than something in control of website. FWIW, we have discussed setting a low session storage limit instead of doing this. That seems more likely to break the end user experience.

True that we cannot do anything if the website chooses to use a lot of session storage. Currently we have no way to tell websites that we are running on constrained devices and use memory efficiently. We are working on getting there. But, for storing 5MB storage database, Chrome stores a cache on the renderer side, a cache on the browser side and a database that could have a cache and the disk space.
Typical browser allocation size on 512MB device is 3-7MB of malloc. Out of this on major websites like google, we could have 2-4MB of just dom storage database.

Since setting a lower limit for the website seems against the general browser behavior, I am trying to optimize the storage better.

Sure, I'm not arguing that there aren't ways we can make (some) improvements in chrome as well. It would just be nice to have a better idea of the big picture: How much of an issue in practice is session storage memory usage? Are we providing the tools to websites so they can do "the right thing" on low memory devices? Do we have any plans to somehow make the user aware that it's a website misbehaving causing poor performance, rather than something chrome does?

And for this specific case, have we considered guaranteeing at most one renderer process per tab on low memory android devices? (Or maybe we're already guaranteeing that?) In that case yeah, we can get rid of a lot of the caching/notification logic in session storage *on low end android devices*.

This original bug seems to imply that getting rid of the cache is the best thing to do in general, while if the goal is instead to fix low memory android devices (while leaving more powerful devices/systems alone) more things might be possible.

LocalStorage.BrowserLocalStorageSizeInKB histogram shows 5% of the databases opened are > 2.2MB on low-end devices. We can assume 5% of the websites have storage > 2MB. But, this could be a local storage or session storage database. So, removing cache would get back 2MB on 5% of low memory devices. We cannot really measure how much this affects the performance because that depends on how the device decides to swap. All Chrome can do is use as little memory as possible.

 Issue 718622  is in progress, but the websites really do not have an incentive to use less memory based on device size from what I heard recently. So, a good solution would be to make the user aware that the website is misbehaving.

We have never discussed about OOPIF on low-memory devices. Maybe it will be a good idea to discuss that once we figure out that OOPIF could create multiple renderers for same origin. We have not made decisions on interventions for low-memory devices. So, all the on going changes are only done without breaking sites.

Thanks for your suggestions.

1) It looks like we're set up to be more aggressive about purging on android devices. Making further adjustments to that logic based on total/available memory would be relatively straight forward.

2) Also special casing the one client case (which is probably the predominant case) to very aggressively purge the browser-side cache might make sense. It's difficult to force that (single renderer) to happen in all cases, but when it does happen, we can certainly take advantage of it. Wen there is one client, we don't need the browser-side cache. We know how many 'connections' are open to areas so detecting the single consumer case is straight forward.

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

commit 41acb7c6d5661509b65a90a2a48f93166c0efa9d
Author: Siddhartha <ssid@chromium.org>
Date: Thu Aug 10 05:31:28 2017

Make storage of values in DOMStorageMap optional

The DOMStorageMap takes a bool |has_only_keys| and stores only keys and
sizes instead of the actual values. This will be used in next CLs when
browser process does not have to store the values.
No functional change, just adding support.

BUG=748218

Change-Id: I23c78e5320caf9684c8e7372e5532f67863262c3
Reviewed-on: https://chromium-review.googlesource.com/589818
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Michael Nordman <michaeln@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493276}
[modify] https://crrev.com/41acb7c6d5661509b65a90a2a48f93166c0efa9d/content/common/dom_storage/dom_storage_map.cc
[modify] https://crrev.com/41acb7c6d5661509b65a90a2a48f93166c0efa9d/content/common/dom_storage/dom_storage_map.h
[modify] https://crrev.com/41acb7c6d5661509b65a90a2a48f93166c0efa9d/content/common/dom_storage/dom_storage_map_unittest.cc

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

commit 5466b8fdc41c9009d94d884a03599769e1f1330f
Author: Siddhartha <ssid@chromium.org>
Date: Tue Sep 05 20:01:30 2017

Dom storage: Change the browser-side cache to keys only when possible on Android

When there is only one process that has the area open, then the values
in the map is not required to be stored in browser. Support enabling and
disabling caching values in DOMStorageArea and set it disbaled when the
database is opened.

Bug:  743187 
Change-Id: I2a6556d88eddb071396db0a6e820bfb795157fd6
Reviewed-on: https://chromium-review.googlesource.com/604688
Reviewed-by: Michael Nordman <michaeln@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499722}
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/browser/dom_storage/dom_storage_area.cc
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/browser/dom_storage/dom_storage_area.h
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/browser/dom_storage/dom_storage_area_unittest.cc
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/browser/dom_storage/dom_storage_context_impl_unittest.cc
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/browser/dom_storage/dom_storage_database.h
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/browser/dom_storage/dom_storage_host.cc
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/browser/dom_storage/dom_storage_namespace.cc
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/browser/dom_storage/dom_storage_namespace.h
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/browser/dom_storage/local_storage_context_mojo_unittest.cc
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/browser/dom_storage/local_storage_database_adapter.h
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/browser/dom_storage/session_storage_database.cc
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/browser/dom_storage/session_storage_database.h
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/browser/dom_storage/session_storage_database_adapter.cc
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/browser/dom_storage/session_storage_database_adapter.h
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/browser/dom_storage/session_storage_database_unittest.cc
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/common/dom_storage/dom_storage_map.cc
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/common/dom_storage/dom_storage_map.h
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/common/dom_storage/dom_storage_map_unittest.cc
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/renderer/dom_storage/dom_storage_cached_area.cc
[modify] https://crrev.com/5466b8fdc41c9009d94d884a03599769e1f1330f/content/renderer/dom_storage/local_storage_cached_area.cc

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

commit e390de623f536cd0e918f52fe75a513063a39b64
Author: Siddhartha <ssid@chromium.org>
Date: Tue Sep 05 23:25:17 2017

DOM storage: avoid copying old value string when unused

The values can be large and every set and remove copies the old value
unnecessarily. DOMStorageMap accepts null input for old values and copy
only happens when needed.

BUG= 735269 

Change-Id: I25f93cc4e05f57170f657ee0e53fcf3e5882fcf6
Reviewed-on: https://chromium-review.googlesource.com/645834
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Michael Nordman <michaeln@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499791}
[modify] https://crrev.com/e390de623f536cd0e918f52fe75a513063a39b64/content/common/dom_storage/dom_storage_map.cc
[modify] https://crrev.com/e390de623f536cd0e918f52fe75a513063a39b64/content/common/dom_storage/dom_storage_map.h
[modify] https://crrev.com/e390de623f536cd0e918f52fe75a513063a39b64/content/common/dom_storage/dom_storage_map_unittest.cc
[modify] https://crrev.com/e390de623f536cd0e918f52fe75a513063a39b64/content/renderer/dom_storage/dom_storage_cached_area.cc
[modify] https://crrev.com/e390de623f536cd0e918f52fe75a513063a39b64/content/renderer/dom_storage/local_storage_cached_area.cc

The system health graphs show improvements in memory after this change:

https://chromeperf.appspot.com/report?sid=a798e0e0e63ac3a84caf8e37bef29055b6ef21c3401b7992d7953d6a6e4ef70d

On high end devices where the commit is not frequent:
Doms storage value changed from 2MB to 1MB on the google search omnibox story. On the search newtab story it changes from 3MB to 2.8MB. This is because the commit was not triggered when taking memory measurement. 

On low end devices:
There is noise in metrics for low-memory devices, since the commit happens at random times. But now since we only store one copy of uncommitted database in memory, the noise is reduced.
The story search newtab shows reduction from 4.4MB to 2.8MB. On Android One the noisy metric reduces by !MB on average on the search stories.

Some other media and news stories show reductions around 300KB.

Can this be marked as fixed now?

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

commit 5af5bdce5bf9c121f691ec9edc7fcdd56b3c667b
Author: Siddhartha <ssid@chromium.org>
Date: Fri Oct 13 05:20:16 2017

Local storage: Add area cache on renderer side

On each navigation the local storage database is cleared and created
again with a mojo message to browser to load the whole database from
disk. To avoid that, keep a cache of the database in renderer.

BUG= 743187 

Change-Id: Ic91144aebd5e1948bbbb1a50b58025a4dd7592c7
Reviewed-on: https://chromium-review.googlesource.com/662117
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508603}
[modify] https://crrev.com/5af5bdce5bf9c121f691ec9edc7fcdd56b3c667b/content/renderer/dom_storage/DEPS
[modify] https://crrev.com/5af5bdce5bf9c121f691ec9edc7fcdd56b3c667b/content/renderer/dom_storage/local_storage_cached_area.cc
[modify] https://crrev.com/5af5bdce5bf9c121f691ec9edc7fcdd56b3c667b/content/renderer/dom_storage/local_storage_cached_area.h
[modify] https://crrev.com/5af5bdce5bf9c121f691ec9edc7fcdd56b3c667b/content/renderer/dom_storage/local_storage_cached_area_unittest.cc
[modify] https://crrev.com/5af5bdce5bf9c121f691ec9edc7fcdd56b3c667b/content/renderer/dom_storage/local_storage_cached_areas.cc
[modify] https://crrev.com/5af5bdce5bf9c121f691ec9edc7fcdd56b3c667b/content/renderer/dom_storage/local_storage_cached_areas.h
[add] https://crrev.com/5af5bdce5bf9c121f691ec9edc7fcdd56b3c667b/content/renderer/dom_storage/local_storage_cached_areas_unittest.cc
[add] https://crrev.com/5af5bdce5bf9c121f691ec9edc7fcdd56b3c667b/content/renderer/dom_storage/mock_leveldb_wrapper.cc
[add] https://crrev.com/5af5bdce5bf9c121f691ec9edc7fcdd56b3c667b/content/renderer/dom_storage/mock_leveldb_wrapper.h
[modify] https://crrev.com/5af5bdce5bf9c121f691ec9edc7fcdd56b3c667b/content/test/BUILD.gn
[modify] https://crrev.com/5af5bdce5bf9c121f691ec9edc7fcdd56b3c667b/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/5af5bdce5bf9c121f691ec9edc7fcdd56b3c667b/tools/metrics/histograms/histograms.xml