Bug 142146 - BreakingContext cleanup
Summary: BreakingContext cleanup
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-01 09:03 PST by Myles C. Maxfield
Modified: 2015-03-03 11:13 PST (History)
9 users (show)

See Also:


Attachments
Patch (6.47 KB, patch)
2015-03-01 09:05 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (9.78 KB, patch)
2015-03-01 10:14 PST, Myles C. Maxfield
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2015-03-01 09:03:29 PST
BreakingContext cleanup
Comment 1 Myles C. Maxfield 2015-03-01 09:05:57 PST
Created attachment 247631 [details]
Patch
Comment 2 Myles C. Maxfield 2015-03-01 10:14:34 PST
Created attachment 247634 [details]
Patch
Comment 3 zalan 2015-03-01 10:20:10 PST
Comment on attachment 247631 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=247631&action=review

> Source/WebCore/rendering/line/BreakingContextInlineHeaders.h:1052
>      if (m_lineBreak == m_resolver.position()) {
>          if (!m_lineBreak.renderer() || !m_lineBreak.renderer()->isBR()) {
>              // we just add as much as possible
> -            if (m_blockStyle.whiteSpace() == PRE && !m_current.offset()) {
> -                m_lineBreak.moveTo(m_lastObject, m_lastObject->isText() ? m_lastObject->length() : 0);
> -            } else if (m_lineBreak.renderer()) {
> +            if (m_blockStyle.whiteSpace() == PRE && !m_current.offset())
> +                commitLineBreakAtCurrentWidth(*m_lastObject, m_lastObject->isText() ? m_lastObject->length() : 0);
> +            else if (m_lineBreak.renderer()) {
>                  // Don't ever break in the middle of a word if we can help it.
>                  // There's no room at all. We just have to be on this line,
>                  // even though we'll spill out.
> -                m_lineBreak.moveTo(m_current.renderer(), m_current.offset());
> +                commitLineBreakAtCurrentWidth(*m_current.renderer(), m_current.offset());

So the extra m_width.commit() here (by calling commitLineBreakAtCurrentWidth()) does not change behavior?
Comment 4 Myles C. Maxfield 2015-03-01 18:10:15 PST
Comment on attachment 247631 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=247631&action=review

>> Source/WebCore/rendering/line/BreakingContextInlineHeaders.h:1052
>> +                commitLineBreakAtCurrentWidth(*m_current.renderer(), m_current.offset());
> 
> So the extra m_width.commit() here (by calling commitLineBreakAtCurrentWidth()) does not change behavior?

No - this function only ever gets called at the very end of LineBreaker::nextLineBreak(), where the LineWidth immediately gets destroyed.
Comment 5 Myles C. Maxfield 2015-03-03 11:13:37 PST
Committed r180944: <http://trac.webkit.org/changeset/180944>