Add signed-in username to feedback reports for Blimp |
||||
Issue descriptionIt 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
,
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).
,
Oct 28 2016
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.
,
Nov 3 2016
CL in review that implements this: https://codereview.chromium.org/2403913003/
,
Nov 4 2016
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 ).
,
Nov 4 2016
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.
,
Nov 4 2016
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.
,
Nov 7 2016
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.
,
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
,
Nov 11 2016
Is this work complete?
,
Nov 14 2016
,
Dec 9 2016
|
||||
►
Sign in to add a comment |
||||
Comment 1 by xingliu@chromium.org
, Oct 14 2016