New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 691897 link

Starred by 6 users
Status: Fixed
Owner:
patricia...@chromium.org
Closed: Aug 2017
Cc:
█ ananta@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocked on:
issue 687349


Sign in to add a comment

Convert existing views code to use LayoutDelegate methods instead of constants

Project Member Reported by pkasting@chromium.org, Feb 14 2017 
Chrome code using constants in layout_constants.h should use the accessor methods in LayoutDelegate instead.  Making these conversions globally will greatly aid in Harmonizing dialogs as in many cases just converting to use these functions is all the dialogs need.

See also similar  bug 687340 .

This is marked as blocked on  bug 687349  because some such accessors may not be easily accessible from code in ui/views/ today, but some accessors are already available, and any code in chrome/ can be fixed immediately, so there's a lot of work that can be done here.

patricialor: Are you available to take this?  It can be done in pieces and doesn't require a lot of spin-up on Harmony stuff, but it would be nice to start it sooner rather than later.  If not, let me know and I'll reassign.

Comment 1 by patricia...@chromium.org, Feb 15 2017 

Yep, thanks for that! I can probably start on this later today or tomorrow, will update the status when I do.

Comment 2 by patricia...@chromium.org, Feb 16 2017

Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 8 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e05be844b2562f3c35ada23f782d2c1aea19adf0

commit e05be844b2562f3c35ada23f782d2c1aea19adf0
Author: patricialor <patricialor@chromium.org>
Date: Wed Mar 08 03:27:26 2017

Views/Harmony: Use LayoutDelegate::GetMetric for views::kPanelVertMargin.

Replace direct references to views::kPanelHorizMargin under
chrome/browser/ui/views/* with a call to LayoutDelegate::GetMetric().

BUG= 691897 

Review-Url: https://codereview.chromium.org/2713803002
Cr-Commit-Position: refs/heads/master@{#455372}

[modify] https://crrev.com/e05be844b2562f3c35ada23f782d2c1aea19adf0/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc
[modify] https://crrev.com/e05be844b2562f3c35ada23f782d2c1aea19adf0/chrome/browser/ui/views/arc_app_dialog_view.cc
[modify] https://crrev.com/e05be844b2562f3c35ada23f782d2c1aea19adf0/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.cc
[modify] https://crrev.com/e05be844b2562f3c35ada23f782d2c1aea19adf0/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc
[modify] https://crrev.com/e05be844b2562f3c35ada23f782d2c1aea19adf0/chrome/browser/ui/views/extensions/extension_uninstall_dialog_view.cc
[modify] https://crrev.com/e05be844b2562f3c35ada23f782d2c1aea19adf0/chrome/browser/ui/views/importer/import_lock_dialog_view.cc
[modify] https://crrev.com/e05be844b2562f3c35ada23f782d2c1aea19adf0/chrome/browser/ui/views/sad_tab_view.cc
[modify] https://crrev.com/e05be844b2562f3c35ada23f782d2c1aea19adf0/chrome/browser/ui/views/sync/profile_signin_confirmation_dialog_views.cc
[modify] https://crrev.com/e05be844b2562f3c35ada23f782d2c1aea19adf0/chrome/browser/ui/views/task_manager_view.cc
[modify] https://crrev.com/e05be844b2562f3c35ada23f782d2c1aea19adf0/chrome/browser/ui/views/webshare/webshare_target_picker_view.cc
[modify] https://crrev.com/e05be844b2562f3c35ada23f782d2c1aea19adf0/chrome/browser/ui/views/website_settings/website_settings_popup_view.cc

Comment 5 by patricia...@chromium.org, Mar 24 2017 

Hi pkasting, I just wanted to confirm which LayoutDelegate metrics map to which constants in layout_constants.h. At the moment I'm using this:

kPanelHorizMargin => PANEL_CONTENT_MARGIN
kPanelVertMargin => PANEL_CONTENT_MARGIN
kPanelVerticalSpacing => ?
kPanelSubVerticalSpacing => ?
kLabelToControlVerticalSpacing => ?
kRelatedControlHorizontalSpacing => RELATED_CONTROL_HORIZONTAL_SPACING
kRelatedControlSmallHorizontalSpacing => RELATED_CONTROL_HORIZONTAL_SPACING_SMALL
kRelatedControlVerticalSpacing => RELATED_CONTROL_VERTICAL_SPACING
kRelatedControlSmallVerticalSpacing => RELATED_CONTROL_VERTICAL_SPACING_SMALL
kUnrelatedControlHorizontalSpacing => UNRELATED_CONTROL_HORIZONTAL_SPACING
kUnrelatedControlLargeHorizontalSpacing => UNRELATED_CONTROL_HORIZONTAL_SPACING_LARGE
kUnrelatedControlVerticalSpacing => UNRELATED_CONTROL_VERTICAL_SPACING
kUnrelatedControlLargeVerticalSpacing => UNRELATED_CONTROL_VERTICAL_SPACING_LARGE
kButtonVEdgeMargin => DIALOG_BUTTON_MARGIN
kButtonHEdgeMargin => DIALOG_BUTTON_MARGIN
kButtonVEdgeMarginNew => DIALOG_BUTTON_MARGIN
kButtonHEdgeMarginNew => DIALOG_BUTTON_MARGIN
kCloseButtonMargin => DIALOG_CLOSE_BUTTON_MARGIN
kRelatedButtonHSpacing => RELATED_BUTTON_HORIZONTAL_SPACING
kCheckboxIndent => SUBSECTION_HORIZONTAL_INDENT
kItemLabelSpacing => RELATED_LABEL_HORIZONTAL_SPACING
kButtonHorizontalPadding => BUTTON_HORIZONTAL_PADDING
kMinimumButtonWidth => BUTTON_MINIMUM_WIDTH
kDialogMinimumButtonWidth => DIALOG_BUTTON_MINIMUM_WIDTH

Is that correct? For the constants which point to question marks, should I be exposing these constants in LayoutDelegate as I go? Are there any specific cases in which we want to merge any of them with existing ones?

Thanks!

Comment 6 by pkasting@chromium.org, Mar 24 2017 

Looks correct offhand.

I'd map kLabelToControlVerticalSpacing to RELATED_CONTROL_VERTICAL_SPACING.

The panel vertical spacing ones are used almost exclusively by SadTabView.  Because of this isolated use, they're effectively file-local, and I'd probably try mapping them to UNRELATED_CONTROL_VERTICAL_SPACING[_LARGE], and seeing how the result looks.  (There's one use in CreateApplicationShortcutView to sanity-check in this case too.)
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 27 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8a8f67e659f819a459a6076e8fcc634e755dabf6

commit 8a8f67e659f819a459a6076e8fcc634e755dabf6
Author: patricialor <patricialor@chromium.org>
Date: Mon Mar 27 05:37:17 2017

Views/Harmony: Replace layout constants in chrome/browser/ui/views/extensions.

Replace all direct references to constants in ui/views/layout/layout_constants.h
under chrome/browser/ui/views/extensions/* with LayoutDelegate metrics.

BUG= 691897 

Review-Url: https://codereview.chromium.org/2753243002
Cr-Commit-Position: refs/heads/master@{#459707}

[modify] https://crrev.com/8a8f67e659f819a459a6076e8fcc634e755dabf6/chrome/browser/ui/views/extensions/bookmark_app_confirmation_view.cc
[modify] https://crrev.com/8a8f67e659f819a459a6076e8fcc634e755dabf6/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc
[modify] https://crrev.com/8a8f67e659f819a459a6076e8fcc634e755dabf6/chrome/browser/ui/views/extensions/extension_installed_bubble_view.cc
[modify] https://crrev.com/8a8f67e659f819a459a6076e8fcc634e755dabf6/chrome/browser/ui/views/extensions/extension_uninstall_dialog_view.cc
[modify] https://crrev.com/8a8f67e659f819a459a6076e8fcc634e755dabf6/chrome/browser/ui/views/extensions/media_galleries_dialog_views.cc
[modify] https://crrev.com/8a8f67e659f819a459a6076e8fcc634e755dabf6/chrome/browser/ui/views/extensions/media_gallery_checkbox_view.cc
[modify] https://crrev.com/8a8f67e659f819a459a6076e8fcc634e755dabf6/chrome/browser/ui/views/harmony/harmony_layout_delegate.cc
[modify] https://crrev.com/8a8f67e659f819a459a6076e8fcc634e755dabf6/chrome/browser/ui/views/harmony/layout_delegate.cc
[modify] https://crrev.com/8a8f67e659f819a459a6076e8fcc634e755dabf6/chrome/browser/ui/views/harmony/layout_delegate.h
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 7 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8b6a69f075e5f117b6a9df69cb42ebd6ffe52fa6

commit 8b6a69f075e5f117b6a9df69cb42ebd6ffe52fa6
Author: patricialor <patricialor@chromium.org>
Date: Fri Apr 07 05:30:27 2017

Views/Harmony: Remove references to layout constants in c/b/ui/views/sync.

Remove unnecessary includes for ui/views/layout/layout_constants.h and do some
refactoring to remove references to layout constants in
ProfileSigninConfirmationDialogViews under chrome/browser/ui/views/sync/*.

See
https://drive.google.com/file/d/0BzEa5HU1aAqBOXBDazdRT245Wk0/view?usp=sharing
for before/after screenshots of ProfileSigninConfirmationDialogViews.

BUG= 691897 

Review-Url: https://codereview.chromium.org/2799163002
Cr-Commit-Position: refs/heads/master@{#462771}

[modify] https://crrev.com/8b6a69f075e5f117b6a9df69cb42ebd6ffe52fa6/chrome/browser/ui/views/sync/bubble_sync_promo_view.cc
[modify] https://crrev.com/8b6a69f075e5f117b6a9df69cb42ebd6ffe52fa6/chrome/browser/ui/views/sync/one_click_signin_dialog_view.cc
[modify] https://crrev.com/8b6a69f075e5f117b6a9df69cb42ebd6ffe52fa6/chrome/browser/ui/views/sync/profile_signin_confirmation_dialog_views.cc

Comment 9 by pkasting@chromium.org, Apr 11 2017

Cc: ananta@chromium.org

Comment 10 by ananta@chromium.org, Apr 14 2017 

Starting out with chrome/browser/ui/views/infobars
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 14 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/82438b804d2a93c7ba02ba543ac278eb7142473c

commit 82438b804d2a93c7ba02ba543ac278eb7142473c
Author: ananta <ananta@chromium.org>
Date: Fri Apr 14 19:50:54 2017

Replace layout constants in chrome/browser/ui/views/infobars.

Replace references to constants in ui/views/layout/layout_constants.h with their
equivalents using ChromeLayoutProvider.

BUG= 691897 

Review-Url: https://codereview.chromium.org/2812243007
Cr-Commit-Position: refs/heads/master@{#464779}

[modify] https://crrev.com/82438b804d2a93c7ba02ba543ac278eb7142473c/chrome/browser/ui/views/infobars/confirm_infobar.cc
[modify] https://crrev.com/82438b804d2a93c7ba02ba543ac278eb7142473c/chrome/browser/ui/views/infobars/infobar_view.cc
Project Member

Comment 12 by bugdroid1@chromium.org, Apr 26 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/66f7a47ed009c67b5b1663af630c9cc4c0cb351f

commit 66f7a47ed009c67b5b1663af630c9cc4c0cb351f
Author: ananta <ananta@chromium.org>
Date: Wed Apr 26 05:32:35 2017

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/2837153003
Cr-Commit-Position: refs/heads/master@{#467245}

[modify] https://crrev.com/66f7a47ed009c67b5b1663af630c9cc4c0cb351f/chrome/browser/ui/views/confirm_bubble_views.cc
[modify] https://crrev.com/66f7a47ed009c67b5b1663af630c9cc4c0cb351f/chrome/browser/ui/views/content_setting_bubble_contents.cc
[modify] https://crrev.com/66f7a47ed009c67b5b1663af630c9cc4c0cb351f/chrome/browser/ui/views/create_application_shortcut_view.cc
[modify] https://crrev.com/66f7a47ed009c67b5b1663af630c9cc4c0cb351f/chrome/browser/ui/views/crypto_module_password_dialog_view.cc
[modify] https://crrev.com/66f7a47ed009c67b5b1663af630c9cc4c0cb351f/chrome/browser/ui/views/crypto_module_password_dialog_view_unittest.cc
[modify] https://crrev.com/66f7a47ed009c67b5b1663af630c9cc4c0cb351f/chrome/browser/ui/views/first_run_bubble.cc
[modify] https://crrev.com/66f7a47ed009c67b5b1663af630c9cc4c0cb351f/chrome/browser/ui/views/first_run_bubble_unittest.cc
[modify] https://crrev.com/66f7a47ed009c67b5b1663af630c9cc4c0cb351f/chrome/browser/ui/views/first_run_dialog.cc
[modify] https://crrev.com/66f7a47ed009c67b5b1663af630c9cc4c0cb351f/chrome/browser/ui/views/network_profile_bubble_view.cc
[modify] https://crrev.com/66f7a47ed009c67b5b1663af630c9cc4c0cb351f/chrome/browser/ui/views/outdated_upgrade_bubble_view.cc
[modify] https://crrev.com/66f7a47ed009c67b5b1663af630c9cc4c0cb351f/chrome/browser/ui/views/session_crashed_bubble_view.cc
Project Member

Comment 13 by bugdroid1@chromium.org, Apr 29 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5ecf11b9c4f09559336a513b4d0486a3f74d2625

commit 5ecf11b9c4f09559336a513b4d0486a3f74d2625
Author: ananta <ananta@chromium.org>
Date: Sat Apr 29 00:45:10 2017

Description: Replace layout constants in chrome/browser/extensions and chrome/browser/first_run

Changes in this patch as below:
1. Replace references to constants in ui/views/layout/layout_constants.h with their
   equivalents using ChromeLayoutProvider.

2. Changed the RequestFileSystemDialogView class to use a simpler FillLayout instead of
   a BoxLayout. This class now subclasses DialogDelegateView instead of DialogDelegate for
   simplicity. Removed the dependency on Extension and Volume datatypes from this class, which
   enables us to easily test the dialog. The extension name, volume label and id are passed in.
   We maintain the contract of invoking the cancel callback if the volume label is empty. That is
   done in the caller ConsentProviderDelegate::ShowDialog().

3. Added browser tests for displaying the TryChromeDialogView and RequestFileSystemDialogView dialogs.

4. Made the TryChromeDialogTest class a friend of the TryChromeDialogView class. This allows it to
   instantiate an instance of this class. The TryChromeDialogView class no longer puts up a modal
   dialog by default. This is controlled by the DialogType enum, which for chrome is always MODAL.
   For the browser tests we pass in MODELESS here, as the loop is run by the test. The ShowModal() method
   is renamed to ShowDialog(). It takes an additional enum UsageType which controls whether we are in testing
   mode. In this case we always want the dialog to be displayed.

BUG= 691897 
TEST=Covered by tests RequestFileSystemDialogTest.InvokeDialog_default and TryChromeDialogTest.InvokeDialog_default
TBR=grt

Review-Url: https://codereview.chromium.org/2816293002
Cr-Commit-Position: refs/heads/master@{#468195}

[modify] https://crrev.com/5ecf11b9c4f09559336a513b4d0486a3f74d2625/chrome/browser/BUILD.gn
[modify] https://crrev.com/5ecf11b9c4f09559336a513b4d0486a3f74d2625/chrome/browser/DEPS
[modify] https://crrev.com/5ecf11b9c4f09559336a513b4d0486a3f74d2625/chrome/browser/chrome_browser_main.cc
[modify] https://crrev.com/5ecf11b9c4f09559336a513b4d0486a3f74d2625/chrome/browser/extensions/BUILD.gn
[modify] https://crrev.com/5ecf11b9c4f09559336a513b4d0486a3f74d2625/chrome/browser/extensions/api/file_system/DEPS
[modify] https://crrev.com/5ecf11b9c4f09559336a513b4d0486a3f74d2625/chrome/browser/extensions/api/file_system/file_system_api.cc
[modify] https://crrev.com/5ecf11b9c4f09559336a513b4d0486a3f74d2625/chrome/browser/first_run/try_chrome_dialog_view_browsertest.cc
[modify] https://crrev.com/5ecf11b9c4f09559336a513b4d0486a3f74d2625/chrome/browser/ui/BUILD.gn
[add] https://crrev.com/5ecf11b9c4f09559336a513b4d0486a3f74d2625/chrome/browser/ui/views/extensions/request_file_system_dialog_browsertest.cc
[rename] https://crrev.com/5ecf11b9c4f09559336a513b4d0486a3f74d2625/chrome/browser/ui/views/extensions/request_file_system_dialog_view.cc
[rename] https://crrev.com/5ecf11b9c4f09559336a513b4d0486a3f74d2625/chrome/browser/ui/views/extensions/request_file_system_dialog_view.h
[rename] https://crrev.com/5ecf11b9c4f09559336a513b4d0486a3f74d2625/chrome/browser/ui/views/try_chrome_dialog_view.cc
[rename] https://crrev.com/5ecf11b9c4f09559336a513b4d0486a3f74d2625/chrome/browser/ui/views/try_chrome_dialog_view.h
[modify] https://crrev.com/5ecf11b9c4f09559336a513b4d0486a3f74d2625/chrome/test/BUILD.gn

Comment 14 by vabr@chromium.org, May 2 2017 

r468195 seems to have caused test failures on 
https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/20886
Find-it tool blames it and I as a sheriff agree enough to try a speculative revert.
Sample log attached.
logs.txt
61.7 KB View Download

Comment 15 by ananta@chromium.org, May 2 2017 

The leaks have been fixed here https://codereview.chromium.org/2855663002

Please don't revert yet
Project Member

Comment 16 by bugdroid1@chromium.org, May 11 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/143169d83f40fb7e9e9d0c50cd2ed3bf353fb58a

commit 143169d83f40fb7e9e9d0c50cd2ed3bf353fb58a
Author: ananta <ananta@chromium.org>
Date: Thu May 11 02:42:48 2017

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}

[modify] https://crrev.com/143169d83f40fb7e9e9d0c50cd2ed3bf353fb58a/chrome/browser/ui/views/arc_app_dialog_view.cc
[modify] https://crrev.com/143169d83f40fb7e9e9d0c50cd2ed3bf353fb58a/chrome/browser/ui/views/certificate_selector.cc
[modify] https://crrev.com/143169d83f40fb7e9e9d0c50cd2ed3bf353fb58a/chrome/browser/ui/views/conflicting_module_view_win.cc
[modify] https://crrev.com/143169d83f40fb7e9e9d0c50cd2ed3bf353fb58a/chrome/browser/ui/views/uninstall_view.cc
Project Member

Comment 17 by bugdroid1@chromium.org, May 18 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f195c43cff20208422063e0d9179b4a29415c28d

commit f195c43cff20208422063e0d9179b4a29415c28d
Author: patricialor <patricialor@chromium.org>
Date: Thu May 18 08:00:50 2017

Views/Harmony: Remove references to layout constants in c/b/u/v/passwords.

Remove unnecessary includes for ui/views/layout/layout_constants.h and do some
refactoring to remove references to layout constants in
chrome/browser/ui/views/passwords/*.

This CL also improves the TestBrowserDialog tests for PasswordDialogViewTest and
ManagePasswordsBubbleDialogViewTest.

BUG= 691897 

Review-Url: https://codereview.chromium.org/2869683003
Cr-Commit-Position: refs/heads/master@{#472733}

[modify] https://crrev.com/f195c43cff20208422063e0d9179b4a29415c28d/chrome/browser/ui/passwords/manage_passwords_test.cc
[modify] https://crrev.com/f195c43cff20208422063e0d9179b4a29415c28d/chrome/browser/ui/views/passwords/account_chooser_dialog_view.cc
[modify] https://crrev.com/f195c43cff20208422063e0d9179b4a29415c28d/chrome/browser/ui/views/passwords/auto_signin_first_run_dialog_view.cc
[modify] https://crrev.com/f195c43cff20208422063e0d9179b4a29415c28d/chrome/browser/ui/views/passwords/credentials_item_view.cc
[modify] https://crrev.com/f195c43cff20208422063e0d9179b4a29415c28d/chrome/browser/ui/views/passwords/credentials_selection_view.cc
[modify] https://crrev.com/f195c43cff20208422063e0d9179b4a29415c28d/chrome/browser/ui/views/passwords/manage_password_items_view.cc
[modify] https://crrev.com/f195c43cff20208422063e0d9179b4a29415c28d/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc
[modify] https://crrev.com/f195c43cff20208422063e0d9179b4a29415c28d/chrome/browser/ui/views/passwords/password_dialog_view_browsertest.cc
Project Member

Comment 20 by bugdroid1@chromium.org, May 27 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cf4f838225c132dee06cc21dd6a576347734589b

commit cf4f838225c132dee06cc21dd6a576347734589b
Author: ananta <ananta@chromium.org>
Date: Sat May 27 03:07:56 2017

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.

BUG= 691897 
TBR=sky

Review-Url: https://codereview.chromium.org/2904443006
Cr-Commit-Position: refs/heads/master@{#475229}

[modify] https://crrev.com/cf4f838225c132dee06cc21dd6a576347734589b/chrome/browser/ui/views/ime/ime_warning_bubble_view.cc
[modify] https://crrev.com/cf4f838225c132dee06cc21dd6a576347734589b/chrome/browser/ui/views/importer/import_lock_dialog_view.cc
[modify] https://crrev.com/cf4f838225c132dee06cc21dd6a576347734589b/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc
[modify] https://crrev.com/cf4f838225c132dee06cc21dd6a576347734589b/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc
[modify] https://crrev.com/cf4f838225c132dee06cc21dd6a576347734589b/chrome/browser/ui/views/profiles/profile_chooser_view.cc
[modify] https://crrev.com/cf4f838225c132dee06cc21dd6a576347734589b/chrome/browser/ui/views/translate/translate_bubble_view.cc
[modify] https://crrev.com/cf4f838225c132dee06cc21dd6a576347734589b/chrome/browser/ui/views/translate/translate_bubble_view_unittest.cc
[modify] https://crrev.com/cf4f838225c132dee06cc21dd6a576347734589b/ui/views/bubble/bubble_dialog_delegate.cc
[modify] https://crrev.com/cf4f838225c132dee06cc21dd6a576347734589b/ui/views/bubble/bubble_frame_view.cc
[modify] https://crrev.com/cf4f838225c132dee06cc21dd6a576347734589b/ui/views/controls/button/label_button.cc
[modify] https://crrev.com/cf4f838225c132dee06cc21dd6a576347734589b/ui/views/controls/message_box_view.cc
[modify] https://crrev.com/cf4f838225c132dee06cc21dd6a576347734589b/ui/views/layout/grid_layout.cc
[modify] https://crrev.com/cf4f838225c132dee06cc21dd6a576347734589b/ui/views/views_delegate.cc
[modify] https://crrev.com/cf4f838225c132dee06cc21dd6a576347734589b/ui/views/window/dialog_delegate.cc
Project Member

Comment 21 by bugdroid1@chromium.org, May 31 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cb36afa5925c2971e27b9653802a74264d76fa2b

commit cb36afa5925c2971e27b9653802a74264d76fa2b
Author: ananta <ananta@chromium.org>
Date: Wed May 31 23:13:01 2017

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.

BUG= 691897 

Review-Url: https://codereview.chromium.org/2907933002
Cr-Commit-Position: refs/heads/master@{#476081}

[modify] https://crrev.com/cb36afa5925c2971e27b9653802a74264d76fa2b/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc
[modify] https://crrev.com/cb36afa5925c2971e27b9653802a74264d76fa2b/chrome/browser/ui/views/apps/app_info_dialog/app_info_footer_panel.cc
[modify] https://crrev.com/cb36afa5925c2971e27b9653802a74264d76fa2b/chrome/browser/ui/views/apps/app_info_dialog/app_info_header_panel.cc
[modify] https://crrev.com/cb36afa5925c2971e27b9653802a74264d76fa2b/chrome/browser/ui/views/apps/app_info_dialog/app_info_panel.cc
[modify] https://crrev.com/cb36afa5925c2971e27b9653802a74264d76fa2b/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel.cc
[modify] https://crrev.com/cb36afa5925c2971e27b9653802a74264d76fa2b/chrome/browser/ui/views/apps/app_info_dialog/app_info_summary_panel.cc
[modify] https://crrev.com/cb36afa5925c2971e27b9653802a74264d76fa2b/chrome/browser/ui/views/apps/app_info_dialog/arc_app_info_links_panel.cc
[modify] https://crrev.com/cb36afa5925c2971e27b9653802a74264d76fa2b/chrome/browser/ui/views/desktop_ios_promotion/desktop_ios_promotion_footnote_view.cc
[modify] https://crrev.com/cb36afa5925c2971e27b9653802a74264d76fa2b/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc
[modify] https://crrev.com/cb36afa5925c2971e27b9653802a74264d76fa2b/chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.cc
[modify] https://crrev.com/cb36afa5925c2971e27b9653802a74264d76fa2b/chrome/browser/ui/views/webshare/webshare_target_picker_view.cc
Project Member

Comment 22 by bugdroid1@chromium.org, Jun 2 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/127b114a4ab9474637dfbe111fac9fadbd7d4b0c

commit 127b114a4ab9474637dfbe111fac9fadbd7d4b0c
Author: ananta <ananta@chromium.org>
Date: Fri Jun 02 02:00:02 2017

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.

BUG= 691897 

Review-Url: https://codereview.chromium.org/2919903002
Cr-Commit-Position: refs/heads/master@{#476520}

[modify] https://crrev.com/127b114a4ab9474637dfbe111fac9fadbd7d4b0c/chrome/browser/chromeos/DEPS
[modify] https://crrev.com/127b114a4ab9474637dfbe111fac9fadbd7d4b0c/chrome/browser/chromeos/attestation/platform_verification_dialog.cc
[modify] https://crrev.com/127b114a4ab9474637dfbe111fac9fadbd7d4b0c/chrome/browser/chromeos/enrollment_dialog_view.cc
[modify] https://crrev.com/127b114a4ab9474637dfbe111fac9fadbd7d4b0c/chrome/browser/chromeos/login/ui/simple_web_view_dialog.cc
[modify] https://crrev.com/127b114a4ab9474637dfbe111fac9fadbd7d4b0c/chrome/browser/chromeos/options/network_config_view.cc
[modify] https://crrev.com/127b114a4ab9474637dfbe111fac9fadbd7d4b0c/chrome/browser/chromeos/power/idle_action_warning_dialog_view.cc
[modify] https://crrev.com/127b114a4ab9474637dfbe111fac9fadbd7d4b0c/chrome/browser/chromeos/profiles/multiprofiles_intro_dialog.cc
[modify] https://crrev.com/127b114a4ab9474637dfbe111fac9fadbd7d4b0c/chrome/browser/chromeos/ui/request_pin_view.cc
[modify] https://crrev.com/127b114a4ab9474637dfbe111fac9fadbd7d4b0c/chrome/browser/ui/views/task_manager_view.cc
Project Member

Comment 23 by bugdroid1@chromium.org, Jun 6 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8c0bf389d78991280270f383043405f0098a9a85

commit 8c0bf389d78991280270f383043405f0098a9a85
Author: ananta <ananta@chromium.org>
Date: Tue Jun 06 21:52:47 2017

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.

BUG= 691897 

Review-Url: https://codereview.chromium.org/2926533002
Cr-Commit-Position: refs/heads/master@{#477424}

[modify] https://crrev.com/8c0bf389d78991280270f383043405f0098a9a85/chrome/browser/ui/views/sad_tab_view.cc
Project Member

Comment 24 by bugdroid1@chromium.org, Jun 9 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/478f3b20593b53ce53fb65e9b70fb3d59f15d17d

commit 478f3b20593b53ce53fb65e9b70fb3d59f15d17d
Author: ananta <ananta@chromium.org>
Date: Fri Jun 09 21:40:09 2017

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.

The approach taken is to push the DISTANCE_UNRELATED_CONTROL_VERTICAL constant down to
views::LayoutProvider. Other approaches considered were to use DISTANCE_RELATED_CONTROL_VERTICAL
here which did not seem correct or define a local constant in the dialog_example.cc

BUG= 691897 

Review-Url: https://codereview.chromium.org/2934503002
Cr-Commit-Position: refs/heads/master@{#478427}

[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/chromeos/enrollment_dialog_view.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/apps/app_info_dialog/app_info_summary_panel.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/autofill/save_card_bubble_views.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/collected_cookies_views.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/content_setting_bubble_contents.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/crypto_module_password_dialog_view.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/desktop_ios_promotion/desktop_ios_promotion_bubble_view.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/extensions/chooser_dialog_view.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/harmony/chrome_layout_provider.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/harmony/chrome_layout_provider.h
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/harmony/harmony_layout_provider.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/login_view.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/profiles/profile_chooser_view.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/sad_tab_view.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/tab_modal_confirm_dialog_views.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/translate/translate_bubble_view.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/try_chrome_dialog_view.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/chrome/browser/ui/views/uninstall_view.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/ui/views/examples/dialog_example.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/ui/views/layout/layout_provider.cc
[modify] https://crrev.com/478f3b20593b53ce53fb65e9b70fb3d59f15d17d/ui/views/layout/layout_provider.h
Project Member

Comment 26 by bugdroid1@chromium.org, Jun 15 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f131ea653e12fd70bcf10dc91ac5a731734e41ea

commit f131ea653e12fd70bcf10dc91ac5a731734e41ea
Author: ananta <ananta@chromium.org>
Date: Thu Jun 15 06:03:14 2017

Remove references to ui/views/layout/layout_constants.h

This patch removes layout_constants.h and inlines them in layout_provider.cc and
chrome_layout_provider.cc. Last step of the set of changes which were around replacing
these constants with their equivalents from the layout provider.

BUG= 691897 
TBR=sky

Review-Url: https://codereview.chromium.org/2942453002
Cr-Commit-Position: refs/heads/master@{#479616}

[modify] https://crrev.com/f131ea653e12fd70bcf10dc91ac5a731734e41ea/chrome/browser/ui/views/harmony/chrome_layout_provider.cc
[modify] https://crrev.com/f131ea653e12fd70bcf10dc91ac5a731734e41ea/ui/views/BUILD.gn
[modify] https://crrev.com/f131ea653e12fd70bcf10dc91ac5a731734e41ea/ui/views/examples/dialog_example.cc
[delete] https://crrev.com/36f87e2389e738d2b6690115d7c212e24ad053e6/ui/views/layout/layout_constants.h
[modify] https://crrev.com/f131ea653e12fd70bcf10dc91ac5a731734e41ea/ui/views/layout/layout_provider.cc

Comment 27 by pkasting@chromium.org, Aug 8 2017

Status: Fixed (was: Started)
►

Sign in to add a comment