[PageLoadMetrics] Use URL rather than origin to get/set website settings. [chromium/src : main]
Christoph Schwering (Gerrit)
Attention is currently required from: Christian Dullweber.
2 comments:
Patchset:
Hi Christian, thanks for your suggestion on the bug. PTAL
File chrome/browser/page_load_metrics/observers/formfill_page_load_metrics_observer.cc:
Just to double check: you suggested *not* to strip path etc, because that's done at a later point anyway, correct?
To view, visit change 4055524. To unsubscribe, or for help writing mail filters, visit settings.
Christian Dullweber (Gerrit)
Attention is currently required from: Christoph Schwering.
Patch set 1:Code-Review +1
2 comments:
Patchset:
lgtm
File chrome/browser/page_load_metrics/observers/formfill_page_load_metrics_observer.cc:
Just to double check: you suggested *not* to strip path etc, because that's done at a later point an […]
Yes, this is correct. SetWebsiteSettingDefaultScope automatically applies the "default scope". Which is "origin" for your setting.
GetWebsiteSetting should always get the full URL. It will then match the setting that matches the URL the closest, which will be your origin setting.
To view, visit change 4055524. To unsubscribe, or for help writing mail filters, visit settings.
Christoph Schwering (Gerrit)
Attention is currently required from: Nicolás Peña.
2 comments:
Patchset:
Nicolas, could you take a look? This is to prevent hitting a DCHECK in GetPatternsForContentSettingsType()
File chrome/browser/page_load_metrics/observers/formfill_page_load_metrics_observer.cc:
Yes, this is correct. SetWebsiteSettingDefaultScope automatically applies the "default scope". […]
Thanks!
To view, visit change 4055524. To unsubscribe, or for help writing mail filters, visit settings.
Nicolás Peña (Gerrit)
To view, visit change 4055524. To unsubscribe, or for help writing mail filters, visit settings.
Christoph Schwering (Gerrit)
Attention is currently required from: Nicolás Peña.
1 comment:
Patchset:
Nicalas, I lost your +1 after enabling the test that was disabled due to this bug.
To view, visit change 4055524. To unsubscribe, or for help writing mail filters, visit settings.
Nicolás Peña (Gerrit)
Attention is currently required from: Christoph Schwering.
Patch set 2:Code-Review +1
Christoph Schwering (Gerrit)
Patch set 2:Commit-Queue +2
1 comment:
Patchset:
THanks!
To view, visit change 4055524. To unsubscribe, or for help writing mail filters, visit settings.
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
[PageLoadMetrics] Use URL rather than origin to get/set website settings.
Rather than passing the origin, which may be null due to opaque origins,
this CL passes the last-committed URL to GetWebsiteSetting() and
SetWebsiteSettingDefaultScope().
Bug: 1392914
Change-Id: Iaf5d86b7adf9f461b700d8b1806a942b1a4569ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4055524
Reviewed-by: Nicolás Peña <n...@chromium.org>
Commit-Queue: Christoph Schwering <schw...@google.com>
Cr-Commit-Position: refs/heads/main@{#1076246}
---
M chrome/browser/autofill/autofill_across_iframes_browsertest.cc
M chrome/browser/page_load_metrics/observers/formfill_page_load_metrics_observer.cc
2 files changed, 26 insertions(+), 12 deletions(-)