Re-styled the App Info Dialog according to UI feedback

Re-styled the App Info Dialog according to UI feedback

Changed the layout of the App Info Dialog according to UI feedback. This
includes removing the tabs, and replacing it with a single, scrollable
dialog that has all the information in a single pane.

BUG=364681, 266739
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=276557

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

[sashab, 2014-06-10 02:50:38]

There is a bug I know about where the dialog content only starts part-way down
the dialog (there is a gap at the top). I am working on fixing that, but it will
be a part of a separate commit.

[sashab, 2014-06-10 02:53:30]

https://codereview.chromium.org/327743002/diff/1/chrome/browser/ui/views/apps...
File chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_tab.cc
(left):

https://codereview.chromium.org/327743002/diff/1/chrome/browser/ui/views/apps...
chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_tab.cc:1: //
Copyright 2014 The Chromium Authors. All rights reserved.
I also need to move the tests in permissions_tab_unittest.cc to their
appropriate classes.

[benwells, 2014-06-10 03:28:11]

https://codereview.chromium.org/327743002/diff/1/chrome/app/generated_resourc...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/327743002/diff/1/chrome/app/generated_resourc...
chrome/app/generated_resources.grd:2169: +        View in store
Shouldn't this be different for title case? Same for some of the others ("It
Can:", "Create Shortcuts", etc.)

[sashab, 2014-06-10 05:24:27]

Matched files this time as well :) Should make the review a bit easier.

Dialog is slightly different now, though - the 'create shortcuts' button and the
launch options now appear in the middle of the dialog. Waiting to hear back from
UI about what to do with these UI elements. In the meantime, review as though
it's not going to change.

https://codereview.chromium.org/327743002/diff/1/chrome/app/generated_resourc...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/327743002/diff/1/chrome/app/generated_resourc...
chrome/app/generated_resources.grd:2169: +        View in store
On 2014/06/10 03:28:10, benwells wrote:
> Shouldn't this be different for title case? Same for some of the others ("It
> Can:", "Create Shortcuts", etc.)

Nope; looking at "use_titlecase" in generated_resources.grd, this is only used
for menu options (and for Mac OS X, it seems).
Update: after looking at the style guide, these definitely need to be separated
:) Done.

[benwells, 2014-06-11 02:52:07]

https://codereview.chromium.org/327743002/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc
(right):

https://codereview.chromium.org/327743002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc:71:
dialog_body_scrollview->ClipHeightTo(kMaxDialogHeight, kMaxDialogHeight);
Why is this needed at all? The comment on ScrollView::ClipHeightTo would imply
this isn't needed.

[sashab, 2014-06-11 05:48:17]

https://codereview.chromium.org/327743002/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc
(right):

https://codereview.chromium.org/327743002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc:71:
dialog_body_scrollview->ClipHeightTo(kMaxDialogHeight, kMaxDialogHeight);
On 2014/06/11 02:52:06, benwells wrote:
> Why is this needed at all? The comment on ScrollView::ClipHeightTo would imply
> this isn't needed.

Putting ClipHeightTo() as a method on ScrollView was a mistake.
BoundedScrollView should really be a separate class, or ClipHeightTo should at
least have a version that takes no arguments and stretches as tall as it can.

ScrollView by itself is not a ScrollView, this is the problem. By "stretch to
fill its outer container", it means stretch to the whole container, not a part
of it.

I think this is a bug that should be fixed in a separate commit: ScrollViews
don't work unless they are the only view in their widget. Either that or a
ScrollView is not supposed to behave anything like a BoundedScrollView.

[benwells, 2014-06-11 07:58:44]

lgtm then. Seems like ScrollView is a trap.

[sashab, 2014-06-11 22:45:52]

The CQ bit was checked by sashab@chromium.org

[commit-bot: I haz the power, 2014-06-11 22:46:45]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sashab@chromium.org/327743002/20001

[commit-bot: I haz the power, 2014-06-12 00:05:24]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  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...)
  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/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/15741)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/18873)

[commit-bot: I haz the power, 2014-06-12 00:07:20]

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

[commit-bot: I haz the power, 2014-06-12 00:07:21]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/15802)

[sashab, 2014-06-12 03:45:03]

The CQ bit was checked by sashab@chromium.org

[commit-bot: I haz the power, 2014-06-12 03:46:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sashab@chromium.org/327743002/20001

[commit-bot: I haz the power, 2014-06-12 07:15:24]

Change committed as 276557

[please use gerrit instead, 2014-06-12 17:16:11]

Appears to have broken
AppListControllerAppInfoDialogBrowserTest.DoShowAppInfoFlow on:
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%2...

I will be reverting via git, because drover and the button above are
experiencing troubles:
https://code.google.com/p/chromium/issues/detail?id=383831

[please use gerrit instead, 2014-06-12 17:49:05]

Reverted in https://codereview.chromium.org/335483003

[sashab, 2014-06-16 01:29:50]

The CQ bit was checked by sashab@chromium.org

[commit-bot: I haz the power, 2014-06-16 01:30:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sashab@chromium.org/327743002/100001

[sashab, 2014-06-16 04:20:56]

The CQ bit was unchecked by sashab@chromium.org

[sashab, 2014-06-17 21:08:46]

The CQ bit was checked by sashab@chromium.org

[commit-bot: I haz the power, 2014-06-17 21:13:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sashab@chromium.org/327743002/130024

[commit-bot: I haz the power, 2014-06-18 06:50:26]

Change committed as 277954