Bug 208175 – IsolatedObject support for ProgressIndicator and Meter.

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

[Andres Gonzalez]

IsolatedObject support for ProgressIndicator and Meter.

[Andres Gonzalez]

Created attachment 391621 [details]
Patch

[Darin Adler]

Comment on attachment 391621 [details]
Patch

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

> Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp:97
> +    if (!gaugeRegionValue.isEmpty()) {
> +        StringBuilder builder;
> +        builder.append(description);
> +        if (builder.length())
> +            builder.appendLiteral(", ");
> +        builder.append(gaugeRegionValue);
> +        description = builder.toString();
> +    }

Here’s a more efficient way to write this:

    if (!gaugeRegionValue.isEmpty())
        description = makeString(description, description.isEmpty() ? "" : ", ", gaugeRegionValue);

Or we could use the + operator if you prefer that to makeString.

    if (!gaugeRegionValue.isEmpty())
        description = description + (description.isEmpty() ? "" : ", ") + gaugeRegionValue;

[Andres Gonzalez]

Created attachment 391644 [details]
Patch

[Andres Gonzalez]

(In reply to Darin Adler from comment #2)
> Comment on attachment 391621 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=391621&action=review
> 
> > Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp:97
> > +    if (!gaugeRegionValue.isEmpty()) {
> > +        StringBuilder builder;
> > +        builder.append(description);
> > +        if (builder.length())
> > +            builder.appendLiteral(", ");
> > +        builder.append(gaugeRegionValue);
> > +        description = builder.toString();
> > +    }
> 
> Here’s a more efficient way to write this:
> 
>     if (!gaugeRegionValue.isEmpty())
>         description = makeString(description, description.isEmpty() ? "" :
> ", ", gaugeRegionValue);
> 
> Or we could use the + operator if you prefer that to makeString.
> 
>     if (!gaugeRegionValue.isEmpty())
>         description = description + (description.isEmpty() ? "" : ", ") +
> gaugeRegionValue;

Thanks very much, Darin. Changed it to:

    if (!gaugeRegionValue.isEmpty())
        description = description.isEmpty() ? gaugeRegionValue : description + ", " + gaugeRegionValue;

So that we don't call operator+ unnecessarily when description.isEmpty. I assumed that the StringBuilder implementation was more efficient than the String concatenation and so I kept it. Good to know that that is not the case. Thanks again.

[WebKit Commit Bot]

Comment on attachment 391644 [details]
Patch

Clearing flags on attachment: 391644

Committed r257356: <https://trac.webkit.org/changeset/257356>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/59765193>