Bug 131180 - Merge ShapeInfo & ShapeOutsideInfo now that ShapeInsideInfo is no more
Summary: Merge ShapeInfo & ShapeOutsideInfo now that ShapeInsideInfo is no more
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: Bem Jones-Bey
URL:
Keywords:
Depends on:
Blocks: 98664
  Show dependency treegraph
 
Reported: 2014-04-03 12:50 PDT by Bem Jones-Bey
Modified: 2014-04-03 16:05 PDT (History)
10 users (show)

See Also:


Attachments
Patch (44.52 KB, patch)
2014-04-03 12:57 PDT, Bem Jones-Bey
no flags Details | Formatted Diff | Diff
Patch (44.42 KB, patch)
2014-04-03 14:40 PDT, Bem Jones-Bey
no flags Details | Formatted Diff | Diff
Patch (44.45 KB, patch)
2014-04-03 15:25 PDT, Bem Jones-Bey
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bem Jones-Bey 2014-04-03 12:50:26 PDT
Merge ShapeInfo & ShapeOutsideInfo now that ShapeInsideInfo is no more
Comment 1 Bem Jones-Bey 2014-04-03 12:57:05 PDT
Created attachment 228533 [details]
Patch
Comment 2 Zoltan Horvath 2014-04-03 13:22:50 PDT
Comment on attachment 228533 [details]
Patch

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

> Source/WebCore/rendering/shapes/ShapeOutsideInfo.cpp:123
> +bool checkShapeImageOrigin(Document& document, CachedImage& cachedImage)

This function is only used by ShapeOutsideInfo::isEnabledFor. Can we just make it static?

> Source/WebCore/rendering/shapes/ShapeOutsideInfo.h:125
> +bool checkShapeImageOrigin(Document&, CachedImage&);

...and we won't need this.

> Source/WebCore/rendering/shapes/ShapeOutsideInfo.cpp:197
> +inline LayoutUnit borderBeforeInWritingMode(const RenderBox& renderer, WritingMode writingMode)

We have some inlines like this, would it make sense to make it inline statics?
Comment 3 Bem Jones-Bey 2014-04-03 14:40:13 PDT
Created attachment 228545 [details]
Patch

Updated for comments
Comment 4 Andreas Kling 2014-04-03 15:07:08 PDT
Comment on attachment 228545 [details]
Patch

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

r=me

> Source/WebCore/rendering/shapes/ShapeOutsideInfo.h:93
> +        InfoMap::AddResult result = infoMap.add(&key, std::make_unique<ShapeOutsideInfo>(key));

I'd use "auto" here.
Comment 5 Bem Jones-Bey 2014-04-03 15:25:59 PDT
Created attachment 228548 [details]
Patch

Patch for landing
Comment 6 WebKit Commit Bot 2014-04-03 16:05:17 PDT
Comment on attachment 228548 [details]
Patch

Clearing flags on attachment: 228548

Committed r166752: <http://trac.webkit.org/changeset/166752>
Comment 7 WebKit Commit Bot 2014-04-03 16:05:24 PDT
All reviewed patches have been landed.  Closing bug.