Input method functions for chrome.languageSettingsPrivate and chrome.inputMethodPrivate

Support adding and removing input methods.

This will be used in chrome://md-settings in a follow-up CL.

Tests to be added in a follow-up CL.

BUG=479043
Committed: https://crrev.com/5a2d5770c697fc7738d25006d4a328fcb6b59900
Cr-Commit-Position: refs/heads/master@{#389389}

[michaelpg, 2016-04-18 18:11:58]

Description was changed from

==========
BUG=
==========

to

==========
Support adding and removing input methods.

This will be used in chrome://md-settings in a follow-up CL.

Needs tests, will add them to this CL.

BUG=479043
==========

[michaelpg, 2016-04-18 21:59:39]

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

[michaelpg, 2016-04-18 21:59:40]

shuchen: PTAL at chrome/browser and chrome/common.
This logic will be used for chrome://md-settings.

It's similar to the logic that is currently used in the
chrome://settings JavaScript.

[Shu Chen, 2016-04-19 02:33:57]

https://codereview.chromium.org/1897913002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/input_method_api.cc (right):

https://codereview.chromium.org/1897913002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/input_method_api.cc:281:
ime_state->GetInputMethodFromId(params->input_method_id);
A null check for |ime| should be necessary.

https://codereview.chromium.org/1897913002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/input_method_api.cc:291:
browser->OpenURL(url_params);
Can we use chrome::ShowSingletonTabOverwritingNTP() here?

[michaelpg, 2016-04-20 02:39:09]

Patchset #3 (id:40001) has been deleted

[michaelpg, 2016-04-20 02:41:09]

https://codereview.chromium.org/1897913002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/input_method_api.cc (right):

https://codereview.chromium.org/1897913002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/input_method_api.cc:281:
ime_state->GetInputMethodFromId(params->input_method_id);
On 2016/04/19 02:33:57, Shu Chen wrote:
> A null check for |ime| should be necessary.

Done.

https://codereview.chromium.org/1897913002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/input_method_api.cc:291:
browser->OpenURL(url_params);
On 2016/04/19 02:33:57, Shu Chen wrote:
> Can we use chrome::ShowSingletonTabOverwritingNTP() here?

This makes the page open in the current tab (Settings). I tried using the
overloaded constructor:
    NavigateParams(Profile* a_profile, const GURL& a_url, ui::PageTransition
a_transition)
which opens a new tab, but it isn't a singleton tab (repeated calls open new
tabs).

[Shu Chen, 2016-04-20 06:38:33]

https://codereview.chromium.org/1897913002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/input_method_api.cc (right):

https://codereview.chromium.org/1897913002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/input_method_api.cc:291:
browser->OpenURL(url_params);
On 2016/04/20 02:41:09, michaelpg wrote:
> On 2016/04/19 02:33:57, Shu Chen wrote:
> > Can we use chrome::ShowSingletonTabOverwritingNTP() here?
> 
> This makes the page open in the current tab (Settings). I tried using the
> overloaded constructor:
>     NavigateParams(Profile* a_profile, const GURL& a_url, ui::PageTransition
> a_transition)
> which opens a new tab, but it isn't a singleton tab (repeated calls open new
> tabs).

ACK.

https://codereview.chromium.org/1897913002/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/input_method_api.cc (right):

https://codereview.chromium.org/1897913002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/input_method_api.cc:283: return
RespondNow(NoArguments());
This should be an error:
  return RespondNow(Error(...));

https://codereview.chromium.org/1897913002/diff/60001/chrome/browser/extensio...
File
chrome/browser/extensions/api/language_settings_private/language_settings_private_api.cc
(right):

https://codereview.chromium.org/1897913002/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/api/language_settings_private/language_settings_private_api.cc:62:
ime_state->GetActiveInputMethodIds());
GetActiveInputMethodIds() can include 3rd-party extension IMEs.

[michaelpg, 2016-04-20 08:07:55]

Description was changed from

==========
Support adding and removing input methods.

This will be used in chrome://md-settings in a follow-up CL.

Needs tests, will add them to this CL.

BUG=479043
==========

to

==========
Support adding and removing input methods.

This will be used in chrome://md-settings in a follow-up CL.

Tests to be added in a follow-up CL.

BUG=479043
==========

[michaelpg, 2016-04-20 08:09:08]

PTAL.

This CL is getting big-ish and these APIs aren't tested yet,
so I would prefer to add tests in a follow-up CL if that's ok.

https://codereview.chromium.org/1897913002/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/input_method_api.cc (right):

https://codereview.chromium.org/1897913002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/input_method_api.cc:283: return
RespondNow(NoArguments());
On 2016/04/20 06:38:33, Shu Chen wrote:
> This should be an error:
>   return RespondNow(Error(...));

Done.

https://codereview.chromium.org/1897913002/diff/60001/chrome/browser/extensio...
File
chrome/browser/extensions/api/language_settings_private/language_settings_private_api.cc
(right):

https://codereview.chromium.org/1897913002/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/api/language_settings_private/language_settings_private_api.cc:62:
ime_state->GetActiveInputMethodIds());
On 2016/04/20 06:38:33, Shu Chen wrote:
> GetActiveInputMethodIds() can include 3rd-party extension IMEs.

Oops. I didn't notice because GetSortedComponentIMEs() was filtering these back
out. And it is the correct behavior for PopulateInputMethodListFromDescriptors.
So I split into two functions which should each do what they say now :-)

[Shu Chen, 2016-04-21 01:48:06]

lgtm

[michaelpg, 2016-04-21 02:46:33]

michaelpg@chromium.org changed reviewers:
+ asvitkine@chromium.org, dbeam@chromium.org

[michaelpg, 2016-04-21 02:46:34]

+asvitkine: extensions/browser/extension_function_histogram_value.h

+dbeam: third_party/closure_compiler/interfaces

[Alexei Svitkine (slow), 2016-04-21 15:05:53]

https://codereview.chromium.org/1897913002/diff/100001/extensions/browser/ext...
File extensions/browser/extension_function_histogram_value.h (right):

https://codereview.chromium.org/1897913002/diff/100001/extensions/browser/ext...
extensions/browser/extension_function_histogram_value.h:1179:
INPUTMETHODPRIVATE_OPENOPTIONSPAGE,
Please follow the instructions on the comment below.

[Dan Beam, 2016-04-21 16:38:28]

lgtm

[michaelpg, 2016-04-21 17:54:29]

https://codereview.chromium.org/1897913002/diff/100001/extensions/browser/ext...
File extensions/browser/extension_function_histogram_value.h (right):

https://codereview.chromium.org/1897913002/diff/100001/extensions/browser/ext...
extensions/browser/extension_function_histogram_value.h:1179:
INPUTMETHODPRIVATE_OPENOPTIONSPAGE,
On 2016/04/21 15:05:53, Alexei Svitkine wrote:
> Please follow the instructions on the comment below.

Done (sry, got lost in a rebase. That seems to happen a lot so I've added a
custom hook to remind me to check before adding you next time!)

[Alexei Svitkine (slow), 2016-04-21 18:04:13]

lgtm

[michaelpg, 2016-04-21 18:12:00]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2016-04-21 18:12:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
shuchen@chromium.org, dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/1897913002/#ps140001
(title: "rebase")

[commit-bot: I haz the power, 2016-04-21 18:12:42]

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

[commit-bot: I haz the power, 2016-04-21 18:12:43]

This CL has an open dependency (Issue 1902893003 Patch 80001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[michaelpg, 2016-04-23 00:34:05]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2016-04-23 00:34:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
shuchen@chromium.org, asvitkine@chromium.org, dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/1897913002/#ps160001
(title: "rebase off origin/master")

[commit-bot: I haz the power, 2016-04-23 00:34:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1897913002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1897913002/160001

[commit-bot: I haz the power, 2016-04-23 00:38:23]

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

[commit-bot: I haz the power, 2016-04-23 00:38:25]

Try jobs failed on following builders:
  ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[michaelpg, 2016-04-23 00:48:54]

The CQ bit was unchecked by michaelpg@chromium.org

[michaelpg, 2016-04-23 00:49:18]

Patchset #8 (id:160001) has been deleted

[michaelpg, 2016-04-23 08:28:23]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2016-04-23 08:28:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
shuchen@chromium.org, asvitkine@chromium.org, dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/1897913002/#ps180001
(title: "rebase")

[commit-bot: I haz the power, 2016-04-23 08:28:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1897913002/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1897913002/180001

[commit-bot: I haz the power, 2016-04-23 08:50:49]

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

[commit-bot: I haz the power, 2016-04-23 08:50:50]

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[michaelpg, 2016-04-23 21:28:50]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2016-04-23 21:28:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
shuchen@chromium.org, asvitkine@chromium.org, dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/1897913002/#ps200001
(title: "fix typo")

[commit-bot: I haz the power, 2016-04-23 21:29:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1897913002/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1897913002/200001

[commit-bot: I haz the power, 2016-04-23 22:37:51]

Description was changed from

==========
Support adding and removing input methods.

This will be used in chrome://md-settings in a follow-up CL.

Tests to be added in a follow-up CL.

BUG=479043
==========

to

==========
Support adding and removing input methods.

This will be used in chrome://md-settings in a follow-up CL.

Tests to be added in a follow-up CL.

BUG=479043
==========

[commit-bot: I haz the power, 2016-04-23 22:37:52]

Committed patchset #9 (id:200001)

[commit-bot: I haz the power, 2016-04-23 22:39:37]

Description was changed from

==========
Support adding and removing input methods.

This will be used in chrome://md-settings in a follow-up CL.

Tests to be added in a follow-up CL.

BUG=479043
==========

to

==========
Support adding and removing input methods.

This will be used in chrome://md-settings in a follow-up CL.

Tests to be added in a follow-up CL.

BUG=479043

Committed: https://crrev.com/5a2d5770c697fc7738d25006d4a328fcb6b59900
Cr-Commit-Position: refs/heads/master@{#389389}
==========

[commit-bot: I haz the power, 2016-04-23 22:39:38]

Patchset 9 (id:??) landed as
https://crrev.com/5a2d5770c697fc7738d25006d4a328fcb6b59900
Cr-Commit-Position: refs/heads/master@{#389389}