Optimize sending sys info to the feedback app

Optimize sending sys info to the feedback app

This change optimizes the processing of sys info before sending it
to the feedback app. It shaves off 10ms from that time on peach_pit.

BUG=681279
Review-Url: https://codereview.chromium.org/2635473003
Cr-Commit-Position: refs/heads/master@{#444113}
Committed: https://chromium.googlesource.com/chromium/src/+/701e43f0444b6bd25f7a58acb5ebe87138ffec04

[afakhry, 2017-01-14 03:12:28]

The CQ bit was checked by afakhry@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-14 03:12:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-14 03:54:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-14 03:54:35]

Dry run: This issue passed the CQ dry run.

[afakhry, 2017-01-17 17:54:41]

afakhry@chromium.org changed reviewers:
+ xiyuan@chromium.org

[afakhry, 2017-01-17 17:54:42]

Xiyuan, could you please review this CL? Thanks.

[xiyuan, 2017-01-17 18:38:15]

https://codereview.chromium.org/2635473003/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/feedback_private/feedback_private_api.cc
(right):

https://codereview.chromium.org/2635473003/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/feedback_private/feedback_private_api.cc:235:
&FeedbackPrivateGetSystemInformationFunction::OnCompleted, this));
Is this Bind safe? |service| and |this| are not bound with each other.

https://codereview.chromium.org/2635473003/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/feedback_private/feedback_private_api.cc:245:
SystemInformation sys_info;
Can you rename this or |sys_info| arg of the function? It is a bit confusing to
read when two vars have the same name.

[afakhry, 2017-01-17 18:50:56]

The CQ bit was checked by afakhry@chromium.org to run a CQ dry run

[afakhry, 2017-01-17 18:51:13]

https://codereview.chromium.org/2635473003/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/feedback_private/feedback_private_api.cc
(right):

https://codereview.chromium.org/2635473003/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/feedback_private/feedback_private_api.cc:235:
&FeedbackPrivateGetSystemInformationFunction::OnCompleted, this));
On 2017/01/17 18:38:14, xiyuan wrote:
> Is this Bind safe? |service| and |this| are not bound with each other.

The ExtensionFunctions are RefCountedThreadSafe, so it should be safe.

https://codereview.chromium.org/2635473003/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/feedback_private/feedback_private_api.cc:245:
SystemInformation sys_info;
On 2017/01/17 18:38:14, xiyuan wrote:
> Can you rename this or |sys_info| arg of the function? It is a bit confusing
to
> read when two vars have the same name.

Oops, that was a copy-paste mistake! :)

Done.

[commit-bot: I haz the power, 2017-01-17 18:51:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[xiyuan, 2017-01-17 19:15:11]

lgtm

https://codereview.chromium.org/2635473003/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/feedback_private/feedback_private_api.cc
(right):

https://codereview.chromium.org/2635473003/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/feedback_private/feedback_private_api.cc:235:
&FeedbackPrivateGetSystemInformationFunction::OnCompleted, this));
On 2017/01/17 18:51:13, afakhry wrote:
> On 2017/01/17 18:38:14, xiyuan wrote:
> > Is this Bind safe? |service| and |this| are not bound with each other.
> 
> The ExtensionFunctions are RefCountedThreadSafe, so it should be safe.

Acknowledged. Forgot that. :p

[afakhry, 2017-01-17 19:21:50]

The CQ bit was unchecked by afakhry@chromium.org

[afakhry, 2017-01-17 19:21:51]

The CQ bit was checked by afakhry@chromium.org

[commit-bot: I haz the power, 2017-01-17 19:22:31]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-17 19:47:25]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484680911271260,
"parent_rev": "bb33e323f188305ac1583c6394e0b5dc1b32f356", "commit_rev":
"701e43f0444b6bd25f7a58acb5ebe87138ffec04"}

[commit-bot: I haz the power, 2017-01-17 19:48:32]

Description was changed from

==========
Optimize sending sys info to the feedback app

This change optimizes the processing of sys info before sending it
to the feedback app. It shaves off 10ms from that time on peach_pit.

BUG=681279
==========

to

==========
Optimize sending sys info to the feedback app

This change optimizes the processing of sys info before sending it
to the feedback app. It shaves off 10ms from that time on peach_pit.

BUG=681279

Review-Url: https://codereview.chromium.org/2635473003
Cr-Commit-Position: refs/heads/master@{#444113}
Committed:
https://chromium.googlesource.com/chromium/src/+/701e43f0444b6bd25f7a58acb5eb...
==========

[commit-bot: I haz the power, 2017-01-17 19:48:34]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/701e43f0444b6bd25f7a58acb5eb...