Bug 138704 - Simple line layout: Move simple line layout RunResolver and LineResolver implementation to SimpleLineLayoutResolver.cpp
Summary: Simple line layout: Move simple line layout RunResolver and LineResolver impl...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-11-13 13:14 PST by zalan
Modified: 2014-11-14 08:20 PST (History)
2 users (show)

See Also:


Attachments
Patch (27.30 KB, patch)
2014-11-13 13:24 PST, zalan
no flags Details | Formatted Diff | Diff
Patch (20.46 KB, patch)
2014-11-13 15:07 PST, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2014-11-13 13:14:25 PST
from SimpleLineLayoutResolver.h.
Comment 1 zalan 2014-11-13 13:24:55 PST
Created attachment 241498 [details]
Patch
Comment 2 Antti Koivisto 2014-11-13 14:22:38 PST
Comment on attachment 241498 [details]
Patch

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

Hope this doesn't make things slower.

> Source/WebCore/rendering/SimpleLineLayoutResolver.h:43
> -    IteratorType begin() const { return m_begin; }
> -    IteratorType end() const { return m_end; }
> +    inline IteratorType begin() const { return m_begin; }
> +    inline IteratorType end() const { return m_end; }

The inline keywords here and elsewhere don't do anything and should not be added.

> Source/WebCore/rendering/SimpleLineLayoutResolver.h:80
> +        inline bool operator==(const Iterator& other) const { ASSERT(&m_resolver == &other.m_resolver); return m_runIndex == other.m_runIndex; }
> +        inline bool operator!=(const Iterator& other) const { return !(*this == other); }
>  
> -        Run operator*() const;
> +        inline Run operator*() const { return Run(*this); }

Multiline function definitions should stay on the bottom of the file. 

I don't generally think anything except trivial accessors should be inlined to declaration.
Comment 3 Antti Koivisto 2014-11-13 14:51:26 PST
r=me but please keep the function bodies that stay in the header as-is.
Comment 4 zalan 2014-11-13 15:07:46 PST
Created attachment 241509 [details]
Patch
Comment 5 WebKit Commit Bot 2014-11-14 08:20:15 PST
Comment on attachment 241509 [details]
Patch

Clearing flags on attachment: 241509

Committed r176123: <http://trac.webkit.org/changeset/176123>
Comment 6 WebKit Commit Bot 2014-11-14 08:20:19 PST
All reviewed patches have been landed.  Closing bug.