Change queryId of launcherSearchProvider API to integer.

Change queryId of launcherSearchProvider API to integer.

BUG=487505
TEST=GYP_GENERATORS=ninja tools/gyp/gyp --depth . ui/file_manager/file_manager/background/js/compiled_resources.gyp && ninja -C out/Default

Committed: https://crrev.com/fd9ae3c13f352699de3e5573988eac1d6f8e13ce
Cr-Commit-Position: refs/heads/master@{#329806}

[yawano, 2015-05-13 07:33:41]

yawano@chromium.org changed reviewers:
+ benwells@chromium.org, fukino@chromium.org, oshima@chromium.org

[yawano, 2015-05-13 07:33:42]

PTAL

@oshima: files under chrome/browser/chromeos.
@benwells: chrome/common/extensions/api/launcher_search_provider.idl.
@fukino: files under ui/file_manager.

Thank you!

https://codereview.chromium.org/1140873002/diff/1/chrome/browser/chromeos/lau...
File chrome/browser/chromeos/launcher_search_provider/service.h (right):

https://codereview.chromium.org/1140873002/diff/1/chrome/browser/chromeos/lau...
chrome/browser/chromeos/launcher_search_provider/service.h:70: int query_id_;
Changed this to int per code style:
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Integer_Types

[benwells, 2015-05-13 07:51:29]

idl lgtm

[yawano, 2015-05-13 07:52:42]

On 2015/05/13 07:51:29, benwells wrote:
> idl lgtm

Thanks.

[fukino, 2015-05-13 08:25:07]

ui/file_manager lgtm

[yawano, 2015-05-13 08:30:41]

On 2015/05/13 08:25:07, fukino wrote:
> ui/file_manager lgtm

Thanks.

[oshima, 2015-05-13 16:02:01]

https://codereview.chromium.org/1140873002/diff/1/chrome/browser/chromeos/lau...
File chrome/browser/chromeos/launcher_search_provider/service.h (right):

https://codereview.chromium.org/1140873002/diff/1/chrome/browser/chromeos/lau...
chrome/browser/chromeos/launcher_search_provider/service.h:56: const int
query_id,
shouldn't this be int64?

[yawano, 2015-05-14 00:44:45]

https://codereview.chromium.org/1140873002/diff/1/chrome/browser/chromeos/lau...
File chrome/browser/chromeos/launcher_search_provider/service.h (right):

https://codereview.chromium.org/1140873002/diff/1/chrome/browser/chromeos/lau...
chrome/browser/chromeos/launcher_search_provider/service.h:56: const int
query_id,
Sorry, why? Type in the generated code from idl is also 'int'.

[Matt Giuca, 2015-05-14 00:56:07]

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

[Matt Giuca, 2015-05-14 00:56:07]

What's with the TEST= line in the CL description? I don't know what that command
does.

TEST= lines are (to my knowledge) supposed to be for manual testing instructions
for user-visible changes. Since this is changing an API, it doesn't need a
TEST=.

Otherwise lgtm.

https://codereview.chromium.org/1140873002/diff/1/chrome/browser/chromeos/lau...
File chrome/browser/chromeos/launcher_search_provider/service.cc (right):

https://codereview.chromium.org/1140873002/diff/1/chrome/browser/chromeos/lau...
chrome/browser/chromeos/launcher_search_provider/service.cc:57: // TODO(yawano):
Consider if we can change it to integer at javascript side.
Remove this comment.

[mtomasz, 2015-05-14 01:05:20]

On 2015/05/14 00:56:07, Matt Giuca wrote:
> What's with the TEST= line in the CL description? I don't know what that
command
> does.
> 
> TEST= lines are (to my knowledge) supposed to be for manual testing
instructions
> for user-visible changes. Since this is changing an API, it doesn't need a
> TEST=.

Let me chime in. We usually write in TEST= either manual instructions or if
there is automation, then a test case name or a shell command to automatically
verify correctness of the patch. Here the command is firing the closure
compiler. I believe it's essential for testers to know if a CL is automatically
tested. If yes, then it doesn't need a heavy manual verification. But this is
just my point of view.

> 
> Otherwise lgtm.
> 
>
https://codereview.chromium.org/1140873002/diff/1/chrome/browser/chromeos/lau...
> File chrome/browser/chromeos/launcher_search_provider/service.cc (right):
> 
>
https://codereview.chromium.org/1140873002/diff/1/chrome/browser/chromeos/lau...
> chrome/browser/chromeos/launcher_search_provider/service.cc:57: //
TODO(yawano):
> Consider if we can change it to integer at javascript side.
> Remove this comment.

[oshima, 2015-05-14 01:26:11]

lgtm

https://codereview.chromium.org/1140873002/diff/1/chrome/browser/chromeos/lau...
File chrome/browser/chromeos/launcher_search_provider/service.h (right):

https://codereview.chromium.org/1140873002/diff/1/chrome/browser/chromeos/lau...
chrome/browser/chromeos/launcher_search_provider/service.h:56: const int
query_id,
On 2015/05/14 00:44:44, yawano wrote:
> Sorry, why? Type in the generated code from idl is also 'int'.

Sorry, I must be sleeping or something. I somehow got confused with proto.
Please disregard.

[yawano, 2015-05-14 03:19:25]

Thank you for the review!

https://codereview.chromium.org/1140873002/diff/1/chrome/browser/chromeos/lau...
File chrome/browser/chromeos/launcher_search_provider/service.h (right):

https://codereview.chromium.org/1140873002/diff/1/chrome/browser/chromeos/lau...
chrome/browser/chromeos/launcher_search_provider/service.h:56: const int
query_id,
On 2015/05/14 01:26:11, oshima wrote:
> On 2015/05/14 00:44:44, yawano wrote:
> > Sorry, why? Type in the generated code from idl is also 'int'.
> 
> Sorry, I must be sleeping or something. I somehow got confused with proto.
> Please disregard. 

Acknowledged.

[yawano, 2015-05-14 03:19:29]

The CQ bit was checked by yawano@chromium.org

[yawano, 2015-05-14 03:19:37]

The CQ bit was unchecked by yawano@chromium.org

[yawano, 2015-05-14 04:08:45]

@mgiuca: As @mtomasz pointed out, the TEST= is a command to run closure compiler
type annotation check. We run this command to confirm that javascript code is
correctly changed or not.

https://codereview.chromium.org/1140873002/diff/1/chrome/browser/chromeos/lau...
File chrome/browser/chromeos/launcher_search_provider/service.cc (right):

https://codereview.chromium.org/1140873002/diff/1/chrome/browser/chromeos/lau...
chrome/browser/chromeos/launcher_search_provider/service.cc:57: // TODO(yawano):
Consider if we can change it to integer at javascript side.
Thank you! I forgot to remove this.

[Matt Giuca, 2015-05-14 04:47:51]

Acknowledged.

[yawano, 2015-05-14 04:49:12]

The CQ bit was checked by yawano@chromium.org

[yawano, 2015-05-14 04:49:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
benwells@chromium.org, oshima@chromium.org, fukino@chromium.org,
mgiuca@chromium.org
Link to the patchset: https://codereview.chromium.org/1140873002/#ps20001
(title: "Remove old TODO.")

[commit-bot: I haz the power, 2015-05-14 04:49:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1140873002/20001

[commit-bot: I haz the power, 2015-05-14 05:20:30]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-05-14 05:21:17]

Patchset 2 (id:??) landed as
https://crrev.com/fd9ae3c13f352699de3e5573988eac1d6f8e13ce
Cr-Commit-Position: refs/heads/master@{#329806}