Bug 142146

Summary: BreakingContext cleanup
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, esprehn+autocc, glenn, jonlee, kondapallykalyan, simon.fraser, thorton, zalan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch dino: review+

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>