New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 754489 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Remove ChromeosMetricsProvider::WriteExternalTouchscreensProto() and related protos

Project Member Reported by sky@chromium.org, Aug 10 2017

Issue description

The implementation of this function is X11 specific. As ChromeOS hasn't shipped an X11 board in a while WriteExternalTouchscreensProto() is a no-op and no longer updates the proto SystemProfileProto::Hardware::TouchScreen. We could either update this for Ozone, or given no one seems to have noticed remove the proto entries.

I'm not sure who can make the call if we care about this metrics anymore. Zel?
 
Owner: marc...@chromium.org
marcheu@, this seems like a left over from our Feon project, can you please route this to appropriate dev
Cc: spang@chromium.org
Labels: -Pri-3 Pri-2
Owner: sky@chromium.org
Yeah I'd just remove it unless Spang thinks we need it.

Comment 3 by spang@chromium.org, Aug 11 2017

It's not hard to implement but I'd lean toward removing it given that nobody noticed it went missing.
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 11 2017

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

commit 84f5db292dcb8dfe926c64dbf4cda4496b1691d3
Author: Scott Violet <sky@chromium.org>
Date: Fri Aug 11 15:05:02 2017

chromeos: remove dead metrics code

This code was updating some protos using X11 specific code that no
longer works. I'm removing the code and in its place putting
NOTIMPLEMENTED(). I filed a bug to decide if we should nuke the protos
and the code entirely, or wire up to InputDeviceManager.

BUG= 754489 
TEST=none
R=kylechar@chromium.org

Change-Id: I883d6e58ac330947cad5ef28398a06e59e4774fe
Reviewed-on: https://chromium-review.googlesource.com/611380
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493742}
[modify] https://crrev.com/84f5db292dcb8dfe926c64dbf4cda4496b1691d3/chrome/browser/metrics/chromeos_metrics_provider.cc
[modify] https://crrev.com/84f5db292dcb8dfe926c64dbf4cda4496b1691d3/chrome/browser/metrics/chromeos_metrics_provider_unittest.cc

Comment 5 by sky@chromium.org, Aug 11 2017

Status: Started (was: Untriaged)
Summary: Remove ChromeosMetricsProvider::WriteExternalTouchscreensProto() and related protos (was: ChromeosMetricsProvider::WriteExternalTouchscreensProto() needs to be implemented for ozone)
Ok. I will remove it. I've updated the description to reflect what should happen.
Components: Internals>Metrics
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 21 2017

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

commit b3fa9de520fff1ed6322ff74b15e80858a67bc69
Author: Scott Violet <sky@chromium.org>
Date: Mon Aug 21 19:15:36 2017

update sysm_profile.proto to reflect deprecated fields

This landed in the g3 side, so pulling into Chrome. Also removing the
NOTIMPLEMENTED() now that we decided we're going to deprecate.

BUG= 754489 
TEST=none

Change-Id: I9bf67257de2d3177582743db548a5c517c1f4b25
Reviewed-on: https://chromium-review.googlesource.com/622242
Commit-Queue: Alexei Svitkine (very slow) <asvitkine@chromium.org>
Reviewed-by: Alexei Svitkine (very slow) <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496015}
[modify] https://crrev.com/b3fa9de520fff1ed6322ff74b15e80858a67bc69/chrome/browser/metrics/chromeos_metrics_provider.cc
[modify] https://crrev.com/b3fa9de520fff1ed6322ff74b15e80858a67bc69/components/metrics/proto/system_profile.proto

Comment 8 by sky@chromium.org, Aug 21 2017

Status: Fixed (was: Started)

Comment 9 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment