Bug 264018 - PlainText(): avoid extra buffer allocation for empty ranges
Summary: PlainText(): avoid extra buffer allocation for empty ranges
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-10-31 18:50 PDT by Ahmad Saleem
Modified: 2023-11-20 05:16 PST (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ahmad Saleem 2023-10-31 18:50:45 PDT
Hi team,

Came across potential improvement from Blink's Commit:

Commit: https://chromium.googlesource.com/chromium/blink/+/0b34a706cb547c672215c108270ffb8d344fb097

WebKit Source: https://searchfox.org/wubkat/source/Source/WebCore/editing/TextIterator.cpp#2531

___

This compiles:

String plainText(const SimpleRange& range, TextIteratorBehaviors defaultBehavior, bool isDisplayString)
{
    TextIterator it(range, defaultBehavior);
    if (it.atEnd())
        return emptyString();
    // The initial buffer size can be critical for performance: https://bugs.webkit.org/show_bug.cgi?id=81192
    constexpr unsigned initialCapacity = 1 << 15;
    Ref document = range.start.document();
    StringBuilder builder;
    builder.reserveCapacity(initialCapacity);
    TextIteratorBehaviors behaviors = defaultBehavior;
    if (!isDisplayString)
        behaviors.add(TextIteratorBehavior::EmitsTextsWithoutTranscoding);
    for (; !it.atEnd(); it.advance())
        it.appendTextToStringBuilder(builder);
    if (builder.isEmpty())
        return emptyString();
    String result = builder.toString();

...

___

Just wanted to raise to get input, whether it is worthwhile to add. I didn't run any test suite to confirm whether it regress anything or not.

Thanks!
Comment 1 Ahmad Saleem 2023-11-01 17:50:29 PDT
PR Attempt: https://github.com/WebKit/WebKit/pull/19831

NOTE - It leads to 'backslash to Yen' hack being disabled and leading to expectation changes in few of test cases:

editing/execCommand/transpose-backslash-with-euc.html
fast/encoding/yentest.html
fast/encoding/yentest2.html
imported/w3c/web-platform-tests/html/semantics/forms/the-option-element/option-text-backslash.html
editing/pasteboard/copy-backslash-with-euc.html
editing/execCommand/transpose-backslash-with-euc.html

___

I don't know whether we have any plans to remove 'backslash to yen' yet but it is removed by Blink.

Just wanted to document here, all these bits.
Comment 2 Radar WebKit Bug Importer 2023-11-07 17:51:14 PST
<rdar://problem/118090616>
Comment 3 Karl Dubost 2023-11-20 05:16:34 PST
Maybe Elika can give an opinion here.