Issue metadata
Sign in to add a comment
|
Migrate ImportantSites to use SiteEngagementService::GetAllDetails() |
||||||||||||||||||||||||
Issue descriptionSince SiteEngagementService fetches a lot more information and exposes it in GetAllDetails(), then we can remove a bunch of redundant fetching and code in ImportantSitesUtil. Plan from Dominick on https://bugs.chromium.org/p/chromium/issues/detail?id=703848: I think a good plan is to: * move the site_engagement.mojom from the ui/ folder to the engagement/ folder * modify the SiteEngagementInfo struct to contain separate doubles for base, notification, installed, and total scores * introduce a new method, SiteEngagementService::GetScores, which returns a vector of SiteEngagementInfo structs * use that new method in important sites and the site engagement web UI. * make the site engagement web UI have separate columns for the separate components of the score Some of this is outdated, but generally accurate still.
,
Apr 12 2017
,
Jul 24 2017
,
Jul 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0971cff34a819b60922470a7b290885839e9ac68 commit 0971cff34a819b60922470a7b290885839e9ac68 Author: Dominick Ng <dominickn@chromium.org> Date: Wed Jul 26 00:25:54 2017 Remove the SiteEngagementService::GetScoreMap API. This CL migrates clients of SiteEngagementService::GetScoreMap to SiteEngagementService::GetAllDetails. The biggest client is Important Sites, which can now use the EngagamentDetails struct directly to determine if a site has notifications enabled. BUG= 711041 Change-Id: I6297f9efce445ad80724f461a664b37949578f7e Reviewed-on: https://chromium-review.googlesource.com/582729 Reviewed-by: Ben Wells <benwells@chromium.org> Reviewed-by: Daniel Murphy <dmurph@chromium.org> Commit-Queue: Dominick Ng <dominickn@chromium.org> Cr-Commit-Position: refs/heads/master@{#489495} [modify] https://crrev.com/0971cff34a819b60922470a7b290885839e9ac68/chrome/browser/engagement/important_sites_util.cc [modify] https://crrev.com/0971cff34a819b60922470a7b290885839e9ac68/chrome/browser/engagement/site_engagement_metrics.cc [modify] https://crrev.com/0971cff34a819b60922470a7b290885839e9ac68/chrome/browser/engagement/site_engagement_metrics.h [modify] https://crrev.com/0971cff34a819b60922470a7b290885839e9ac68/chrome/browser/engagement/site_engagement_service.cc [modify] https://crrev.com/0971cff34a819b60922470a7b290885839e9ac68/chrome/browser/engagement/site_engagement_service.h [modify] https://crrev.com/0971cff34a819b60922470a7b290885839e9ac68/chrome/browser/engagement/site_engagement_service_unittest.cc
,
Nov 10 2017
,
Nov 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ea772d325f0d5bcf73ff8041ce9095c95091f41f commit ea772d325f0d5bcf73ff8041ce9095c95091f41f Author: Dominick Ng <dominickn@chromium.org> Date: Tue Nov 14 00:23:13 2017 Use the site engagement details to determine installed state for Important Sites. The EngagementDetails struct allows consumers to determine if a particular origin is added to home screen. The querying is equivalent to that implemented in Important Sites - both use a 10 day recency cutoff. All recently launched sites are recorded by the engagement service at the exact same time as the content setting used by the Important Sites code. This CL removes the custom querying for installed state in Important Sites and simply checks the EngagementDetails struct. BUG= 711041 Change-Id: I7747f0fbd08e6912cc5966f3ced678b717114547 Reviewed-on: https://chromium-review.googlesource.com/765555 Reviewed-by: Daniel Murphy <dmurph@chromium.org> Commit-Queue: Dominick Ng <dominickn@chromium.org> Cr-Commit-Position: refs/heads/master@{#516111} [modify] https://crrev.com/ea772d325f0d5bcf73ff8041ce9095c95091f41f/chrome/browser/engagement/important_sites_util.cc
,
Nov 14 2017
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by dmu...@chromium.org
, Apr 12 2017