Extension settings API: force through changes that come from sync (ignoring

Extension settings API: force through changes that come from sync (ignoring
local quota) by adding a WriteOptions flag to Set and using FORCE when
writing from sync.

Otherwise sync will effectively be permanently disabled when the settings are
over quota.


BUG=103514
TEST=*ExtensionSetting* (unit_tests, browser_tests)


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110852

[not at google - send to devlin, 2011-11-17 07:02:52]

This is actually very few changes.  I've commented where the non-vacuous ones
area, "vacuous" being those which are propagating that WriteOptions thing
through each Storage implementation.

By the way, if you can think of a better way of doing this... please tell :)

http://codereview.chromium.org/8587025/diff/1/chrome/browser/extensions/setti...
File chrome/browser/extensions/settings/settings_quota_unittest.cc (right):

http://codereview.chromium.org/8587025/diff/1/chrome/browser/extensions/setti...
chrome/browser/extensions/settings/settings_quota_unittest.cc:529:
QuotaBytesPerSettingWithInitialSettingsForced) {
Change #1: a test.

http://codereview.chromium.org/8587025/diff/1/chrome/browser/extensions/setti...
File chrome/browser/extensions/settings/settings_storage.h (right):

http://codereview.chromium.org/8587025/diff/1/chrome/browser/extensions/setti...
chrome/browser/extensions/settings/settings_storage.h:93: enum WriteOptions {
Change #2: the WriteOptions enum and changes to the Storage interface.

http://codereview.chromium.org/8587025/diff/1/chrome/browser/extensions/setti...
File chrome/browser/extensions/settings/settings_storage_quota_enforcer.cc
(right):

http://codereview.chromium.org/8587025/diff/1/chrome/browser/extensions/setti...
chrome/browser/extensions/settings/settings_storage_quota_enforcer.cc:123: if
(options != FORCE) {
Change #3: not enforcing quota from Set (here and below)

http://codereview.chromium.org/8587025/diff/1/chrome/browser/extensions/setti...
File chrome/browser/extensions/settings/settings_sync_unittest.cc (right):

http://codereview.chromium.org/8587025/diff/1/chrome/browser/extensions/setti...
chrome/browser/extensions/settings/settings_sync_unittest.cc:1253:
LargeOutgoingChangeRejectedButIncomingAccepted) {
Change #4: another test

http://codereview.chromium.org/8587025/diff/1/chrome/browser/extensions/setti...
File chrome/browser/extensions/settings/syncable_settings_storage.cc (right):

http://codereview.chromium.org/8587025/diff/1/chrome/browser/extensions/setti...
chrome/browser/extensions/settings/syncable_settings_storage.cc:399: WriteResult
result = delegate_->Set(FORCE, key, *new_value);
Change #5: forcing changes that come from sync (here and 2 below)

[Matt Perry, 2011-11-17 19:53:51]

http://codereview.chromium.org/8587025/diff/1/chrome/browser/extensions/setti...
File chrome/browser/extensions/settings/settings_storage_quota_enforcer.cc
(right):

http://codereview.chromium.org/8587025/diff/1/chrome/browser/extensions/setti...
chrome/browser/extensions/settings/settings_storage_quota_enforcer.cc:123: if
(options != FORCE) {
On 2011/11/17 07:02:53, kalman wrote:
> Change #3: not enforcing quota from Set (here and below)

It looks like FORCE only has an effect for Set(). Do you really need it for
Remove/Clear?

[not at google - send to devlin, 2011-11-18 03:35:20]

http://codereview.chromium.org/8587025/diff/1/chrome/browser/extensions/setti...
File chrome/browser/extensions/settings/settings_storage_quota_enforcer.cc
(right):

http://codereview.chromium.org/8587025/diff/1/chrome/browser/extensions/setti...
chrome/browser/extensions/settings/settings_storage_quota_enforcer.cc:123: if
(options != FORCE) {
On 2011/11/17 19:53:51, Matt Perry wrote:
> On 2011/11/17 07:02:53, kalman wrote:
> > Change #3: not enforcing quota from Set (here and below)
> 
> It looks like FORCE only has an effect for Set(). Do you really need it for
> Remove/Clear?

Yep.  I added it to Remove and Clear for consistency but I agree that it's
unnecessary.  Reverted those changes, just made it Set.

[Matt Perry, 2011-11-18 19:29:41]

LGTM - but update the CL description now that Remove/Clear don't have
WriteOptions.

[commit-bot: I haz the power, 2011-11-19 01:00:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/8587025/9001

[commit-bot: I haz the power, 2011-11-19 02:38:24]

Try job failure for 8587025-9001 (retry) (retry) on linux_rel for step
"ui_tests".
It's a second try, previously, step "ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2011-11-20 00:06:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/8587025/9001

[commit-bot: I haz the power, 2011-11-20 01:01:55]

Change committed as 110852