From d22d873921f2f657b483a01a398ddce3001747a1 Mon Sep 17 00:00:00 2001 From: "estade@chromium.org" Date: Tue, 4 May 2010 19:24:42 +0000 Subject: Fix various uses of release() that did not check its value BUG=42904 TEST=compile, unit tests Review URL: http://codereview.chromium.org/1730024 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@46376 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/bookmarks/bookmark_model.cc | 7 +++---- chrome/browser/bookmarks/bookmark_storage.h | 11 +++++------ chrome/browser/browser_main.cc | 2 +- chrome/browser/gtk/certificate_dialogs.cc | 2 +- chrome/browser/gtk/location_bar_view_gtk.cc | 16 ++++++++-------- .../browser/printing/cloud_print/cloud_print_helpers.cc | 15 ++++++--------- .../browser/renderer_host/render_widget_host_unittest.cc | 2 +- chrome/browser/search_engines/template_url_model.cc | 14 +++++++------- chrome/browser/sync/engine/syncapi_unittest.cc | 3 ++- chrome/browser/sync/syncable/directory_backing_store.cc | 14 +++++++------- 10 files changed, 41 insertions(+), 45 deletions(-) (limited to 'chrome/browser') diff --git a/chrome/browser/bookmarks/bookmark_model.cc b/chrome/browser/bookmarks/bookmark_model.cc index be78ef5..a5c87bf 100644 --- a/chrome/browser/bookmarks/bookmark_model.cc +++ b/chrome/browser/bookmarks/bookmark_model.cc @@ -580,8 +580,6 @@ void BookmarkModel::DoneLoading( return; } - bookmark_bar_node_ = details->bb_node(); - other_node_ = details->other_folder_node(); next_node_id_ = details->max_id(); if (details->computed_checksum() != details->stored_checksum()) SetFileChanged(); @@ -594,8 +592,9 @@ void BookmarkModel::DoneLoading( if (store_.get()) store_->ScheduleSave(); } - index_.reset(details->index()); - details->release(); + bookmark_bar_node_ = details->release_bb_node(); + other_node_ = details->release_other_folder_node(); + index_.reset(details->release_index()); // WARNING: order is important here, various places assume bookmark bar then // other node. diff --git a/chrome/browser/bookmarks/bookmark_storage.h b/chrome/browser/bookmarks/bookmark_storage.h index 033a849..bc55219 100644 --- a/chrome/browser/bookmarks/bookmark_storage.h +++ b/chrome/browser/bookmarks/bookmark_storage.h @@ -40,15 +40,14 @@ class BookmarkLoadDetails { ids_reassigned_(false) { } - void release() { - bb_node_.release(); - other_folder_node_.release(); - index_.release(); - } - BookmarkNode* bb_node() { return bb_node_.get(); } + BookmarkNode* release_bb_node() { return bb_node_.release(); } BookmarkNode* other_folder_node() { return other_folder_node_.get(); } + BookmarkNode* release_other_folder_node() { + return other_folder_node_.release(); + } BookmarkIndex* index() { return index_.get(); } + BookmarkIndex* release_index() { return index_.release(); } // Max id of the nodes. void set_max_id(int64 max_id) { max_id_ = max_id; } diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc index b2b4489..6caace3 100644 --- a/chrome/browser/browser_main.cc +++ b/chrome/browser/browser_main.cc @@ -1241,7 +1241,7 @@ int BrowserMain(const MainFunctionParams& parameters) { // browser_shutdown takes care of deleting browser_process, so we need to // release it. - browser_process.release(); + ignore_result(browser_process.release()); browser_shutdown::Shutdown(); return result_code; diff --git a/chrome/browser/gtk/certificate_dialogs.cc b/chrome/browser/gtk/certificate_dialogs.cc index e181ad4..ba12f6d 100644 --- a/chrome/browser/gtk/certificate_dialogs.cc +++ b/chrome/browser/gtk/certificate_dialogs.cc @@ -126,7 +126,7 @@ std::string GetCMSString(std::vector cert_chain, size_t start, NSSCMSContentInfo *cinfo = NSS_CMSMessage_GetContentInfo(message.get()); if (NSS_CMSContentInfo_SetContent_SignedData( message.get(), cinfo, signed_data.get()) == SECSuccess) { - signed_data.release(); + ignore_result(signed_data.release()); } else { LOG(ERROR) << "NSS_CMSMessage_GetContentInfo failed"; return ""; diff --git a/chrome/browser/gtk/location_bar_view_gtk.cc b/chrome/browser/gtk/location_bar_view_gtk.cc index e31262c..7a1749cc 100644 --- a/chrome/browser/gtk/location_bar_view_gtk.cc +++ b/chrome/browser/gtk/location_bar_view_gtk.cc @@ -469,9 +469,9 @@ void LocationBarViewGtk::Update(const TabContents* contents) { } void LocationBarViewGtk::OnAutocompleteAccept(const GURL& url, - WindowOpenDisposition disposition, - PageTransition::Type transition, - const GURL& alternate_nav_url) { + WindowOpenDisposition disposition, + PageTransition::Type transition, + const GURL& alternate_nav_url) { if (!url.is_valid()) return; @@ -487,8 +487,8 @@ void LocationBarViewGtk::OnAutocompleteAccept(const GURL& url, return; } - scoped_ptr fetcher( - new AlternateNavURLFetcher(alternate_nav_url)); + AlternateNavURLFetcher* fetcher = + new AlternateNavURLFetcher(alternate_nav_url); // The AlternateNavURLFetcher will listen for the pending navigation // notification that will be issued as a result of the "open URL." It // will automatically install itself into that navigation controller. @@ -496,10 +496,10 @@ void LocationBarViewGtk::OnAutocompleteAccept(const GURL& url, if (fetcher->state() == AlternateNavURLFetcher::NOT_STARTED) { // I'm not sure this should be reachable, but I'm not also sure enough // that it shouldn't to stick in a NOTREACHED(). In any case, this is - // harmless; we can simply let the fetcher get deleted here and it will - // clean itself up properly. + // harmless. + delete fetcher; } else { - fetcher.release(); // The navigation controller will delete the fetcher. + // The navigation controller will delete the fetcher. } } diff --git a/chrome/browser/printing/cloud_print/cloud_print_helpers.cc b/chrome/browser/printing/cloud_print/cloud_print_helpers.cc index 336a203..3fe81c5 100644 --- a/chrome/browser/printing/cloud_print/cloud_print_helpers.cc +++ b/chrome/browser/printing/cloud_print/cloud_print_helpers.cc @@ -84,23 +84,20 @@ bool CloudPrintHelpers::ParseResponseJSON( const std::string& response_data, bool* succeeded, DictionaryValue** response_dict) { scoped_ptr message_value(base::JSONReader::Read(response_data, false)); - DCHECK(message_value.get()); if (!message_value.get()) { + NOTREACHED(); return false; } if (!message_value->IsType(Value::TYPE_DICTIONARY)) { NOTREACHED(); return false; } - DictionaryValue* response_dict_local = - static_cast(message_value.get()); - if (succeeded) { + scoped_ptr response_dict_local( + static_cast(message_value.release())); + if (succeeded) response_dict_local->GetBoolean(kSuccessValue, succeeded); - } - if (response_dict) { - *response_dict = response_dict_local; - message_value.release(); - } + if (response_dict) + *response_dict = response_dict_local.release(); return true; } diff --git a/chrome/browser/renderer_host/render_widget_host_unittest.cc b/chrome/browser/renderer_host/render_widget_host_unittest.cc index 28be917..2cfd968 100644 --- a/chrome/browser/renderer_host/render_widget_host_unittest.cc +++ b/chrome/browser/renderer_host/render_widget_host_unittest.cc @@ -42,7 +42,7 @@ class RenderWidgetHostProcess : public MockRenderProcessHost { ~RenderWidgetHostProcess() { // We don't want to actually delete the channel, since it's not a real // pointer. - channel_.release(); + ignore_result(channel_.release()); delete current_update_buf_; } diff --git a/chrome/browser/search_engines/template_url_model.cc b/chrome/browser/search_engines/template_url_model.cc index d6a8171..361bfb8 100644 --- a/chrome/browser/search_engines/template_url_model.cc +++ b/chrome/browser/search_engines/template_url_model.cc @@ -732,6 +732,7 @@ void TemplateURLModel::MergeEnginesFromPrepopulateData() { std::set updated_ids; for (size_t i = 0; i < loaded_urls.size(); ++i) { + // We take ownership of |t_url|. scoped_ptr t_url(loaded_urls[i]); int t_url_id = t_url->prepopulate_id(); if (!t_url_id || updated_ids.count(t_url_id)) { @@ -745,22 +746,21 @@ void TemplateURLModel::MergeEnginesFromPrepopulateData() { const TemplateURL* existing_url = existing_url_iter->second; if (!existing_url->safe_for_autoreplace()) { // User edited the entry, preserve the keyword and description. - loaded_urls[i]->set_safe_for_autoreplace(false); - loaded_urls[i]->set_keyword(existing_url->keyword()); - loaded_urls[i]->set_autogenerate_keyword( + t_url->set_safe_for_autoreplace(false); + t_url->set_keyword(existing_url->keyword()); + t_url->set_autogenerate_keyword( existing_url->autogenerate_keyword()); - loaded_urls[i]->set_short_name(existing_url->short_name()); + t_url->set_short_name(existing_url->short_name()); } - Replace(existing_url, loaded_urls[i]); + Replace(existing_url, t_url.release()); id_to_turl.erase(existing_url_iter); } else { - Add(loaded_urls[i]); + Add(t_url.release()); } if (i == default_search_index && !default_search_provider_) SetDefaultSearchProvider(loaded_urls[i]); updated_ids.insert(t_url_id); - t_url.release(); } // Remove any prepopulated engines which are no longer in the master list, as diff --git a/chrome/browser/sync/engine/syncapi_unittest.cc b/chrome/browser/sync/engine/syncapi_unittest.cc index b89e0de..70157b5 100644 --- a/chrome/browser/sync/engine/syncapi_unittest.cc +++ b/chrome/browser/sync/engine/syncapi_unittest.cc @@ -26,7 +26,8 @@ class SyncApiTest : public testing::Test { } virtual void TearDown() { - share_.dir_manager.release(); + // |share_.dir_manager| does not actually own its value. + ignore_result(share_.dir_manager.release()); setter_upper_.TearDown(); } diff --git a/chrome/browser/sync/syncable/directory_backing_store.cc b/chrome/browser/sync/syncable/directory_backing_store.cc index c22566b..18ae46a 100644 --- a/chrome/browser/sync/syncable/directory_backing_store.cc +++ b/chrome/browser/sync/syncable/directory_backing_store.cc @@ -181,24 +181,25 @@ bool DirectoryBackingStore::OpenAndConfigureHandleHelper( sqlite3** handle) const { if (SQLITE_OK == OpenSqliteDb(backing_filepath_, handle)) { sqlite_utils::scoped_sqlite_db_ptr scoped_handle(*handle); - sqlite3_busy_timeout(*handle, std::numeric_limits::max()); + sqlite3_busy_timeout(scoped_handle.get(), std::numeric_limits::max()); { SQLStatement statement; - statement.prepare(*handle, "PRAGMA fullfsync = 1"); + statement.prepare(scoped_handle.get(), "PRAGMA fullfsync = 1"); if (SQLITE_DONE != statement.step()) { - LOG(ERROR) << sqlite3_errmsg(*handle); + LOG(ERROR) << sqlite3_errmsg(scoped_handle.get()); return false; } } { SQLStatement statement; - statement.prepare(*handle, "PRAGMA synchronous = 2"); + statement.prepare(scoped_handle.get(), "PRAGMA synchronous = 2"); if (SQLITE_DONE != statement.step()) { - LOG(ERROR) << sqlite3_errmsg(*handle); + LOG(ERROR) << sqlite3_errmsg(scoped_handle.get()); return false; } } - sqlite3_busy_timeout(*handle, kDirectoryBackingStoreBusyTimeoutMs); + sqlite3_busy_timeout(scoped_handle.release(), + kDirectoryBackingStoreBusyTimeoutMs); #if defined(OS_WIN) // Do not index this file. Scanning can occur every time we close the file, // which causes long delays in SQLite's file locking. @@ -208,7 +209,6 @@ bool DirectoryBackingStore::OpenAndConfigureHandleHelper( attrs | FILE_ATTRIBUTE_NOT_CONTENT_INDEXED); #endif - scoped_handle.release(); return true; } return false; -- cgit v1.1