Automatically trigger Never-Translate in new translation infobar

Automatically trigger Never-Translate in new translation infobar
In the new translation infobar, when the number of consecutive
translation denied reaches a threshold, it will trigger 'Never-Translate
this language' automatically.  This CL is trying to implement this
feature without altering the behavior of the existing infobars.

A translation denied is counted when user click on the close button
without ever translated or clicking on any menu item.

BUG=703887
TBR=groby@chromium.org

Review-Url: https://codereview.chromium.org/2874393002
Cr-Commit-Position: refs/heads/master@{#472011}
Committed: https://chromium.googlesource.com/chromium/src/+/cd21f782ca83751c89fc9ebc0ef233e736ff9c88

[Marti Wong, 2017-05-12 04:53:12]

martiw@chromium.org changed reviewers:
+ groby@chromium.org, mdjones@chromium.org

[Marti Wong, 2017-05-12 04:53:13]

PTAL thanks~!

[Leo, 2017-05-12 05:09:06]

googleo@chromium.org changed reviewers:
+ googleo@chromium.org

[Leo, 2017-05-12 05:09:07]

Logic in the TranslateCompactInfoBar is not easily understandable.

https://codereview.chromium.org/2874393002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java
(right):

https://codereview.chromium.org/2874393002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:202:
if (!mUserInteracted && !mTriggeredAutoNeverByCloseButton) {
Logic here is unclear and not straightforward. Please refactor the code here to
make it understandable.

https://codereview.chromium.org/2874393002/diff/1/components/translate/core/b...
File components/translate/core/browser/translate_infobar_delegate.cc (right):

https://codereview.chromium.org/2874393002/diff/1/components/translate/core/b...
components/translate/core/browser/translate_infobar_delegate.cc:314: bool
TranslateInfoBarDelegate::ShouldAutoNeverTranslate() {
Can we move all the CompactUIEnabled logic to
ui/android/infobars/translate_compact_infobar.cc

[Marti Wong, 2017-05-12 07:16:11]

PTAL again. thx

https://codereview.chromium.org/2874393002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java
(right):

https://codereview.chromium.org/2874393002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:202:
if (!mUserInteracted && !mTriggeredAutoNeverByCloseButton) {
On 2017/05/12 05:09:07, Leo wrote:
> Logic here is unclear and not straightforward. Please refactor the code here
to
> make it understandable.

Done some refactoring. see if it's good now.  thx

https://codereview.chromium.org/2874393002/diff/1/components/translate/core/b...
File components/translate/core/browser/translate_infobar_delegate.cc (right):

https://codereview.chromium.org/2874393002/diff/1/components/translate/core/b...
components/translate/core/browser/translate_infobar_delegate.cc:314: bool
TranslateInfoBarDelegate::ShouldAutoNeverTranslate() {
On 2017/05/12 05:09:07, Leo wrote:
> Can we move all the CompactUIEnabled logic to
> ui/android/infobars/translate_compact_infobar.cc

Done.

[Marti Wong, 2017-05-12 07:27:56]

https://codereview.chromium.org/2874393002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java
(right):

https://codereview.chromium.org/2874393002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:40:
public static final int ACTION_AUTO_NEVER_LANGUAGE = 4;
Created Action IDs so that snackbar will know if the action is caused by
overflow menu or by auto-behavior.  This allows me to remove the flag
mTriggeredAutoNeverByCloseButton in my last patch.  And also, these action are
needed when we need to separately track auto-always, auto-never from
manual-always, manual-never later.

https://codereview.chromium.org/2874393002/diff/20001/chrome/browser/ui/andro...
File chrome/browser/ui/android/infobars/translate_compact_infobar.h (right):

https://codereview.chromium.org/2874393002/diff/20001/chrome/browser/ui/andro...
chrome/browser/ui/android/infobars/translate_compact_infobar.h:69: 
Rachel, I moved the logic back to translate_compact_infobar.
Sorry for the back and forth.  But putting compact UI related stuff together now
could make the future refactoring easier to follow.

[mdjones, 2017-05-12 17:12:01]

lgtm % punctuation nits

https://codereview.chromium.org/2874393002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java
(right):

https://codereview.chromium.org/2874393002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:38:
// Actions triggered automatically (when translation or denied count reaches the
threshold)
nit: punctuation.

https://codereview.chromium.org/2874393002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:74:
// the infobar even user cancels the "Never Translate".
nit: "even if the user"

https://codereview.chromium.org/2874393002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:337:
// after applying this option, the infobar will dismiss
nit: "after"->"After" and punctuation. Same for below.

[groby-ooo-7-16, 2017-05-12 17:46:45]

I'm not sure why we need to have helpers that just forward to prefs?

https://codereview.chromium.org/2874393002/diff/40001/components/translate/co...
File components/translate/core/browser/translate_infobar_delegate.h (right):

https://codereview.chromium.org/2874393002/diff/40001/components/translate/co...
components/translate/core/browser/translate_infobar_delegate.h:155: int
GetTranslationAcceptedCount();
These are both already available via TranslatePrefs?

[Marti Wong, 2017-05-14 04:30:47]

Thank Matthew and Rachel~!
PTAL

https://codereview.chromium.org/2874393002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java
(right):

https://codereview.chromium.org/2874393002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:38:
// Actions triggered automatically (when translation or denied count reaches the
threshold)
On 2017/05/12 17:12:01, mdjones wrote:
> nit: punctuation.

Done.

https://codereview.chromium.org/2874393002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:74:
// the infobar even user cancels the "Never Translate".
On 2017/05/12 17:12:00, mdjones wrote:
> nit: "even if the user"

Done.

https://codereview.chromium.org/2874393002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:337:
// after applying this option, the infobar will dismiss
On 2017/05/12 17:12:00, mdjones wrote:
> nit: "after"->"After" and punctuation. Same for below.

Done.

https://codereview.chromium.org/2874393002/diff/40001/components/translate/co...
File components/translate/core/browser/translate_infobar_delegate.h (right):

https://codereview.chromium.org/2874393002/diff/40001/components/translate/co...
components/translate/core/browser/translate_infobar_delegate.h:155: int
GetTranslationAcceptedCount();
On 2017/05/12 17:46:45, groby wrote:
> These are both already available via TranslatePrefs?

Sorry. These 2 lines should be removed and I forgot to do so.
Those functions were moved to translate_compact_infobar.cc already.
My bad.

[groby1, 2017-05-15 20:22:24]

groby@google.com changed reviewers:
+ groby@google.com

[groby1, 2017-05-15 20:22:24]

lgtm

[Marti Wong, 2017-05-16 00:31:26]

The CQ bit was checked by martiw@chromium.org

[Marti Wong, 2017-05-16 00:31:27]

The patchset sent to the CQ was uploaded after l-g-t-m from mdjones@chromium.org
Link to the patchset: https://codereview.chromium.org/2874393002/#ps80001
(title: "sync and merge")

[commit-bot: I haz the power, 2017-05-16 00:33:33]

CQ is trying da patch.

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

[Marti Wong, 2017-05-16 00:33:40]

On 2017/05/15 20:22:24, groby1 wrote:
> lgtm

Thanks Rachel and Matthew!  Committing.

[commit-bot: I haz the power, 2017-05-16 01:19:19]

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

[commit-bot: I haz the power, 2017-05-16 01:19:20]

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...)

[Marti Wong, 2017-05-16 03:51:15]

On 2017/05/16 01:19:20, commit-bot: I haz the power wrote:
> 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...)

Unable to commit due to missing LGTM from an OWNER for these files:
    components/translate/core/browser/translate_infobar_delegate.cc
    components/translate/core/browser/translate_infobar_delegate.h

Hi Rachel, I guess I need your chromium account's LGTM (instead of google
account's).
Thanks~!

[Marti Wong, 2017-05-16 03:53:33]

Description was changed from

==========
Automatically trigger Never-Translate in new translation infobar
In the new translation infobar, when the number of consecutive
translation denied reaches a threshold, it will trigger 'Never-Translate
this language' automatically.  This CL is trying to implement this
feature without altering the behavior of the existing infobars.

A translation denied is counted when user click on the close button
without ever translated or clicking on any menu item.

BUG=703887
==========

to

==========
Automatically trigger Never-Translate in new translation infobar
In the new translation infobar, when the number of consecutive
translation denied reaches a threshold, it will trigger 'Never-Translate
this language' automatically.  This CL is trying to implement this
feature without altering the behavior of the existing infobars.

A translation denied is counted when user click on the close button
without ever translated or clicking on any menu item.

BUG=703887
TBR=groby@chromium.org
==========

[Marti Wong, 2017-05-16 03:53:40]

The CQ bit was checked by martiw@chromium.org

[commit-bot: I haz the power, 2017-05-16 03:54:08]

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-16 04:00:07]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1494906820270200,
"parent_rev": "22667180463b25a9059df17350b495b037e5a300", "commit_rev":
"cd21f782ca83751c89fc9ebc0ef233e736ff9c88"}

[commit-bot: I haz the power, 2017-05-16 04:00:22]

Description was changed from

==========
Automatically trigger Never-Translate in new translation infobar
In the new translation infobar, when the number of consecutive
translation denied reaches a threshold, it will trigger 'Never-Translate
this language' automatically.  This CL is trying to implement this
feature without altering the behavior of the existing infobars.

A translation denied is counted when user click on the close button
without ever translated or clicking on any menu item.

BUG=703887
TBR=groby@chromium.org
==========

to

==========
Automatically trigger Never-Translate in new translation infobar
In the new translation infobar, when the number of consecutive
translation denied reaches a threshold, it will trigger 'Never-Translate
this language' automatically.  This CL is trying to implement this
feature without altering the behavior of the existing infobars.

A translation denied is counted when user click on the close button
without ever translated or clicking on any menu item.

BUG=703887
TBR=groby@chromium.org

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

[commit-bot: I haz the power, 2017-05-16 04:00:23]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/cd21f782ca83751c89fc9ebc0ef2...