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

commit ef0fdeeaa50d1badc47bdac7aaf095e213fe7a67
Author: Gyuyoung Kim <gyuyoung.kim@lge.com>
Date: Thu Mar 22 08:21:58 2018

Pass the maximium disk cache size of appcache in the command line to appcache thread

Embedded devices have limited storage capacity generally. So current maximum disk
cache size(250MB) can make their storage fully used in the embedded devices.
For the embedder, it would be good if we give the power them to set the maximum
disk cache size through the command line.

Bug:  824619 
Change-Id: Ib4fae34934eadf23f1165907b5c547fff205b827
Reviewed-on: https://chromium-review.googlesource.com/974804
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Gyuyoung Kim <gyuyoung.kim@lge.com>
Cr-Commit-Position: refs/heads/master@{#545010}
[modify] https://crrev.com/ef0fdeeaa50d1badc47bdac7aaf095e213fe7a67/content/browser/appcache/appcache_storage_impl.cc
[modify] https://crrev.com/ef0fdeeaa50d1badc47bdac7aaf095e213fe7a67/content/public/common/content_switches.cc
[modify] https://crrev.com/ef0fdeeaa50d1badc47bdac7aaf095e213fe7a67/content/public/common/content_switches.h

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

commit d143b1cc5e86eef3b2d7e8dacd629d24f58a5b6b
Author: Gyuyoung Kim <gyuyoung.kim@lge.com>
Date: Tue Mar 27 14:31:13 2018

Set the maximum cache size per an origin in the command line for application cache

Embedded devices have limited storage capacity generally. But current default
hard coded value is 5MB per an origin in application cache storage. But embedders
often want to reduce the default hard coded value per an origin as well. So it would
be good if we give the power them to set the maximum cache size per an origin for
application cache through the command line.

Bug:  824619 
Change-Id: I078365ec412834c024a92ff5ec4ad5d00452e2a7
Reviewed-on: https://chromium-review.googlesource.com/979877
Commit-Queue: Gyuyoung Kim <gyuyoung.kim@lge.com>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546105}
[modify] https://crrev.com/d143b1cc5e86eef3b2d7e8dacd629d24f58a5b6b/content/browser/appcache/appcache_storage_impl.cc
[modify] https://crrev.com/d143b1cc5e86eef3b2d7e8dacd629d24f58a5b6b/content/public/common/content_switches.cc
[modify] https://crrev.com/d143b1cc5e86eef3b2d7e8dacd629d24f58a5b6b/content/public/common/content_switches.h

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

commit c97d6fd554e16bbf9935220cf12147055e24de29
Author: Gyuyoung Kim <gyuyoung.kim@lge.com>
Date: Thu Sep 06 23:39:50 2018

appcache: Check if new appcache usage exceeds the maximum cache size

In AppCacheStorageImpl::StoreGroupAndCacheTask::Run(), it has not checked
whether new appcache usage exceeds the maximium cache size. This CL adds
the check condition to AppCacheStorageImpl::StoreGroupAndCacheTask::Run().

Bug:  824619 
Change-Id: Ic289f69cebcc60f06fc852072e9c50af831b8737
Reviewed-on: https://chromium-review.googlesource.com/1203752
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Gyuyoung Kim <gyuyoung.kim@lge.com>
Cr-Commit-Position: refs/heads/master@{#589365}
[modify] https://crrev.com/c97d6fd554e16bbf9935220cf12147055e24de29/content/browser/appcache/appcache_storage_impl.cc

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

commit 231fbf936ffa5f983640ee099bf71bd6fd9944f5
Author: Victor Costan <pwnall@chromium.org>
Date: Tue Oct 09 23:19:40 2018

Revert "appcache: Check if new appcache usage exceeds the maximum cache size"

This reverts commit c97d6fd554e16bbf9935220cf12147055e24de29.

Reason for revert: Broke AppCache integration with Quota Manager.  https://crbug.com/892547 

Original change's description:
> appcache: Check if new appcache usage exceeds the maximum cache size
> 
> In AppCacheStorageImpl::StoreGroupAndCacheTask::Run(), it has not checked
> whether new appcache usage exceeds the maximium cache size. This CL adds
> the check condition to AppCacheStorageImpl::StoreGroupAndCacheTask::Run().
> 
> Bug:  824619 
> Change-Id: Ic289f69cebcc60f06fc852072e9c50af831b8737
> Reviewed-on: https://chromium-review.googlesource.com/1203752
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Commit-Queue: Gyuyoung Kim <gyuyoung.kim@lge.com>
> Cr-Commit-Position: refs/heads/master@{#589365}

TBR=piman@chromium.org,gyuyoung.kim@lge.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  824619 
Change-Id: I17f16c39c997ae148279818afaf477584136f101
Reviewed-on: https://chromium-review.googlesource.com/c/1271879
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598126}
[modify] https://crrev.com/231fbf936ffa5f983640ee099bf71bd6fd9944f5/content/browser/appcache/appcache_storage_impl.cc

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

commit 1f05f839927e4ef9792d923d8ed0979785dbc137
Author: Victor Costan <pwnall@chromium.org>
Date: Wed Oct 10 02:08:53 2018

Revert "Set the maximum cache size per an origin in the command line for application cache"

This reverts commit d143b1cc5e86eef3b2d7e8dacd629d24f58a5b6b.

Reason for revert: Broke AppCache integration with Quota Manager.  https://crbug.com/892547 

Original change's description:
> Set the maximum cache size per an origin in the command line for application cache
> 
> Embedded devices have limited storage capacity generally. But current default
> hard coded value is 5MB per an origin in application cache storage. But embedders
> often want to reduce the default hard coded value per an origin as well. So it would
> be good if we give the power them to set the maximum cache size per an origin for
> application cache through the command line.
> 
> Bug:  824619 
> Change-Id: I078365ec412834c024a92ff5ec4ad5d00452e2a7
> Reviewed-on: https://chromium-review.googlesource.com/979877
> Commit-Queue: Gyuyoung Kim <gyuyoung.kim@lge.com>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#546105}

TBR=piman@chromium.org,gyuyoung.kim@lge.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  824619 
Change-Id: I66fb8f36f7d08409af95a288512e2d751c59a88f
Reviewed-on: https://chromium-review.googlesource.com/c/1272444
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598178}
[modify] https://crrev.com/1f05f839927e4ef9792d923d8ed0979785dbc137/content/browser/appcache/appcache_storage_impl.cc
[modify] https://crrev.com/1f05f839927e4ef9792d923d8ed0979785dbc137/content/public/common/content_switches.cc
[modify] https://crrev.com/1f05f839927e4ef9792d923d8ed0979785dbc137/content/public/common/content_switches.h

gyuyoung.kim@lge.com: Sorry, I'm reverting all the patches in this bug because of the quota issue they've created.

This is not meant to reflect any sort of judgement on the goal behind the patches. I'd like to understand what you need to accomplish, and figure out a path forward for your work.

Please let me know if you'd like to discuss here, or over e-mail.

I've cc-ed myself and added the correct component so the bug is correctly tracked. Added all the OSes that will get impacted by changes to AppCache or Quota.

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

commit 985f0f78988115db1ca753698e599ea46e90a747
Author: Victor Costan <pwnall@chromium.org>
Date: Wed Oct 10 05:42:17 2018

Revert "Pass the maximium disk cache size of appcache in the command line to appcache thread"

This reverts commit ef0fdeeaa50d1badc47bdac7aaf095e213fe7a67.

Reason for revert: Broke AppCache integration with Quota Manager.  https://crbug.com/892547 

Original change's description:
> Pass the maximium disk cache size of appcache in the command line to appcache thread
> 
> Embedded devices have limited storage capacity generally. So current maximum disk
> cache size(250MB) can make their storage fully used in the embedded devices.
> For the embedder, it would be good if we give the power them to set the maximum
> disk cache size through the command line.
> 
> Bug:  824619 
> Change-Id: Ib4fae34934eadf23f1165907b5c547fff205b827
> Reviewed-on: https://chromium-review.googlesource.com/974804
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Commit-Queue: Gyuyoung Kim <gyuyoung.kim@lge.com>
> Cr-Commit-Position: refs/heads/master@{#545010}

TBR=dgozman@chromium.org,piman@chromium.org,gyuyoung.kim@lge.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  824619 
Change-Id: I1b924434c52510f7008d03835670ca5d77a9b630
Reviewed-on: https://chromium-review.googlesource.com/c/1272804
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598220}
[modify] https://crrev.com/985f0f78988115db1ca753698e599ea46e90a747/content/browser/appcache/appcache_storage_impl.cc
[modify] https://crrev.com/985f0f78988115db1ca753698e599ea46e90a747/content/public/common/content_switches.cc
[modify] https://crrev.com/985f0f78988115db1ca753698e599ea46e90a747/content/public/common/content_switches.h

pwnall@, I succeed to reproduce the problem locally. And I found what CL has caused this problem. The last one caused this issue - https://chromium-review.googlesource.com/c/chromium/src/+/1203752. I guess the code modified the original behavior unintentionally. I will prepare a fix soon.

BTW, the first two CLs were to give embedders the power to set the maximum appcache sizes. Because there are various embedded devices which have been using Chromium. Most of them have different memory sizes. IMHO, the CLs will continue to help embedders to control their memory usage. So I hope that the first two CLs will be merged again. WDYT?

This is what I wanted to talk about. Again, feel free to switch the conversation to my e-mail if you're not comfortable sharing the answers publicly.

Do you have a reason for specifically targeting AppCache?

If not... the best way to limit application storage is to change the quota system. AppCache listens to the quota system, so you'll indirectly change the AppCache limit. At the same time, you also limit all other storage systems --examples: Cache Storage, IndexedDB, FileSystem, WebSQL.

WDYT? Will changing quota meet your needs better, or do you still want to specifically target AppCache?

Thank you for your suggestion! It would be great if we can limit all other storage systems together for low-end devices. BTW, I wonder if we can continue to keep the AppCache switches in order to avoid that web applications try to use much more disk size for AppCache abnormally. Although we limit all other storage systems, abnormal AppCache usage is able to influence the quota for other storage systems. But it's useless, I prefer to limit all other storage systems together definitely.

Today I took a look at this issue. As I mentioned in C#12, 1203752 has caused this problem. I thought that we need to check if new origin usage should not exceed the maximum appcache size(Default value is 5MB). Because it had been checking only when there is no available space. So this problem is what I intended. The CL generated the failure message because the fail.html tries to load 6MB appcache data. Could you please let me know why we should check the appcache quata only when then is no available space?

pwnall@, any comment on my question?

(re-posting #17 from correct account)

Sorry, I missed this earlier.

Please follow the changes applied to "space_available_" in appcache_storage_impl.cc -- a good way to do that is to open up "https://cs.chromium.org/chromium/src/content/browser/appcache/appcache_storage_impl.cc" search for the member name and look for all the hits.

You'll see the following:
1) space_available_ starts out at -1 in the StoreGroupAndCacheTask constructor
2) GetQuotaThenSchedule() sets space_available_ to a very large value if there is no quota manager and the origin gets infinite quota
3) If there is a quota manager, OnQuotaCallback() sets space_available_ to a value based on the quota manager's return.
4) In Run(), if space_available_ is -1 (no quota manager, no unlimited storage for the origin), kDefaultQuota is used as a cap.

So, kDefaultQuota is the *default* ceiling when a QuotaManager isn't wired into AppCache. When a QuotaManager is wired, that overrides any static default.

I'll follow up with a recommendation on the CL you're putting together.

Thank you for the answer. As I talked in the mail, I understand that -1 means that no QuotaMangager is not wired. If so, it seems it's reasonable to check the maximum origin cache size only when |space_available_| is -1. So I'd like to give up restoring the 3rd CL.

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

commit 6efb4dae8ab5d42f5ffcd9b9185c3b7af72925e5
Author: Gyuyoung Kim <gyuyoung.kim@lge.com>
Date: Sat Nov 03 01:27:17 2018

AppCache: Add command-line flags for default quota and disk size.

This CL introduces the following command-line switches:
- max-appcache-disk-cache-size-mb - limits the size of the HTTP disk
  cache used by AppCache for all origins
- max-appcache-origin-cache-size-mb - limits the per-origin AppCache
  quota when no QuotaManager is present

The CL is heavily based on the following reverted commits:
1. https://crrev.com/c/974804 - Set the maximum cache size per an origin
    in the command line for application cache
2. https://crrev.com/c/979877 - Pass the maximium disk cache size of
    appcache in the command line to appcache thread

Bug:  824619 , 895825
Change-Id: I4fb424fa740ef9875d0584def4e9390aef366916
Reviewed-on: https://chromium-review.googlesource.com/c/1282685
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Gyuyoung Kim <gyuyoung.kim@lge.com>
Cr-Commit-Position: refs/heads/master@{#605118}
[modify] https://crrev.com/6efb4dae8ab5d42f5ffcd9b9185c3b7af72925e5/content/browser/appcache/appcache_storage_impl.cc
[modify] https://crrev.com/6efb4dae8ab5d42f5ffcd9b9185c3b7af72925e5/content/public/common/content_switches.cc
[modify] https://crrev.com/6efb4dae8ab5d42f5ffcd9b9185c3b7af72925e5/content/public/common/content_switches.h

I'd like to close this bug because "AppCache: Add command-line flags for default quota and disk size." was landed. But, as mentioned in the comment, this patch will be reverted, and 895825 will follow the revert.