Make app list return to previous page after search.

Make app list return to previous page after search.

This CL makes the app list return to the previously displayed page when the
search box query is cleared.

BUG=349727
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=283403

[Matt Giuca, 2014-07-09 04:42:07]

https://codereview.chromium.org/377783006/diff/20001/ui/app_list/views/app_li...
File ui/app_list/views/app_list_view_unittest.cc (right):

https://codereview.chromium.org/377783006/diff/20001/ui/app_list/views/app_li...
ui/app_list/views/app_list_view_unittest.cc:447:
EXPECT_TRUE(main_view->search_box_view()->visible());
Why did IsViewAtOrigin go away?

https://codereview.chromium.org/377783006/diff/20001/ui/app_list/views/conten...
File ui/app_list/views/contents_view.cc (right):

https://codereview.chromium.org/377783006/diff/20001/ui/app_list/views/conten...
ui/app_list/views/contents_view.cc:169: show ? NAMED_PAGE_SEARCH_RESULTS :
NAMED_PAGE_APPS;
show is always true.. see later.

https://codereview.chromium.org/377783006/diff/20001/ui/app_list/views/conten...
ui/app_list/views/contents_view.cc:172: page =
GetPageIndexForNamedPage(new_named_page);
Can just collapse the entire body of this if statement to:
page =
GetPageIndexForNamedPage(app_list::switches::IsExperimentalAppListEnabled() ?
NAMED_PAGE_START : NAMED_PAGE_SEARCH_RESULTS);

https://codereview.chromium.org/377783006/diff/20001/ui/app_list/views/conten...
ui/app_list/views/contents_view.cc:173: }
else { page = page_before_search_; }

I find it easier to read a two-branch if statement, rather than initialize it to
the false case.

[calamity, 2014-07-11 04:22:17]

https://codereview.chromium.org/377783006/diff/20001/ui/app_list/views/app_li...
File ui/app_list/views/app_list_view_unittest.cc (right):

https://codereview.chromium.org/377783006/diff/20001/ui/app_list/views/app_li...
ui/app_list/views/app_list_view_unittest.cc:447:
EXPECT_TRUE(main_view->search_box_view()->visible());
On 2014/07/09 04:42:07, Matt Giuca wrote:
> Why did IsViewAtOrigin go away?

Wasn't necessary because of ShowContentsViewPageAndVerify(). Should have been
removed when that was added.

https://codereview.chromium.org/377783006/diff/20001/ui/app_list/views/conten...
File ui/app_list/views/contents_view.cc (right):

https://codereview.chromium.org/377783006/diff/20001/ui/app_list/views/conten...
ui/app_list/views/contents_view.cc:169: show ? NAMED_PAGE_SEARCH_RESULTS :
NAMED_PAGE_APPS;
On 2014/07/09 04:42:07, Matt Giuca wrote:
> show is always true.. see later.

Acknowledged.

https://codereview.chromium.org/377783006/diff/20001/ui/app_list/views/conten...
ui/app_list/views/contents_view.cc:172: page =
GetPageIndexForNamedPage(new_named_page);
On 2014/07/09 04:42:07, Matt Giuca wrote:
> Can just collapse the entire body of this if statement to:
> page =
> GetPageIndexForNamedPage(app_list::switches::IsExperimentalAppListEnabled() ?
> NAMED_PAGE_START : NAMED_PAGE_SEARCH_RESULTS);

Acknowledged.

https://codereview.chromium.org/377783006/diff/20001/ui/app_list/views/conten...
ui/app_list/views/contents_view.cc:173: }
On 2014/07/09 04:42:07, Matt Giuca wrote:
> else { page = page_before_search_; }
> 
> I find it easier to read a two-branch if statement, rather than initialize it
to
> the false case.

Changed the whole thing. Should be nicer now.

[Matt Giuca, 2014-07-11 07:37:56]

lgtm

[calamity, 2014-07-14 05:37:07]

The CQ bit was checked by calamity@chromium.org

[commit-bot: I haz the power, 2014-07-14 05:37:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/calamity@chromium.org/377783006/40001

[commit-bot: I haz the power, 2014-07-14 06:23:22]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/9...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/20...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win8_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win8_chromium_rel/bui...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/28090)

[commit-bot: I haz the power, 2014-07-14 06:24:39]

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

[commit-bot: I haz the power, 2014-07-14 06:24:40]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/28107)

[calamity, 2014-07-16 06:29:59]

The CQ bit was checked by calamity@chromium.org

[commit-bot: I haz the power, 2014-07-16 06:31:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/calamity@chromium.org/377783006/60001

[commit-bot: I haz the power, 2014-07-16 15:52:00]

Change committed as 283403