New issue
Advanced search Search tips

Issue 653721 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Feature



Sign in to add a comment

Add signed-in username to feedback reports for Blimp

Project Member Reported by nyquist@chromium.org, Oct 6 2016

Issue description

It is important to know which account was signed in when feedback reports are filed for Blimp. We should add another field to the feedback data about this.

Key: "Blimp Account"
Value: The currently signed in account, e.g. foo@example.com.

This should be part of the cross platform code in CreateBlimpFeedbackData:
https://cs.chromium.org/chromium/src/blimp/client/core/feedback/blimp_feedback_data.h?dr=CSs&sq=package:chromium&l=21
 
Waiting on if we can send user name on the wire.

Comment 2 by mdw@chromium.org, Oct 14 2016

I thought that Feedback reports included Gaia already, assuming they are going through the standard Google Feedback library? For example, when I look at Feedback reports in Pulse, there is often a Gaia ID there (which is redacted on the report, but presumably in the actual data).



Yes, you are right in that it already sends GAIA. The thing is that we don't know whether that is the signed in account or not. To be honest, I'm not really sure how the Google Feedback system chooses to populate that field, other than it's from the list of available accounts on your device.

This bug aims to add the account you are in fact using Blimp with. That is; the account you are signed in to Chrome with. This means that with this field there would be no possible confusion as to which account this bug report is about for the purposes of Blimp.
CL in review that implements this: https://codereview.chromium.org/2403913003/
Discussed with Chrome privacy about this, and we will add this field for now, but we will remove it later when we get a better engine identifier.

We will also aim to see if it's possible to crash or at least remove the data if the build is stable, beta or dev.

We will aim to remove this field as part of  issue 661360  which is blocking the bug to add a better identifier ( issue 661337 ).
Oh, and for reading the channel, we have to do that in //chrome land. In Java we can do it here:

We can read the data in the //chrome layer here:
https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java

We can pass it through here:
https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/feedback/FeedbackCollector.java?q=feedbackcollector&sq=package:chromium&dr=CSs&l=294

In native code it would be:
https://cs.chromium.org/chromium/src/chrome/common/channel_info.h?q=GetChannel&sq=package:chromium&dr=CSs&l=21

But we initiate the feedback from Java, so I believe the other option is better.
Should we remove all the other user data if build version is not local or canary?

In this way we can just bail out from FeedbackCollector.addBlimpData in java if the build channel check fails.

Another way is passing a bool into native, but it doesn't make sense to send any blimp feedback in those channels.
Well, the important part is to not send the user identifiable data, but I think you're right that the easiest fix for now is to just send nothing on those channels at all, in FeedbackCollector.addBlimpData. So let's go with that for now, since it reduces the need for a lot of temporary plumbing.
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 8 2016

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

commit 23665d9436ba067872f0b2c652dce17404393657
Author: xingliu <xingliu@chromium.org>
Date: Tue Nov 08 22:35:12 2016

Add user name in the feedback data.

Added a map entry for user name in the feedback data. The key is
"Blimp Account", and the value will be Google account name of the current
user, such as someone@gmail.com.

When the user is not logged in, an empty string will be filled in as
the value in the feedback data map.

For early connect, the username can be catched in the feedback data.

BUG= 653721 

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

[modify] https://crrev.com/23665d9436ba067872f0b2c652dce17404393657/blimp/client/core/context/blimp_client_context_impl.cc
[modify] https://crrev.com/23665d9436ba067872f0b2c652dce17404393657/blimp/client/core/feedback/blimp_feedback_data.cc
[modify] https://crrev.com/23665d9436ba067872f0b2c652dce17404393657/blimp/client/core/feedback/blimp_feedback_data.h
[modify] https://crrev.com/23665d9436ba067872f0b2c652dce17404393657/blimp/client/core/feedback/blimp_feedback_data_unittest.cc
[modify] https://crrev.com/23665d9436ba067872f0b2c652dce17404393657/blimp/client/core/session/identity_source.cc
[modify] https://crrev.com/23665d9436ba067872f0b2c652dce17404393657/blimp/client/core/session/identity_source.h
[modify] https://crrev.com/23665d9436ba067872f0b2c652dce17404393657/blimp/client/core/session/identity_source_unittest.cc
[modify] https://crrev.com/23665d9436ba067872f0b2c652dce17404393657/chrome/android/java/src/org/chromium/chrome/browser/feedback/FeedbackCollector.java

Comment 10 by w...@chromium.org, Nov 11 2016

Labels: M-56
Status: Started (was: Untriaged)
Is this work complete?
Status: Fixed (was: Started)
Labels: Archive-Blimp

Sign in to add a comment