Replaces the use of Value::SetIntPath() with
Value::Dict::SetByDottedPath(). This require changing other similar
values (e.g Value::SetStringKey() to Value::Dict::Set())
No functionality changed.
Bug: 1187001
Change-Id: I83708e72cf0243c28cf2e7aae8705a3cf3fafe5a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4356431
Commit-Queue: Emilia Paz <emiliapaz@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1119992}
Although the histogram is expired, we are still receiving data from
one of its "variant" on the serverside. In particular:
Settings.JsonDataReadSizeKilobytes.Network_Persistent_State
This is because the "Network_Persistent_State" suffix is not in
histogram_suffixes_list.xml, so it is not being detected as
expired, and is hence being sent.
Since the histogram is intended to be expired, just remove it.
For reference, documented old values @ go/etqpo (Google-internal).
Bug: 1423446
Change-Id: Ia7be074927eaae6f2f46ec1b53b23237aad64e9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4327823
Commit-Queue: Luc Nguyen <lucnguyen@google.com>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Auto-Submit: Luc Nguyen <lucnguyen@google.com>
Cr-Commit-Position: refs/heads/main@{#1116706}
This CL removes the use of an out param in
ImportantFileWriter::SerializeData in favour of returning an optional.
Additionally, this CL removes usages of JSONStringValueSerializer from
the files in question.
Bug: 1421354
Change-Id: I32e59e2e49c4e226eae85cbcfe280ab34c39a661
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4321452
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Claudio DeSouza <cdesouza@igalia.com>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1116324}
This CL is part of a batch of CLs to replace the use of the less
friendly base::WriteFile call with simpler variants, which are easier to
read, and less prone to mistakes.
Changes in this particular CL address files under the path
/components/prefs.
This CL was uploaded by git cl split.
R=gab@chromium.org
AX-Relnotes: n/a.
Bug: 418837
Change-Id: I9df41c2bd5bfa2f11f03b2bf83c3c0ed7c557cb0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4288223
Auto-Submit: Claudio DeSouza <cdesouza@igalia.com>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1110354}
PrefModelAssociator is the "bridge" between Sync and the native
Preferences model. It synchronizes user pref values (as opposed to
policy-controlled values, default values, etc) between this Chrome
instance and the Sync server.
Before this CL, PrefModelAssociator was hooked up to PrefService.
After this CL, it's instead hooked up directly to the user-controlled
PrefStore.
This is simpler/more natural in some ways:
Only user prefs should be synced, so hooking up directly to those
makes sense. E.g. PrefModelAssociator now no longer needs to filter
out pref changes if the pref has a non-user-controlled value.
There is one behavior change, when the user-controlled value of a pref
is changed while a "higher-level" pref value also exists (e.g. policy-
or extension-controlled):
Before this CL, the pref change would be ignored.
After this CL, the user pref value will still get sent to the Sync
server.
This should be a very rare situation and not have any notable impact.
Bug: 1404937
Change-Id: Ia9e72effb14b52dd470007bed902441bf6f0aeb3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4219172
Reviewed-by: Maksim Moskvitin <mmoskvitin@google.com>
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1105099}
This is a reland of commit 5d8d0d2168
Remove the failing test. See patchset 1..2.
Original change's description:
> CHECK on duplicate AddObserver() calls in ObserverList
>
> There're observed crashes because of duplicate AddObserver() calls. See
> crbug.com/1410397#c2. This may lead to obscure bugs if not captured,
> therefore replace NOTREACHED with a CHECK so that it will always crash
> on release builds.
>
> Bug: None
> Change-Id: I23053675d33d2d6fc9cb2ecc7d083f03b6ec60c0
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4197957
> Reviewed-by: Peter Boström <pbos@chromium.org>
> Commit-Queue: Keren Zhu <kerenzhu@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1097657}
Bug: None
Change-Id: I88e819cc101cadc5e24f9780e2fa686a883c1297
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4197922
Commit-Queue: Keren Zhu <kerenzhu@chromium.org>
Owners-Override: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1097919}
Turns out there's a lot of includes, so these will have to be removed
before deleting the implementation of the task runner handles.
To allow the deletion of the task runner handle headers, add
the sequenced/thread task runner handles where they are used in
the codebase with scripts.
This was done with an automated change, with a few touchups afterwards.
The code for the mass-refactor changes are here:
python:
https://paste.googleplex.com/5534570878337024
shell:
https://paste.googleplex.com/6466750748033024
In terms of touchups:
- add sequenced/thread task runner handles to
the third_party/blink/public/DEPS, because multiple files were using
it transitively anyways.
- rewrite certain parts of the codebase which used
ThreadTaskRunnerHandles instead of CurrentDefaultHandles.
- fix a compile issue with forward-declaration in
extensions/browser/extension_file_task_runner.h.
AX-Relnotes: n/a.
Bug: 1026641
Change-Id: I737ef32aee4e77c21eaa3a2bdc403a28322cf1b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4133323
Owners-Override: Gabriel Charette <gab@chromium.org>
Commit-Queue: Sean Maher <spvw@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1090532}
Romves some usage of deprecated base::ListValue and base::DictionaryValue, replacing them with base::Value.
This CL also removed unused declarations.
Bug: 1187062, 1187061
Change-Id: I970e9ab9d2aa7d463609035b82133bd282294d11
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4122593
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Jonathan Njeunje <njeunje@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1086760}
This CL migrates from `DictionaryValue` and `SetInteger()` to
`base::Value::Dict` and `Set()`, or `Value::SetIntPath()` in
components/sync_preferences/pref_model_associator_unittest.cc
Bug: 1187026
Change-Id: Iab5ae213553b15b42c816096a65d5d76a3a0d001
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4113185
Commit-Queue: Shunya Shishido <sisidovski@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1084860}
Since TestingPrefServiceBase::SetPref() already owns the
std::unique_ptr<base::Value> argument, just convert it to base::Value
instead of cloning it.
Change-Id: I7bbef6d43951ee383a0071cafa8fa52872a62625
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4109930
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1084491}
These compilation units had a transitive dependency on ostream via
base/numerics/safe_conversions.h. The ostream include in
safe_conversions.h is however unused and should be removed.
This CL was uploaded by git cl split.
R=battre@chromium.org
Bug: 1270812, 1372522
Change-Id: Iec69dd485d658c0766e1d3bceee44f250ce1974e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4057312
Auto-Submit: Jean-Philippe Gravel <jpgravel@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Jean-Philippe Gravel <jpgravel@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1076004}
PrefService::ClearMutableValues() (and thus all the various, mostly
empty, implementations in the PrefStore subclasses) was unused, so this
CL removes it.
Bug: none
Change-Id: I95722d6aec4c8fc68dc7560f6f6693310edf74d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4005952
Commit-Queue: Dominic Battré <battre@chromium.org>
Auto-Submit: Marc Treib <treib@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1068034}
This patch must be a no-op with all the common build flags.
Add the "DanglingUntriaged" raw_ptr annotation. It indicates a
raw_ptr becomes dangling, and it should be triaged/fixed. This will also disable dangling protection for those pointers, once enabled.
These were identified by running the CQ bots with DPD activated (both build + runtime here: https://crrev.com/c/3941825)
Bug: 1291138
Change-Id: I07e3d85b1c629f7d49c928f1fca42a538c64ef3b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3959615
Owners-Override: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Pâris Meuleman <pmeuleman@chromium.org>
Commit-Queue: Ali Hijazi <ahijazi@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1061867}
This cl migrates ProxyConfigDictionary (see
components/proxy_config/proxy_config_dictionary.h) to use
base::Value::Dict, most of the changes are directly propagated from
there. Besides that it expands TestingPrefServiceBase (see
components/prefs/testing_pref_service.h) interface with setters taking
base::Value::Dict and base::Value::List which allows to avoid manual
conversion to the base::Value on the call side in many tests.
LOW_COVERAGE_REASON=Code Health type migration, nothing new to test
Bug: 1187061, 1187001
Change-Id: I0dbca1a78b3c0a4f313f364f3d2fc35378ccf273
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3922202
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Andrey Davydov <andreydav@google.com>
Cr-Commit-Position: refs/heads/main@{#1053030}
Currently, WriteablePrefStore takes `std::unique_ptr<base::Value>`
arguments for `SetValue` and `SetValueSilently`. However, the
implementations actually store `base::Value`. This leads to a lot of
unnecessary memory allocations and releases for values that are
created on the heap just to be moved away immediately.
This CL changes the API of WriteablePrefStore to accept `base::Value`.
This avoids the superfluous allocations and release.
Bug: 1368265
Change-Id: I698f5e4efa5006db0c62436add745712001c9b11
Tests: Existing unit tests
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3913938
Commit-Queue: Roland Bock <rbock@google.com>
Owners-Override: Gabriel Charette <gab@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1052757}
Currently, PrefService::GetValue and related functions take path
arguments as `const std::string&`. Since most call sites actually
pass `const char*` parameters, this leads to a lot of string constructions and (potentially) memory allocations, depending on the size of the string.
This CL replaces the argument type with `base::StringPiece`, which reduces the Android binary size by about 11kB.
In order to look up `base::StringPiece` keys in
`std::set<std::string>` or `std::map<std::string, base::Value>` we
need to replace the Compare function object with `std::less<void>`, which allows transparent comparison.
This is defined in the new header components/prefs/pref_name_set.h
Bug: 1366031
Change-Id: I386f032849fb75c8416047e48ecc92b567faf450
Tests: Existing unit tests
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3902702
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Igor <igorcov@chromium.org>
Reviewed-by: Sean Topping <seantopping@chromium.org>
Commit-Queue: Roland Bock <rbock@google.com>
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Michael Bai <michaelbai@chromium.org>
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1051089}
pref_registry_->defaults() is a better fallback than nullptr.
Note that this could only ever have an effect in case
pref_registry_->defaults()
!= pref_value_store_->GetStore(DEFAULT_STORE)
So in the long run, we might want to add a guarantee for equality.
Bug: 1364529
Change-Id: I63f380fab0839afc1e1931629a9c804f2fed687c
Tests: Existing unit tests
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3899758
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Roland Bock <rbock@google.com>
Cr-Commit-Position: refs/heads/main@{#1050840}
This CL adds a method to the bridge that allows checking whether
a pref with a specified key is using its default value.
This needed in the process of migrating existing SharedPreferences
to PrefService prefs (as an equivalent of the "contains" method).
Bug: 1362553
Change-Id: I986d7e662215c906dbdd7231dfb09f47e56f935a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3904382
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Jan Keitel <jkeitel@google.com>
Cr-Commit-Position: refs/heads/main@{#1049049}
The new ScopedDictPrefUpdate and ScopedListPrefUpdate class only
expose Value::Dicts and Value::Lists, respectively. This makes it
more straightforward to use the new Dict/List classes, and prevents
callers from using deprecated value manipulation methods with the
APIs.
The new API also returns references instead of pointers, to prevent
unnecessary null checking.
Bug: 1362719
Change-Id: I322f4b037571c39beb741a4434d0cee1eb1276d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3900657
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1048082}
As of today, PrefService::GetString yields a std::string value. In case
the requested path contains a string, this is a copy of the actual pref
value. Otherwise it is a freshly constructed string.
However, the documentation clearly assumes that the requested pref has
to be a string value. There is no unit test that validates the behavior
for non-string values. And an analysis has revealed no usage that would
rely on the fallback either.
PrefService::GetString is therefore needlessly expensive.
Following the same logic as go/trust-your-prefs, note that if, there were a call site that called GetString with a key that did not point to a string, it would /always/ receive an empty string and needs to be refactored.
This CL removes the fallback for PrefService::GetString and makes it
return a reference to the stored value.
It also removes the fallbacks in GetBool, GetInt, and GetDouble for the
same reasons.
Bug: 1344857
Change-Id: Ibd70e206d75e23d84f7e6ef5e0307c1518e56515
Tests: Existing unit tests
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3810628
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Roland Bock <rbock@google.com>
Cr-Commit-Position: refs/heads/main@{#1047861}
Semi-final step go/trust-your-prefs#heading=h.jjuz6fybcx8y
Introduce
- PrefService::GetDict and
- PrefService::GetList
as drop-in replacements for
- PrefService::GetValueDict and
- PrefService::GetValueList
and replace all known call sites via
sed -i 's/\bGetValueDict\b/GetDict/g' $(ag -l -s '\bGetValueDict\b')
sed -i 's/\bGetValueList\b/GetList/g' $(ag -l -s '\bGetValueList\b')
The final step will be to remove GetValueDict and GetValueList in a
follow-up CL.
Bug: 1334665
Test: Existing unit tests
Change-Id: I093e81af89628a02216501e59c1de7f1e5e22b9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3887802
Auto-Submit: Roland Bock <rbock@google.com>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Owners-Override: Gabriel Charette <gab@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1045895}
This is a reland of commit 0dbb10a87d
The code that caused the revert was migrated in
crrev.com/c/3798165/4..5
Original change's description:
> Remove PrefService::Get
>
> PrefService::Get has been replaced by PrefService::GetValue.
>
> Bug: 1334665
> Test: Existing unit tests
> Change-Id: Iec3458bdde235172ca86549328e24006dd584727
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3827209
> Commit-Queue: Roland Bock <rbock@google.com>
> Reviewed-by: Dominic Battré <battre@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1038763}
Bug: 1334665
Change-Id: I939a9254af25727aa97b95cabdb859b62798c79d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3857319
Commit-Queue: Roland Bock <rbock@google.com>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1040951}