Skip to content

Commit

Permalink
Enable MSVC warning for unused locals.
Browse files Browse the repository at this point in the history
There is seemingly a bug in the compiler where it occasionally claims a local is
unused when it isn't.  This forces a few places to either inline such locals or
mark them ALLOW_UNUSED_LOCAL.

BUG=81439
TEST=none
R=brettw@chromium.org, cpu@chromium.org, jamesr@chromium.org, rvargas@chromium.org, sievers@chromium.org, sky@chromium.org, vitalybuka@chromium.org, wolenetz@chromium.org

Review URL: https://codereview.chromium.org/731373002

Cr-Commit-Position: refs/heads/master@{#305108}
  • Loading branch information
pkasting committed Nov 20, 2014
1 parent ce67606 commit be940e9
Show file tree
Hide file tree
Showing 42 changed files with 125 additions and 177 deletions.
11 changes: 5 additions & 6 deletions ash/display/display_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,18 +95,17 @@ void DisplayInfo::SetUse125DSFForUIScaling(bool enable) {
// static
DisplayInfo DisplayInfo::CreateFromSpecWithID(const std::string& spec,
int64 id) {
// Default bounds for a display.
const int kDefaultHostWindowX = 200;
const int kDefaultHostWindowY = 200;
const int kDefaultHostWindowWidth = 1366;
const int kDefaultHostWindowHeight = 768;

// Use larger than max int to catch overflow early.
static int64 synthesized_display_id = 2200000000LL;

#if defined(OS_WIN)
gfx::Rect bounds_in_native(aura::WindowTreeHost::GetNativeScreenSize());
#else
// Default bounds for a display.
const int kDefaultHostWindowX = 200;
const int kDefaultHostWindowY = 200;
const int kDefaultHostWindowWidth = 1366;
const int kDefaultHostWindowHeight = 768;
gfx::Rect bounds_in_native(kDefaultHostWindowX, kDefaultHostWindowY,
kDefaultHostWindowWidth, kDefaultHostWindowHeight);
#endif
Expand Down
3 changes: 1 addition & 2 deletions ash/root_window_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,7 @@ TEST_F(RootWindowControllerTest, ModalContainerNotLoggedInLoggedIn) {

TEST_F(RootWindowControllerTest, ModalContainerBlockedSession) {
UpdateDisplay("600x600");
Shell* shell = Shell::GetInstance();
RootWindowController* controller = shell->GetPrimaryRootWindowController();
RootWindowController* controller = Shell::GetPrimaryRootWindowController();
aura::Window* lock_container =
controller->GetContainer(kShellWindowId_LockScreenContainer);
for (int block_reason = FIRST_BLOCK_REASON;
Expand Down
3 changes: 1 addition & 2 deletions base/metrics/statistics_recorder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -289,9 +289,8 @@ StatisticsRecorder::StatisticsRecorder() {
void StatisticsRecorder::DumpHistogramsToVlog(void* instance) {
DCHECK(VLOG_IS_ON(1));

StatisticsRecorder* me = reinterpret_cast<StatisticsRecorder*>(instance);
string output;
me->WriteGraph(std::string(), &output);
StatisticsRecorder::WriteGraph(std::string(), &output);
VLOG(1) << output;
}

Expand Down
4 changes: 3 additions & 1 deletion base/security_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,9 @@ TEST(SecurityTest, MAYBE_NewOverflow) {
}
// On windows, the compiler prevents static array sizes of more than
// 0x7fffffff (error C2148).
#if !defined(OS_WIN) || !defined(ARCH_CPU_64_BITS)
#if defined(OS_WIN) && defined(ARCH_CPU_64_BITS)
ALLOW_UNUSED_LOCAL(kDynamicArraySize);
#else
{
scoped_ptr<char[][kArraySize2]> array_pointer(new (nothrow)
char[kDynamicArraySize][kArraySize2]);
Expand Down
1 change: 0 additions & 1 deletion build/common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -5417,7 +5417,6 @@
# removed as code is fixed.
4100, # Unreferenced formal parameter
4121, # Alignment of a member was sensitive to packing
4189, # Local variable is initialized but not referenced
4244, # Conversion from 'type1' to 'type2', possible loss of data
4481, # Nonstandard extension used: override specifier 'keyword'
4505, # Unreferenced local function has been removed
Expand Down
1 change: 0 additions & 1 deletion build/config/compiler/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,6 @@ config("default_warnings") {

"/wd4100", # Unreferenced formal function parameter.
"/wd4121", # Alignment of a member was sensitive to packing.
"/wd4189", # A variable was declared and initialized but never used.
"/wd4244", # Conversion: possible loss of data.
"/wd4481", # Nonstandard extension: override specifier.
"/wd4505", # Unreferenced local function has been removed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -914,36 +914,32 @@ TEST_F(HostContentSettingsMapTest, GetContentSetting) {
}

TEST_F(HostContentSettingsMapTest, ShouldAllowAllContent) {
TestingProfile profile;
HostContentSettingsMap* host_content_settings_map =
profile.GetHostContentSettingsMap();

GURL http_host("http://example.com/");
GURL https_host("https://example.com/");
GURL embedder("chrome://foo");
GURL extension("chrome-extension://foo");
EXPECT_FALSE(host_content_settings_map->ShouldAllowAllContent(
EXPECT_FALSE(HostContentSettingsMap::ShouldAllowAllContent(
http_host, embedder, CONTENT_SETTINGS_TYPE_NOTIFICATIONS));
EXPECT_FALSE(host_content_settings_map->ShouldAllowAllContent(
EXPECT_FALSE(HostContentSettingsMap::ShouldAllowAllContent(
http_host, embedder, CONTENT_SETTINGS_TYPE_GEOLOCATION));
EXPECT_FALSE(host_content_settings_map->ShouldAllowAllContent(
EXPECT_FALSE(HostContentSettingsMap::ShouldAllowAllContent(
http_host, embedder, CONTENT_SETTINGS_TYPE_COOKIES));
EXPECT_TRUE(host_content_settings_map->ShouldAllowAllContent(
EXPECT_TRUE(HostContentSettingsMap::ShouldAllowAllContent(
https_host, embedder, CONTENT_SETTINGS_TYPE_COOKIES));
EXPECT_TRUE(host_content_settings_map->ShouldAllowAllContent(
EXPECT_TRUE(HostContentSettingsMap::ShouldAllowAllContent(
https_host, embedder, CONTENT_SETTINGS_TYPE_COOKIES));
EXPECT_TRUE(host_content_settings_map->ShouldAllowAllContent(
EXPECT_TRUE(HostContentSettingsMap::ShouldAllowAllContent(
embedder, http_host, CONTENT_SETTINGS_TYPE_COOKIES));
#if defined(ENABLE_EXTENSIONS)
EXPECT_TRUE(host_content_settings_map->ShouldAllowAllContent(
EXPECT_TRUE(HostContentSettingsMap::ShouldAllowAllContent(
extension, extension, CONTENT_SETTINGS_TYPE_COOKIES));
#else
EXPECT_FALSE(host_content_settings_map->ShouldAllowAllContent(
EXPECT_FALSE(HostContentSettingsMap::ShouldAllowAllContent(
extension, extension, CONTENT_SETTINGS_TYPE_COOKIES));
#endif
EXPECT_FALSE(host_content_settings_map->ShouldAllowAllContent(
EXPECT_FALSE(HostContentSettingsMap::ShouldAllowAllContent(
extension, extension, CONTENT_SETTINGS_TYPE_PLUGINS));
EXPECT_FALSE(host_content_settings_map->ShouldAllowAllContent(
EXPECT_FALSE(HostContentSettingsMap::ShouldAllowAllContent(
extension, http_host, CONTENT_SETTINGS_TYPE_COOKIES));
}

Expand Down
2 changes: 0 additions & 2 deletions chrome/browser/enumerate_modules_model_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -979,8 +979,6 @@ void EnumerateModulesModel::MaybePostScanningTask() {
static bool done = false;
if (!done) {
done = true;

const CommandLine& cmd_line = *CommandLine::ForCurrentProcess();
if (base::win::GetVersion() == base::win::VERSION_XP) {
check_modules_timer_.Start(FROM_HERE,
base::TimeDelta::FromMilliseconds(kModuleCheckDelayMs),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,10 +336,9 @@ IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest,
EXPECT_EQ(TRUE, ::IsWindow(hwnd));

// Create a new browser window to prevent the message loop from terminating.
Browser* new_browser = chrome::FindBrowserWithWebContents(
browser()->OpenURL(content::OpenURLParams(
GURL("about:"), content::Referrer(), NEW_WINDOW,
ui::PAGE_TRANSITION_TYPED, false)));
browser()->OpenURL(content::OpenURLParams(GURL("about:"), content::Referrer(),
NEW_WINDOW,
ui::PAGE_TRANSITION_TYPED, false));

// Forcibly closing the browser HWND should not cause a crash.
EXPECT_EQ(TRUE, ::CloseWindow(hwnd));
Expand Down
60 changes: 28 additions & 32 deletions chrome/browser/extensions/external_provider_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -465,34 +465,17 @@ void ExternalProviderImpl::CreateExternalProviders(
#endif

bool is_chromeos_demo_session = false;
#if !defined(OS_WIN)
int bundled_extension_creation_flags = Extension::NO_FLAGS;
#endif
#if defined(OS_CHROMEOS)
user_manager::UserManager* user_manager = user_manager::UserManager::Get();
is_chromeos_demo_session =
user_manager && user_manager->IsLoggedInAsDemoUser() &&
connector->GetDeviceMode() == policy::DEVICE_MODE_RETAIL_KIOSK;
bundled_extension_creation_flags = Extension::FROM_WEBSTORE |
Extension::WAS_INSTALLED_BY_DEFAULT;
#endif

#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
if (!profile->IsSupervised()) {
provider_list->push_back(
linked_ptr<ExternalProviderInterface>(
new ExternalProviderImpl(
service,
new ExternalPrefLoader(
chrome::DIR_STANDALONE_EXTERNAL_EXTENSIONS,
ExternalPrefLoader::NONE,
NULL),
profile,
Manifest::EXTERNAL_PREF,
Manifest::EXTERNAL_PREF_DOWNLOAD,
bundled_extension_creation_flags)));
}
#endif

#if defined(OS_CHROMEOS)
if (!is_chromeos_demo_session && !is_chrome_os_public_session) {
int external_apps_path_id = profile->IsSupervised() ?
chrome::DIR_SUPERVISED_USERS_DEFAULT_APPS :
Expand Down Expand Up @@ -538,10 +521,35 @@ void ExternalProviderImpl::CreateExternalProviders(
Manifest::INVALID_LOCATION,
Extension::NO_FLAGS)));
}
#elif defined(OS_LINUX)
if (!profile->IsSupervised()) {
provider_list->push_back(
linked_ptr<ExternalProviderInterface>(
new ExternalProviderImpl(
service,
new ExternalPrefLoader(
chrome::DIR_STANDALONE_EXTERNAL_EXTENSIONS,
ExternalPrefLoader::NONE,
NULL),
profile,
Manifest::EXTERNAL_PREF,
Manifest::EXTERNAL_PREF_DOWNLOAD,
bundled_extension_creation_flags)));
}
#endif

if (!profile->IsSupervised() && !is_chromeos_demo_session) {
#if !defined(OS_WIN)
#if defined(OS_WIN)
provider_list->push_back(
linked_ptr<ExternalProviderInterface>(
new ExternalProviderImpl(
service,
new ExternalRegistryLoader,
profile,
Manifest::EXTERNAL_REGISTRY,
Manifest::EXTERNAL_PREF_DOWNLOAD,
Extension::NO_FLAGS)));
#else
provider_list->push_back(
linked_ptr<ExternalProviderInterface>(
new ExternalProviderImpl(
Expand All @@ -553,7 +561,6 @@ void ExternalProviderImpl::CreateExternalProviders(
Manifest::EXTERNAL_PREF,
Manifest::EXTERNAL_PREF_DOWNLOAD,
bundled_extension_creation_flags)));
#endif

// Define a per-user source of external extensions.
#if defined(OS_MACOSX)
Expand All @@ -569,17 +576,6 @@ void ExternalProviderImpl::CreateExternalProviders(
Manifest::EXTERNAL_PREF_DOWNLOAD,
Extension::NO_FLAGS)));
#endif

#if defined(OS_WIN)
provider_list->push_back(
linked_ptr<ExternalProviderInterface>(
new ExternalProviderImpl(
service,
new ExternalRegistryLoader,
profile,
Manifest::EXTERNAL_REGISTRY,
Manifest::EXTERNAL_PREF_DOWNLOAD,
Extension::NO_FLAGS)));
#endif

#if !defined(OS_CHROMEOS)
Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/history/in_memory_url_index_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -983,12 +983,11 @@ TEST_F(InMemoryURLIndexTest, WhitelistedURLs) {
{ "xmpp://guest@example.com", false },
};

URLIndexPrivateData& private_data(*GetPrivateData());
const std::set<std::string>& whitelist(scheme_whitelist());
for (size_t i = 0; i < arraysize(data); ++i) {
GURL url(data[i].url_spec);
EXPECT_EQ(data[i].expected_is_whitelisted,
private_data.URLSchemeIsWhitelisted(url, whitelist));
URLIndexPrivateData::URLSchemeIsWhitelisted(url, whitelist));
}
}

Expand Down
1 change: 0 additions & 1 deletion chrome/browser/process_singleton_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ bool ProcessLaunchNotification(
return false;

// Handle the WM_COPYDATA message from another process.
HWND hwnd = reinterpret_cast<HWND>(wparam);
const COPYDATASTRUCT* cds = reinterpret_cast<COPYDATASTRUCT*>(lparam);

CommandLine parsed_command_line(CommandLine::NO_PROGRAM);
Expand Down
16 changes: 4 additions & 12 deletions chrome/browser/search/hotword_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,10 @@ TEST_P(HotwordServiceTest, IsHotwordAllowedBadFieldTrial) {
TestingProfile::Builder profile_builder;
scoped_ptr<TestingProfile> profile = profile_builder.Build();

HotwordServiceFactory* hotword_service_factory =
HotwordServiceFactory::GetInstance();

// Check that the service exists so that a NULL service be ruled out in
// following tests.
HotwordService* hotword_service =
hotword_service_factory->GetForProfile(profile.get());
HotwordServiceFactory::GetForProfile(profile.get());
EXPECT_TRUE(hotword_service != NULL);

// When the field trial is empty or Disabled, it should not be allowed.
Expand All @@ -137,7 +134,7 @@ TEST_P(HotwordServiceTest, IsHotwordAllowedBadFieldTrial) {
hotword_internal::kHotwordFieldTrialDisabledGroupName));
group = base::FieldTrialList::FindFullName(
hotword_internal::kHotwordFieldTrialName);
EXPECT_TRUE(group ==hotword_internal::kHotwordFieldTrialDisabledGroupName);
EXPECT_TRUE(group == hotword_internal::kHotwordFieldTrialDisabledGroupName);
EXPECT_FALSE(HotwordServiceFactory::IsHotwordAllowed(profile.get()));

// Set a valid locale with invalid field trial to be sure it is
Expand All @@ -154,13 +151,10 @@ TEST_P(HotwordServiceTest, IsHotwordAllowedLocale) {
TestingProfile::Builder profile_builder;
scoped_ptr<TestingProfile> profile = profile_builder.Build();

HotwordServiceFactory* hotword_service_factory =
HotwordServiceFactory::GetInstance();

// Check that the service exists so that a NULL service be ruled out in
// following tests.
HotwordService* hotword_service =
hotword_service_factory->GetForProfile(profile.get());
HotwordServiceFactory::GetForProfile(profile.get());
EXPECT_TRUE(hotword_service != NULL);

// Set the field trial to a valid one.
Expand Down Expand Up @@ -194,10 +188,8 @@ TEST_P(HotwordServiceTest, AudioLoggingPrefSetCorrectly) {
TestingProfile::Builder profile_builder;
scoped_ptr<TestingProfile> profile = profile_builder.Build();

HotwordServiceFactory* hotword_service_factory =
HotwordServiceFactory::GetInstance();
HotwordService* hotword_service =
hotword_service_factory->GetForProfile(profile.get());
HotwordServiceFactory::GetForProfile(profile.get());
EXPECT_TRUE(hotword_service != NULL);

// If it's a fresh profile, although the default value is true,
Expand Down
7 changes: 2 additions & 5 deletions chrome/browser/search/suggestions/suggestions_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,8 @@ std::string SuggestionsSource::GetSource() const {
void SuggestionsSource::StartDataRequest(
const std::string& path, int render_process_id, int render_frame_id,
const content::URLDataSource::GotDataCallback& callback) {
SuggestionsServiceFactory* suggestions_service_factory =
SuggestionsServiceFactory::GetInstance();

SuggestionsService* suggestions_service(
suggestions_service_factory->GetForProfile(profile_));
SuggestionsService* suggestions_service =
SuggestionsServiceFactory::GetForProfile(profile_);

if (!suggestions_service) {
callback.Run(NULL);
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/views/certificate_viewer_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ void ShowCertificateViewerImpl(content::WebContents* web_contents,
base::MessageLoop::current());
// This next call blocks but keeps processing windows messages, making it
// modal to the browser window.
BOOL rv = ::CryptUIDlgViewCertificate(&view_info, &properties_changed);
::CryptUIDlgViewCertificate(&view_info, &properties_changed);

CertFreeCertificateContext(cert_list);
}
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/views/panels/panel_frame_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ void PanelFrameView::SetWindowCornerStyle(panel::CornerStyle corner_style) {
// window region if the region really differs.
HWND native_window = views::HWNDForWidget(panel_view_->window());
base::win::ScopedRegion current_region(::CreateRectRgn(0, 0, 0, 0));
int current_region_result = ::GetWindowRgn(native_window, current_region);
::GetWindowRgn(native_window, current_region);

gfx::Path window_mask;
GetWindowMask(size(), &window_mask);
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ui/webui/sync_internals_message_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,6 @@ void SyncInternalsMessageHandler::SendAboutInfo() {
// May return NULL (e.g., if sync is disabled on the command line).
ProfileSyncService* SyncInternalsMessageHandler::GetProfileSyncService() {
Profile* profile = Profile::FromWebUI(web_ui());
ProfileSyncServiceFactory* factory = ProfileSyncServiceFactory::GetInstance();
return factory->GetForProfile(profile->GetOriginalProfile());
return ProfileSyncServiceFactory::GetForProfile(
profile->GetOriginalProfile());
}
1 change: 0 additions & 1 deletion chrome/installer/setup/setup_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1532,7 +1532,6 @@ InstallStatus InstallProductsHelper(
COMPILE_ASSERT(BrowserDistribution::NUM_TYPES == 4,
add_support_for_new_products_here_);
const uint32 kBrowserBit = 1 << BrowserDistribution::CHROME_BROWSER;
const uint32 kAppHostBit = 1 << BrowserDistribution::CHROME_APP_HOST;
int message_id = 0;

proceed_with_installation = false;
Expand Down
6 changes: 0 additions & 6 deletions chrome/installer/util/installation_validator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ BrowserDistribution::Type
void InstallationValidator::ChromeRules::AddUninstallSwitchExpectations(
const ProductContext& ctx,
SwitchExpectations* expectations) const {
const bool is_multi_install =
ctx.state.uninstall_command().HasSwitch(switches::kMultiInstall);

// --chrome should be present for uninstall iff --multi-install. This wasn't
// the case in Chrome 10 (between r68996 and r72497), though, so consider it
// optional.
Expand All @@ -40,9 +37,6 @@ void InstallationValidator::ChromeRules::AddUninstallSwitchExpectations(
void InstallationValidator::ChromeRules::AddRenameSwitchExpectations(
const ProductContext& ctx,
SwitchExpectations* expectations) const {
const bool is_multi_install =
ctx.state.uninstall_command().HasSwitch(switches::kMultiInstall);

// --chrome should not be present for rename. It was for a time, so we'll be
// lenient so that mini_installer tests pass.

Expand Down
1 change: 0 additions & 1 deletion chrome/installer/util/installer_state_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,6 @@ TEST_F(InstallerStateTest, WithProduct) {

TEST_F(InstallerStateTest, InstallerResult) {
const bool system_level = true;
bool multi_install = false;
HKEY root = system_level ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;

RegKey key;
Expand Down
Loading

0 comments on commit be940e9

Please sign in to comment.