Bug 115765 – Begin unraveling the mess that is QuotesData

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Anders Carlsson]

Begin unraveling the mess that is QuoteData

[Anders Carlsson]

Created attachment 200987 [details]
Patch

[Andreas Kling]

Comment on attachment 200987 [details]
Patch

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

r=me, but you'll need to unbreak the RenderStyle optimization.

> Source/WebCore/ChangeLog:3
> +        Begin unraveling the mess that is QuoteData

QuoteData => QuotesData

> Source/WebCore/rendering/style/QuotesData.cpp:30
> +    quotes.reserveCapacity(2);

You should use reserveInitialCapacity() here.

> Source/WebCore/rendering/style/QuotesData.h:35
> +    static PassRefPtr<QuotesData> create(const Vector<std::pair<String, String> >& quotes);

This could also be QuotesData::adopt(Voctor<>&) and use swap() to avoid copying the data.

> Source/WebCore/rendering/style/RenderStyle.cpp:755
> -    if (QuotesData::equals(rareInheritedData->quotes.get(), q.get()))
> -        return;
>      rareInheritedData.access()->quotes = q;

You are removing an optimization here!
rareInheritedData.access() is DataRef<T>'s copy-on-write accessor, this means that setQuotes() will always detach from shared rareInheritedData.

[Anders Carlsson]

Committed r149700: <http://trac.webkit.org/changeset/149700>

[Roger Fong]

This caused a lot of test failures:
http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK2%20(Tests)/r149700%20(8675)/results.html

[Alexey Proskuryakov]

> This caused a lot of test failures:

What happened with this? Looks like tests got happier after r149707, but still not quite: <http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK2%20(Tests)/r149708%20(8680)/results.html>.

[Ryosuke Niwa]

fast/css-generated-content/close-quote-negative-depth.html is still failing:

http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fcss-generated-content%2Fclose-quote-negative-depth.html

[Alexey Proskuryakov]

Ryosuke filed bug 115776 about that.