New issue
Advanced search Search tips

Issue 699381 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

SiteEngagementService::ResetScoreForURL is confusingly named

Project Member Reported by dominickn@chromium.org, Mar 8 2017

Issue description

This method resets the *base score*, not including any bonus. For instance, if you try resetting the score to 50, but you have a bonus from having a web app installed, the actual score you'll get back later will be higher than 50.

We should rename the method to make this clearer.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 10 2017

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

commit e062cff433a3c2ee02cb9e45d480093a7b958272
Author: dominickn <dominickn@chromium.org>
Date: Fri Mar 10 00:14:52 2017

Rename SiteEngagementService::ResetScoreForURL.

This method only resets the base score for a URL. It does not affect the
bonus score that an origin might receive, e.g. from being installed to
the user's desktop or homescreen. This means that getting the score of
an origin immediately after resetting it might return an unexpected
number.

This CL renames the method and adds a comment explicitly calling out the
potential difference.

BUG= 699381 
TBR=bauerb@chromium.org
TBR=dbeam@chromium.org

Review-Url: https://codereview.chromium.org/2735333002
Cr-Commit-Position: refs/heads/master@{#455918}

[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/android/java/src/org/chromium/chrome/browser/engagement/SiteEngagementService.java
[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java
[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/android/javatests/src/org/chromium/chrome/browser/engagement/SiteEngagementServiceTest.java
[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/browser/banners/app_banner_manager_browsertest.cc
[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/browser/banners/app_banner_settings_helper_unittest.cc
[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/browser/budget_service/budget_database_unittest.cc
[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/browser/budget_service/budget_manager_browsertest.cc
[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/browser/budget_service/budget_manager_unittest.cc
[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/browser/engagement/important_sites_util.cc
[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/browser/engagement/important_sites_util_unittest.cc
[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/browser/engagement/site_engagement_service.cc
[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/browser/engagement/site_engagement_service.h
[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/browser/engagement/site_engagement_service_android.cc
[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/browser/engagement/site_engagement_service_android.h
[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/browser/engagement/site_engagement_service_unittest.cc
[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/browser/plugins/chrome_plugin_service_filter_unittest.cc
[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/browser/push_messaging/push_messaging_browsertest.cc
[modify] https://crrev.com/e062cff433a3c2ee02cb9e45d480093a7b958272/chrome/browser/ui/webui/engagement/site_engagement_ui.cc

Status: Fixed (was: Started)

Sign in to add a comment