New issue
Advanced search Search tips

Issue 680752 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

[Remoting Android] Refactor ClientTelemetryLogger

Project Member Reported by yuweih@chromium.org, Jan 12 2017

Issue description

Currently both the log writer and the logger have the lifetime of the whole app, which is bad since logging states within a particular session can leak into the next session if the code has any bug.

While the log writer itself is bound to the lifetime of the app, ClientTelemetryLogger should only have a lifetime of each session.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 23 2017

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

commit 72b3d43cd95b330a4056e06371399af57b07bacd
Author: yuweih <yuweih@chromium.org>
Date: Mon Jan 23 04:19:13 2017

[Remoting Android] Refactor ClientTelemetryLogger

This CL refactors ClientTelemetryLogger such that it is bound to one session
to avoid vulnerability of leaking session state if the logger has any bug.
TelemetryLogWriter will still have the same lifetime as the app.

Also made a ConnectToHostInfo struct to avoid passing 17 arguments to
ChromotingJniInstance.

BUG= 680752 

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

[modify] https://crrev.com/72b3d43cd95b330a4056e06371399af57b07bacd/remoting/android/BUILD.gn
[modify] https://crrev.com/72b3d43cd95b330a4056e06371399af57b07bacd/remoting/base/telemetry_log_writer.cc
[modify] https://crrev.com/72b3d43cd95b330a4056e06371399af57b07bacd/remoting/base/telemetry_log_writer.h
[modify] https://crrev.com/72b3d43cd95b330a4056e06371399af57b07bacd/remoting/client/client_telemetry_logger.cc
[modify] https://crrev.com/72b3d43cd95b330a4056e06371399af57b07bacd/remoting/client/client_telemetry_logger.h
[modify] https://crrev.com/72b3d43cd95b330a4056e06371399af57b07bacd/remoting/client/client_telemetry_logger_unittest.cc
[modify] https://crrev.com/72b3d43cd95b330a4056e06371399af57b07bacd/remoting/client/jni/chromoting_jni_instance.cc
[modify] https://crrev.com/72b3d43cd95b330a4056e06371399af57b07bacd/remoting/client/jni/chromoting_jni_instance.h
[modify] https://crrev.com/72b3d43cd95b330a4056e06371399af57b07bacd/remoting/client/jni/chromoting_jni_runtime.cc
[modify] https://crrev.com/72b3d43cd95b330a4056e06371399af57b07bacd/remoting/client/jni/chromoting_jni_runtime.h
[add] https://crrev.com/72b3d43cd95b330a4056e06371399af57b07bacd/remoting/client/jni/connect_to_host_info.cc
[add] https://crrev.com/72b3d43cd95b330a4056e06371399af57b07bacd/remoting/client/jni/connect_to_host_info.h
[modify] https://crrev.com/72b3d43cd95b330a4056e06371399af57b07bacd/remoting/client/jni/jni_client.cc
[modify] https://crrev.com/72b3d43cd95b330a4056e06371399af57b07bacd/remoting/client/jni/jni_client.h

Comment 2 by yuweih@chromium.org, Jan 23 2017

Status: Fixed (was: Assigned)

Comment 3 by joedow@chromium.org, Jan 27 2017

Labels: -M-57 M-58
Updating milestone since this did not get checked into the M57 branch.

Sign in to add a comment