New issue
Advanced search Search tips

Issue 655187 link

Starred by 1 user
Status: Available
Owner: ----
Cc:
engedy@chromium.org
pkasting@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Change registry_controlled_domains API to use StringPiece instead of std::string

Project Member Reported by pkalinnikov@chromium.org, Oct 12 2016 
Now the methods of the registry_controlled_domains [1] library use std::string internally and return strings when it is not necessary.

It would be good to change the interface so that it doesn't allocate strings where it's not necessary. For instance, the GetDomainAndRegistry(const GURL&, ...) function could return base::StringPiece pointing to a piece of the GURL.

1. https://cs.chromium.org/chromium/src/net/base/registry_controlled_domains/registry_controlled_domain.h
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 13 2016 

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

commit e0a1c1bd1456928a05a582c78210a81f8fe30859
Author: pkalinnikov <pkalinnikov@chromium.org>
Date: Thu Oct 13 18:55:05 2016

Avoid unnecessary std::string allocations.

When checking SameDomainOrHost, it is unnecessary to allocate std::string's
when retrieving DomainAndRegistry from URLs. It is actually unnecessary
elsewhere, but this can be addressed in a different CL.

BUG=655187

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

[modify] https://crrev.com/e0a1c1bd1456928a05a582c78210a81f8fe30859/net/base/registry_controlled_domains/registry_controlled_domain.cc

Comment 2 by pkalinnikov@chromium.org, Oct 13 2016

Description: Show this description

Comment 3 by benhenry@chromium.org, Aug 2 2017 

Ping! This bug is assigned and has an owner, but hasn't been updated in over 180 days. Is this still something we want to do?

Comment 4 by pkalinnikov@chromium.org, Aug 3 2017

Owner: ----
Status: Available (was: Assigned)
I won't be working on this bug in the near future, so I'm leaving it available.

One thing that could be addressed is this function:
GetDomainAndRegistry(const GURL& gurl, PrivateRegistryFilter filter);
It could return base::StringPiece instead of std::string. I'm not sure how valuable improvement it would be. Feel free to mark it WontFix.
Project Member

Comment 5 by sheriffbot@chromium.org, Aug 3

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 6 by dtapu...@chromium.org, Aug 24

Components: Internals>Network

Comment 7 by davidben@chromium.org, Aug 24

Status: Available (was: Untriaged)
Skimming things, it seems like it might save a copy, but we'd have to be careful about code like this which acts on a temporary GURL.
https://cs.chromium.org/chromium/src/chrome/browser/plugins/plugin_info_host_impl.cc?rcl=c9437e66ce32af03af67988fa734d180efbba6f8&l=491

The one that takes a StringPiece as input does a canonicalization step (whereas I guess GURL can be assumed canonicalized), so it needs to return std::string.

I suspect this is questionably useful, but there are quite a few callers? *shrug*
►

Sign in to add a comment