0

Fix a bunch of invalid uses of StringPiece::data()

A StringPiece is a pointer/length pair and not guaranteed to be
NUL-terminated. Passing .data() into a function that expects a
NUL-terminated const char * is invalid and will go out-of-bounds
(security bug) if the StringPiece didn't happen to created from a
NUL-terminated value.

Fortunately, most StringPieces are created from buffers that happen to
be NUL-terminated, but we cannot safely rely on this. This fixes a
bunch of instances I found looking through code search, most of it in
ash. Some observations:

- Some code converts StringPiece to string by calling .data(). This is
  unnecessary because there is already a conversion defined.

- StringPrintf does not work well with StringPiece. I converted to
  string or used StrCat.

- When functions take const std::string& and the caller has a
  StringPiece, it is a common mistake to call .data(). This is
  incorrect, but. To reduce the risk of people making that mistake
  again, I converted a few functions to StringPiece where it was easy.

Change-Id: I9978c11adbc6f36acf0401e765e4495bdbe8c470
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4573916
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: David Benjamin <davidben@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1152307}
This commit is contained in:
David Benjamin
2023-06-02 01:53:42 +00:00
committed by Chromium LUCI CQ
parent 0182a9c089
commit 41724714b8
21 changed files with 93 additions and 91 deletions

@ -4,6 +4,8 @@
#include "ash/accelerators/accelerator_tracker.h" #include "ash/accelerators/accelerator_tracker.h"
#include <string>
#include "base/metrics/user_metrics.h" #include "base/metrics/user_metrics.h"
#include "ui/events/event.h" #include "ui/events/event.h"
@ -25,7 +27,7 @@ void AcceleratorTracker::OnKeyEvent(ui::KeyEvent* event) {
event->key_code(), event->flags()); event->key_code(), event->flags());
const auto it = accelerator_tracker_map_.find(trackerData); const auto it = accelerator_tracker_map_.find(trackerData);
if (it != accelerator_tracker_map_.end()) { if (it != accelerator_tracker_map_.end()) {
base::RecordAction(base::UserMetricsAction(it->second.data())); base::RecordComputedAction(std::string(it->second));
} }
} }

@ -41,43 +41,43 @@ TEST_F(AcceleratorTrackerTest, TrackKeyEvent) {
base::UserActionTester user_action_tester; base::UserActionTester user_action_tester;
// The metric is not recorded before the event is fired. // The metric is not recorded before the event is fired.
EXPECT_EQ(0, user_action_tester.GetActionCount(intended_user_action.data())); EXPECT_EQ(0, user_action_tester.GetActionCount(intended_user_action));
// The metric is recorded after the event is fired. // The metric is recorded after the event is fired.
ui::KeyEvent key_event(ui::ET_KEY_PRESSED, intended_key_code, ui::KeyEvent key_event(ui::ET_KEY_PRESSED, intended_key_code,
intended_modifier); intended_modifier);
accelerator_tracker.OnKeyEvent(&key_event); accelerator_tracker.OnKeyEvent(&key_event);
EXPECT_EQ(1, user_action_tester.GetActionCount(intended_user_action.data())); EXPECT_EQ(1, user_action_tester.GetActionCount(intended_user_action));
// Fire a similar key event won't trigger this metric. // Fire a similar key event won't trigger this metric.
// An event with same key code and modifier, but different key states. // An event with same key code and modifier, but different key states.
ui::KeyEvent key_event1(ui::ET_KEY_RELEASED, intended_key_code, ui::KeyEvent key_event1(ui::ET_KEY_RELEASED, intended_key_code,
intended_modifier); intended_modifier);
accelerator_tracker.OnKeyEvent(&key_event1); accelerator_tracker.OnKeyEvent(&key_event1);
EXPECT_EQ(1, user_action_tester.GetActionCount(intended_user_action.data())); EXPECT_EQ(1, user_action_tester.GetActionCount(intended_user_action));
// An event with same key code and key state, but superset of modifiers. // An event with same key code and key state, but superset of modifiers.
ui::KeyEvent key_event2(ui::ET_KEY_PRESSED, intended_key_code, ui::KeyEvent key_event2(ui::ET_KEY_PRESSED, intended_key_code,
intended_modifier | ui::EF_ALT_DOWN); intended_modifier | ui::EF_ALT_DOWN);
accelerator_tracker.OnKeyEvent(&key_event2); accelerator_tracker.OnKeyEvent(&key_event2);
EXPECT_EQ(1, user_action_tester.GetActionCount(intended_user_action.data())); EXPECT_EQ(1, user_action_tester.GetActionCount(intended_user_action));
// An event with same key code and key state, but subset of modifiers. // An event with same key code and key state, but subset of modifiers.
ui::KeyEvent key_event3(ui::ET_KEY_PRESSED, intended_key_code, ui::KeyEvent key_event3(ui::ET_KEY_PRESSED, intended_key_code,
ui::EF_CONTROL_DOWN); ui::EF_CONTROL_DOWN);
accelerator_tracker.OnKeyEvent(&key_event3); accelerator_tracker.OnKeyEvent(&key_event3);
EXPECT_EQ(1, user_action_tester.GetActionCount(intended_user_action.data())); EXPECT_EQ(1, user_action_tester.GetActionCount(intended_user_action));
// An event with same key code and key state, but different modifiers. // An event with same key code and key state, but different modifiers.
ui::KeyEvent key_event4(ui::ET_KEY_PRESSED, intended_key_code, ui::KeyEvent key_event4(ui::ET_KEY_PRESSED, intended_key_code,
ui::EF_ALT_DOWN); ui::EF_ALT_DOWN);
accelerator_tracker.OnKeyEvent(&key_event4); accelerator_tracker.OnKeyEvent(&key_event4);
EXPECT_EQ(1, user_action_tester.GetActionCount(intended_user_action.data())); EXPECT_EQ(1, user_action_tester.GetActionCount(intended_user_action));
// An event with same key state and modifiers, but different key codes. // An event with same key state and modifiers, but different key codes.
ui::KeyEvent key_event5(ui::ET_KEY_PRESSED, ui::VKEY_B, intended_modifier); ui::KeyEvent key_event5(ui::ET_KEY_PRESSED, ui::VKEY_B, intended_modifier);
accelerator_tracker.OnKeyEvent(&key_event5); accelerator_tracker.OnKeyEvent(&key_event5);
EXPECT_EQ(1, user_action_tester.GetActionCount(intended_user_action.data())); EXPECT_EQ(1, user_action_tester.GetActionCount(intended_user_action));
} }
} // namespace ash } // namespace ash

@ -104,7 +104,7 @@ void AmbientUiSettings::WriteToPrefService(PrefService& pref_service) const {
} }
std::string AmbientUiSettings::ToString() const { std::string AmbientUiSettings::ToString() const {
std::string output(ash::ToString(theme_).data()); std::string output(ash::ToString(theme_));
if (theme_ == AmbientTheme::kVideo) { if (theme_ == AmbientTheme::kVideo) {
CHECK(video_); CHECK(video_);
base::StrAppend(&output, {".", ash::ToString(*video_)}); base::StrAppend(&output, {".", ash::ToString(*video_)});

@ -69,10 +69,10 @@ std::string GetHistogramName(const char* prefix, bool tablet_mode) {
return histogram; return histogram;
} }
void RecordEngagementTime(base::StringPiece histogram_name, void RecordEngagementTime(const std::string& histogram_name,
base::TimeDelta engagement_time) { base::TimeDelta engagement_time) {
base::UmaHistogramCustomTimes( base::UmaHistogramCustomTimes(
histogram_name.data(), histogram_name,
/*sample=*/engagement_time, /*sample=*/engagement_time,
// There is no value in bucketing engagement times that are on the order // There is no value in bucketing engagement times that are on the order
// of milliseconds. A 1 second minimum is imposed here but not in the // of milliseconds. A 1 second minimum is imposed here but not in the

@ -31,8 +31,8 @@ namespace {
// 2 -> "CrOS_AttributionNode2" // 2 -> "CrOS_AttributionNode2"
// ... // ...
std::string BuildAttributionNodeName(int idx) { std::string BuildAttributionNodeName(int idx) {
return base::StringPrintf("%s_Attribution_Text%d", return base::StrCat({kLottieCustomizableIdPrefix, "_Attribution_Text",
kLottieCustomizableIdPrefix.data(), idx); base::NumberToString(idx)});
} }
// Not all text nodes in the animation are necessarily ones that should hold // Not all text nodes in the animation are necessarily ones that should hold

@ -23,9 +23,8 @@ std::string GenerateLottieCustomizableIdForTesting(int unique_id) {
std::string GenerateLottieDynamicAssetIdForTesting(base::StringPiece position, std::string GenerateLottieDynamicAssetIdForTesting(base::StringPiece position,
int idx) { int idx) {
CHECK(!position.empty()); CHECK(!position.empty());
return base::StringPrintf("%s_Photo_Position%s_%d", return base::StrCat({kLottieCustomizableIdPrefix, "_Photo_Position", position,
kLottieCustomizableIdPrefix.data(), position.data(), "_", base::NumberToString(idx)});
idx);
} }
AmbientPhotoConfig GenerateAnimationConfigWithNAssets(int num_assets) { AmbientPhotoConfig GenerateAnimationConfigWithNAssets(int num_assets) {

@ -62,7 +62,7 @@ std::u16string GetTimezoneId(const icu::TimeZone& timezone) {
base::Time ToUTCTime(base::StringPiece utc_time_str) { base::Time ToUTCTime(base::StringPiece utc_time_str) {
base::Time time; base::Time time;
CHECK(base::Time::FromUTCString(utc_time_str.data(), &time)) CHECK(base::Time::FromUTCString(std::string(utc_time_str).c_str(), &time))
<< "Invalid UTC time string specified: " << utc_time_str; << "Invalid UTC time string specified: " << utc_time_str;
return time; return time;
} }

@ -22,12 +22,12 @@ UserActionTester::~UserActionTester() {
base::RemoveActionCallback(action_callback_); base::RemoveActionCallback(action_callback_);
} }
int UserActionTester::GetActionCount(const std::string& user_action) const { int UserActionTester::GetActionCount(base::StringPiece user_action) const {
return times_map_.count(user_action); return times_map_.count(user_action);
} }
std::vector<TimeTicks> UserActionTester::GetActionTimes( std::vector<TimeTicks> UserActionTester::GetActionTimes(
const std::string& user_action) const { base::StringPiece user_action) const {
std::vector<TimeTicks> result; std::vector<TimeTicks> result;
auto range = times_map_.equal_range(user_action); auto range = times_map_.equal_range(user_action);
for (auto it = range.first; it != range.second; it++) { for (auto it = range.first; it != range.second; it++) {

@ -11,6 +11,7 @@
#include "base/memory/scoped_refptr.h" #include "base/memory/scoped_refptr.h"
#include "base/metrics/user_metrics.h" #include "base/metrics/user_metrics.h"
#include "base/strings/string_piece.h"
#include "base/task/single_thread_task_runner.h" #include "base/task/single_thread_task_runner.h"
#include "base/time/time.h" #include "base/time/time.h"
@ -31,17 +32,17 @@ class UserActionTester {
~UserActionTester(); ~UserActionTester();
// Returns the number of times the given |user_action| occurred. // Returns the number of times the given |user_action| occurred.
int GetActionCount(const std::string& user_action) const; int GetActionCount(base::StringPiece user_action) const;
// Returns the time values at which the given |user_action| has occurred. // Returns the time values at which the given |user_action| has occurred.
// The order of returned values is unspecified. // The order of returned values is unspecified.
std::vector<TimeTicks> GetActionTimes(const std::string& user_action) const; std::vector<TimeTicks> GetActionTimes(base::StringPiece user_action) const;
// Resets all user action counts to 0. // Resets all user action counts to 0.
void ResetCounts(); void ResetCounts();
private: private:
typedef std::multimap<std::string, TimeTicks> UserActionTimesMap; typedef std::multimap<std::string, TimeTicks, std::less<>> UserActionTimesMap;
// The callback that is notified when a user actions occurs. // The callback that is notified when a user actions occurs.
void OnUserAction(const std::string& user_action, TimeTicks action_time); void OnUserAction(const std::string& user_action, TimeTicks action_time);

@ -140,9 +140,10 @@ bool TouchFile(const base::FilePath& path,
base::StringPiece mtime_string, base::StringPiece mtime_string,
base::StringPiece atime_string) { base::StringPiece atime_string) {
base::Time mtime, atime; base::Time mtime, atime;
auto result = base::Time::FromString(mtime_string.data(), &mtime) && auto result =
base::Time::FromString(atime_string.data(), &atime) && base::Time::FromString(std::string(mtime_string).c_str(), &mtime) &&
base::TouchFile(path, atime, mtime); base::Time::FromString(std::string(atime_string).c_str(), &atime) &&
base::TouchFile(path, atime, mtime);
return result; return result;
} }

@ -573,7 +573,7 @@ struct AddEntriesMessage {
// Maps |value| to base::Time. Returns true on success. // Maps |value| to base::Time. Returns true on success.
static bool MapStringToTime(base::StringPiece value, base::Time* time) { static bool MapStringToTime(base::StringPiece value, base::Time* time) {
return base::Time::FromString(value.data(), time); return base::Time::FromString(std::string(value).c_str(), time);
} }
}; };
}; };

@ -1199,7 +1199,7 @@ IN_PROC_BROWSER_TEST_P(InputMethodEngineBrowserTest, APIArgumentTest) {
ASSERT_TRUE( ASSERT_TRUE(
content::ExecJs(browser()->tab_strip_model()->GetActiveWebContents(), content::ExecJs(browser()->tab_strip_model()->GetActiveWebContents(),
password_field_change_to_text_script.data())); password_field_change_to_text_script));
ASSERT_TRUE(focus_listener.WaitUntilSatisfied()); ASSERT_TRUE(focus_listener.WaitUntilSatisfied());
ASSERT_TRUE(focus_listener.was_satisfied()); ASSERT_TRUE(focus_listener.was_satisfied());

@ -17,6 +17,7 @@
#include "base/functional/bind.h" #include "base/functional/bind.h"
#include "base/json/json_reader.h" #include "base/json/json_reader.h"
#include "base/json/json_writer.h" #include "base/json/json_writer.h"
#include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/strings/string_piece_forward.h" #include "base/strings/string_piece_forward.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
@ -395,9 +396,8 @@ void DeviceCommandFetchSupportPacketJob::OnEventEnqueued(
return; return;
} }
std::string error_message = std::string error_message = base::StrCat(
base::StringPrintf("Couldn't enqueue event to reporting queue: %s", {"Couldn't enqueue event to reporting queue: ", status.error_message()});
status.error_message().data());
SYSLOG(ERROR) << error_message; SYSLOG(ERROR) << error_message;
std::move(result_callback_).Run(ResultType::kFailure, error_message); std::move(result_callback_).Run(ResultType::kFailure, error_message);

@ -339,8 +339,8 @@ void SystemExtensionsApiBrowserTest::MaybeInstallSystemExtension(
} }
// Write manifest. // Write manifest.
const std::string manifest_str = const std::string manifest_str = base::StringPrintf(
base::StringPrintf(manifest_template_.data(), test_file_name.data()); manifest_template_.c_str(), std::string(test_file_name).c_str());
CHECK(base::WriteFile(system_extension_path.AppendASCII("manifest.json"), CHECK(base::WriteFile(system_extension_path.AppendASCII("manifest.json"),
manifest_str)) manifest_str))
<< "Failed to write the manifest."; << "Failed to write the manifest.";

@ -39,7 +39,7 @@ void AppendAmbientVideoAlbums(AmbientVideo currently_selected_video,
std::vector<mojom::AmbientModeAlbumPtr>& output) { std::vector<mojom::AmbientModeAlbumPtr>& output) {
for (const VideoAlbumInfo& video_album_info : kAllVideoAlbumInfo) { for (const VideoAlbumInfo& video_album_info : kAllVideoAlbumInfo) {
mojom::AmbientModeAlbumPtr album = mojom::AmbientModeAlbum::New(); mojom::AmbientModeAlbumPtr album = mojom::AmbientModeAlbum::New();
album->id = video_album_info.id.data(); album->id = std::string(video_album_info.id);
album->checked = currently_selected_video == video_album_info.video; album->checked = currently_selected_video == video_album_info.video;
album->title = l10n_util::GetStringUTF8(video_album_info.title_resource_id); album->title = l10n_util::GetStringUTF8(video_album_info.title_resource_id);
// Product name does not need to be translated. // Product name does not need to be translated.

@ -316,10 +316,11 @@ class PersonalizationAppAmbientProviderImplTest : public ash::AshTestBase {
ambient_provider_->SetTopicSource(topic_source); ambient_provider_->SetTopicSource(topic_source);
} }
void SetAlbumSelected(const std::string& id, void SetAlbumSelected(base::StringPiece id,
ash::AmbientModeTopicSource topic_source, ash::AmbientModeTopicSource topic_source,
bool selected) { bool selected) {
ambient_provider_->SetAlbumSelected(id, topic_source, selected); ambient_provider_->SetAlbumSelected(std::string(id), topic_source,
selected);
} }
void FetchPreviewImages() { ambient_provider_->FetchPreviewImages(); } void FetchPreviewImages() { ambient_provider_->FetchPreviewImages(); }
@ -927,19 +928,17 @@ TEST_F(PersonalizationAppAmbientProviderImplTest, TestSetSelectedVideo) {
/*new_mexico_selected=*/true); /*new_mexico_selected=*/true);
// Switch to clouds. // Switch to clouds.
SetAlbumSelected(kCloudsAlbumId.data(), AmbientModeTopicSource::kVideo, true); SetAlbumSelected(kCloudsAlbumId, AmbientModeTopicSource::kVideo, true);
expect_videos_selected(/*clouds_selected=*/true, expect_videos_selected(/*clouds_selected=*/true,
/*new_mexico_selected=*/false); /*new_mexico_selected=*/false);
// Switch back to new mexico. // Switch back to new mexico.
SetAlbumSelected(kNewMexicoAlbumId.data(), AmbientModeTopicSource::kVideo, SetAlbumSelected(kNewMexicoAlbumId, AmbientModeTopicSource::kVideo, true);
true);
expect_videos_selected(/*clouds_selected=*/false, expect_videos_selected(/*clouds_selected=*/false,
/*new_mexico_selected=*/true); /*new_mexico_selected=*/true);
// Should never be in a state where there are no videos selected. // Should never be in a state where there are no videos selected.
SetAlbumSelected(kNewMexicoAlbumId.data(), AmbientModeTopicSource::kVideo, SetAlbumSelected(kNewMexicoAlbumId, AmbientModeTopicSource::kVideo, false);
false);
expect_videos_selected(/*clouds_selected=*/false, expect_videos_selected(/*clouds_selected=*/false,
/*new_mexico_selected=*/true); /*new_mexico_selected=*/true);

@ -99,7 +99,7 @@ void AppendRedirect(std::vector<std::string>* redirects,
redirects->push_back(base::StringPrintf( redirects->push_back(base::StringPrintf(
"[%zu/%zu] %s -> %s (%s) -> %s", redirect_index + 1, chain.length, "[%zu/%zu] %s -> %s (%s) -> %s", redirect_index + 1, chain.length,
FormatURL(chain.initial_url).c_str(), FormatURL(redirect.url).c_str(), FormatURL(chain.initial_url).c_str(), FormatURL(redirect.url).c_str(),
SiteDataAccessTypeToString(redirect.access_type).data(), std::string(SiteDataAccessTypeToString(redirect.access_type)).c_str(),
FormatURL(chain.final_url).c_str())); FormatURL(chain.final_url).c_str()));
} }

@ -192,8 +192,8 @@ void RunAddHostPermission(Profile* profile,
auto function = auto function =
base::MakeRefCounted<api::DeveloperPrivateAddHostPermissionFunction>(); base::MakeRefCounted<api::DeveloperPrivateAddHostPermissionFunction>();
std::string args = base::StringPrintf(R"(["%s", "%s"])", std::string args = base::StringPrintf(
extension.id().c_str(), host.data()); R"(["%s", "%s"])", extension.id().c_str(), std::string(host).c_str());
if (should_succeed) { if (should_succeed) {
EXPECT_TRUE(api_test_utils::RunFunction(function.get(), args, profile)) EXPECT_TRUE(api_test_utils::RunFunction(function.get(), args, profile))
<< function->GetError(); << function->GetError();

@ -222,7 +222,7 @@ void CastActivityManager::DoLaunchSession(DoLaunchSessionParams params) {
base::TimeDelta launch_timeout = cast_source.launch_timeout(); base::TimeDelta launch_timeout = cast_source.launch_timeout();
std::vector<std::string> type_str; std::vector<std::string> type_str;
for (ReceiverAppType type : cast_source.supported_app_types()) { for (ReceiverAppType type : cast_source.supported_app_types()) {
type_str.push_back(cast_util::EnumToString(type).value().data()); type_str.emplace_back(cast_util::EnumToString(type).value());
} }
logger_->LogInfo(mojom::LogCategory::kRoute, kLoggerComponent, logger_->LogInfo(mojom::LogCategory::kRoute, kLoggerComponent,
"Sent a Launch Session request.", sink.id(), "Sent a Launch Session request.", sink.id(),

@ -645,7 +645,7 @@ bool BeginNavigateToURLFromRenderer(const ToRenderFrameHost& adapter,
} }
bool NavigateIframeToURL(WebContents* web_contents, bool NavigateIframeToURL(WebContents* web_contents,
const std::string& iframe_id, base::StringPiece iframe_id,
const GURL& url) { const GURL& url) {
TestNavigationObserver load_observer(web_contents); TestNavigationObserver load_observer(web_contents);
bool result = BeginNavigateIframeToURL(web_contents, iframe_id, url); bool result = BeginNavigateIframeToURL(web_contents, iframe_id, url);
@ -654,13 +654,11 @@ bool NavigateIframeToURL(WebContents* web_contents,
} }
bool BeginNavigateIframeToURL(WebContents* web_contents, bool BeginNavigateIframeToURL(WebContents* web_contents,
const std::string& iframe_id, base::StringPiece iframe_id,
const GURL& url) { const GURL& url) {
std::string script = base::StringPrintf( std::string script =
"setTimeout(\"" base::StrCat({"setTimeout(\"var iframes = document.getElementById('",
"var iframes = document.getElementById('%s');iframes.src='%s';" iframe_id, "');iframes.src='", url.spec(), "';\",0)"});
"\",0)",
iframe_id.c_str(), url.spec().c_str());
return ExecJs(web_contents, script, EXECUTE_SCRIPT_NO_USER_GESTURE); return ExecJs(web_contents, script, EXECUTE_SCRIPT_NO_USER_GESTURE);
} }
@ -701,7 +699,7 @@ void NavigateToURLBlockUntilNavigationsComplete(
} }
GURL GetFileUrlWithQuery(const base::FilePath& path, GURL GetFileUrlWithQuery(const base::FilePath& path,
const std::string& query_string) { base::StringPiece query_string) {
GURL url = net::FilePathToFileURL(path); GURL url = net::FilePathToFileURL(path);
if (!query_string.empty()) { if (!query_string.empty()) {
GURL::Replacements replacements; GURL::Replacements replacements;
@ -935,7 +933,7 @@ void SimulateMouseClickAt(WebContents* web_contents,
gfx::PointF GetCenterCoordinatesOfElementWithId( gfx::PointF GetCenterCoordinatesOfElementWithId(
content::WebContents* web_contents, content::WebContents* web_contents,
const std::string& id) { base::StringPiece id) {
float x = EvalJs(web_contents, float x = EvalJs(web_contents,
JsReplace("const bounds = " JsReplace("const bounds = "
"document.getElementById($1)." "document.getElementById($1)."
@ -954,7 +952,7 @@ gfx::PointF GetCenterCoordinatesOfElementWithId(
} }
void SimulateMouseClickOrTapElementWithId(content::WebContents* web_contents, void SimulateMouseClickOrTapElementWithId(content::WebContents* web_contents,
const std::string& id) { base::StringPiece id) {
gfx::Point point = gfx::ToFlooredPoint( gfx::Point point = gfx::ToFlooredPoint(
GetCenterCoordinatesOfElementWithId(web_contents, id)); GetCenterCoordinatesOfElementWithId(web_contents, id));
@ -1358,7 +1356,7 @@ RenderFrameHost* ConvertToRenderFrameHost(RenderFrameHost* render_frame_host) {
} }
void ExecuteScriptAsync(const ToRenderFrameHost& adapter, void ExecuteScriptAsync(const ToRenderFrameHost& adapter,
const std::string& script) { base::StringPiece script) {
// Prerendering pages will never have user gesture. // Prerendering pages will never have user gesture.
if (adapter.render_frame_host()->GetLifecycleState() == if (adapter.render_frame_host()->GetLifecycleState() ==
RenderFrameHost::LifecycleState::kPrerendering) { RenderFrameHost::LifecycleState::kPrerendering) {
@ -1370,13 +1368,13 @@ void ExecuteScriptAsync(const ToRenderFrameHost& adapter,
} }
void ExecuteScriptAsyncWithoutUserGesture(const ToRenderFrameHost& adapter, void ExecuteScriptAsyncWithoutUserGesture(const ToRenderFrameHost& adapter,
const std::string& script) { base::StringPiece script) {
adapter.render_frame_host()->ExecuteJavaScriptForTests( adapter.render_frame_host()->ExecuteJavaScriptForTests(
base::UTF8ToUTF16(script), base::NullCallback()); base::UTF8ToUTF16(script), base::NullCallback());
} }
// EvalJsResult methods. // EvalJsResult methods.
EvalJsResult::EvalJsResult(base::Value value, const std::string& error) EvalJsResult::EvalJsResult(base::Value value, base::StringPiece error)
: value(error.empty() ? std::move(value) : base::Value()), error(error) {} : value(error.empty() ? std::move(value) : base::Value()), error(error) {}
EvalJsResult::EvalJsResult(const EvalJsResult& other) EvalJsResult::EvalJsResult(const EvalJsResult& other)
@ -1450,9 +1448,9 @@ namespace {
// //
// TODO(nick): Elide snippets to 80 chars, since it is common for sources to not // TODO(nick): Elide snippets to 80 chars, since it is common for sources to not
// include newlines. // include newlines.
std::string AnnotateAndAdjustJsStackTraces(const std::string& js_error, std::string AnnotateAndAdjustJsStackTraces(base::StringPiece js_error,
std::string source_name, std::string source_name,
const std::string& source, base::StringPiece source,
int column_adjustment_for_line_one) { int column_adjustment_for_line_one) {
// Escape wildcards in |source_name| for use in MatchPattern. // Escape wildcards in |source_name| for use in MatchPattern.
base::ReplaceChars(source_name, "\\", "\\\\", &source_name); base::ReplaceChars(source_name, "\\", "\\\\", &source_name);
@ -1610,8 +1608,8 @@ class ExecuteJavaScriptForTestsWaiter : public WebContentsObserver {
}; };
EvalJsResult EvalJsRunner(const ToRenderFrameHost& execution_target, EvalJsResult EvalJsRunner(const ToRenderFrameHost& execution_target,
const std::string& script, base::StringPiece script,
const std::string& source_url, base::StringPiece source_url,
int options, int options,
int32_t world_id) { int32_t world_id) {
RenderFrameHostImpl* rfh = RenderFrameHostImpl* rfh =
@ -1648,8 +1646,9 @@ EvalJsResult EvalJsRunner(const ToRenderFrameHost& execution_target,
CHECK(result_value.is_string() && !result_value.GetString().empty()); CHECK(result_value.is_string() && !result_value.GetString().empty());
std::string error_text = std::string error_text =
"a JavaScript error: \"" + result_value.GetString() + "\""; "a JavaScript error: \"" + result_value.GetString() + "\"";
return EvalJsResult(base::Value(), AnnotateAndAdjustJsStackTraces( return EvalJsResult(base::Value(),
error_text, source_url, script, 0)); AnnotateAndAdjustJsStackTraces(
error_text, std::string(source_url), script, 0));
} }
return EvalJsResult(result_value.Clone(), std::string()); return EvalJsResult(result_value.Clone(), std::string());
@ -1658,7 +1657,7 @@ EvalJsResult EvalJsRunner(const ToRenderFrameHost& execution_target,
} // namespace } // namespace
::testing::AssertionResult ExecJs(const ToRenderFrameHost& execution_target, ::testing::AssertionResult ExecJs(const ToRenderFrameHost& execution_target,
const std::string& script, base::StringPiece script,
int options, int options,
int32_t world_id) { int32_t world_id) {
// TODO(nick): Do we care enough about folks shooting themselves in the foot // TODO(nick): Do we care enough about folks shooting themselves in the foot
@ -1674,7 +1673,7 @@ EvalJsResult EvalJsRunner(const ToRenderFrameHost& execution_target,
} }
EvalJsResult EvalJs(const ToRenderFrameHost& execution_target, EvalJsResult EvalJs(const ToRenderFrameHost& execution_target,
const std::string& script, base::StringPiece script,
int options, int options,
int32_t world_id) { int32_t world_id) {
TRACE_EVENT1("test", "EvalJs", "script", script); TRACE_EVENT1("test", "EvalJs", "script", script);
@ -1687,8 +1686,8 @@ EvalJsResult EvalJs(const ToRenderFrameHost& execution_target,
// let/const don't leak outside the code being run, but vars will float to // let/const don't leak outside the code being run, but vars will float to
// the outer scope. // the outer scope.
const char* kSourceURL = "__const_std::string&_script__"; const char* kSourceURL = "__const_std::string&_script__";
std::string modified_script = base::StringPrintf("{%s\n}\n//# sourceURL=%s", std::string modified_script =
script.c_str(), kSourceURL); base::StrCat({"{", script, "\n}\n//# sourceURL=", kSourceURL});
return EvalJsRunner(execution_target, modified_script, kSourceURL, options, return EvalJsRunner(execution_target, modified_script, kSourceURL, options,
world_id); world_id);
@ -1696,8 +1695,8 @@ EvalJsResult EvalJs(const ToRenderFrameHost& execution_target,
EvalJsResult EvalJsAfterLifecycleUpdate( EvalJsResult EvalJsAfterLifecycleUpdate(
const ToRenderFrameHost& execution_target, const ToRenderFrameHost& execution_target,
const std::string& raf_script, base::StringPiece raf_script,
const std::string& script, base::StringPiece script,
int options, int options,
int32_t world_id) { int32_t world_id) {
TRACE_EVENT2("test", "EvalJsAfterLifecycleUpdate", "raf_script", raf_script, TRACE_EVENT2("test", "EvalJsAfterLifecycleUpdate", "raf_script", raf_script,
@ -1707,11 +1706,11 @@ EvalJsResult EvalJsAfterLifecycleUpdate(
const char* kWrapperURL = "__const_std::string&_EvalJsAfterLifecycleUpdate__"; const char* kWrapperURL = "__const_std::string&_EvalJsAfterLifecycleUpdate__";
std::string modified_raf_script; std::string modified_raf_script;
if (raf_script.length()) { if (raf_script.length()) {
modified_raf_script = base::StringPrintf("%s;\n//# sourceURL=%s", modified_raf_script =
raf_script.c_str(), kSourceURL); base::StrCat({raf_script, ";\n//# sourceURL=", kSourceURL});
} }
std::string modified_script = std::string modified_script =
base::StringPrintf("%s;\n//# sourceURL=%s", script.c_str(), kSourceURL); base::StrCat({script, ";\n//# sourceURL=", kSourceURL});
// This runner_script delays running the argument scripts until just before // This runner_script delays running the argument scripts until just before
// (|raf_script|) and after (|script|) a rendering update. // (|raf_script|) and after (|script|) a rendering update.
@ -1765,7 +1764,7 @@ RenderFrameHost* FrameMatchingPredicate(
return rfh; return rfh;
} }
bool FrameMatchesName(const std::string& name, RenderFrameHost* frame) { bool FrameMatchesName(base::StringPiece name, RenderFrameHost* frame) {
return frame->GetFrameName() == name; return frame->GetFrameName() == name;
} }
@ -2039,7 +2038,7 @@ ui::AXNodeData GetFocusedAccessibilityNodeInfo(WebContents* web_contents) {
} }
bool AccessibilityTreeContainsNodeWithName(BrowserAccessibility* node, bool AccessibilityTreeContainsNodeWithName(BrowserAccessibility* node,
const std::string& name) { base::StringPiece name) {
// If an image annotation is set, it plays the same role as a name, so it // If an image annotation is set, it plays the same role as a name, so it
// makes sense to check both in the same test helper. // makes sense to check both in the same test helper.
if (node->GetStringAttribute(ax::mojom::StringAttribute::kName) == name || if (node->GetStringAttribute(ax::mojom::StringAttribute::kName) == name ||
@ -2060,7 +2059,7 @@ void WaitForAccessibilityTreeToChange(WebContents* web_contents) {
} }
void WaitForAccessibilityTreeToContainNodeWithName(WebContents* web_contents, void WaitForAccessibilityTreeToContainNodeWithName(WebContents* web_contents,
const std::string& name) { base::StringPiece name) {
WebContentsImpl* web_contents_impl = static_cast<WebContentsImpl*>( WebContentsImpl* web_contents_impl = static_cast<WebContentsImpl*>(
web_contents); web_contents);
RenderFrameHostImpl* main_frame = static_cast<RenderFrameHostImpl*>( RenderFrameHostImpl* main_frame = static_cast<RenderFrameHostImpl*>(
@ -2356,7 +2355,7 @@ void RenderProcessHostWatcher::RenderProcessHostDestroyed(
RenderProcessHostKillWaiter::RenderProcessHostKillWaiter( RenderProcessHostKillWaiter::RenderProcessHostKillWaiter(
RenderProcessHost* render_process_host, RenderProcessHost* render_process_host,
const std::string& uma_name) base::StringPiece uma_name)
: exit_watcher_(render_process_host, : exit_watcher_(render_process_host,
RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT), RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT),
uma_name_(uma_name) {} uma_name_(uma_name) {}

@ -22,6 +22,7 @@
#include "base/process/process.h" #include "base/process/process.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/scoped_observation.h" #include "base/scoped_observation.h"
#include "base/strings/string_piece.h"
#include "base/template_util.h" #include "base/template_util.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/types/strong_alias.h" #include "base/types/strong_alias.h"
@ -243,18 +244,18 @@ void NavigateToURLBlockUntilNavigationsComplete(
// necessary, a user activation can be triggered right before calling this // necessary, a user activation can be triggered right before calling this
// method, e.g. by calling |ExecJs(frame_tree_node, "")|. // method, e.g. by calling |ExecJs(frame_tree_node, "")|.
bool NavigateIframeToURL(WebContents* web_contents, bool NavigateIframeToURL(WebContents* web_contents,
const std::string& iframe_id, base::StringPiece iframe_id,
const GURL& url); const GURL& url);
// Similar to |NavigateIframeToURL()| but returns as soon as the navigation is // Similar to |NavigateIframeToURL()| but returns as soon as the navigation is
// initiated. // initiated.
bool BeginNavigateIframeToURL(WebContents* web_contents, bool BeginNavigateIframeToURL(WebContents* web_contents,
const std::string& iframe_id, base::StringPiece iframe_id,
const GURL& url); const GURL& url);
// Generate a URL for a file path including a query string. // Generate a URL for a file path including a query string.
GURL GetFileUrlWithQuery(const base::FilePath& path, GURL GetFileUrlWithQuery(const base::FilePath& path,
const std::string& query_string); base::StringPiece query_string);
// Checks whether the page type of the last committed navigation entry matches // Checks whether the page type of the last committed navigation entry matches
// |page_type|. // |page_type|.
@ -341,12 +342,12 @@ void SimulateMouseClickAt(WebContents* web_contents,
// friendly by taking zooming into account. // friendly by taking zooming into account.
gfx::PointF GetCenterCoordinatesOfElementWithId( gfx::PointF GetCenterCoordinatesOfElementWithId(
content::WebContents* web_contents, content::WebContents* web_contents,
const std::string& id); base::StringPiece id);
// Retrieves the center coordinates of the element with id |id| and simulates a // Retrieves the center coordinates of the element with id |id| and simulates a
// mouse click there using SimulateMouseClickAt(). // mouse click there using SimulateMouseClickAt().
void SimulateMouseClickOrTapElementWithId(content::WebContents* web_contents, void SimulateMouseClickOrTapElementWithId(content::WebContents* web_contents,
const std::string& id); base::StringPiece id);
// Simulates asynchronously a mouse enter/move/leave event. The mouse event is // Simulates asynchronously a mouse enter/move/leave event. The mouse event is
// routed through RenderWidgetHostInputEventRouter and thus can target OOPIFs. // routed through RenderWidgetHostInputEventRouter and thus can target OOPIFs.
@ -571,12 +572,12 @@ RenderFrameHost* ConvertToRenderFrameHost(WebContents* web_contents);
// - EvalJs (if you want to retrieve a value) // - EvalJs (if you want to retrieve a value)
// - DOMMessageQueue (to manually wait for domAutomationController.send(...)) // - DOMMessageQueue (to manually wait for domAutomationController.send(...))
void ExecuteScriptAsync(const ToRenderFrameHost& adapter, void ExecuteScriptAsync(const ToRenderFrameHost& adapter,
const std::string& script); base::StringPiece script);
// Same as `content::ExecuteScriptAsync()`, but doesn't send a user gesture to // Same as `content::ExecuteScriptAsync()`, but doesn't send a user gesture to
// the renderer. // the renderer.
void ExecuteScriptAsyncWithoutUserGesture(const ToRenderFrameHost& adapter, void ExecuteScriptAsyncWithoutUserGesture(const ToRenderFrameHost& adapter,
const std::string& script); base::StringPiece script);
// JsLiteralHelper is a helper class that determines what types are legal to // JsLiteralHelper is a helper class that determines what types are legal to
// pass to StringifyJsLiteral. Legal types include int, string, StringPiece, // pass to StringifyJsLiteral. Legal types include int, string, StringPiece,
@ -692,7 +693,7 @@ struct EvalJsResult {
// Creates an EvalJs result. If |error| is non-empty, |value| will be // Creates an EvalJs result. If |error| is non-empty, |value| will be
// ignored. // ignored.
EvalJsResult(base::Value value, const std::string& error); EvalJsResult(base::Value value, base::StringPiece error);
// Copy ctor. // Copy ctor.
EvalJsResult(const EvalJsResult& value); EvalJsResult(const EvalJsResult& value);
@ -844,7 +845,7 @@ enum EvalJsOptions {
// //
// It is guaranteed that EvalJs works even when the target frame is frozen. // It is guaranteed that EvalJs works even when the target frame is frozen.
[[nodiscard]] EvalJsResult EvalJs(const ToRenderFrameHost& execution_target, [[nodiscard]] EvalJsResult EvalJs(const ToRenderFrameHost& execution_target,
const std::string& script, base::StringPiece script,
int options = EXECUTE_SCRIPT_DEFAULT_OPTIONS, int options = EXECUTE_SCRIPT_DEFAULT_OPTIONS,
int32_t world_id = ISOLATED_WORLD_ID_GLOBAL); int32_t world_id = ISOLATED_WORLD_ID_GLOBAL);
@ -855,8 +856,8 @@ enum EvalJsOptions {
// processed by the browser. // processed by the browser.
[[nodiscard]] EvalJsResult EvalJsAfterLifecycleUpdate( [[nodiscard]] EvalJsResult EvalJsAfterLifecycleUpdate(
const ToRenderFrameHost& execution_target, const ToRenderFrameHost& execution_target,
const std::string& raf_script, base::StringPiece raf_script,
const std::string& script, base::StringPiece script,
int options = EXECUTE_SCRIPT_DEFAULT_OPTIONS, int options = EXECUTE_SCRIPT_DEFAULT_OPTIONS,
int32_t world_id = ISOLATED_WORLD_ID_GLOBAL); int32_t world_id = ISOLATED_WORLD_ID_GLOBAL);
@ -870,7 +871,7 @@ enum EvalJsOptions {
// until it resolves (by default). // until it resolves (by default).
[[nodiscard]] ::testing::AssertionResult ExecJs( [[nodiscard]] ::testing::AssertionResult ExecJs(
const ToRenderFrameHost& execution_target, const ToRenderFrameHost& execution_target,
const std::string& script, base::StringPiece script,
int options = EXECUTE_SCRIPT_DEFAULT_OPTIONS, int options = EXECUTE_SCRIPT_DEFAULT_OPTIONS,
int32_t world_id = ISOLATED_WORLD_ID_GLOBAL); int32_t world_id = ISOLATED_WORLD_ID_GLOBAL);
@ -889,7 +890,7 @@ RenderFrameHost* FrameMatchingPredicate(
base::RepeatingCallback<bool(RenderFrameHost*)> predicate); base::RepeatingCallback<bool(RenderFrameHost*)> predicate);
// Predicates for use with FrameMatchingPredicate[OrNullPtr](). // Predicates for use with FrameMatchingPredicate[OrNullPtr]().
bool FrameMatchesName(const std::string& name, RenderFrameHost* frame); bool FrameMatchesName(base::StringPiece name, RenderFrameHost* frame);
bool FrameIsChildOfMainFrame(RenderFrameHost* frame); bool FrameIsChildOfMainFrame(RenderFrameHost* frame);
bool FrameHasSourceUrl(const GURL& url, RenderFrameHost* frame); bool FrameHasSourceUrl(const GURL& url, RenderFrameHost* frame);
@ -1023,7 +1024,7 @@ void WaitForAccessibilityTreeToChange(WebContents* web_contents);
// WaitForAccessibilityTreeToChange, above, and then checks again. // WaitForAccessibilityTreeToChange, above, and then checks again.
// Keeps looping until the text is found (or the test times out). // Keeps looping until the text is found (or the test times out).
void WaitForAccessibilityTreeToContainNodeWithName(WebContents* web_contents, void WaitForAccessibilityTreeToContainNodeWithName(WebContents* web_contents,
const std::string& name); base::StringPiece name);
// Get a snapshot of a web page's accessibility tree. // Get a snapshot of a web page's accessibility tree.
ui::AXTreeUpdate GetAccessibilityTreeSnapshot(WebContents* web_contents); ui::AXTreeUpdate GetAccessibilityTreeSnapshot(WebContents* web_contents);
@ -1193,7 +1194,7 @@ class RenderProcessHostKillWaiter {
// |uma_name| is the name of the histogram from which the |bad_message_reason| // |uma_name| is the name of the histogram from which the |bad_message_reason|
// can be extracted. // can be extracted.
RenderProcessHostKillWaiter(RenderProcessHost* render_process_host, RenderProcessHostKillWaiter(RenderProcessHost* render_process_host,
const std::string& uma_name); base::StringPiece uma_name);
RenderProcessHostKillWaiter(const RenderProcessHostKillWaiter&) = delete; RenderProcessHostKillWaiter(const RenderProcessHostKillWaiter&) = delete;
RenderProcessHostKillWaiter& operator=(const RenderProcessHostKillWaiter&) = RenderProcessHostKillWaiter& operator=(const RenderProcessHostKillWaiter&) =