Include a timestamp with and add a keyboard shortcut for Feedback.

Include a timestamp with and add a keyboard shortcut for Feedback.
Everytime a user clicks on the Feedback menu item or presses ctrl+alt+i on ChromeOS, record the exact time of that happening and send it in the feedback report the user submits.


R=zelidrag@chromium.org,adlr@chromium.org
BUG=chromium-os:25678
TEST=Tested with sending several reports invoked both by the shortcut and the menu item.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=119971

[zel, 2012-01-31 16:38:03]

https://chromiumcodereview.appspot.com/9104030/diff/8/chrome/browser/feedback...
File chrome/browser/feedback/feedback_data.h (right):

https://chromiumcodereview.appspot.com/9104030/diff/8/chrome/browser/feedback...
chrome/browser/feedback/feedback_data.h:37: , const std::string& timestamp
string? why don't you pass this as base::Time and do the formatting when you
feed the list?

https://chromiumcodereview.appspot.com/9104030/diff/8/chrome/browser/ui/webui...
File chrome/browser/ui/webui/feedback_ui.cc (right):

https://chromiumcodereview.appspot.com/9104030/diff/8/chrome/browser/ui/webui...
chrome/browser/ui/webui/feedback_ui.cc:145: std::string timestamp =
base::DoubleToString(now.ToDoubleT());
format this to string where you really need the string, pass it as base::Time in
all other places

[rkc, 2012-01-31 18:16:45]

https://chromiumcodereview.appspot.com/9104030/diff/8/chrome/browser/feedback...
File chrome/browser/feedback/feedback_data.h (right):

https://chromiumcodereview.appspot.com/9104030/diff/8/chrome/browser/feedback...
chrome/browser/feedback/feedback_data.h:37: , const std::string& timestamp
On 2012/01/31 16:38:03, zel wrote:
> string? why don't you pass this as base::Time and do the formatting when you
> feed the list?

The reason we are converting the time to a string and passing it as such is to
avoid a conversion back and forth - we need to convert it to a string to pass it
via the URL query params, after which we'd need to convert it back, which seems
unnecessary since we never use it for anything except for storing it.

https://chromiumcodereview.appspot.com/9104030/diff/8/chrome/browser/ui/webui...
File chrome/browser/ui/webui/feedback_ui.cc (right):

https://chromiumcodereview.appspot.com/9104030/diff/8/chrome/browser/ui/webui...
chrome/browser/ui/webui/feedback_ui.cc:145: std::string timestamp =
base::DoubleToString(now.ToDoubleT());
On 2012/01/31 16:38:03, zel wrote:
> format this to string where you really need the string, pass it as base::Time
in
> all other places

Same reason as given in the other comment.

[zel, 2012-01-31 18:26:17]

lgtm

[sky, 2012-01-31 18:57:59]

browser/ui/viewss LGTM

[rkc, 2012-01-31 19:00:17]

Added reviewers for browser/ui/webui/

[arv (Not doing code reviews), 2012-01-31 19:02:24]

LGTM

rubber stamp

[commit-bot: I haz the power, 2012-01-31 19:09:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rkc@chromium.org/9104030/8

[commit-bot: I haz the power, 2012-01-31 22:15:44]

Change committed as 119971