Enable content/ code to get SourceId for a web contents |
|||||
Issue descriptionRecently we added ukm::GetSourceIdForWebContentsDocument which allows code above content/ to get a SourceId for a WebContents. Long term our goal is to get as much code as possible to use this, instead of calling UpdateSourceURL. Code in content/ that wants to log UKM, such as https://chromium-review.googlesource.com/c/chromium/src/+/833021, can't invoke ukm::GetSourceIdForWebContentsDocument since it is exposed via components/. We should figure out how to expose a variant of this method to content/ code. Since UKM is intended to be used from content/ it seems reasonable to consider moving parts of components/ukm into content. Alternatively, we could expose an interface to content/ code which delegates to the existing ukm::GetSourceIdForWebContentsDocument method.
,
Dec 25 2017
RE: moving this code to content/, we already have methods like WebContentsImpl::UpdateUrlForUkmSource in content/ so it seems like UKM is already very much a part of content code. Making this change would allow us to remove the call to UpdateSourceURL in the earlier linked CL, as well as in content/browser/plugin_service_impl.cc. I think we could also claim that having just 2 callers is acceptable, but if we intend to allow code in content/ to log UKM we should expect the number of callers in content/ to grow over time.
,
Jan 2 2018
FWIW, //content is allowed to depend on some //components. For example: https://cs.chromium.org/chromium/src/content/browser/DEPS?l=2-22
,
May 18 2018
,
Jun 4 2018
,
Jun 15 2018
,
Jun 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/380413484b2001fd9f6b6c37571f46e6fc0cf6a9 commit 380413484b2001fd9f6b6c37571f46e6fc0cf6a9 Author: Steven Holte <holte@google.com> Date: Fri Jun 22 22:00:57 2018 Add UKM SourceID getter to WebContentsImpl. Bug: 797586 Change-Id: I357e5b945886e51043d662973c9a1960f9b46929 Reviewed-on: https://chromium-review.googlesource.com/1086413 Commit-Queue: Steven Holte <holte@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Min Qin <qinmin@chromium.org> Cr-Commit-Position: refs/heads/master@{#569799} [modify] https://crrev.com/380413484b2001fd9f6b6c37571f46e6fc0cf6a9/components/download/internal/common/download_ukm_helper.cc [modify] https://crrev.com/380413484b2001fd9f6b6c37571f46e6fc0cf6a9/components/download/public/common/download_ukm_helper.h [modify] https://crrev.com/380413484b2001fd9f6b6c37571f46e6fc0cf6a9/content/browser/download/download_resource_handler.cc [modify] https://crrev.com/380413484b2001fd9f6b6c37571f46e6fc0cf6a9/content/browser/download/save_package.cc [modify] https://crrev.com/380413484b2001fd9f6b6c37571f46e6fc0cf6a9/content/browser/loader/cross_site_document_resource_handler.cc [modify] https://crrev.com/380413484b2001fd9f6b6c37571f46e6fc0cf6a9/content/browser/plugin_service_impl.cc [modify] https://crrev.com/380413484b2001fd9f6b6c37571f46e6fc0cf6a9/content/browser/plugin_service_impl_unittest.cc [modify] https://crrev.com/380413484b2001fd9f6b6c37571f46e6fc0cf6a9/content/browser/renderer_host/input/render_widget_host_latency_tracker.cc [modify] https://crrev.com/380413484b2001fd9f6b6c37571f46e6fc0cf6a9/content/browser/renderer_host/input/render_widget_host_latency_tracker.h [modify] https://crrev.com/380413484b2001fd9f6b6c37571f46e6fc0cf6a9/content/browser/renderer_host/input/render_widget_host_latency_tracker_unittest.cc [modify] https://crrev.com/380413484b2001fd9f6b6c37571f46e6fc0cf6a9/content/browser/renderer_host/render_widget_host_delegate.cc [modify] https://crrev.com/380413484b2001fd9f6b6c37571f46e6fc0cf6a9/content/browser/renderer_host/render_widget_host_delegate.h [modify] https://crrev.com/380413484b2001fd9f6b6c37571f46e6fc0cf6a9/content/browser/web_contents/web_contents_impl.cc [modify] https://crrev.com/380413484b2001fd9f6b6c37571f46e6fc0cf6a9/content/browser/web_contents/web_contents_impl.h
,
Jun 22 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by bmcquade@chromium.org
, Dec 25 2017