New issue
Advanced search Search tips

Issue 856271 link

Starred by 1 user
Status: Fixed
Owner:
johnidel@chromium.org
Closed: Jul 9
Cc:
csharrison@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

DelayUnsafeAds experiment should only be "activated" if AdTagging is enabled

Project Member Reported by csharrison@chromium.org, Jun 25 2018 
Right now, we attempt to do this by only querying for base::Feature::IsEnabled("DelayUnsafeAds") if "AdTagging" is enabled, but we need to do the same sort of check for variation parameters.

https://cs.chromium.org/chromium/src/components/subresource_filter/content/common/ad_delay_throttle.cc?rcl=da0c64a5292fa2d930ce74f525023b708e6f3dc1&l=113

The benefit here is that since the experiment is useless without ad tagging, those users without ad tagging enabled will not introduce noise to the finch metrics.
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 28 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c507789d78bd54ff98f1e9bb53f080735b13d0a3

commit c507789d78bd54ff98f1e9bb53f080735b13d0a3
Author: John Delaney <johnidel@chromium.org>
Date: Thu Jun 28 12:59:20 2018

Activate DelayUnsafeAds only if AdTagging enabled

The DelayUnsafeAds field trial parameters should only be queried if the client has
AdTagging enabled.

Bug:  856271 
Change-Id: Ie4c19bb735c8b20c2454da21f12c3c7ce7cd4e02
Reviewed-on: https://chromium-review.googlesource.com/1117326
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Commit-Queue: John Delaney <johnidel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571094}
[modify] https://crrev.com/c507789d78bd54ff98f1e9bb53f080735b13d0a3/components/subresource_filter/content/common/ad_delay_throttle.cc
[modify] https://crrev.com/c507789d78bd54ff98f1e9bb53f080735b13d0a3/components/subresource_filter/content/common/ad_delay_throttle.h
[modify] https://crrev.com/c507789d78bd54ff98f1e9bb53f080735b13d0a3/components/subresource_filter/content/common/ad_delay_throttle_unittest.cc

Comment 2 by johnidel@chromium.org, Jul 9

Status: Fixed (was: Assigned)
►

Sign in to add a comment