|
|
DescriptionOnly perform dialog Layout() once during Widget::Init().
For Widgets that require a non-client view Layout is currently performed
twice during Widget::Init():
- once before the initial bounds of the Widget are determined
(when setting the RootView's ContentsView), and
- once after (when setting the initial bounds).
The first Layout() is unnecessary and can lead to bugs when the RootView
attempts a layout at an invalid size. Layout of text in particular can
be affected since giving it a size influences text wrapping, and
subsequent answers for Label::GetPreferredSize() will change.
In particular, on Mac and Linux, zero-sized windows are not valid, so a
NativeWidget may take on a size of 1x1 until its initial bounds is
determined. If RootView performs a layout at this size, it can "lock" a
Label to an non-preferred, but non-zero, size. This causes Label (and
gfx::RenderText) to report a new preferred size of 0x0 (since no text
will fit). So when Widget initialization later tries to determine what
the initial bounds of the Widget should be, the Label is not accounted
for. Keeping the Label with a 0x0 size instead causes
Label::GetPreferredSize() to report a meaningful preferred size in more
cases.
But also layout is expensive and shouldn't be done multiple times
whenever a Widget is created if we can avoid it.
BUG=671820
Review-Url: https://codereview.chromium.org/2712383002
Cr-Commit-Position: refs/heads/master@{#453365}
Committed: https://chromium.googlesource.com/chromium/src/+/23e39591317d71645dda43792fadd858bdf892ab
Patch Set 1 : Just the test - it currently fails #Patch Set 2 : With the fix - linux/chromeos don't like this one #Patch Set 3 : Initial layout? #Patch Set 4 : test desktop widgets #Patch Set 5 : selfnits #
Messages
Total messages: 28 (24 generated)
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
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_...)
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Only perform RootView::Layout() once during Widget::Init. For Widgets that require a non-client view it's currently performed twice: - once before the initial bounds of the Widget are determined, and - once after (when setting the inital bounds). The first Layout() is unnecessary and can lead to bugs when the RootView attempts a layout at an invalid size. Layout of text in particular can be affected since giving it a size influences text wrapping, and later results of Label::GetPreferredSize() change. In particular, on Mac and Linux, zero-sized windows are not valid, so a NativeWidget may take on a size of 1x1 until its inital bounds is determined. If RootView performs a layout at this size, it can "lock" a Label to an non-preferred, but non-zero, size. This causes Label (and gfx::RenderText) to report a new preferred size of 0x0 (since no text will fit). So when Widget initialization later tries to determine what the initial bounds of the Widget should be, the Label is not accounted for. BUG=671820 ========== to ========== Only perform dialog Layout() once during Widget::Init(). For Widgets that require a non-client view it's currently performed twice: - once before the initial bounds of the Widget are determined, and - once after (when setting the inital bounds). The first Layout() is unnecessary and can lead to bugs when the RootView attempts a layout at an invalid size. Layout of text in particular can be affected since giving it a size influences text wrapping, and later results of Label::GetPreferredSize() change. In particular, on Mac and Linux, zero-sized windows are not valid, so a NativeWidget may take on a size of 1x1 until its inital bounds is determined. If RootView performs a layout at this size, it can "lock" a Label to an non-preferred, but non-zero, size. This causes Label (and gfx::RenderText) to report a new preferred size of 0x0 (since no text will fit). So when Widget initialization later tries to determine what the initial bounds of the Widget should be, the Label is not accounted for. BUG=671820 ==========
Description was changed from ========== Only perform dialog Layout() once during Widget::Init(). For Widgets that require a non-client view it's currently performed twice: - once before the initial bounds of the Widget are determined, and - once after (when setting the inital bounds). The first Layout() is unnecessary and can lead to bugs when the RootView attempts a layout at an invalid size. Layout of text in particular can be affected since giving it a size influences text wrapping, and later results of Label::GetPreferredSize() change. In particular, on Mac and Linux, zero-sized windows are not valid, so a NativeWidget may take on a size of 1x1 until its inital bounds is determined. If RootView performs a layout at this size, it can "lock" a Label to an non-preferred, but non-zero, size. This causes Label (and gfx::RenderText) to report a new preferred size of 0x0 (since no text will fit). So when Widget initialization later tries to determine what the initial bounds of the Widget should be, the Label is not accounted for. BUG=671820 ========== to ========== Only perform dialog Layout() once during Widget::Init(). For Widgets that require a non-client view Layout is currently performed twice during Widget::Init: - once before the initial bounds of the Widget are determined, and - once after (when setting the initial bounds). The first Layout() is unnecessary and can lead to bugs when the RootView attempts a layout at an invalid size. Layout of text in particular can be affected since giving it a size influences text wrapping, and subsequent answers for Label::GetPreferredSize() will change. In particular, on Mac and Linux, zero-sized windows are not valid, so a NativeWidget may take on a size of 1x1 until its initial bounds is determined. If RootView performs a layout at this size, it can "lock" a Label to an non-preferred, but non-zero, size. This causes Label (and gfx::RenderText) to report a new preferred size of 0x0 (since no text will fit). So when Widget initialization later tries to determine what the initial bounds of the Widget should be, the Label is not accounted for. BUG=671820 ==========
Description was changed from ========== Only perform dialog Layout() once during Widget::Init(). For Widgets that require a non-client view Layout is currently performed twice during Widget::Init: - once before the initial bounds of the Widget are determined, and - once after (when setting the initial bounds). The first Layout() is unnecessary and can lead to bugs when the RootView attempts a layout at an invalid size. Layout of text in particular can be affected since giving it a size influences text wrapping, and subsequent answers for Label::GetPreferredSize() will change. In particular, on Mac and Linux, zero-sized windows are not valid, so a NativeWidget may take on a size of 1x1 until its initial bounds is determined. If RootView performs a layout at this size, it can "lock" a Label to an non-preferred, but non-zero, size. This causes Label (and gfx::RenderText) to report a new preferred size of 0x0 (since no text will fit). So when Widget initialization later tries to determine what the initial bounds of the Widget should be, the Label is not accounted for. BUG=671820 ========== to ========== Only perform dialog Layout() once during Widget::Init(). For Widgets that require a non-client view Layout is currently performed twice during Widget::Init: - once before the initial bounds of the Widget are determined, and - once after (when setting the initial bounds). The first Layout() is unnecessary and can lead to bugs when the RootView attempts a layout at an invalid size. Layout of text in particular can be affected since giving it a size influences text wrapping, and subsequent answers for Label::GetPreferredSize() will change. In particular, on Mac and Linux, zero-sized windows are not valid, so a NativeWidget may take on a size of 1x1 until its initial bounds is determined. If RootView performs a layout at this size, it can "lock" a Label to an non-preferred, but non-zero, size. This causes Label (and gfx::RenderText) to report a new preferred size of 0x0 (since no text will fit). So when Widget initialization later tries to determine what the initial bounds of the Widget should be, the Label is not accounted for. But also layout is expensive and shouldn't be done multiple times whenever a Widget is created. BUG=671820 ==========
tapted@chromium.org changed reviewers: + sky@chromium.org
Description was changed from ========== Only perform dialog Layout() once during Widget::Init(). For Widgets that require a non-client view Layout is currently performed twice during Widget::Init: - once before the initial bounds of the Widget are determined, and - once after (when setting the initial bounds). The first Layout() is unnecessary and can lead to bugs when the RootView attempts a layout at an invalid size. Layout of text in particular can be affected since giving it a size influences text wrapping, and subsequent answers for Label::GetPreferredSize() will change. In particular, on Mac and Linux, zero-sized windows are not valid, so a NativeWidget may take on a size of 1x1 until its initial bounds is determined. If RootView performs a layout at this size, it can "lock" a Label to an non-preferred, but non-zero, size. This causes Label (and gfx::RenderText) to report a new preferred size of 0x0 (since no text will fit). So when Widget initialization later tries to determine what the initial bounds of the Widget should be, the Label is not accounted for. But also layout is expensive and shouldn't be done multiple times whenever a Widget is created. BUG=671820 ========== to ========== Only perform dialog Layout() once during Widget::Init(). For Widgets that require a non-client view Layout is currently performed twice during Widget::Init: - once before the initial bounds of the Widget are determined, and - once after (when setting the initial bounds). The first Layout() is unnecessary and can lead to bugs when the RootView attempts a layout at an invalid size. Layout of text in particular can be affected since giving it a size influences text wrapping, and subsequent answers for Label::GetPreferredSize() will change. In particular, on Mac and Linux, zero-sized windows are not valid, so a NativeWidget may take on a size of 1x1 until its initial bounds is determined. If RootView performs a layout at this size, it can "lock" a Label to an non-preferred, but non-zero, size. This causes Label (and gfx::RenderText) to report a new preferred size of 0x0 (since no text will fit). So when Widget initialization later tries to determine what the initial bounds of the Widget should be, the Label is not accounted for. But also layout is expensive and shouldn't be done multiple times whenever a Widget is created if we can avoid it. BUG=671820 ==========
Description was changed from ========== Only perform dialog Layout() once during Widget::Init(). For Widgets that require a non-client view Layout is currently performed twice during Widget::Init: - once before the initial bounds of the Widget are determined, and - once after (when setting the initial bounds). The first Layout() is unnecessary and can lead to bugs when the RootView attempts a layout at an invalid size. Layout of text in particular can be affected since giving it a size influences text wrapping, and subsequent answers for Label::GetPreferredSize() will change. In particular, on Mac and Linux, zero-sized windows are not valid, so a NativeWidget may take on a size of 1x1 until its initial bounds is determined. If RootView performs a layout at this size, it can "lock" a Label to an non-preferred, but non-zero, size. This causes Label (and gfx::RenderText) to report a new preferred size of 0x0 (since no text will fit). So when Widget initialization later tries to determine what the initial bounds of the Widget should be, the Label is not accounted for. But also layout is expensive and shouldn't be done multiple times whenever a Widget is created if we can avoid it. BUG=671820 ========== to ========== Only perform dialog Layout() once during Widget::Init(). For Widgets that require a non-client view Layout is currently performed twice during Widget::Init: - once before the initial bounds of the Widget are determined, and - once after (when setting the initial bounds). The first Layout() is unnecessary and can lead to bugs when the RootView attempts a layout at an invalid size. Layout of text in particular can be affected since giving it a size influences text wrapping, and subsequent answers for Label::GetPreferredSize() will change. In particular, on Mac and Linux, zero-sized windows are not valid, so a NativeWidget may take on a size of 1x1 until its initial bounds is determined. If RootView performs a layout at this size, it can "lock" a Label to an non-preferred, but non-zero, size. This causes Label (and gfx::RenderText) to report a new preferred size of 0x0 (since no text will fit). So when Widget initialization later tries to determine what the initial bounds of the Widget should be, the Label is not accounted for. Keeping the Label with a 0x0 size instead causes Label::GetPreferredSize() to report a meaningful preferred size in more cases. But also layout is expensive and shouldn't be done multiple times whenever a Widget is created if we can avoid it. BUG=671820 ==========
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Only perform dialog Layout() once during Widget::Init(). For Widgets that require a non-client view Layout is currently performed twice during Widget::Init: - once before the initial bounds of the Widget are determined, and - once after (when setting the initial bounds). The first Layout() is unnecessary and can lead to bugs when the RootView attempts a layout at an invalid size. Layout of text in particular can be affected since giving it a size influences text wrapping, and subsequent answers for Label::GetPreferredSize() will change. In particular, on Mac and Linux, zero-sized windows are not valid, so a NativeWidget may take on a size of 1x1 until its initial bounds is determined. If RootView performs a layout at this size, it can "lock" a Label to an non-preferred, but non-zero, size. This causes Label (and gfx::RenderText) to report a new preferred size of 0x0 (since no text will fit). So when Widget initialization later tries to determine what the initial bounds of the Widget should be, the Label is not accounted for. Keeping the Label with a 0x0 size instead causes Label::GetPreferredSize() to report a meaningful preferred size in more cases. But also layout is expensive and shouldn't be done multiple times whenever a Widget is created if we can avoid it. BUG=671820 ========== to ========== Only perform dialog Layout() once during Widget::Init(). For Widgets that require a non-client view Layout is currently performed twice during Widget::Init(): - once before the initial bounds of the Widget are determined (when setting the RootView's ContentsView), and - once after (when setting the initial bounds). The first Layout() is unnecessary and can lead to bugs when the RootView attempts a layout at an invalid size. Layout of text in particular can be affected since giving it a size influences text wrapping, and subsequent answers for Label::GetPreferredSize() will change. In particular, on Mac and Linux, zero-sized windows are not valid, so a NativeWidget may take on a size of 1x1 until its initial bounds is determined. If RootView performs a layout at this size, it can "lock" a Label to an non-preferred, but non-zero, size. This causes Label (and gfx::RenderText) to report a new preferred size of 0x0 (since no text will fit). So when Widget initialization later tries to determine what the initial bounds of the Widget should be, the Label is not accounted for. Keeping the Label with a 0x0 size instead causes Label::GetPreferredSize() to report a meaningful preferred size in more cases. But also layout is expensive and shouldn't be done multiple times whenever a Widget is created if we can avoid it. BUG=671820 ==========
Hi sky, please take a look (Patchset1 has tryjobs without the fix - fails on all platforms). (cc pkasting FYI - this is from https://codereview.chromium.org/2706423002/diff/240001/ui/views/examples/dial... )
LGTM
The CQ bit was checked by tapted@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 80001, "attempt_start_ts": 1488234631363210, "parent_rev": "67f96fcba8901815194004f7c5778550bf51e3e1", "commit_rev": "23e39591317d71645dda43792fadd858bdf892ab"}
Message was sent while issue was closed.
Description was changed from ========== Only perform dialog Layout() once during Widget::Init(). For Widgets that require a non-client view Layout is currently performed twice during Widget::Init(): - once before the initial bounds of the Widget are determined (when setting the RootView's ContentsView), and - once after (when setting the initial bounds). The first Layout() is unnecessary and can lead to bugs when the RootView attempts a layout at an invalid size. Layout of text in particular can be affected since giving it a size influences text wrapping, and subsequent answers for Label::GetPreferredSize() will change. In particular, on Mac and Linux, zero-sized windows are not valid, so a NativeWidget may take on a size of 1x1 until its initial bounds is determined. If RootView performs a layout at this size, it can "lock" a Label to an non-preferred, but non-zero, size. This causes Label (and gfx::RenderText) to report a new preferred size of 0x0 (since no text will fit). So when Widget initialization later tries to determine what the initial bounds of the Widget should be, the Label is not accounted for. Keeping the Label with a 0x0 size instead causes Label::GetPreferredSize() to report a meaningful preferred size in more cases. But also layout is expensive and shouldn't be done multiple times whenever a Widget is created if we can avoid it. BUG=671820 ========== to ========== Only perform dialog Layout() once during Widget::Init(). For Widgets that require a non-client view Layout is currently performed twice during Widget::Init(): - once before the initial bounds of the Widget are determined (when setting the RootView's ContentsView), and - once after (when setting the initial bounds). The first Layout() is unnecessary and can lead to bugs when the RootView attempts a layout at an invalid size. Layout of text in particular can be affected since giving it a size influences text wrapping, and subsequent answers for Label::GetPreferredSize() will change. In particular, on Mac and Linux, zero-sized windows are not valid, so a NativeWidget may take on a size of 1x1 until its initial bounds is determined. If RootView performs a layout at this size, it can "lock" a Label to an non-preferred, but non-zero, size. This causes Label (and gfx::RenderText) to report a new preferred size of 0x0 (since no text will fit). So when Widget initialization later tries to determine what the initial bounds of the Widget should be, the Label is not accounted for. Keeping the Label with a 0x0 size instead causes Label::GetPreferredSize() to report a meaningful preferred size in more cases. But also layout is expensive and shouldn't be done multiple times whenever a Widget is created if we can avoid it. BUG=671820 Review-Url: https://codereview.chromium.org/2712383002 Cr-Commit-Position: refs/heads/master@{#453365} Committed: https://chromium.googlesource.com/chromium/src/+/23e39591317d71645dda43792fad... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/23e39591317d71645dda43792fad... |