Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(26)

Issue 889463003: GURL::Replacements methods accept a StringPiece instead of std::string&. (Closed)

Created:
5 years, 10 months ago by Matt Giuca
Modified:
5 years, 10 months ago
Reviewers:
brettw
CC:
chromium-reviews, skanuj+watch_chromium.org, nkostylev+watch_chromium.org, tburkard+watch_chromium.org, dhollowa+watch_chromium.org, dougw+watch_chromium.org, nasko+codewatch_chromium.org, dcheng, dmazzoni+watch_chromium.org, stevenjb+watch_chromium.org, extensions-reviews_chromium.org, cbentzel+watch_chromium.org, melevin+watch_chromium.org, aboxhall+watch_chromium.org, grt+watch_chromium.org, dzhioev+watch_chromium.org, jam, darin-cc_chromium.org, jkarlin+watch_chromium.org, chromium-apps-reviews_chromium.org, creis+watch_chromium.org, dtseng+watch_chromium.org, yuzo+watch_chromium.org, gavinp+prer_chromium.org, jfweitz+watch_chromium.org, pam+watch_chromium.org, oshima+watch_chromium.org, Jered, davidben+watch_chromium.org, plundblad+watch_chromium.org, donnd+watch_chromium.org, David Black, samarth+watch_chromium.org, kmadhusu+watch_chromium.org, davemoore+watch_chromium.org, chrome-apps-syd-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

GURL::Replacements methods accept a StringPiece instead of std::string&. Previously, it was unsafe to pass a char* because it would be implicitly converted to a string with a very short lifetime. Now, you can safely pass a char* as long as the memory pointed to by the char* outlives the Replacements (which is always true for string literals). All existing usage of Replacements should continue to work as usual. Updated every call to Replacements Set*Str methods that that could be simplified by the new StringPiece API (for example, passing string literals directly, instead of copying them into a string object). Internally, renamed StdStringReplacements to StringPieceReplacements. BUG=454274 Committed: https://crrev.com/77752c3e55c59747a62a362799ac6de5ff56fd45 Cr-Commit-Position: refs/heads/master@{#314771}

Patch Set 1 #

Patch Set 2 : Clean up some more calls from recent and not-yet-committed CLs. #

Patch Set 3 : const char[]s are now static (else the compiler can place the data on the stack). #

Patch Set 4 : Rebase (fixed some merge conflicts). #

Unified diffs Side-by-side diffs Delta from patch set Stats (+140 lines, -224 lines) Patch
M chrome/browser/android/logo_service.cc View 1 2 3 1 chunk +1 line, -4 lines 0 comments Download
M chrome/browser/apps/app_browsertest.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/apps/guest_view/web_view_browsertest.cc View 1 2 3 4 chunks +4 lines, -8 lines 0 comments Download
M chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/captive_portal/captive_portal_browsertest.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/chromeos/app_mode/fake_cws.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/chromeos/login/signin/oauth2_browsertest.cc View 1 2 3 1 chunk +2 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/login/test/oobe_base_test.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/extensions/api/cloud_print_private/cloud_print_private_apitest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/extensions/api/streams_private/streams_private_apitest.cc View 1 2 3 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/extensions/app_process_apitest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/extensions/chrome_app_api_browsertest.cc View 1 2 4 chunks +26 lines, -26 lines 0 comments Download
M chrome/browser/extensions/content_script_apitest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/extensions/extension_dom_clipboard_apitest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/extensions/extension_nacl_browsertest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/extensions/extension_resource_request_policy_apitest.cc View 1 chunk +2 lines, -4 lines 0 comments Download
M chrome/browser/extensions/isolated_app_browsertest.cc View 6 chunks +6 lines, -12 lines 0 comments Download
M chrome/browser/extensions/process_management_browsertest.cc View 2 chunks +2 lines, -4 lines 0 comments Download
M chrome/browser/extensions/process_manager_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/window_open_apitest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/net/cookie_policy_browsertest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/net/proxy_browsertest.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/net/websocket_browsertest.cc View 3 chunks +4 lines, -8 lines 0 comments Download
M chrome/browser/prerender/prerender_browsertest.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/search/search.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/ssl/ssl_browser_tests.cc View 4 chunks +4 lines, -8 lines 0 comments Download
M chrome/browser/supervised_user/experimental/supervised_user_async_url_checker.cc View 2 chunks +8 lines, -12 lines 0 comments Download
M chrome/browser/task_manager/task_manager_browsertest.cc View 3 chunks +3 lines, -6 lines 0 comments Download
M chrome/browser/ui/app_list/start_page_service.cc View 1 2 3 1 chunk +1 line, -4 lines 0 comments Download
M chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/ui/login/login_prompt_browsertest.cc View 1 2 4 chunks +5 lines, -7 lines 0 comments Download
M chrome/renderer/safe_browsing/phishing_classifier_browsertest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M components/google/core/browser/google_util.cc View 1 chunk +2 lines, -4 lines 0 comments Download
M components/omnibox/autocomplete_match.cc View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M components/search_engines/search_terms_data.cc View 1 chunk +1 line, -5 lines 0 comments Download
M components/search_engines/template_url.cc View 2 chunks +2 lines, -1 line 0 comments Download
M content/browser/accessibility/site_per_process_accessibility_browsertest.cc View 1 2 3 1 chunk +2 lines, -5 lines 0 comments Download
M content/browser/cross_site_transfer_browsertest.cc View 8 chunks +10 lines, -22 lines 0 comments Download
M content/browser/renderer_host/render_process_host_browsertest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M content/browser/shared_worker/worker_browsertest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M content/browser/web_contents/opened_by_dom_browsertest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M content/browser/web_contents/web_contents_impl_browsertest.cc View 1 2 2 chunks +3 lines, -3 lines 0 comments Download
M content/public/test/browser_test_utils.cc View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M content/test/browser_test_utils_browsertest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M extensions/browser/guest_view/web_view/web_view_apitest.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M net/http/http_stream_factory_impl.cc View 1 2 3 1 chunk +3 lines, -4 lines 0 comments Download
M net/url_request/url_request_unittest.cc View 1 chunk +2 lines, -4 lines 0 comments Download
M net/websockets/websocket_end_to_end_test.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M url/gurl.h View 1 chunk +2 lines, -2 lines 0 comments Download
M url/url_canon_stdstring.h View 2 chunks +13 lines, -12 lines 0 comments Download

Messages

Total messages: 11 (4 generated)
Matt Giuca
See bug for an explanation. brettw: For an initial review (in particular, of the url/* ...
5 years, 10 months ago (2015-02-02 06:33:53 UTC) #2
brettw
lgtm
5 years, 10 months ago (2015-02-03 01:27:12 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/889463003/40001
5 years, 10 months ago (2015-02-04 00:24:28 UTC) #5
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/30217)
5 years, 10 months ago (2015-02-04 00:28:03 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/889463003/60001
5 years, 10 months ago (2015-02-05 06:33:34 UTC) #9
commit-bot: I haz the power
Committed patchset #4 (id:60001)
5 years, 10 months ago (2015-02-05 07:31:56 UTC) #10
commit-bot: I haz the power
5 years, 10 months ago (2015-02-05 07:32:47 UTC) #11
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/77752c3e55c59747a62a362799ac6de5ff56fd45
Cr-Commit-Position: refs/heads/master@{#314771}

Powered by Google App Engine
This is Rietveld 408576698