Refactoring ConstructLanguageDetectionEvent.

Refactoring ConstructLanguageDetectionEvent.

Refactoring the code so it can also be used by the iOS client.
Also add a unit test.


BUG=722679
Review-Url: https://codereview.chromium.org/2912843003
Cr-Commit-Position: refs/heads/master@{#476135}
Committed: https://chromium.googlesource.com/chromium/src/+/f1e6a615e165c9ad4e33b4f50dd23e9ee9fab5be

[renjieliu1, 2017-05-30 08:33:32]

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

[commit-bot: I haz the power, 2017-05-30 08:33:43]

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-05-30 09:59:24]

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

[commit-bot: I haz the power, 2017-05-30 09:59:25]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[renjieliu1, 2017-05-30 23:56:23]

Description was changed from

==========
Refactoring ConstructLanguageDetectionEvent.

Refactoring the code so it can also be used by the iOS client.
Also add a unit test.


BUG=722679
==========

to

==========
Refactoring ConstructLanguageDetectionEvent.

Refactoring the code so it can also be used by the iOS client.
Also add a unit test.


BUG=722679
==========

[renjieliu1, 2017-05-30 23:56:24]

renjieliu@chromium.org changed reviewers:
+ napper@chromium.org

[renjieliu1, 2017-05-30 23:57:38]

renjieliu@chromium.org changed reviewers:
+ amoylan@chromium.org

[napper, 2017-05-31 05:00:06]

https://codereview.chromium.org/2912843003/diff/1/components/translate/core/c...
File
components/translate/core/common/language_detection_logging_helper_unittest.cc
(right):

https://codereview.chromium.org/2912843003/diff/1/components/translate/core/c...
components/translate/core/common/language_detection_logging_helper_unittest.cc:16:
TEST_F(LanguageDetectionLoggingHelperTest, ConstructUserEventSpecifics) {
Why not just use TEST()?

https://codereview.chromium.org/2912843003/diff/1/components/translate/core/c...
components/translate/core/common/language_detection_logging_helper_unittest.cc:27:
std::unique_ptr<sync_pb::UserEventSpecifics> user_event =
const

https://codereview.chromium.org/2912843003/diff/1/components/translate/core/c...
components/translate/core/common/language_detection_logging_helper_unittest.cc:45:
std::unique_ptr<sync_pb::UserEventSpecifics> user_event =
const

[renjieliu1, 2017-05-31 05:12:00]

https://codereview.chromium.org/2912843003/diff/1/components/translate/core/c...
File
components/translate/core/common/language_detection_logging_helper_unittest.cc
(right):

https://codereview.chromium.org/2912843003/diff/1/components/translate/core/c...
components/translate/core/common/language_detection_logging_helper_unittest.cc:16:
TEST_F(LanguageDetectionLoggingHelperTest, ConstructUserEventSpecifics) {
On 2017/05/31 05:00:06, napper wrote:
> Why not just use TEST()?

Done.

https://codereview.chromium.org/2912843003/diff/1/components/translate/core/c...
components/translate/core/common/language_detection_logging_helper_unittest.cc:27:
std::unique_ptr<sync_pb::UserEventSpecifics> user_event =
On 2017/05/31 05:00:06, napper wrote:
> const

Done.

https://codereview.chromium.org/2912843003/diff/1/components/translate/core/c...
components/translate/core/common/language_detection_logging_helper_unittest.cc:45:
std::unique_ptr<sync_pb::UserEventSpecifics> user_event =
On 2017/05/31 05:00:06, napper wrote:
> const

Done.

[napper, 2017-05-31 06:53:08]

https://codereview.chromium.org/2912843003/diff/20001/components/translate/co...
File
components/translate/core/common/language_detection_logging_helper_unittest.cc
(right):

https://codereview.chromium.org/2912843003/diff/20001/components/translate/co...
components/translate/core/common/language_detection_logging_helper_unittest.cc:13:
typedef testing::Test LanguageDetectionLoggingHelperTest;
I don't think you need this

https://codereview.chromium.org/2912843003/diff/20001/components/translate/co...
components/translate/core/common/language_detection_logging_helper_unittest.cc:13:
typedef testing::Test LanguageDetectionLoggingHelperTest;
I don't think you need this

https://codereview.chromium.org/2912843003/diff/20001/components/translate/co...
components/translate/core/common/language_detection_logging_helper_unittest.cc:16:
TEST(LanguageDetectionLoggingHelperTest, ConstructUserEventSpecifics) {
Can these be in namespace translate {?

[renjieliu1, 2017-05-31 07:02:07]

https://codereview.chromium.org/2912843003/diff/20001/components/translate/co...
File
components/translate/core/common/language_detection_logging_helper_unittest.cc
(right):

https://codereview.chromium.org/2912843003/diff/20001/components/translate/co...
components/translate/core/common/language_detection_logging_helper_unittest.cc:13:
typedef testing::Test LanguageDetectionLoggingHelperTest;
On 2017/05/31 06:53:08, napper wrote:
> I don't think you need this

Done.

https://codereview.chromium.org/2912843003/diff/20001/components/translate/co...
components/translate/core/common/language_detection_logging_helper_unittest.cc:13:
typedef testing::Test LanguageDetectionLoggingHelperTest;
On 2017/05/31 06:53:07, napper wrote:
> I don't think you need this

you're right, thanks!

https://codereview.chromium.org/2912843003/diff/20001/components/translate/co...
components/translate/core/common/language_detection_logging_helper_unittest.cc:16:
TEST(LanguageDetectionLoggingHelperTest, ConstructUserEventSpecifics) {
On 2017/05/31 06:53:07, napper wrote:
> Can these be in namespace translate {?

Done.

[napper, 2017-05-31 07:06:24]

lgtm

[renjieliu1, 2017-05-31 07:08:52]

renjieliu@chromium.org changed reviewers:
+ groby@chromium.org

[renjieliu1, 2017-05-31 07:08:53]

Hi Rachel,

I'm refactoring the ConstructLanguageDetectionEvent so the code can be shared
with iOS client implementation. PTAL.

Thanks a lot! :D

[groby-ooo-7-16, 2017-05-31 20:01:46]

RS LGTM

[renjieliu1, 2017-05-31 23:43:04]

The CQ bit was checked by renjieliu@chromium.org

[commit-bot: I haz the power, 2017-05-31 23:43:55]

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-05-31 23:54:13]

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

[commit-bot: I haz the power, 2017-05-31 23:54:15]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[renjieliu1, 2017-05-31 23:56:26]

renjieliu@chromium.org changed reviewers:
+ skym@chromium.org

[renjieliu1, 2017-05-31 23:57:44]

Hi Sky,

Can you take a quick review on the cl as this cl adds
"components/sync/protocol" as dependency.

Thanks a lot!

[skym, 2017-06-01 00:00:10]

lgtm

[renjieliu1, 2017-06-01 00:02:05]

The CQ bit was checked by renjieliu@chromium.org

[commit-bot: I haz the power, 2017-06-01 00:03:57]

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-06-01 01:36:01]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496275325762250,
"parent_rev": "4c218107068b3358f3c65c729b412fdd4dc2cc45", "commit_rev":
"f1e6a615e165c9ad4e33b4f50dd23e9ee9fab5be"}

[commit-bot: I haz the power, 2017-06-01 01:36:10]

Description was changed from

==========
Refactoring ConstructLanguageDetectionEvent.

Refactoring the code so it can also be used by the iOS client.
Also add a unit test.


BUG=722679
==========

to

==========
Refactoring ConstructLanguageDetectionEvent.

Refactoring the code so it can also be used by the iOS client.
Also add a unit test.


BUG=722679

Review-Url: https://codereview.chromium.org/2912843003
Cr-Commit-Position: refs/heads/master@{#476135}
Committed:
https://chromium.googlesource.com/chromium/src/+/f1e6a615e165c9ad4e33b4f50dd2...
==========

[commit-bot: I haz the power, 2017-06-01 01:36:11]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/f1e6a615e165c9ad4e33b4f50dd2...