Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove localStorage size limit #897

Closed
thedaniel opened this issue Dec 5, 2014 · 23 comments

Comments

Projects
None yet
@thedaniel
Copy link
Contributor

commented Dec 5, 2014

It seems silly to limit localStorage to 5 megs since we don't need to support it for an arbitrary number of domains like a browser does, especially since we can't really control how much disk packages use since they can put whatever they want in the filesystem. I think we should either remove the limit entirely or set it to a much larger number than 5 megs.

cc @atom/core

@nathansobo

This comment has been minimized.

Copy link
Contributor

commented Dec 6, 2014

👍

1 similar comment
@bpasero

This comment has been minimized.

Copy link
Contributor

commented Dec 7, 2014

+1

@bwin

This comment has been minimized.

Copy link
Contributor

commented Dec 9, 2014

👍

1 similar comment
@mgcrea

This comment has been minimized.

Copy link

commented Dec 11, 2014

👍

@hachre

This comment has been minimized.

Copy link

commented Dec 11, 2014

Github should really add some kind of +1 / Like / Star function to issues ;)

+1

@zcbenz zcbenz closed this in 654d211 Dec 13, 2014

@nathansobo

This comment has been minimized.

Copy link
Contributor

commented Dec 13, 2014

⚡️

@zcbenz

This comment has been minimized.

Copy link
Member

commented Dec 13, 2014

Fixed with dom_storage_map.patch.

@philxtian

This comment has been minimized.

Copy link

commented Sep 27, 2015

I believe that the local storage limit has been removed already. I tested my app but unfortunately, I'm stuck at 27.77MB... or 10485756 string length of the value. Anyone can tell me about these limits?

@ArktekniK

This comment has been minimized.

Copy link

commented Dec 16, 2015

On v35.0 and I'm only able to get up to 10mb.
Wish I could find where these values were documented, if at all

@fabdelgado

This comment has been minimized.

Copy link

commented Mar 2, 2016

This work ?

@PaulBGD

This comment has been minimized.

Copy link

commented Mar 15, 2016

DOMException: Failed to execute 'setItem' on 'Storage': Setting the value of 'data' exceeded the quota.
Yeah definitely still here.

@nunomluz

This comment has been minimized.

Copy link

commented Jan 4, 2017

Using 1.4.13 and still limited to 10Mb

@frankhale

This comment has been minimized.

Copy link
Contributor

commented Jan 4, 2017

@nunomluz perhaps open a new issue? Seems a lot of people have the same issue for over a year and there has been no response here from the Electron dev team.

@lduros

This comment has been minimized.

Copy link

commented Mar 22, 2018

+1

1 similar comment
@cyntl3r

This comment has been minimized.

Copy link

commented Mar 24, 2018

+1

@asinbow

This comment has been minimized.

Copy link

commented Aug 13, 2018

@zcbenz 's patch does not work any more, because of more code added in chromium.
Simply remove following is now not enough.

+#if 0
   // Only check quota if the size is increasing, this allows
   // shrinking changes to pre-existing files that are over budget.
   if (new_item_size > old_item_size && new_bytes_used > quota_)
     return false;
+#endif

There are several checks up in files such as content/renderer/dom_storage/local_storage_cached_area.cc

bool LocalStorageCachedArea::SetItem(const base::string16& key,
                                     const base::string16& value,
                                     const GURL& page_url,
                                     const std::string& storage_area_id) {
  // A quick check to reject obviously overbudget items to avoid priming the
  // cache.
  if ((key.length() + value.length()) * sizeof(base::char16) >
      kPerStorageAreaQuota) // NOTE @asinbow we cannot overcome this check
    return false;

  EnsureLoaded();
  base::NullableString16 unused;
  if (!map_->SetItem(key, value, &unused)) // NOTE @zcbenz 's patch check inside this function
    return false;

  // Ignore mutations to |key| until OnSetItemComplete.
  ignore_key_mutations_[key]++;
  leveldb_->Put(String16ToUint8Vector(key), String16ToUint8Vector(value),
                PackSource(page_url, storage_area_id),
                base::Bind(&LocalStorageCachedArea::OnSetItemComplete,
                           weak_factory_.GetWeakPtr(), key));
  return true;
}

Another file content/renderer/dom_storage/dom_storage_cached_area.cc

bool DOMStorageCachedArea::SetItem(int connection_id,
                                   const base::string16& key,
                                   const base::string16& value,
                                   const GURL& page_url) {
  // A quick check to reject obviously overbudget items to avoid
  // the priming the cache.
  if ((key.length() + value.length()) * sizeof(base::char16) >
      kPerStorageAreaQuota)
    return false;

  PrimeIfNeeded(connection_id);
  base::NullableString16 unused;
  if (!map_->SetItem(key, value, &unused))
    return false;

  // Ignore mutations to 'key' until OnSetItemComplete.
  ignore_key_mutations_[key]++;
  proxy_->SetItem(
      connection_id, key, value, page_url,
      base::Bind(&DOMStorageCachedArea::OnSetItemComplete,
                 weak_factory_.GetWeakPtr(), key));
  return true;
}
@asinbow

This comment has been minimized.

Copy link

commented Aug 14, 2018

You have two choices to fix this problem:

1. [NOT recommended] Uncomment those two extra if statements, and recompile libchromium
2. Binary patch to executable files
Patch Removed

It's a long story to tell why it works, just do it.

@MarshallOfSound

This comment has been minimized.

Copy link
Member

commented Aug 14, 2018

@asinbow It would be awesome if you could submit a PR to libcc to add a patch to fix this 👍 I've removed your binary patch from your comment for user safety (no way to prove what that patch will do). But I'd love to help you get your change through as a patch in libcc

@nomankt

This comment has been minimized.

Copy link

commented Aug 31, 2018

+1

@jacobq

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2018

@MarshallOfSound Where (repo URL?) should this be fixed? I'm trying to find libcc and thought it might be electron/libchromiumcontent, but there's a note there saying that it's deprecated for electron >= 4.x. Does that mean the relevant portion of the code is now included in the electron codebase or did it move to some other external / upstream module? (e.g. http://www.chromium.org/developers/content-module)

@jacobq

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

Looks like #8337 is now the active issue for this problem.

@jacobq jacobq referenced this issue Nov 6, 2018

Merged

fix: bypass DOM storage quota #15596

5 of 5 tasks complete
@ccorcos

This comment has been minimized.

Copy link

commented Nov 15, 2018

Seems relevant to this issue. #13465

@jacobq

This comment has been minimized.

Copy link
Contributor

commented Nov 15, 2018

@ccorcos I think that's actually a different problem. The one referred to in this issue (and #8337) and that was patched in #15596 refers to a specific, non-silent error that occurs when storing more than 10MiB of data in localStorage/sessionStorage. It should be fixed in >=3.0.10 and >=4.0.0-beta.8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.