third_party/WebKit/Source/modules/bluetooth/DEPS - Issue 2771893002: Move Bluetooth.Web.FunctionCall.Count UMA from browser to WebKit

Side by Side Diff: third_party/WebKit/Source/modules/bluetooth/DEPS

Issue 2771893002: Move Bluetooth.Web.FunctionCall.Count UMA from browser to WebKit (Closed)

Patch Set: move Bluetooth.Web.FunctionCall.Count UMA from browser to WebKit Created 3 years, 9 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

« third_party/WebKit/Source/modules/bluetooth/BluetoothMetrics.h ('K') | « third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTService.cpp ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 include_rules = [	1 include_rules = [

	2 "+base/metrics/histogram_macros.h",
	ortuno 2017/03/24 05:48:04 I'm surprised we are allowed to add this. If I und I'm surprised we are allowed to add this. If I understand correctly we are not allowed to use base in blink. Looking around the modules folder I don't see any other deps that have base in them. There have been many threads in the past whenever someone tried to add a new dependency on base: https://groups.google.com/a/chromium.org/d/msg/blink-dev/pScbOlA-1AE/rZK9Hba1... https://groups.google.com/a/chromium.org/d/msg/blink-dev/rjy64WpOfSc/3ZRA09iD... https://groups.google.com/a/chromium.org/d/msg/blink-dev/klpywegl4Nc/xUrBycH-... This was also the main reason we ended up doing all the histogramming on the browser side. scheib 2017/03/24 19:53:31 However, if we did want to preserve our old UMA va Show quoted text On 2017/03/24 05:48:04, ortuno wrote: > I'm surprised we are allowed to add this. If I understand correctly we are not > allowed to use base in blink. Looking around the modules folder I don't see any > other deps that have base in them. There have been many threads in the past > whenever someone tried to add a new dependency on base: > > https://groups.google.com/a/chromium.org/d/msg/blink-dev/pScbOlA-1AE/rZK9Hba1... > > https://groups.google.com/a/chromium.org/d/msg/blink-dev/rjy64WpOfSc/3ZRA09iD... > > https://groups.google.com/a/chromium.org/d/msg/blink-dev/klpywegl4Nc/xUrBycH-... > > This was also the main reason we ended up doing all the histogramming on the > browser side. However, if we did want to preserve our old UMA values, and use the initial approach of this CL, I think it would be worth a discussion with OWNERS about adding a dep to base/metrics: WebKit/Source/Platform already uses metrics. https://cs.chromium.org/search/?q=f:webkit+f:deps+base%5C/metrics&type=cs Before we did the repo-merge we exposed histograms to WebKit via wrappers. WebKit/Source/platform/Histogram.h has them now, but it also seems to be extra layers we shouldn't use now that we've integrated the codebase. juncai 2017/03/27 19:08:57 I added a post at blink-dev: https://groups.google Show quoted text On 2017/03/24 19:53:31, scheib wrote: > On 2017/03/24 05:48:04, ortuno wrote: > > I'm surprised we are allowed to add this. If I understand correctly we are not > > allowed to use base in blink. Looking around the modules folder I don't see > any > > other deps that have base in them. There have been many threads in the past > > whenever someone tried to add a new dependency on base: > > > > > https://groups.google.com/a/chromium.org/d/msg/blink-dev/pScbOlA-1AE/rZK9Hba1... > > > > > https://groups.google.com/a/chromium.org/d/msg/blink-dev/rjy64WpOfSc/3ZRA09iD... > > > > > https://groups.google.com/a/chromium.org/d/msg/blink-dev/klpywegl4Nc/xUrBycH-... > > > > This was also the main reason we ended up doing all the histogramming on the > > browser side. > > However, if we did want to preserve our old UMA values, and use the initial > approach of this CL, I think it would be worth a discussion with OWNERS about > adding a dep to base/metrics: > > WebKit/Source/Platform already uses metrics. > https://cs.chromium.org/search/?q=f:webkit+f:deps+base%5C/metrics&type=cs > > Before we did the repo-merge we exposed histograms to WebKit via wrappers. > WebKit/Source/platform/Histogram.h has them now, but it also seems to be extra > layers we shouldn't use now that we've integrated the codebase. I added a post at blink-dev: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/qX_3Uavx96U
2 "+bindings",	3 "+bindings",

3 "+core",	4 "+core",

4 "+device/bluetooth/public/interfaces",	5 "+device/bluetooth/public/interfaces",

5 "-modules",	6 "-modules",

6 "+modules/bluetooth",	7 "+modules/bluetooth",

7 "+modules/EventTargetModules.h",	8 "+modules/EventTargetModules.h",

8 "+mojo/public/cpp/bindings",	9 "+mojo/public/cpp/bindings",

9 "-web",	10 "-web",

10 ]	11 ]

OLD	NEW