Bug 115765 - Begin unraveling the mess that is QuotesData
Summary: Begin unraveling the mess that is QuotesData
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Anders Carlsson
Depends on:
Reported: 2013-05-07 15:25 PDT by Anders Carlsson
Modified: 2013-05-08 00:14 PDT (History)
9 users (show)

See Also:

Patch (11.26 KB, patch)
2013-05-07 15:29 PDT, Anders Carlsson
kling: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2013-05-07 15:25:15 PDT
Begin unraveling the mess that is QuoteData
Comment 1 Anders Carlsson 2013-05-07 15:29:43 PDT
Created attachment 200987 [details]
Comment 2 Andreas Kling 2013-05-07 15:38:52 PDT
Comment on attachment 200987 [details]

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.
Comment 3 Anders Carlsson 2013-05-07 16:06:26 PDT
Committed r149700: <http://trac.webkit.org/changeset/149700>
Comment 5 Alexey Proskuryakov 2013-05-07 23:20:51 PDT
> 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>.
Comment 6 Ryosuke Niwa 2013-05-07 23:54:58 PDT
fast/css-generated-content/close-quote-negative-depth.html is still failing:

Comment 7 Alexey Proskuryakov 2013-05-08 00:14:29 PDT
Ryosuke filed bug 115776 about that.