Skip to content

Commit

Permalink
chrome: Fixes various compiler warnings
Browse files Browse the repository at this point in the history
For various reasons some compiler warnings have been disabled (and
are still disabled until all this fixed). I'm in the process of reenabling them. This patch fixes some of the code that should
trigger warnings:

. Unnecessary semicolons.
. Unused functions.
. Ensure functions are behind the appropriate ifdefs (otherwise they are
  unused on some platforms).
. Ignoring return values that are marked [[nodiscard]]. I largely
  went with using std::ignore, which is effectively what the code was
  doing before this change. I will file separate bugs for the areas
  I used this in.
. Adding operator=. If the class defines a copy constructor, then it
  must also define an assignment operator.

Bug: 1491776

Change-Id: Ibb1cfa76df9ad8f30b71b7fbabe0e90ff0ebdae0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4938696
Commit-Queue: Scott Violet <sky@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1209514}
  • Loading branch information
Scott Violet authored and Chromium LUCI CQ committed Oct 13, 2023
1 parent b69599a commit d3432eb
Show file tree
Hide file tree
Showing 50 changed files with 133 additions and 145 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ IN_PROC_BROWSER_TEST_F(SensitivityPersistedTabDataAndroidBrowserTest,
TestOnPageContentAnnotatedSensitivePage) {
const GURL sensitive_url =
embedded_test_server()->GetURL("localhost", kSensitiveRelUrl);
content::NavigateToURL(web_contents(), sensitive_url);
// TODO: handle return value.
std::ignore = content::NavigateToURL(web_contents(), sensitive_url);

TabAndroid* tab_android = TabAndroid::FromWebContents(web_contents());
SensitivityPersistedTabDataAndroid* sptda =
Expand All @@ -167,7 +168,8 @@ IN_PROC_BROWSER_TEST_F(SensitivityPersistedTabDataAndroidBrowserTest,
TestOnPageContentAnnotatedNonSensitivePage) {
const GURL non_sensitive_url =
embedded_test_server()->GetURL("localhost", kNonSensitiveRelUrl);
content::NavigateToURL(web_contents(), non_sensitive_url);
// TODO: handle return value.
std::ignore = content::NavigateToURL(web_contents(), non_sensitive_url);

TabAndroid* tab_android = TabAndroid::FromWebContents(web_contents());
SensitivityPersistedTabDataAndroid* sptda =
Expand All @@ -189,7 +191,8 @@ IN_PROC_BROWSER_TEST_F(SensitivityPersistedTabDataAndroidBrowserTest,
const GURL non_sensitive_url2 =
embedded_test_server()->GetURL("localhost", kNonSensitiveRelUrl2);

content::NavigateToURL(web_contents(), sensitive_url);
// TODO: handle return value.
std::ignore = content::NavigateToURL(web_contents(), sensitive_url);
TabAndroid* tab_android = TabAndroid::FromWebContents(web_contents());
SensitivityPersistedTabDataAndroid* sptda =
new SensitivityPersistedTabDataAndroid(tab_android);
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/android/webapk/webapk_database_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class WebApkDatabaseTest : public ::testing::Test {
database_factory_ = std::make_unique<FakeWebApkDatabaseFactory>();

web_contents_ = web_contents_factory_.CreateWebContents(profile());
};
}

// Creates a random WebApkProto based off of a suffix.
static std::unique_ptr<WebApkProto> CreateWebApkProto(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class TestPublisher : public AppPublisher {
LaunchSource launch_source,
WindowInfoPtr window_info) override {}
void LaunchAppWithParams(AppLaunchParams&& params,
LaunchCallback callback) override{};
LaunchCallback callback) override {}

// These methods are private in AppPublisher, we expose them here for unit
// testing.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ void FocusApp() override {}
void ReopenApp() override {}
void FilesOpened(const std::vector<base::FilePath>& files) override {}
void ProfileSelectedFromMenu(const base::FilePath& profile_path) override {}
void OpenAppSettings() override{};
void OpenAppSettings() override {}
void UrlsOpened(const std::vector<GURL>& urls) override {}
void OpenAppWithOverrideUrl(const GURL& override_url) override {}
void ApplicationWillTerminate() override {}
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/apps/intent_helper/intent_picker_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
namespace apps {
namespace {

#if BUILDFLAG(IS_MAC)
std::vector<apps::IntentPickerAppInfo> CombinePossibleMacAppWithOtherApps(
std::vector<apps::IntentPickerAppInfo> apps,
absl::optional<apps::IntentPickerAppInfo> mac_app) {
Expand All @@ -45,6 +46,7 @@ std::vector<apps::IntentPickerAppInfo> CombinePossibleMacAppWithOtherApps(
}
return apps;
}
#endif

PickerEntryType GetPickerEntryType(AppType app_type) {
PickerEntryType picker_entry_type = PickerEntryType::kUnknown;
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/autofill/autofill_interactive_uitest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2606,9 +2606,9 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest,
// Hide autofill preview.
content::RenderWidgetHost* render_widget_host =
GetWebContents()->GetRenderWidgetHostView()->GetRenderWidgetHost();
SendKeyToPopupAndWait(ui::DomKey::ESCAPE,
{ObservedUiEvents::kSuggestionsHidden},
render_widget_host);
ASSERT_TRUE(SendKeyToPopupAndWait(ui::DomKey::ESCAPE,
{ObservedUiEvents::kSuggestionsHidden},
render_widget_host));
ASSERT_FALSE(IsPopupShown());

// Select element on `other` and wait for `onchange` event.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,10 @@ class ChromeBrowsingDataModelDelegateTest : public testing::Test {
// Get salts for test keys, so that they are stored in the service.
base::test::TestFuture<const std::string&> future;
media_device_salt_service()->GetSalt(StorageKey1(), future.GetCallback());
future.Wait();
ASSERT_TRUE(future.Wait());
future.Clear();
media_device_salt_service()->GetSalt(StorageKey2(), future.GetCallback());
future.Wait();
ASSERT_TRUE(future.Wait());

base::test::TestFuture<std::vector<blink::StorageKey>> all_keys_future;
media_device_salt_service()->GetAllStorageKeys(
Expand Down Expand Up @@ -177,7 +177,7 @@ TEST_F(ChromeBrowsingDataModelDelegateTest, RemoveDataKeyForMediaDeviceSalts) {
{static_cast<BrowsingDataModel::StorageType>(
ChromeBrowsingDataModelDelegate::StorageType::kMediaDeviceSalt)},
done_future.GetCallback());
done_future.Wait();
ASSERT_TRUE(done_future.Wait());

base::test::TestFuture<std::vector<blink::StorageKey>> all_keys_future;
media_device_salt_service()->GetAllStorageKeys(all_keys_future.GetCallback());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

namespace {

#if BUILDFLAG(ENABLE_NACL)
bool ShouldNaClBeAllowed() {
// Enabled by policy.
if (g_browser_process->local_state()->GetBoolean(
Expand All @@ -24,6 +25,7 @@ bool ShouldNaClBeAllowed() {
}
return base::FeatureList::IsEnabled(kNaclAllow);
}
#endif

} // namespace

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,10 @@ class WebContentsCloseWaiter : public content::WebContentsObserver {
WebContentsCloseWaiter(const WebContentsCloseWaiter&) = delete;
WebContentsCloseWaiter& operator=(const WebContentsCloseWaiter&) = delete;

void Wait() { future_.Wait(); }
void Wait() {
// TODO: handle return value.
std::ignore = future_.Wait();
}

private:
// content::WebContentsObserver overrides.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class ChromeDeviceAuthenticatorFactoryTest : public testing::Test {
device_reauth::DeviceAuthSource::kPasswordManager) {}

void SetUp() override {
profile_manager_.SetUp();
ASSERT_TRUE(profile_manager_.SetUp());

profile_ptr1_ = profile_manager_.CreateTestingProfile("test_profile1");
profile_ptr2_ = profile_manager_.CreateTestingProfile("test_profile2");
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/download/download_ui_safe_browsing_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class Profile;

namespace download {
class DownloadItem;
};
}

// Utilities for determining how to display a download in the desktop UI based
// on Safe Browsing state and verdict.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,9 @@ class EnterpriseSigninServiceTest : public InteractiveBrowserTest {
}

auto Navigate(Browser* browser, GURL dest) {
return Steps(
Do([browser, dest]() { ui_test_utils::NavigateToURL(browser, dest); }));
return Steps(Do([browser, dest]() {
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser, dest));
}));
}

private:
Expand Down
28 changes: 0 additions & 28 deletions chrome/browser/extensions/api/autofill_private/autofill_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,34 +40,6 @@ namespace autofill_private = extensions::api::autofill_private;

namespace {

// Get the multi-valued element for |type| and return it as a |vector|.
// TODO(khorimoto): remove this function since multi-valued types are
// deprecated.
std::vector<std::string> GetList(const autofill::AutofillProfile& profile,
autofill::ServerFieldType type) {
std::vector<std::string> list;

std::vector<std::u16string> values;
if (autofill::GroupTypeOfServerFieldType(type) ==
autofill::FieldTypeGroup::kName) {
values.push_back(
profile.GetInfo(autofill::AutofillType(type),
g_browser_process->GetApplicationLocale()));
} else {
values.push_back(profile.GetRawInfo(type));
}

// |Get[Raw]MultiInfo()| always returns at least one, potentially empty, item.
// If this is the case, there is no info to return, so return an empty vector.
if (values.size() == 1 && values.front().empty())
return list;

for (const std::u16string& value16 : values)
list.push_back(base::UTF16ToUTF8(value16));

return list;
}

// Gets the string corresponding to |type| from |profile|.
std::string GetStringFromProfile(const autofill::AutofillProfile& profile,
const autofill::ServerFieldType& type) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1704,7 +1704,7 @@ class PasswordsPrivateDelegateImplMockTaskEnvironmentTest
: profile_manager_(TestingBrowserProcess::GetGlobal()) {}

void SetUp() override {
profile_manager_.SetUp();
ASSERT_TRUE(profile_manager_.SetUp());

profile_ = profile_manager_.CreateTestingProfile("test_profile");
web_contents_ = web_contents_factory_.CreateWebContents(profile_);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class ReadingListEventRouter : public KeyedService,

private:
// ReadingListModelObserver:
void ReadingListModelLoaded(const ReadingListModel* model) override{};
void ReadingListModelLoaded(const ReadingListModel* model) override {}
void ReadingListDidAddEntry(const ReadingListModel* model,
const GURL& url,
reading_list::EntrySource source) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,15 @@ class IpProtectionConfigGetterInterceptor
token_(std::move(token)),
expiration_(expiration),
should_intercept_(should_intercept) {
getter_->receivers_for_testing().SwapImplForTesting(receiver_id_, this);
// TODO: handle return value.
std::ignore =
getter_->receivers_for_testing().SwapImplForTesting(receiver_id_, this);
}

~IpProtectionConfigGetterInterceptor() override {
getter_->receivers_for_testing().SwapImplForTesting(receiver_id_, getter_);
// TODO: handle return value.
std::ignore = getter_->receivers_for_testing().SwapImplForTesting(
receiver_id_, getter_);
}

network::mojom::IpProtectionConfigGetter* GetForwardingInterface() override {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ class DownloadStatusUpdaterBrowserTest : public DownloadTestBase {
// TestDownloadManagerDelegate turns it into DANGEROUS_URL.
GURL url =
embedded_test_server()->GetURL("/downloads/dangerous/dangerous.swf");
ui_test_utils::NavigateToURL(browser, url);
EXPECT_TRUE(ui_test_utils::NavigateToURL(browser, url));
waiter->WaitForFinished();

std::vector<download::DownloadItem*> items;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,13 @@ gfx::Rect GetControlBoundsInRoot(content::WebContents* web_contents,
// Use var instead of const or let so that this can be executed many
// times within a context on different elements without Javascript
// variable redeclaration errors.
content::ExecJs(web_contents, base::StringPrintf(R"(
// TODO: handle return value.
std::ignore =
content::ExecJs(web_contents, base::StringPrintf(R"(
var element = document.getElementById('%s');
var bounds = element.getBoundingClientRect();
)",
field_id.c_str()));
field_id.c_str()));
int top = content::EvalJs(web_contents, "bounds.top").ExtractInt();
int left = content::EvalJs(web_contents, "bounds.left").ExtractInt();
int width = content::EvalJs(web_contents, "bounds.width").ExtractInt();
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/media/webrtc/thumbnail_capturer_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ class API_AVAILABLE(macos(13.2)) ThumbnailCapturerMac
: public ThumbnailCapturer {
public:
ThumbnailCapturerMac();
~ThumbnailCapturerMac() override{};
~ThumbnailCapturerMac() override {}

void Start(Consumer* callback) override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ StructuredMetricsMixin::StructuredMetricsMixin(
InProcessBrowserTestMixinHost* host,
bool setup_profile)
: InProcessBrowserTestMixin(host), setup_profile_(setup_profile) {
temp_dir_.CreateUniqueTempDir();
EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
}

StructuredMetricsMixin::~StructuredMetricsMixin() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ NavigationPredictorMetricsDocumentData::UserInteractionsData::
UserInteractionsData() = default;
NavigationPredictorMetricsDocumentData::UserInteractionsData::
UserInteractionsData(const UserInteractionsData&) = default;
NavigationPredictorMetricsDocumentData::UserInteractionsData&
NavigationPredictorMetricsDocumentData::UserInteractionsData::operator=(
const UserInteractionsData&) = default;

NavigationPredictorMetricsDocumentData::NavigationPredictorMetricsDocumentData(
content::RenderFrameHost* render_frame_host)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ class NavigationPredictorMetricsDocumentData
struct UserInteractionsData {
UserInteractionsData();
UserInteractionsData(const UserInteractionsData&);
UserInteractionsData& operator=(const UserInteractionsData&);

// True if the anchor element is still in viewport, otherwise false.
bool is_in_viewport = false;
Expand Down
4 changes: 0 additions & 4 deletions chrome/browser/nearby_sharing/nearby_share_metrics.cc
Original file line number Diff line number Diff line change
Expand Up @@ -268,10 +268,6 @@ std::string GetDirectionSubcategoryName(bool is_incoming) {
return is_incoming ? ".Receive" : ".Send";
}

std::string GetIsKnownSubcategoryName(bool is_known) {
return is_known ? ".Contact" : ".NonContact";
}

std::string GetShareTargetTypeSubcategoryName(
nearby_share::mojom::ShareTargetType type) {
switch (type) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,6 @@ class MockNotificationProvider
(override));
};

std::vector<mac_notifications::mojom::NotificationIdentifierPtr>
CreateOneNotificationList() {
std::vector<mac_notifications::mojom::NotificationIdentifierPtr>
notifications;
auto profile_id = mac_notifications::mojom::ProfileIdentifier::New(
kProfileId, /*incognito=*/true);
notifications.push_back(mac_notifications::mojom::NotificationIdentifier::New(
kNotificationId, std::move(profile_id)));
return notifications;
}

class FakeMacNotificationProviderFactory
: public MacNotificationProviderFactory,
public mac_notifications::mojom::MacNotificationProvider {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -883,7 +883,8 @@ class NotificationPlatformBridgeLinuxImpl
kMethodCloseNotification);
dbus::MessageWriter writer(&method_call);
writer.AppendUint32(data->dbus_id);
notification_proxy_->CallMethodAndBlock(
// TODO: resolve if std::ignore is ok.
std::ignore = notification_proxy_->CallMethodAndBlock(
&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT);
to_erase.push_back(data);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,8 @@ IN_PROC_BROWSER_TEST_P(SoftNavigationTest, LayoutShift) {
LayoutShiftOnlyInMainFrame,
/*num_layout_shifts=*/1);

EvalJs(web_contents(), "triggerLayoutShift()");
// TODO: handle return value.
std::ignore = EvalJs(web_contents(), "triggerLayoutShift()");

waiter->Wait();

Expand Down Expand Up @@ -641,8 +642,9 @@ IN_PROC_BROWSER_TEST_P(SoftNavigationTest, LayoutShift) {
LayoutShiftOnlyInMainFrame,
/*num_layout_shifts=*/1);

EvalJs(web_contents(),
"triggerLayoutShift(" + base::NumberToString(1.5) + ")");
// TODO: handle return value.
std::ignore = EvalJs(web_contents(),
"triggerLayoutShift(" + base::NumberToString(1.5) + ")");

waiter->Wait();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ IN_PROC_BROWSER_TEST_F(BookmarkBarPageLoadMetricsBrowserTest,
/*kFinalStatusActivated*/ 0, 1);

// Navigate away to flush the metrics and check.
ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL));
ASSERT_TRUE(
ui_test_utils::NavigateToURL(browser(), GURL(url::kAboutBlankURL)));
histogram_tester.ExpectTotalCount(
"Bookmarks.BookmarkBar.PrerenderNavigationToActivation", 0);
}
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ TEST_F(ChromePasswordManagerClientTest, ReceivesAutofillPredictions) {
{autofill::AutofillManagerEvent::kFormsSeen});
autofill_driver->renderer_events().FormsSeen(/*updated_forms=*/{form},
/*removed_forms=*/{});
waiter.Wait(/*num_awaiting_calls=*/1);
ASSERT_TRUE(waiter.Wait(/*num_awaiting_calls=*/1));
}

// Simulate that the field types have been determined, since server
Expand Down Expand Up @@ -673,7 +673,7 @@ TEST_F(ChromePasswordManagerClientTest,
/*removed_forms=*/{});
child_driver->renderer_events().FormsSeen(/*updated_forms=*/{child_form},
/*removed_forms=*/{});
waiter.Wait(/*num_awaiting_calls=*/2);
ASSERT_TRUE(waiter.Wait(/*num_awaiting_calls=*/2));
}

// Simulate that the field types have been determined, since server
Expand Down

0 comments on commit d3432eb

Please sign in to comment.