New issue
Advanced search Search tips

Issue 661895 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

WebString <-> ASCII conversion does not need to go through base::string16

Project Member Reported by kinuko@chromium.org, Nov 3 2016

Issue description

Currently one of the common way to get ASCII string from WebString is to do base::UTF16ToASCII(webString), but this actually first converts WebString (WTF::StringImpl) to base::string16 and then std::string.  This could be wasteful if the original WebString is really ASCII-only (which is expected) and 8-bit string.

We should avoid unnecessary string16 conversions for such patterns.
 
Components: Blink>Internals
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a0d5ac61931de6d57d2f7c2a619177986d4d5e0e

commit a0d5ac61931de6d57d2f7c2a619177986d4d5e0e
Author: kinuko <kinuko@chromium.org>
Date: Fri Nov 04 08:49:59 2016

Skip base::string16 if not necessary for WebString <-> ASCII conversion

One common way to get ASCII string from WebString is to do
base::UTF16ToASCII(webString)

but this always goes through string16 even when webString only
contains ASCII and is already a 8-bit string (which is expected).

This patch adds following WebString methods to help optimizing
WebString <-> ASCII conversions:

- WebString::ascii() ... returns ascii std::string, which skips string16 conversion
  for 8-bit cases. Internally DCHECKs if it contains non-ascii chars
- WebString::containsOnlyASCII() ... should be more efficient than calling
  base::IsStringASCII() on WebString (which incurs string16 conversion)
- WebString::fromASCII() ... does same as fromLatin1() but with DCHECK

Split from https://codereview.chromium.org/2444873002/

BUG= 661895 

Review-Url: https://codereview.chromium.org/2469353003
Cr-Commit-Position: refs/heads/master@{#429834}

[modify] https://crrev.com/a0d5ac61931de6d57d2f7c2a619177986d4d5e0e/content/child/simple_webmimeregistry_impl.cc
[modify] https://crrev.com/a0d5ac61931de6d57d2f7c2a619177986d4d5e0e/content/renderer/cache_storage/cache_storage_dispatcher.cc
[modify] https://crrev.com/a0d5ac61931de6d57d2f7c2a619177986d4d5e0e/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/a0d5ac61931de6d57d2f7c2a619177986d4d5e0e/content/renderer/render_view_browsertest.cc
[modify] https://crrev.com/a0d5ac61931de6d57d2f7c2a619177986d4d5e0e/content/renderer/render_view_impl.cc
[modify] https://crrev.com/a0d5ac61931de6d57d2f7c2a619177986d4d5e0e/content/renderer/renderer_blink_platform_impl.cc
[modify] https://crrev.com/a0d5ac61931de6d57d2f7c2a619177986d4d5e0e/content/renderer/webclipboard_impl.cc
[modify] https://crrev.com/a0d5ac61931de6d57d2f7c2a619177986d4d5e0e/media/blink/key_system_config_selector.cc
[modify] https://crrev.com/a0d5ac61931de6d57d2f7c2a619177986d4d5e0e/media/blink/webcontentdecryptionmodulesession_impl.cc
[modify] https://crrev.com/a0d5ac61931de6d57d2f7c2a619177986d4d5e0e/media/blink/webencryptedmediaclient_impl.cc
[modify] https://crrev.com/a0d5ac61931de6d57d2f7c2a619177986d4d5e0e/third_party/WebKit/Source/platform/exported/WebString.cpp
[modify] https://crrev.com/a0d5ac61931de6d57d2f7c2a619177986d4d5e0e/third_party/WebKit/public/platform/WebString.h

Status: Fixed (was: Assigned)

Sign in to add a comment