Bug 115765

Summary: Begin unraveling the mess that is QuotesData
Product: WebKit Reporter: Anders Carlsson <andersca>
Component: New BugsAssignee: Anders Carlsson <andersca>
Status: RESOLVED FIXED    
Severity: Normal CC: allan.jensen, ap, commit-queue, esprehn+autocc, glenn, macpherson, menard, rniwa, roger_fong
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch kling: review+

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]
Patch
Comment 2 Andreas Kling 2013-05-07 15:38:52 PDT
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.
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:

http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fcss-generated-content%2Fclose-quote-negative-depth.html
Comment 7 Alexey Proskuryakov 2013-05-08 00:14:29 PDT
Ryosuke filed bug 115776 about that.