Remove shape-inside support

CSS Shapes Level 1 (CR) only contains shape-outside. We are focusing our efforts on finalizing
the implementation of shape-outside, it's worth to remove shape-inside code at this point for now.

A list of reasons for the removal:

- Shape-inside is only part of Shapes Level 2, which needs to be improved on some topics.
- Shape-inside is lack of new shapes support (e.g. inset).
- Deprecated shapes are removed from the code (e.g. rectangle), which affects shape-inside.
- The current shape-inside code spreads across the layout code.
- The current shape-inside implementation is experimental in some areas, and the partially implemented code can have security implications.
- Removal of shape-inside opens possibilities for code complexity and performance optimizations for shape-outside. (e.g. simpler geometry code) 

BUG=356778
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170260

[rwlbuis, 2014-03-26 20:45:45]

Publish for feedback :)

[Zoltan, 2014-03-26 21:01:08]

https://codereview.chromium.org/209443007/diff/90001/Source/core/rendering/Re...
File Source/core/rendering/RenderBlock.cpp (right):

https://codereview.chromium.org/209443007/diff/90001/Source/core/rendering/Re...
Source/core/rendering/RenderBlock.cpp:1366: bool
RenderBlock::updateRegionsAndShapesLogicalSize(RenderFlowThread* flowThread)
We could probably remove this function, since no there is regions anymore.

https://codereview.chromium.org/209443007/diff/90001/Source/core/rendering/Re...
File Source/core/rendering/RenderBlockFlow.cpp (right):

https://codereview.chromium.org/209443007/diff/90001/Source/core/rendering/Re...
Source/core/rendering/RenderBlockFlow.cpp:2140: // FIXME Bug 102948: This only
works for shape outside directly set on this block.
We can remove the FIXME.

https://codereview.chromium.org/209443007/diff/90001/Source/core/rendering/li...
File Source/core/rendering/line/LineBreaker.cpp (right):

https://codereview.chromium.org/209443007/diff/90001/Source/core/rendering/li...
Source/core/rendering/line/LineBreaker.cpp:62: return nextSegmentBreak(resolver,
lineInfo, renderTextInfo, lastFloatFromPreviousLine, consecutiveHyphenatedLines,
wordMeasurements);
You can remove LineBreaker::nextLineBreak, and call nextSegmentBreak at
nextLineBreak's call site directly.

[rwlbuis, 2014-03-26 22:35:44]

On 2014/03/26 21:01:08, Zoltan wrote:
>
https://codereview.chromium.org/209443007/diff/90001/Source/core/rendering/Re...
> File Source/core/rendering/RenderBlock.cpp (right):
> 
>
https://codereview.chromium.org/209443007/diff/90001/Source/core/rendering/Re...
> Source/core/rendering/RenderBlock.cpp:1366: bool
> RenderBlock::updateRegionsAndShapesLogicalSize(RenderFlowThread* flowThread)
> We could probably remove this function, since no there is regions anymore.
> 
>
https://codereview.chromium.org/209443007/diff/90001/Source/core/rendering/Re...
> File Source/core/rendering/RenderBlockFlow.cpp (right):
> 
>
https://codereview.chromium.org/209443007/diff/90001/Source/core/rendering/Re...
> Source/core/rendering/RenderBlockFlow.cpp:2140: // FIXME Bug 102948: This only
> works for shape outside directly set on this block.
> We can remove the FIXME.
> 
>
https://codereview.chromium.org/209443007/diff/90001/Source/core/rendering/li...
> File Source/core/rendering/line/LineBreaker.cpp (right):
> 
>
https://codereview.chromium.org/209443007/diff/90001/Source/core/rendering/li...
> Source/core/rendering/line/LineBreaker.cpp:62: return
nextSegmentBreak(resolver,
> lineInfo, renderTextInfo, lastFloatFromPreviousLine,
consecutiveHyphenatedLines,
> wordMeasurements);
> You can remove LineBreaker::nextLineBreak, and call nextSegmentBreak at
> nextLineBreak's call site directly.

Nice suggestions, thanks! Patch set 5 tries to fix them.

[Zoltan, 2014-03-26 22:58:15]

lgtm

[Bem Jones-Bey (adobe), 2014-03-27 00:09:46]

Thanks for doing this!

https://codereview.chromium.org/209443007/diff/130001/Source/core/frame/UseCo...
File Source/core/frame/UseCounter.cpp (left):

https://codereview.chromium.org/209443007/diff/130001/Source/core/frame/UseCo...
Source/core/frame/UseCounter.cpp:395: case CSSPropertyShapeInside: return 346;
I notice that the other cases that have been removed are commented out to
reserve the number. Should we do that here as well?

https://codereview.chromium.org/209443007/diff/130001/Source/core/rendering/R...
File Source/core/rendering/RenderBlock.cpp (right):

https://codereview.chromium.org/209443007/diff/130001/Source/core/rendering/R...
Source/core/rendering/RenderBlock.cpp:1359: void
RenderBlock::imageChanged(WrappedImagePtr image, const IntRect*)
Why is this method even here? It doesn't seem to do anything now except call the
superclass method.

https://codereview.chromium.org/209443007/diff/130001/Source/core/rendering/R...
Source/core/rendering/RenderBlock.cpp:1363: if (!parent() || !everHadLayout())
Even if this method needs to stay, this if statement seems pointless.

https://codereview.chromium.org/209443007/diff/130001/Source/core/rendering/R...
Source/core/rendering/RenderBlock.cpp:1367: void
RenderBlock::updateRegionsAndShapesAfterChildLayout(RenderFlowThread*
flowThread, bool heightChanged)
You should remove this method and replace all calls to it to be calls to
computeRegionRangeForBlock.

https://codereview.chromium.org/209443007/diff/130001/Source/core/rendering/R...
File Source/core/rendering/RenderBlockFlow.cpp (right):

https://codereview.chromium.org/209443007/diff/130001/Source/core/rendering/R...
Source/core/rendering/RenderBlockFlow.cpp:439:
updateRegionsAndShapesAfterChildLayout(flowThreadContainingBlock(),
heightChanged);
As mentioned before, this should just be
computeRegionRangeForBlock(flowThreadContainingBlock());

https://codereview.chromium.org/209443007/diff/130001/Source/core/rendering/R...
File Source/core/rendering/RenderBlockLineLayout.cpp (right):

https://codereview.chromium.org/209443007/diff/130001/Source/core/rendering/R...
Source/core/rendering/RenderBlockLineLayout.cpp:833:
constructBidiRunsForSegment(topResolver, bidiRuns, endOfLine, override,
previousLineBrokeCleanly, isNewUBAParagraph);
You should remove constructBidiRunsForSegment and put the body in
constructBidiRunsForLine, as we don't have to worry about segments anymore.

https://codereview.chromium.org/209443007/diff/130001/Source/core/rendering/R...
Source/core/rendering/RenderBlockLineLayout.cpp:994: endOfLine =
lineBreaker.nextSegmentBreak(resolver, layoutState.lineInfo(), renderTextInfo,
lastFloatFromPreviousLine, consecutiveHyphenatedLines, wordMeasurements);
Shouldn't this be nextLineBreak? We shouldn't have segment breaks anymore after
shape inside has been removed.

https://codereview.chromium.org/209443007/diff/130001/Source/core/rendering/l...
File Source/core/rendering/line/BreakingContext.cpp (right):

https://codereview.chromium.org/209443007/diff/130001/Source/core/rendering/l...
Source/core/rendering/line/BreakingContext.cpp:45: // make sure we consume at
least one char/object.
This comment is obsolete, it should be removed.

https://codereview.chromium.org/209443007/diff/130001/Source/core/rendering/l...
File Source/core/rendering/line/LineBreaker.h (right):

https://codereview.chromium.org/209443007/diff/130001/Source/core/rendering/l...
Source/core/rendering/line/LineBreaker.h:55: InlineIterator
nextSegmentBreak(InlineBidiResolver&, LineInfo&, RenderTextInfo&,
FloatingObject* lastFloatFromPreviousLine, unsigned consecutiveHyphenatedLines,
WordMeasurements&);
nextLineBreak is the correct name for this method.

[eseidel, 2014-03-27 20:48:37]

lgtm

[rwlbuis, 2014-03-27 21:21:02]

The CQ bit was checked by rob.buis@samsung.com

[commit-bot: I haz the power, 2014-03-27 21:21:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rob.buis@samsung.com/209443007/170001

[commit-bot: I haz the power, 2014-03-27 21:40:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-27 21:40:36]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[rwlbuis, 2014-03-27 21:43:27]

The CQ bit was checked by rob.buis@samsung.com

[commit-bot: I haz the power, 2014-03-27 21:43:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rob.buis@samsung.com/209443007/170001

[commit-bot: I haz the power, 2014-03-27 22:28:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-27 22:29:00]

Try jobs failed on following builders:
  tryserver.blink on win_blink_rel

[rwlbuis, 2014-03-27 22:34:16]

The CQ bit was checked by rob.buis@samsung.com

[commit-bot: I haz the power, 2014-03-27 22:34:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rob.buis@samsung.com/209443007/170001

[commit-bot: I haz the power, 2014-03-27 23:57:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-27 23:57:05]

Try jobs failed on following builders:
  tryserver.blink on win_blink_rel

[rwlbuis, 2014-03-28 02:04:51]

The CQ bit was checked by rob.buis@samsung.com

[commit-bot: I haz the power, 2014-03-28 02:05:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rob.buis@samsung.com/209443007/170001

[commit-bot: I haz the power, 2014-03-28 02:36:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 02:36:07]

Try jobs failed on following builders:
  tryserver.blink on win_blink_rel

[rwlbuis, 2014-03-28 03:10:26]

The CQ bit was checked by rob.buis@samsung.com

[commit-bot: I haz the power, 2014-03-28 03:10:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rob.buis@samsung.com/209443007/170001

[commit-bot: I haz the power, 2014-03-28 03:56:27]

Change committed as 170260