Description: Views/Harmony Remove references to ui/views/layout/layout_constants.h

Description: Views/Harmony Remove references to ui/views/layout/layout_constants.h

Replace references to constants in ui/views/layout/layout_constants.h with their
equivalents using ChromeLayoutProvider. Removed some stale includes of
layout_constants.h

BUG=691897
Review-Url: https://codereview.chromium.org/2867253003
Cr-Commit-Position: refs/heads/master@{#470781}
Committed: https://chromium.googlesource.com/chromium/src/+/143169d83f40fb7e9e9d0c50cd2ed3bf353fb58a

[ananta, 2017-05-09 02:35:42]

ananta@chromium.org changed reviewers:
+ pkasting@chromium.org

[ananta, 2017-05-09 02:35:45]

https://codereview.chromium.org/2867253003/diff/1/chrome/browser/ui/views/arc...
File chrome/browser/ui/views/arc_app_dialog_view.cc (right):

https://codereview.chromium.org/2867253003/diff/1/chrome/browser/ui/views/arc...
chrome/browser/ui/views/arc_app_dialog_view.cc:146:
DISTANCE_PANEL_CONTENT_MARGIN),
Wondering if we should simplify this to a CreatePanel with panel insets?

[ananta, 2017-05-09 02:36:16]

The CQ bit was checked by ananta@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-09 02:37:51]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-09 02:51:43]

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

[commit-bot: I haz the power, 2017-05-09 02:51:45]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ananta, 2017-05-09 03:23:29]

The CQ bit was checked by ananta@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-09 03:24:21]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ananta, 2017-05-09 03:41:14]

The CQ bit was checked by ananta@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-09 03:42:26]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-09 04:34:18]

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

[commit-bot: I haz the power, 2017-05-09 04:34:19]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2017-05-10 22:47:43]

LGTM

https://codereview.chromium.org/2867253003/diff/1/chrome/browser/ui/views/arc...
File chrome/browser/ui/views/arc_app_dialog_view.cc (right):

https://codereview.chromium.org/2867253003/diff/1/chrome/browser/ui/views/arc...
chrome/browser/ui/views/arc_app_dialog_view.cc:146:
DISTANCE_PANEL_CONTENT_MARGIN),
On 2017/05/09 02:35:42, ananta wrote:
> Wondering if we should simplify this to a CreatePanel with panel insets?

Right now CreatePanel() is only defined on GridLayout.  We probably don't want
to switch to GridLayout here.

You could do a separate CL that defines a similar function on BoxLayout and
switches everywhere applicable in the codebase to use it.  That would make sense
if there are multiple places wanting to do this, which I think there are.

You may end up wanting a BoxLayout constructor that takes insets rather than
just horizontal/vertical values while you're at it.

https://codereview.chromium.org/2867253003/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/conflicting_module_view_win.cc (right):

https://codereview.chromium.org/2867253003/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/conflicting_module_view_win.cc:142:
views::DISTANCE_RELATED_CONTROL_HORIZONTAL)));
Seems like this should be the icon-to-label spacing, given how it's used below.

https://codereview.chromium.org/2867253003/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/uninstall_view.cc (right):

https://codereview.chromium.org/2867253003/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/uninstall_view.cc:65: const int
unrelated_vertical_spacing = provider->GetDistanceMetric(
Nit: I probably wouldn't have pulled out to temps things that are only used
once, since I dunno that it's actually a readability win.  Up to you I guess.

[ananta, 2017-05-11 00:42:04]

https://codereview.chromium.org/2867253003/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/conflicting_module_view_win.cc (right):

https://codereview.chromium.org/2867253003/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/conflicting_module_view_win.cc:142:
views::DISTANCE_RELATED_CONTROL_HORIZONTAL)));
On 2017/05/10 22:47:43, Peter Kasting wrote:
> Seems like this should be the icon-to-label spacing, given how it's used
below.

Done.

https://codereview.chromium.org/2867253003/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/uninstall_view.cc (right):

https://codereview.chromium.org/2867253003/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/uninstall_view.cc:65: const int
unrelated_vertical_spacing = provider->GetDistanceMetric(
On 2017/05/10 22:47:43, Peter Kasting wrote:
> Nit: I probably wouldn't have pulled out to temps things that are only used
> once, since I dunno that it's actually a readability win.  Up to you I guess.

Leaving this as is, as it felt better to avoid so many provider calls.

[ananta, 2017-05-11 00:42:14]

The CQ bit was checked by ananta@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-11 00:43:09]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-11 02:21:50]

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

[commit-bot: I haz the power, 2017-05-11 02:21:51]

Dry run: This issue passed the CQ dry run.

[ananta, 2017-05-11 02:33:20]

The CQ bit was checked by ananta@chromium.org

[ananta, 2017-05-11 02:33:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/2867253003/#ps60001
(title: "Address comments")

[commit-bot: I haz the power, 2017-05-11 02:34:52]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-11 02:42:51]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1494470000097580,
"parent_rev": "9326522b744f847e47787b2623b155d1ee2f2c77", "commit_rev":
"143169d83f40fb7e9e9d0c50cd2ed3bf353fb58a"}

[commit-bot: I haz the power, 2017-05-11 02:43:03]

Description was changed from

==========
Description: Views/Harmony Remove references to
ui/views/layout/layout_constants.h

Replace references to constants in ui/views/layout/layout_constants.h with their
equivalents using ChromeLayoutProvider. Removed some stale includes of
layout_constants.h

BUG=691897
==========

to

==========
Description: Views/Harmony Remove references to
ui/views/layout/layout_constants.h

Replace references to constants in ui/views/layout/layout_constants.h with their
equivalents using ChromeLayoutProvider. Removed some stale includes of
layout_constants.h

BUG=691897

Review-Url: https://codereview.chromium.org/2867253003
Cr-Commit-Position: refs/heads/master@{#470781}
Committed:
https://chromium.googlesource.com/chromium/src/+/143169d83f40fb7e9e9d0c50cd2e...
==========

[commit-bot: I haz the power, 2017-05-11 02:43:04]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/143169d83f40fb7e9e9d0c50cd2e...