MD Settings: Use a browser proxy from the languages page. Re-enable test.

MD Settings: Use a browser proxy from the languages page. Re-enable test.

 - Add a languages_browser_proxy.js
 - Put chrome.send, languageSettingsPrivate and inputMethodsPrivate behind
   the proxy.
 - Use the proxy to interact with the browser (both chrome.send and private APIs).
 - Implement a fake InputMethodPrivate API and update tests to use it (previously
   tests had been calling to the prod chrome.inputMethodPrivate API).
 - Re-enable CrSettingsLanguagesTest.Languages.

BUG=692356
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2827553002
Cr-Commit-Position: refs/heads/master@{#465853}
Committed: https://chromium.googlesource.com/chromium/src/+/3a1f5619eb05b1d0833c061edd68e7294878604e

[dpapad, 2017-04-17 21:36:33]

Description was changed from

==========
MD Settings: Use a browser proxy from the languages page.

BUG=692356
==========

to

==========
MD Settings: Use a browser proxy from the languages page.

BUG=692356
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dpapad, 2017-04-18 18:00:27]

Patchset #3 (id:40001) has been deleted

[dpapad, 2017-04-18 18:00:29]

Patchset #2 (id:20001) has been deleted

[dpapad, 2017-04-18 18:00:34]

Patchset #1 (id:1) has been deleted

[dpapad, 2017-04-18 18:02:44]

Description was changed from

==========
MD Settings: Use a browser proxy from the languages page.

BUG=692356
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Use a browser proxy from the languages page.

 - Add a languages_browser_proxy.js
 - Put chrome.send, languageSettingsPrivate and inputMethodsPrivate behind
   the proxy.
 - Use the proxy to interact with the browser (chrome.send or private APIs).
 - Implement a fake InputMethodPrivate API and update tests to use it
(previously
   tests had been calling to the prod chrome.inputMethodPrivate API).
 - Re-enable CrSettingsLanguagesTest.Languages.

BUG=692356
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dpapad, 2017-04-18 18:03:19]

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

[commit-bot: I haz the power, 2017-04-18 18:03:48]

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

[dpapad, 2017-04-18 18:42:19]

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

[dpapad, 2017-04-18 18:42:41]

Description was changed from

==========
MD Settings: Use a browser proxy from the languages page.

 - Add a languages_browser_proxy.js
 - Put chrome.send, languageSettingsPrivate and inputMethodsPrivate behind
   the proxy.
 - Use the proxy to interact with the browser (chrome.send or private APIs).
 - Implement a fake InputMethodPrivate API and update tests to use it
(previously
   tests had been calling to the prod chrome.inputMethodPrivate API).
 - Re-enable CrSettingsLanguagesTest.Languages.

BUG=692356
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Use a browser proxy from the languages page.

 - Add a languages_browser_proxy.js
 - Put chrome.send, languageSettingsPrivate and inputMethodsPrivate behind
   the proxy.
 - Use the proxy to interact with the browser (both chrome.send and private
APIs).
 - Implement a fake InputMethodPrivate API and update tests to use it
(previously
   tests had been calling to the prod chrome.inputMethodPrivate API).
 - Re-enable CrSettingsLanguagesTest.Languages.

BUG=692356
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2017-04-18 18:42:55]

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

[dpapad, 2017-04-18 19:42:47]

dpapad@chromium.org changed reviewers:
+ michaelpg@chromium.org

[commit-bot: I haz the power, 2017-04-18 20:22:17]

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

[commit-bot: I haz the power, 2017-04-18 20:22:18]

Dry run: This issue passed the CQ dry run.

[michaelpg, 2017-04-18 23:07:39]

The proxy will definitely make testing simpler. I like that the "model"
(languages.js) will be using it. I'm not sure the UI "views" should be using it
directly, though.

Today the "views", like languages_page, don't call languageSettingsPrivate,
inputMethodPrivate, or chrome.send.[1] These APIs are only used through the
LanguageHelper interface, so only languages.js is directly manipulating the
browser state. languages.js has to maintain several non-trivial transformations
to get from the browser's data to what the UI needs to display; if other classes
can muck with that data, languages.js loses control over the state (and
"caches") it builds.

yyushkina@ is spearheading some language design changes; I expect the UI to
change soon. The more UI work that the "views" do directly, the more work it
will take to shuffle UI the around.

TLDR - I think LanguageHelper should still be the control point for the UI, and
can outsource calls to the stateless BrowserProxy. Would that be in line with
making the tests easier to reason about?



[1] - the exception being edit_dictionary_page.js which does unrelated work.

[dpapad, 2017-04-19 01:29:14]

dpapad@chromium.org changed reviewers:
+ dbeam@chromium.org

[dpapad, 2017-04-19 01:29:15]

I am not 100% sure of the changes you are requesting. Perhaps more clarification
would help. Also using browser proxies from UI components is the pattern we use
extensively in Settings codebase. It makes code easy to test. What makes
language related "views" different? Can you elaborate on your concern? AFAIU
this is a mature pattern that has served as well so far.

Also LanguagesHelper has been acting as an unnecessary wrapper in some cases,
see setCurrentInputMethod and openInputMethodOptions methods which just forward
the call to the private API. I understand that LanguagesHelper has its own
state, and it exists for a good reason, but I don't see why it needs to wrap
calls made to the private APIs.

This CL takes an incomplete and not so widely used pattern of setting global
variables languageSettingsPrivateApiForTest and inputMethodPrivateApiForTest
during test, and utilizes the existing "proxy" pattern to provide test
implementations.

Regarding the upcoming changes you are mentioning in the Languages UI, I think
it is to everyone's benefit to modify a UI for which tests exist, and run on the
bots, than modifying UI were disabled test exist and provide a false sense of
coverage.

[michaelpg, 2017-04-19 02:37:30]

On 2017/04/19 01:29:15, dpapad wrote:
> I am not 100% sure of the changes you are requesting. Perhaps more
clarification
> would help. Also using browser proxies from UI components is the pattern we
use
> extensively in Settings codebase. It makes code easy to test. What makes
> language related "views" different? Can you elaborate on your concern? AFAIU
> this is a mature pattern that has served as well so far.

Take the language list as an example. This needs to be an array of information
about each enabled language. This information is contextual (dependent on other
settings) and opinionated (Settings shows it in a particular way that isn't
dependent on how Chrome stores it).
 * Get enabled language codes from intl.accept_languages (or
settings.language.preferred_languages on Chrome OS)
 * Hide language codes that aren't supported on this system (may be carried over
from a different device)
 * Cross-reference the remaining codes with those in the spellcheck.dictionaries
pref and the translate_blocked_languages pref to determine which should provide
spellcheck or translate options in the overflow menu. Check each language code
and the translate_blocked_languages list against whether the language actually
supports translate, whether the language is the one the user wants to translate
*to* or the UI language (if it exists on this platform). Make sure to apply the
same transformations Chrome does to hack around locale issues. (See lines
400-405 of languages.js at the end of getEnabledLanguageStates_.)
 * Check whether we need to show the restart button next to this language, to
signal that the user has selected this language but hasn't restarted since then.

The user can choose to move any of those languages up or down in the list, which
means going back to the comma-separated intl.accept_languages pref (or
settings.language.preferred_languages on Chrome OS) and moving the given
language code forward or backward (skipping over any enabled language codes that
aren't supported, since those wouldn't be shown in the UI so moving past one of
those would appear to have no effect, to the user).

That's one list. We also have the input method list (whose UI is dependent on
the enabled languages list, too) and the spellcheck list.

This is why languages.js is big. We shouldn't write that logic ad-hoc in
languages_page.js and manage_input_methods_page.js; those are large anyway to
handle all the UI affordances we have.

> 
> Also LanguagesHelper has been acting as an unnecessary wrapper in some cases,
> see setCurrentInputMethod and openInputMethodOptions methods which just
forward
> the call to the private API. I understand that LanguagesHelper has its own
> state, and it exists for a good reason, but I don't see why it needs to wrap
> calls made to the private APIs.

Yes, those methods just forward to the private API. 

Note that languages.js'

> 
> This CL takes an incomplete and not so widely used pattern of setting global
> variables languageSettingsPrivateApiForTest and inputMethodPrivateApiForTest
> during test, and utilizes the existing "proxy" pattern to provide test
> implementations.
> 
> Regarding the upcoming changes you are mentioning in the Languages UI, I think
> it is to everyone's benefit to modify a UI for which tests exist, and run on
the
> bots, than modifying UI were disabled test exist and provide a false sense of
> coverage.


We're in agreement there; I'm not trying to impede test development.

https://codereview.chromium.org/2827553002/diff/80001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/cr_settings_browsertest.js (right):

https://codereview.chromium.org/2827553002/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/cr_settings_browsertest.js:1098: // Flaky on Win
and Linux, see http://crbug/692356.
remove comment? or change to a TODO specifying what point we'd be comfortable
closing that bug.

[michaelpg, 2017-04-19 02:38:13]

gah, hit Tab+Enter accidentally. I'll finish the message and send it out again.

[michaelpg, 2017-04-19 02:53:57]

OK, take 2:

Again, I like the browser proxy pattern. It works well for providing an
interface and subbing a test class for that interface. What I'm not clear on is
what we gain from replacing calls to the LanguageHelper class (which should now
use the browser proxy) with direct calls the browser proxy itself. And the
reason I'm wary of that is because it's simpler for other parts of the UI to
know that they have to call LanguageHelper to do anything, rather than calling
LanguageHelper for some things and the private APIs directly for other things,
especially with those things all being in the same domain of language settings.

E.g., letting languages_page.js use 4 interfaces (two private APIs, chrome.send,
languageHelper) instead of 1 (languageHelper) complicates that code without
improving testability. Testability is improved by using the BrowserProxy
*instead of* the private APIs and chrome.send, which doesn't require changing
languages_page.js. This CL is already improving testability in that way.


On 2017/04/19 01:29:15, dpapad wrote:
> I am not 100% sure of the changes you are requesting. Perhaps more
clarification
> would help.

I'm simply requesting we not remove the forwarding methods from languages.js,
and revert the changes to languages_page.js so languages_page.js continues to
call languageHelper.

Where the removed "forwarding" methods from languages.js called a private API,
replacing that with the browser proxy is great. I'm suggesting making those
changes in-place, inside languages.js, instead of moving those calls to
languages_page.js.

> Also using browser proxies from UI components is the pattern we use
> extensively in Settings codebase. It makes code easy to test. What makes
> language related "views" different? Can you elaborate on your concern? AFAIU
> this is a mature pattern that has served as well so far.

It's different because it takes a lot of pre-processing of several
inter-dependent preferences. If we shard out these settings to different classes
we'd lose the ability to understand how changing one pref in the UI affects the
whole system.

Take the language list as an example. This needs to be an array of information
about each enabled language. This information is contextual (dependent on other
settings) and opinionated (Settings shows it in a particular way that isn't
dependent on how Chrome stores it).
 * Get enabled language codes from intl.accept_languages (or
settings.language.preferred_languages on Chrome OS)
 * Hide language codes that aren't supported on this system (may be carried over
from a different device)
 * Cross-reference the remaining codes with those in the spellcheck.dictionaries
pref and the translate_blocked_languages pref to determine which should provide
spellcheck or translate options in the overflow menu. Check each language code
and the translate_blocked_languages list against whether the language actually
supports translate, whether the language is the one the user wants to translate
*to* or the UI language (if it exists on this platform). Make sure to apply the
same transformations Chrome does to hack around locale issues. (See lines
400-405 of languages.js at the end of getEnabledLanguageStates_.)
 * Check whether we need to show the restart button next to this language, to
signal that the user has selected this language but hasn't restarted since then.

The user can choose to move any of those languages up or down in the list, which
means going back to the comma-separated intl.accept_languages pref (or
settings.language.preferred_languages on Chrome OS) and moving the given
language code forward or backward (skipping over any enabled language codes that
aren't supported, since those wouldn't be shown in the UI so moving past one of
those would appear to have no effect, to the user).

That's one list. We also have the input method list (whose UI is dependent on
the enabled languages list, too) and the spellcheck list.

This is why languages.js is big. We shouldn't write that logic ad-hoc in
languages_page.js and manage_input_methods_page.js; those are large anyway to
handle all the UI affordances we have.

> 
> Also LanguagesHelper has been acting as an unnecessary wrapper in some cases,
> see setCurrentInputMethod and openInputMethodOptions methods which just
forward
> the call to the private API. I understand that LanguagesHelper has its own
> state, and it exists for a good reason, but I don't see why it needs to wrap
> calls made to the private APIs.

Yes, some of those methods just forward to the private API. Others (like
disableLanguage) do a lot more. Keeping the methods that just forward to the
private API gives us flexibility to add more logic there, like if we decide we
should change some other setting before forwarding the call (something I expect
to happen in reality with whatever changes are made to the mocks). It also gives
languages_page.js and input_methods_page.js a single interface instead of four
different ones.

Note that languages.js' |languages| property is |readOnly|. This is because
making arbitrary changes to the model can break other parts of the state. There
are 6 different Polymer-style property observers in languages.js, and other
event listeners that update |languages| too. I realize that the methods this CL
is removing doesn't directly affect those, but there doesn't seem to be a
principal behind which methods can be forwarded and which ones can't.

If you want to look for some pattern, like "anything that sets this.prefs.*
should happen in languages.js, anything that calls a private API can call the
private API directly", then by all means, that would be an improvement. I
haven't found a partition like that in the current code.

> 
> This CL takes an incomplete and not so widely used pattern of setting global
> variables languageSettingsPrivateApiForTest and inputMethodPrivateApiForTest
> during test, and utilizes the existing "proxy" pattern to provide test
> implementations.
> 
> Regarding the upcoming changes you are mentioning in the Languages UI, I think
> it is to everyone's benefit to modify a UI for which tests exist, and run on
the
> bots, than modifying UI were disabled test exist and provide a false sense of
> coverage.

We're in agreement there! I'm not trying to impede test development, and I know
I bear most of the responsibility for the language tests that either don't exist
or are flaky. I'm trying to voice concern over one aspect of this CL that I
think will make development and tests more difficult in the long run.

[Dan Beam, 2017-04-19 16:47:36]

dpapad@: in reading language helper, it seems that it accomplishes the goal of
moving IPC behind an abstraction and does not have as much of a luxury to
separate the data sources it manages (direct chrome.send(), cr.sendWithPromise()
calls and 2 private extension APIs).  this is because it combines information
from these 3 sources and presents it in a simplified way to the UI (I think?). 
as in: if the UI wants a row for English and with the understanding of whether
it can be used for spellchecking, has 1+ IMEs, and is enabled (and possibly
going to be switched to when the user restarts), the languages helper makes that
happen.

I was going to suggest splitting across boundaries of concerns/data sources, but
I think the UI would just have to redo all this work instead of in the helper.

There are not that many places in settings that require this sorry of
complexity, as far as I know, or are either managing it more successfully or
pushing the logic into C++.  are there other examples of the data model
differing vastly from the UI organization? maybe site settings?

michaelpg@: is there a reason we must use these multiple sources from JS
(specifically the 2 private extension APIs), rather than interact more directly
with global services and just send dictionaries from C++ -> JS given our
privileged webui context?  seems like it could simplify the code in the JS...

[dpapad, 2017-04-19 16:52:05]

Patchset #3 (id:100001) has been deleted

[dpapad, 2017-04-19 17:07:54]

@michaelpg: Thanks for the clarification and for giving more context. I believe
your feedback is addressed in patch#3, PTAL.

 - languages.js (aka LanugageHelper) is now using a browser proxy to talk to the
browser (chrome.send or private APIs).
 - languages_page.js is unchanged in this CL, which means that it keeps using
LanguageHelper for everything (no direct calls to the new browser proxy).

[dpapad, 2017-04-19 17:12:30]

On 2017/04/19 at 17:07:54, dpapad wrote:
> @michaelpg: Thanks for the clarification and for giving more context. I
believe your feedback is addressed in patch#3, PTAL.
> 
>  - languages.js (aka LanugageHelper) is now using a browser proxy to talk to
the browser (chrome.send or private APIs).
>  - languages_page.js is unchanged in this CL, which means that it keeps using
LanguageHelper for everything (no direct calls to the new browser proxy).

Having said that, the original bug report was showing flaky failures in both
win_chromium_rel_ng and linux_chromium_chromeos_rel_ng bots. My investigation
showed that only CrOS was making calls to real chrome.* APIs during testing.
Unfortunately the old logs are not available yet, and I can't tell whether the
test was timing out or it was failing in some other way.

So it is likely that there is more to do. Will keep an eye on the bots after
this CL lands, and we can re-disable the tests if necessary.

[dpapad, 2017-04-19 17:13:51]

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

[commit-bot: I haz the power, 2017-04-19 17:14:18]

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

[Dan Beam, 2017-04-19 17:46:52]

https://codereview.chromium.org/2827553002/diff/140001/chrome/test/data/webui...
File chrome/test/data/webui/settings/languages_tests.js (right):

https://codereview.chromium.org/2827553002/diff/140001/chrome/test/data/webui...
chrome/test/data/webui/settings/languages_tests.js:18:
this.languageSettingsPrivate_ = null;
this.languageSettingsPrivate_ = new settings.FakeLanguageSettingsPrivate;

https://codereview.chromium.org/2827553002/diff/140001/chrome/test/data/webui...
chrome/test/data/webui/settings/languages_tests.js:32:
this.languageSettingsPrivate_ = private;
i would not recommend private as a var name

https://codereview.chromium.org/2827553002/diff/140001/chrome/test/data/webui...
chrome/test/data/webui/settings/languages_tests.js:147: var
languageSettingsPrivate = new settings.FakeLanguageSettingsPrivate();
why not just instantiate these directly in TestLanguageBrowserProxy's
constructor?

[dpapad, 2017-04-19 18:11:30]

https://codereview.chromium.org/2827553002/diff/140001/chrome/test/data/webui...
File chrome/test/data/webui/settings/languages_tests.js (right):

https://codereview.chromium.org/2827553002/diff/140001/chrome/test/data/webui...
chrome/test/data/webui/settings/languages_tests.js:18:
this.languageSettingsPrivate_ = null;
On 2017/04/19 at 17:46:52, Dan Beam wrote:
> this.languageSettingsPrivate_ = new settings.FakeLanguageSettingsPrivate;

See related last comment in this file.

https://codereview.chromium.org/2827553002/diff/140001/chrome/test/data/webui...
chrome/test/data/webui/settings/languages_tests.js:32:
this.languageSettingsPrivate_ = private;
On 2017/04/19 at 17:46:52, Dan Beam wrote:
> i would not recommend private as a var name

Done.

https://codereview.chromium.org/2827553002/diff/140001/chrome/test/data/webui...
chrome/test/data/webui/settings/languages_tests.js:147: var
languageSettingsPrivate = new settings.FakeLanguageSettingsPrivate();
On 2017/04/19 at 17:46:52, Dan Beam wrote:
> why not just instantiate these directly in TestLanguageBrowserProxy's
constructor?

The next line calls setSettingsPrefs() which is a method only existing on
FakeLanguageSettingsPrivate, but not on the interface LanguageSettingsPrivate
returned by LanguagesBrowserProxy. So technically I think this is more correct.
In practice it does not matter, because we don't currently type check our tests.

Besides type checking, from a design point of view, a test browser proxy is
meant to be a super thin object that does no work in its constructor. Clients of
the test proxy are supposed to configure the proxy by calling set*() methods on
it, in order to force a certain response.

[Dan Beam, 2017-04-19 18:31:56]

https://codereview.chromium.org/2827553002/diff/140001/chrome/test/data/webui...
File chrome/test/data/webui/settings/languages_tests.js (right):

https://codereview.chromium.org/2827553002/diff/140001/chrome/test/data/webui...
chrome/test/data/webui/settings/languages_tests.js:147: var
languageSettingsPrivate = new settings.FakeLanguageSettingsPrivate();
On 2017/04/19 18:11:30, dpapad wrote:
> On 2017/04/19 at 17:46:52, Dan Beam wrote:
> > why not just instantiate these directly in TestLanguageBrowserProxy's
> constructor?
> 
> The next line calls setSettingsPrefs() which is a method only existing on
> FakeLanguageSettingsPrivate, but not on the interface LanguageSettingsPrivate
> returned by LanguagesBrowserProxy. So technically I think this is more
correct.
> In practice it does not matter, because we don't currently type check our
tests.
> 
> Besides type checking, from a design point of view, a test browser proxy is
> meant to be a super thin object that does no work in its constructor. Clients
of
> the test proxy are supposed to configure the proxy by calling set*() methods
on
> it, in order to force a certain response.

i don't see that as a requirement, and am likely not to do this when it
complicates the code

[michaelpg, 2017-04-19 20:32:14]

lgtm

https://codereview.chromium.org/2827553002/diff/160001/chrome/browser/resourc...
File
chrome/browser/resources/settings/languages_page/languages_browser_proxy.html
(right):

https://codereview.chromium.org/2827553002/diff/160001/chrome/browser/resourc...
chrome/browser/resources/settings/languages_page/languages_browser_proxy.html:1:
<script src="languages_browser_proxy.js"></script>
import cr.html

[dpapad, 2017-04-19 20:50:12]

> i don't see that as a requirement, and am likely not to do this when it
complicates the code

Done.

[dpapad, 2017-04-19 20:50:30]

https://codereview.chromium.org/2827553002/diff/160001/chrome/browser/resourc...
File
chrome/browser/resources/settings/languages_page/languages_browser_proxy.html
(right):

https://codereview.chromium.org/2827553002/diff/160001/chrome/browser/resourc...
chrome/browser/resources/settings/languages_page/languages_browser_proxy.html:1:
<script src="languages_browser_proxy.js"></script>
On 2017/04/19 at 20:32:14, michaelpg wrote:
> import cr.html

Done.

[dpapad, 2017-04-19 20:51:16]

Description was changed from

==========
MD Settings: Use a browser proxy from the languages page.

 - Add a languages_browser_proxy.js
 - Put chrome.send, languageSettingsPrivate and inputMethodsPrivate behind
   the proxy.
 - Use the proxy to interact with the browser (both chrome.send and private
APIs).
 - Implement a fake InputMethodPrivate API and update tests to use it
(previously
   tests had been calling to the prod chrome.inputMethodPrivate API).
 - Re-enable CrSettingsLanguagesTest.Languages.

BUG=692356
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Use a browser proxy from the languages page. Re-enable test.

 - Add a languages_browser_proxy.js
 - Put chrome.send, languageSettingsPrivate and inputMethodsPrivate behind
   the proxy.
 - Use the proxy to interact with the browser (both chrome.send and private
APIs).
 - Implement a fake InputMethodPrivate API and update tests to use it
(previously
   tests had been calling to the prod chrome.inputMethodPrivate API).
 - Re-enable CrSettingsLanguagesTest.Languages.

BUG=692356
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2017-04-19 21:13:40]

lgtm

[dpapad, 2017-04-19 21:55:46]

The CQ bit was checked by dpapad@chromium.org

[dpapad, 2017-04-19 21:55:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/2827553002/#ps170001
(title: "Address")

[commit-bot: I haz the power, 2017-04-19 21:56:32]

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-04-19 22:57:04]

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

[commit-bot: I haz the power, 2017-04-19 22:57:06]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dpapad, 2017-04-20 00:22:16]

The CQ bit was checked by dpapad@chromium.org

[dpapad, 2017-04-20 00:22:16]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org,
michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/2827553002/#ps190001
(title: "Remove accidental usage of getInputMethodPrivate() on non-Cros")

[commit-bot: I haz the power, 2017-04-20 00:22:50]

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-04-20 01:41:38]

CQ is committing da patch.

Bot data: {"patchset_id": 190001, "attempt_start_ts": 1492647736521330,
"parent_rev": "54b67506cfed057ffdbaa138dc1c7dc7b0bf39f8", "commit_rev":
"3a1f5619eb05b1d0833c061edd68e7294878604e"}

[commit-bot: I haz the power, 2017-04-20 01:41:51]

Description was changed from

==========
MD Settings: Use a browser proxy from the languages page. Re-enable test.

 - Add a languages_browser_proxy.js
 - Put chrome.send, languageSettingsPrivate and inputMethodsPrivate behind
   the proxy.
 - Use the proxy to interact with the browser (both chrome.send and private
APIs).
 - Implement a fake InputMethodPrivate API and update tests to use it
(previously
   tests had been calling to the prod chrome.inputMethodPrivate API).
 - Re-enable CrSettingsLanguagesTest.Languages.

BUG=692356
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Use a browser proxy from the languages page. Re-enable test.

 - Add a languages_browser_proxy.js
 - Put chrome.send, languageSettingsPrivate and inputMethodsPrivate behind
   the proxy.
 - Use the proxy to interact with the browser (both chrome.send and private
APIs).
 - Implement a fake InputMethodPrivate API and update tests to use it
(previously
   tests had been calling to the prod chrome.inputMethodPrivate API).
 - Re-enable CrSettingsLanguagesTest.Languages.

BUG=692356
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2827553002
Cr-Commit-Position: refs/heads/master@{#465853}
Committed:
https://chromium.googlesource.com/chromium/src/+/3a1f5619eb05b1d0833c061edd68...
==========

[commit-bot: I haz the power, 2017-04-20 01:41:53]

Committed patchset #7 (id:190001) as
https://chromium.googlesource.com/chromium/src/+/3a1f5619eb05b1d0833c061edd68...