Fix app list item indexing bug.

ui/app_list/test/app_list_test_view_delegate.h

Index: ui/app_list/test/app_list_test_view_delegate.h
diff --git a/ui/app_list/test/app_list_test_view_delegate.h b/ui/app_list/test/app_list_test_view_delegate.h
index 5cafa04f75baabc63c6762d83be0b90b1ee16463..b83ed15ad03090fa2bd4de9eee765d02e38e1039 100644
--- a/ui/app_list/test/app_list_test_view_delegate.h
+++ b/ui/app_list/test/app_list_test_view_delegate.h
@@ -31,7 +31,7 @@ class AppListTestViewDelegate : public AppListViewDelegate {
 
   int dismiss_count() { return dismiss_count_; }
   int open_search_result_count() { return open_search_result_count_; }
-  std::map<size_t, int> open_search_result_counts() {
+  std::map<size_t, int> open_search_result_counts() const {

[khmel, 2017/07/18 15:04:01]

nit: is any reason to return copy of this? would const std::map<size_t, int>&
work?

[Jiaquan He, 2017/07/18 15:58:16]

Just tried. Other callers cannot access operator[] and function clear from a
'const std::map<size_t, int>'.
But we can return std::map<size_t, int>&. And I'm going to remove the const
since other callers might use nonconst [] and clear.

[oshima, 2017/07/18 16:55:26]

Hmm, won't it change the behavior?

Looks like this is used only by a test, and the test may not be working as
intended.
(clear won't clear what's in the delegate)

Can you also rename to open_search_result_counts_for_test() ?

[Jiaquan He, 2017/07/18 17:41:36]

So I think it makes sense to return a reference here. Is that correct? (And I
checked. It won't break any existing test.)
Just to confirm: Actually this whole file is used for test. Is it necessary to
rename this method?

[Jiaquan He, 2017/07/19 21:02:46]

For why it doesn't change the behavior of the existing tests:

Currently only two tests are using this method:

(1)
https://cs.chromium.org/chromium/src/ui/app_list/views/search_result_answer_c...
(2)
https://cs.chromium.org/chromium/src/ui/app_list/views/search_result_list_vie...

And they *try to clear* the map. But after calling |clear|, they don't check
again whether the map is really cleared or not. Actually I checked that the map
in the delegate is not cleared at all.

[oshima, 2017/07/19 21:31:21]

oh sorry, my mistake. Please ignore rename comment.

[oshima, 2017/07/19 21:31:21]

Can you make sure the ranking exist in the map? then lgtm

[Jiaquan He, 2017/07/19 21:54:23]

Please check the new PS.

     return open_search_result_counts_;
   }