Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2883523002:
Reduce the memory usage of bookmarks storage (Closed)
DescriptionReduce the memory usage of bookmarks storage
This CL does:
1. Url index uses flat_set instead of set.
2. Do not load image data and page data from bookmarks metadata into
memory these are deprecated.
3. BookmarkNode stores ptr to icon_url instead of GURL
BUG=691698
Review-Url: https://codereview.chromium.org/2883523002
Cr-Commit-Position: refs/heads/master@{#476158}
Committed: https://chromium.googlesource.com/chromium/src/+/144fb7b80b8112e948a83c3fe64d45738b4450df
Patch Set 1 #
Total comments: 12
Patch Set 2 : address comments and fixes. #
Total comments: 2
Patch Set 3 : Move to client. #Patch Set 4 : Move to client. #
Total comments: 10
Patch Set 5 : Fixes. #Patch Set 6 : fixes. #Patch Set 7 : remove client interface #
Total comments: 4
Patch Set 8 : string in codec.cc #
Total comments: 2
Patch Set 9 : MakeUnique #Messages
Total messages: 53 (35 generated)
Description was changed from ========== Reduce the memory usage of bookmarks storage This CL does: 1. Url index uses flat_set instead of set. 2. Do not load image data and page data from bookmarks metadata into memory. 3. BookmarkNode stores ptr to icon_url instead of GURL BUG=691698 ========== to ========== Reduce the memory usage of bookmarks storage This CL does: 1. Url index uses flat_set instead of set. 2. Do not load image data and page data from bookmarks metadata into memory for low end devices. 3. BookmarkNode stores ptr to icon_url instead of GURL BUG=691698 ==========
Patchset #1 (id:1) has been deleted
ssid@chromium.org changed reviewers: + sky@chromium.org
+sky wdyt?
https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... File components/bookmarks/browser/bookmark_codec.cc (right): https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... components/bookmarks/browser/bookmark_codec.cc:438: if (base::SysInfo::IsLowEndDevice() && If we're going to support disabling this feature I would rather the disabling is explicit rather than keying off IsLowEndDevice. Similarly, the set of keys that are not needed should be passed in, not hard coded here. https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... File components/bookmarks/browser/bookmark_node.h (right): https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... components/bookmarks/browser/bookmark_node.h:72: const std::unique_ptr<GURL>& icon_url() const { return icon_url_; } This should return a const GURL*. https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... components/bookmarks/browser/bookmark_node.h:182: std::unique_ptr<GURL> icon_url_; What is the actual savings of this? What is sizeof(GURL)? https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... File components/bookmarks/browser/titled_url_index.h (right): https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... components/bookmarks/browser/titled_url_index.h:53: using TitledUrlNodeSet = base::flat_set<const TitledUrlNode*>; I'm worried about using flat_set here as there is no guarantee that the set is small. I think they generally are, but that isn't always the case. Do you have data backing up they *always* are small enough? https://codereview.chromium.org/2883523002/diff/20001/components/sync_bookmar... File components/sync_bookmarks/bookmark_change_processor.cc (right): https://codereview.chromium.org/2883523002/diff/20001/components/sync_bookmar... components/sync_bookmarks/bookmark_change_processor.cc:972: bookmark_node->icon_url() ? bookmark_node->icon_url()->spec() : ""); "" -> std::string()
The CQ bit was checked by ssid@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by ssid@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
replied inline. ptal, Thanks! https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... File components/bookmarks/browser/bookmark_codec.cc (right): https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... components/bookmarks/browser/bookmark_codec.cc:438: if (base::SysInfo::IsLowEndDevice() && On 2017/05/12 20:00:13, sky wrote: > If we're going to support disabling this feature I would rather the disabling is > explicit rather than keying off IsLowEndDevice. Similarly, the set of keys that > are not needed should be passed in, not hard coded here. So, I moved the strings to storage.cc and passed it as argument to this class when needed. Do you mean I should disable these meta info for all platforms and all types of devices instead of just low-end devices? Sorry I did not fully understand your comment. https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... File components/bookmarks/browser/bookmark_node.h (right): https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... components/bookmarks/browser/bookmark_node.h:72: const std::unique_ptr<GURL>& icon_url() const { return icon_url_; } On 2017/05/12 20:00:13, sky wrote: > This should return a const GURL*. Sorry, fixed! https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... components/bookmarks/browser/bookmark_node.h:182: std::unique_ptr<GURL> icon_url_; On 2017/05/12 20:00:13, sky wrote: > What is the actual savings of this? What is sizeof(GURL)? The GURL is 95 bytes I think. for 500 urls it turned out to be 50KiB. https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... File components/bookmarks/browser/titled_url_index.h (right): https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... components/bookmarks/browser/titled_url_index.h:53: using TitledUrlNodeSet = base::flat_set<const TitledUrlNode*>; On 2017/05/12 20:00:13, sky wrote: > I'm worried about using flat_set here as there is no guarantee that the set is > small. I think they generally are, but that isn't always the case. Do you have > data backing up they *always* are small enough? The flat_set uses a std::vector as backing type. So, it must be always smaller than using a tree data structure in the case of std::set. The tree uses a lot more overhead in each node whereas vector directly stores only the pointers. In the case I observe the memory overhead of std::set<void*> is 20% payload and 80% overhead. I checked the memory usage patterns with multiple insertion sizes from 10-100000 (random elements) and in all cases the flat_map allocates 4 times lesser memory than std::set. The performance of insertion regressed by 6ms for the case of 10000 insertions and 500ms for the case of 100,000 insertions. The number of people with so many bookmarks is <0.01% https://codereview.chromium.org/2883523002/diff/20001/components/sync_bookmar... File components/sync_bookmarks/bookmark_change_processor.cc (right): https://codereview.chromium.org/2883523002/diff/20001/components/sync_bookmar... components/sync_bookmarks/bookmark_change_processor.cc:972: bookmark_node->icon_url() ? bookmark_node->icon_url()->spec() : ""); On 2017/05/12 20:00:13, sky wrote: > "" -> std::string() Done.
Patchset #2 (id:40001) has been deleted
https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... File components/bookmarks/browser/titled_url_index.h (right): https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... components/bookmarks/browser/titled_url_index.h:53: using TitledUrlNodeSet = base::flat_set<const TitledUrlNode*>; On 2017/05/25 00:27:27, ssid wrote: > On 2017/05/12 20:00:13, sky wrote: > > I'm worried about using flat_set here as there is no guarantee that the set is > > small. I think they generally are, but that isn't always the case. Do you have > > data backing up they *always* are small enough? > > The flat_set uses a std::vector as backing type. So, it must be always smaller > than using a tree data structure in the case of std::set. The tree uses a lot > more overhead in each node whereas vector directly stores only the pointers. In > the case I observe the memory overhead of std::set<void*> is 20% payload and 80% > overhead. > I checked the memory usage patterns with multiple insertion sizes from 10-100000 > (random elements) and in all cases the flat_map allocates 4 times lesser memory > than std::set. > The performance of insertion regressed by 6ms for the case of 10000 insertions > and 500ms for the case of 100,000 insertions. The number of people with so many > bookmarks is <0.01% My question is that the data populating these sets comes from user data. How do you know it is going to be small(ish)? flat_set is best used for small data sets. https://codereview.chromium.org/2883523002/diff/60001/components/bookmarks/br... File components/bookmarks/browser/bookmark_storage.cc (right): https://codereview.chromium.org/2883523002/diff/60001/components/bookmarks/br... components/bookmarks/browser/bookmark_storage.cc:77: if (base::SysInfo::IsLowEndDevice()) { What I was getting at with my earlier comment is that rather than hard coding that low-end-device == ignore certain keys that you pass into BookmarkModel this information. Basically BookmarkModel (probably via the client interface) has a way to specify what meta keys should be ignored. It's up to the actual consumer (by way of the BookmarkClient interface) to specify what those keys are. I'm saying you add something like std::set<std::string> ExecludedMetaKeys() to the BookmarkClient interface.
The CQ bit was checked by ssid@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
The CQ bit was checked by ssid@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thanks, made changes! https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... File components/bookmarks/browser/titled_url_index.h (right): https://codereview.chromium.org/2883523002/diff/20001/components/bookmarks/br... components/bookmarks/browser/titled_url_index.h:53: using TitledUrlNodeSet = base::flat_set<const TitledUrlNode*>; On 2017/05/25 02:54:02, sky wrote: > On 2017/05/25 00:27:27, ssid wrote: > > On 2017/05/12 20:00:13, sky wrote: > > > I'm worried about using flat_set here as there is no guarantee that the set > is > > > small. I think they generally are, but that isn't always the case. Do you > have > > > data backing up they *always* are small enough? > > > > The flat_set uses a std::vector as backing type. So, it must be always smaller > > than using a tree data structure in the case of std::set. The tree uses a lot > > more overhead in each node whereas vector directly stores only the pointers. > In > > the case I observe the memory overhead of std::set<void*> is 20% payload and > 80% > > overhead. > > I checked the memory usage patterns with multiple insertion sizes from > 10-100000 > > (random elements) and in all cases the flat_map allocates 4 times lesser > memory > > than std::set. > > The performance of insertion regressed by 6ms for the case of 10000 insertions > > and 500ms for the case of 100,000 insertions. The number of people with so > many > > bookmarks is <0.01% > > My question is that the data populating these sets comes from user data. How do > you know it is going to be small(ish)? flat_set is best used for small data > sets. The uma for the number of bookmarks used: https://uma.googleplex.com/p/chrome/histograms/?endDate=20170524&dayCount=1&h... Shows that user at 99th percentile uses 1000 bookmarks and 0.01% of the population uses more than 12K bookmarks. We do not regress the performance for these users. Median user uses 9 bookmarks. https://codereview.chromium.org/2883523002/diff/60001/components/bookmarks/br... File components/bookmarks/browser/bookmark_storage.cc (right): https://codereview.chromium.org/2883523002/diff/60001/components/bookmarks/br... components/bookmarks/browser/bookmark_storage.cc:77: if (base::SysInfo::IsLowEndDevice()) { On 2017/05/25 02:54:03, sky wrote: > What I was getting at with my earlier comment is that rather than hard coding > that low-end-device == ignore certain keys that you pass into BookmarkModel this > information. Basically BookmarkModel (probably via the client interface) has a > way to specify what meta keys should be ignored. It's up to the actual consumer > (by way of the BookmarkClient interface) to specify what those keys are. I'm > saying you add something like std::set<std::string> ExecludedMetaKeys() to the > BookmarkClient interface. I think I have made changes as you suggested.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
This is indeed what I asked for. Thanks! One question though. What effects does this have on sync? Lets say we drop some keys, then the user modifies the title. Are the keys erased on the server side too? https://codereview.chromium.org/2883523002/diff/100001/components/bookmarks/b... File components/bookmarks/browser/bookmark_client.h (right): https://codereview.chromium.org/2883523002/diff/100001/components/bookmarks/b... components/bookmarks/browser/bookmark_client.h:70: // Metadata keys to be excluded from bookmark nodes in the storage. How about: Returns the set of keys that are not restored when reading bookmarks back in. https://codereview.chromium.org/2883523002/diff/100001/components/bookmarks/b... components/bookmarks/browser/bookmark_client.h:71: virtual std::vector<std::string> ExecludedMetaKeys(); GetExcludedMetaDataKeys(). https://codereview.chromium.org/2883523002/diff/100001/components/bookmarks/b... File components/bookmarks/browser/bookmark_codec.cc (right): https://codereview.chromium.org/2883523002/diff/100001/components/bookmarks/b... components/bookmarks/browser/bookmark_codec.cc:16: #include "base/sys_info.h" You shouldn't need this key anymore. https://codereview.chromium.org/2883523002/diff/100001/components/bookmarks/b... File components/bookmarks/browser/bookmark_codec.h (right): https://codereview.chromium.org/2883523002/diff/100001/components/bookmarks/b... components/bookmarks/browser/bookmark_codec.h:92: void set_excluded_meta_info_keys(const std::vector<std::string>& keys) { Be consistent: meta_data_keys_. https://codereview.chromium.org/2883523002/diff/100001/components/bookmarks/b... components/bookmarks/browser/bookmark_codec.h:92: void set_excluded_meta_info_keys(const std::vector<std::string>& keys) { Wouldn't a set be a better data structure here?
On 2017/05/26 03:07:39, sky wrote: > This is indeed what I asked for. Thanks! > > One question though. What effects does this have on sync? Lets say we drop some > keys, then the user modifies the title. Are the keys erased on the server side > too? > Thanks for raising this concern. Found this CL that removes these keys. So, it is safe to remove all keys with "stars." prefix. https://codereview.chromium.org/163841300 https://codereview.chromium.org/2883523002/diff/100001/components/bookmarks/b... File components/bookmarks/browser/bookmark_client.h (right): https://codereview.chromium.org/2883523002/diff/100001/components/bookmarks/b... components/bookmarks/browser/bookmark_client.h:70: // Metadata keys to be excluded from bookmark nodes in the storage. On 2017/05/26 03:07:38, sky wrote: > How about: Returns the set of keys that are not restored when reading bookmarks > back in. Done. https://codereview.chromium.org/2883523002/diff/100001/components/bookmarks/b... components/bookmarks/browser/bookmark_client.h:71: virtual std::vector<std::string> ExecludedMetaKeys(); On 2017/05/26 03:07:38, sky wrote: > GetExcludedMetaDataKeys(). Codec uses MetaInfo everywhere. Changed it to MetaInfo in all chnages in this cl. https://codereview.chromium.org/2883523002/diff/100001/components/bookmarks/b... File components/bookmarks/browser/bookmark_codec.cc (right): https://codereview.chromium.org/2883523002/diff/100001/components/bookmarks/b... components/bookmarks/browser/bookmark_codec.cc:16: #include "base/sys_info.h" On 2017/05/26 03:07:38, sky wrote: > You shouldn't need this key anymore. Done. https://codereview.chromium.org/2883523002/diff/100001/components/bookmarks/b... File components/bookmarks/browser/bookmark_codec.h (right): https://codereview.chromium.org/2883523002/diff/100001/components/bookmarks/b... components/bookmarks/browser/bookmark_codec.h:92: void set_excluded_meta_info_keys(const std::vector<std::string>& keys) { On 2017/05/26 03:07:38, sky wrote: > Wouldn't a set be a better data structure here? set uses around 4 extra pointers for each element present. We only use this container for iteration and not for searching. vector is cheaper. https://codereview.chromium.org/2883523002/diff/100001/components/bookmarks/b... components/bookmarks/browser/bookmark_codec.h:92: void set_excluded_meta_info_keys(const std::vector<std::string>& keys) { On 2017/05/26 03:07:39, sky wrote: > Be consistent: meta_data_keys_. I think meta_info is consistent with DecodeMetaInfo and SetMetaInfo and friends.
On 2017/05/30 21:39:14, ssid wrote: > On 2017/05/26 03:07:39, sky wrote: > > This is indeed what I asked for. Thanks! > > > > One question though. What effects does this have on sync? Lets say we drop > some > > keys, then the user modifies the title. Are the keys erased on the server side > > too? > > > > Thanks for raising this concern. Found this CL that removes these keys. So, it > is safe to remove all keys with "stars." prefix. > https://codereview.chromium.org/163841300 I meant crrev.com/1638413003
In that case, how about we explicitly *always* drop the keys? By that I mean don't bother with the client interface and look for the specific set like you had early on. Sorry for the run around, but I thought the keys were still in use. On Tue, May 30, 2017 at 2:40 PM, <ssid@chromium.org> wrote: > On 2017/05/30 21:39:14, ssid wrote: > > On 2017/05/26 03:07:39, sky wrote: > > > This is indeed what I asked for. Thanks! > > > > > > One question though. What effects does this have on sync? Lets say we > drop > > some > > > keys, then the user modifies the title. Are the keys erased on the > server > side > > > too? > > > > > > > Thanks for raising this concern. Found this CL that removes these keys. > So, it > > is safe to remove all keys with "stars." prefix. > > https://codereview.chromium.org/163841300 > > I meant crrev.com/1638413003 > > > https://codereview.chromium.org/2883523002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
The CQ bit was checked by ssid@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/05/30 23:11:44, sky wrote: > In that case, how about we explicitly *always* drop the keys? By that I > mean don't bother with the client interface and look for the specific set > like you had early on. Sorry for the run around, but I thought the keys > were still in use. > I removed the client interface. I kept the list in bookmark_storage since it can later be used by model.cc also if needed. If you like i can move it to codec.cc.
Description was changed from ========== Reduce the memory usage of bookmarks storage This CL does: 1. Url index uses flat_set instead of set. 2. Do not load image data and page data from bookmarks metadata into memory for low end devices. 3. BookmarkNode stores ptr to icon_url instead of GURL BUG=691698 ========== to ========== Reduce the memory usage of bookmarks storage This CL does: 1. Url index uses flat_set instead of set. 2. Do not load image data and page data from bookmarks metadata into memory these are deprecated. 3. BookmarkNode stores ptr to icon_url instead of GURL BUG=691698 ==========
ssid@chromium.org changed reviewers: + skym@chromium.org
+skym for components/sync_bookmarks/bookmark_change_processor.cc No functional change. Just adapting to the change in api.
lgtm
https://codereview.chromium.org/2883523002/diff/160001/components/bookmarks/b... File components/bookmarks/browser/bookmark_codec.cc (right): https://codereview.chromium.org/2883523002/diff/160001/components/bookmarks/b... components/bookmarks/browser/bookmark_codec.cc:437: for (const auto& excluded : excluded_meta_info_keys_) { Sorry if I wasn't clear. I'm suggesting you harcode the list of bogus values here. https://codereview.chromium.org/2883523002/diff/160001/components/bookmarks/b... components/bookmarks/browser/bookmark_codec.cc:438: if (it.key().find(excluded) != std::string::npos) You don't want a find, you want a starts with, right?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #8 (id:180001) has been deleted
sorry. hardcoded the string in the decode function. ptal https://codereview.chromium.org/2883523002/diff/160001/components/bookmarks/b... File components/bookmarks/browser/bookmark_codec.cc (right): https://codereview.chromium.org/2883523002/diff/160001/components/bookmarks/b... components/bookmarks/browser/bookmark_codec.cc:437: for (const auto& excluded : excluded_meta_info_keys_) { On 2017/05/31 21:29:14, sky wrote: > Sorry if I wasn't clear. I'm suggesting you harcode the list of bogus values > here. No problem. fixed it. Made it a single string. if we want more in future we can make it a list. https://codereview.chromium.org/2883523002/diff/160001/components/bookmarks/b... components/bookmarks/browser/bookmark_codec.cc:438: if (it.key().find(excluded) != std::string::npos) On 2017/05/31 21:29:13, sky wrote: > You don't want a find, you want a starts with, right? Done.
LGTM https://codereview.chromium.org/2883523002/diff/200001/components/bookmarks/b... File components/bookmarks/browser/bookmark_node.h (right): https://codereview.chromium.org/2883523002/diff/200001/components/bookmarks/b... components/bookmarks/browser/bookmark_node.h:137: icon_url_.reset(new GURL(icon_url)); Use MakeUnique (in base/memory/ptr_util.h) (see threads on chormium-dev for the why).
The CQ bit was checked by ssid@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
thanks fixed https://codereview.chromium.org/2883523002/diff/200001/components/bookmarks/b... File components/bookmarks/browser/bookmark_node.h (right): https://codereview.chromium.org/2883523002/diff/200001/components/bookmarks/b... components/bookmarks/browser/bookmark_node.h:137: icon_url_.reset(new GURL(icon_url)); On 2017/05/31 22:35:49, sky wrote: > Use MakeUnique (in base/memory/ptr_util.h) (see threads on chormium-dev for the > why). Done.
The CQ bit was checked by ssid@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from skym@chromium.org, sky@chromium.org Link to the patchset: https://codereview.chromium.org/2883523002/#ps200002 (title: "MakeUnique")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 200002, "attempt_start_ts": 1496280396424800,
"parent_rev": "6dfadefe0e66c5d0f820d33dd1a8b273ad5258ee", "commit_rev":
"144fb7b80b8112e948a83c3fe64d45738b4450df"}
Message was sent while issue was closed.
Description was changed from ========== Reduce the memory usage of bookmarks storage This CL does: 1. Url index uses flat_set instead of set. 2. Do not load image data and page data from bookmarks metadata into memory these are deprecated. 3. BookmarkNode stores ptr to icon_url instead of GURL BUG=691698 ========== to ========== Reduce the memory usage of bookmarks storage This CL does: 1. Url index uses flat_set instead of set. 2. Do not load image data and page data from bookmarks metadata into memory these are deprecated. 3. BookmarkNode stores ptr to icon_url instead of GURL BUG=691698 Review-Url: https://codereview.chromium.org/2883523002 Cr-Commit-Position: refs/heads/master@{#476158} Committed: https://chromium.googlesource.com/chromium/src/+/144fb7b80b8112e948a83c3fe64d... ==========
Message was sent while issue was closed.
Committed patchset #9 (id:200002) as https://chromium.googlesource.com/chromium/src/+/144fb7b80b8112e948a83c3fe64d... |
