Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(411)

Issue 2881183003: Add views::View::set_preferred_size, use it in a few places. (Closed)

Created:
3 years, 7 months ago by Evan Stade
Modified:
3 years, 7 months ago
Reviewers:
sky
CC:
chromium-reviews, kalyank, sadrul, derat+watch_chromium.org, tfarina
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Add views::View::set_preferred_size, use it in a few places. This sets a preferred size which will override the one provided by the layout manager or a subclass's GetPreferredSize override. It also renames the override-able function to CalculatePreferredSize and makes it protected. Eventually all GetPreferredSize overrides should be renamed. This patch also gives precedence to LayoutManager::GetPreferredSize over View::CalculatePreferredSize, although this distinction probably rarely crops up. BUG=none Review-Url: https://codereview.chromium.org/2881183003 Cr-Commit-Position: refs/heads/master@{#472155} Committed: https://chromium.googlesource.com/chromium/src/+/ae01bc6a3a50e49466bd2c1455ff011f2d006bf4

Patch Set 1 #

Total comments: 2

Patch Set 2 : discourage usage #

Patch Set 3 : auto* #

Unified diffs Side-by-side diffs Delta from patch set Stats (+42 lines, -94 lines) Patch
M ash/BUILD.gn View 1 chunk +0 lines, -2 lines 0 comments Download
M ash/system/power/power_status_view.cc View 1 chunk +0 lines, -1 line 0 comments Download
M ash/system/screen_layout_observer.cc View 1 chunk +0 lines, -1 line 0 comments Download
D ash/system/tray/fixed_sized_image_view.h View 1 chunk +0 lines, -31 lines 0 comments Download
D ash/system/tray/fixed_sized_image_view.cc View 1 chunk +0 lines, -20 lines 0 comments Download
M ash/system/tray/system_menu_button.cc View 2 chunks +1 line, -5 lines 0 comments Download
M ash/system/tray/tray_popup_utils.cc View 1 2 2 chunks +6 lines, -5 lines 0 comments Download
M ash/system/tray_tracing.cc View 1 chunk +0 lines, -1 line 0 comments Download
M ash/system/update/tray_update.cc View 1 chunk +0 lines, -1 line 0 comments Download
M ash/system/user/user_card_view.cc View 1 2 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/ui/views/profiles/profile_chooser_view.cc View 2 chunks +3 lines, -16 lines 0 comments Download
M ui/views/controls/image_view.h View 1 chunk +1 line, -1 line 0 comments Download
M ui/views/controls/image_view.cc View 2 chunks +6 lines, -6 lines 0 comments Download
M ui/views/view.h View 1 3 chunks +15 lines, -0 lines 0 comments Download
M ui/views/view.cc View 2 chunks +7 lines, -1 line 0 comments Download

Messages

Total messages: 21 (12 generated)
Evan Stade
there are more classes that this would obviate, and updating more of them is left ...
3 years, 7 months ago (2017-05-15 21:23:45 UTC) #2
sky
LGTM https://codereview.chromium.org/2881183003/diff/1/ui/views/view.h File ui/views/view.h (right): https://codereview.chromium.org/2881183003/diff/1/ui/views/view.h#newcode283 ui/views/view.h:283: // may differ. Please also add a comment ...
3 years, 7 months ago (2017-05-15 21:56:16 UTC) #5
Evan Stade
https://codereview.chromium.org/2881183003/diff/1/ui/views/view.h File ui/views/view.h (right): https://codereview.chromium.org/2881183003/diff/1/ui/views/view.h#newcode283 ui/views/view.h:283: // may differ. On 2017/05/15 21:56:16, sky wrote: > ...
3 years, 7 months ago (2017-05-15 22:04:04 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2881183003/20001
3 years, 7 months ago (2017-05-15 22:13:53 UTC) #9
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/426675)
3 years, 7 months ago (2017-05-15 22:49:01 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2881183003/20001
3 years, 7 months ago (2017-05-16 00:18:18 UTC) #13
commit-bot: I haz the power
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_chromeos_ozone_rel_ng/builds/385073)
3 years, 7 months ago (2017-05-16 00:59:57 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2881183003/40001
3 years, 7 months ago (2017-05-16 15:05:08 UTC) #18
commit-bot: I haz the power
3 years, 7 months ago (2017-05-16 18:01:46 UTC) #21
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/ae01bc6a3a50e49466bd2c1455ff...

Powered by Google App Engine
This is Rietveld 408576698