DCHECK failed when adding a SESSION_ONLY cookie exception |
||||
Issue descriptionChrome Version: HEAD What steps will reproduce the problem? (1) Go to chrome://settings/content/cookies (2) Add a "Clear on exit" exception for e.g. google.com What is the expected result? Chrome should add a SESSION_ONLY exception for google.com. What happens instead? web_site_settings_uma_util fails with a DCHECK because it doesn't know about SESSION_ONLY. [148295:148295:0828/150125.025866:FATAL:web_site_settings_uma_util.cc(28)] Check failed: false. Requested to log permission change 0 to 4 #0 0x7fc4a1bd5637 base::debug::StackTrace::StackTrace() #1 0x7fc4a1bfc271 logging::LogMessage::~LogMessage() #2 0x5602b2b9afb7 WebSiteSettingsUmaUtil::LogPermissionChange() #3 0x5602b2549073 settings::SiteSettingsHandler::HandleSetCategoryPermissionForPattern()
,
Aug 28 2017
,
Aug 28 2017
+emilyschechter@, this seems to be a result of supporting session-only content setting exceptions for cookies. Some time ago, you proposed that we would only support session-only cookies as a global switch. Has that changed? Or was that only on mobile? For the record, I am a fan of keeping the session-only value for exceptions in the MD settings, same way as it has always been in the old WebUI settings, but wanted to clarify with you.
,
Aug 28 2017
I assume my proposal was coming from an Eraser-esque place of removing very little-used exceptions surfaces for greater overall Settings simplicity, and was part of the MD-settings work. In Chrome canary on Mac desktop today, this looks like it's still available, so I'm guessing this was not removed with the MD-settings work. I still would like to remove this in the future but TBH I don't think it's the most important thing to do right now since the MD-settings project has already shipped.
,
Sep 1 2017
This should be super quick to fix.
,
Sep 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/40189823856fc9d5372ea5dbd82b6b2cc5eeebea commit 40189823856fc9d5372ea5dbd82b6b2cc5eeebea Author: Timothy Loh <timloh@chromium.org> Date: Thu Sep 14 08:28:00 2017 Fix DCHECK when adding session-only cookie exception Currently adding a session-only cookie exception hits a DCHECK in the UMA logging code, where we only expect actions add/block/reset. This patch adds handling for the session-only case and a corresponding histogram table. Note the table is technically redundant as the values can be calculated from (all - add - block - reset), but it seems nicer to consistently log actions here. TBR=raymes Bug: 759544 Change-Id: I2361a540924cb6eef0c9817375468f6da2010d88 Reviewed-on: https://chromium-review.googlesource.com/657792 Commit-Queue: Timothy Loh <timloh@chromium.org> Reviewed-by: Dave Schuyler <dschuyler@chromium.org> Reviewed-by: Ilya Sherman <isherman@chromium.org> Reviewed-by: Patti <patricialor@chromium.org> Cr-Commit-Position: refs/heads/master@{#501900} [modify] https://crrev.com/40189823856fc9d5372ea5dbd82b6b2cc5eeebea/chrome/browser/content_settings/web_site_settings_uma_util.cc [modify] https://crrev.com/40189823856fc9d5372ea5dbd82b6b2cc5eeebea/chrome/browser/ui/webui/settings/site_settings_handler.h [modify] https://crrev.com/40189823856fc9d5372ea5dbd82b6b2cc5eeebea/chrome/browser/ui/webui/settings/site_settings_handler_unittest.cc [modify] https://crrev.com/40189823856fc9d5372ea5dbd82b6b2cc5eeebea/tools/metrics/histograms/histograms.xml
,
Sep 15 2017
|
||||
►
Sign in to add a comment |
||||
Comment 1 by dullweber@chromium.org
, Aug 28 2017