Skip to content

Commit

Permalink
Fix even more remaining uses of WeakPtr<T>'s operator T* conversion
Browse files Browse the repository at this point in the history
These cases weren't caught by the automated pass and/or needed to be
solved in another way than using .get().

BUG=245942
TBR=darin@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@204036 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
akalin@chromium.org committed Jun 4, 2013
1 parent 35e10e9 commit eab9d01
Show file tree
Hide file tree
Showing 11 changed files with 52 additions and 37 deletions.
2 changes: 1 addition & 1 deletion base/memory/weak_ptr_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ TEST(WeakPtrFactoryTest, Comparison) {
WeakPtrFactory<int> factory(&data);
WeakPtr<int> ptr = factory.GetWeakPtr();
WeakPtr<int> ptr2 = ptr;
EXPECT_EQ(ptr, ptr2);
EXPECT_EQ(ptr.get(), ptr2.get());
}

TEST(WeakPtrFactoryTest, OutOfScope) {
Expand Down
22 changes: 12 additions & 10 deletions chrome/browser/extensions/extension_action_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ TEST(ExtensionActionTest, Visibility) {
action.SetAppearance(100, ExtensionAction::ACTIVE);
ASSERT_FALSE(action.GetIsVisible(1));
ASSERT_TRUE(action.GetIsVisible(100));
EXPECT_FALSE(action.GetIconAnimation(100))
EXPECT_FALSE(action.GetIconAnimation(100).get())
<< "Page actions should not animate.";

action.ClearAllValuesForTab(100);
Expand All @@ -63,22 +63,24 @@ TEST(ExtensionActionTest, ScriptBadgeAnimation) {

ExtensionAction script_badge(
std::string(), ActionInfo::TYPE_SCRIPT_BADGE, ActionInfo());
EXPECT_FALSE(script_badge.GetIconAnimation(ExtensionAction::kDefaultTabId));
EXPECT_FALSE(
script_badge.GetIconAnimation(ExtensionAction::kDefaultTabId).get());
script_badge.SetAppearance(ExtensionAction::kDefaultTabId,
ExtensionAction::ACTIVE);
EXPECT_FALSE(script_badge.GetIconAnimation(ExtensionAction::kDefaultTabId))
EXPECT_FALSE(
script_badge.GetIconAnimation(ExtensionAction::kDefaultTabId).get())
<< "Showing the default tab should not animate script badges.";

script_badge.SetAppearance(ExtensionAction::kDefaultTabId,
ExtensionAction::INVISIBLE);
EXPECT_FALSE(script_badge.GetIconAnimation(1))
EXPECT_FALSE(script_badge.GetIconAnimation(1).get())
<< "Making a script badge invisible should not show its animation.";
script_badge.SetAppearance(1, ExtensionAction::ACTIVE);
EXPECT_TRUE(script_badge.GetIconAnimation(1))
EXPECT_TRUE(script_badge.GetIconAnimation(1).get())
<< "Making a script badge visible should show its animation.";

script_badge.ClearAllValuesForTab(1);
EXPECT_FALSE(script_badge.GetIconAnimation(100));
EXPECT_FALSE(script_badge.GetIconAnimation(100).get());
}

TEST(ExtensionActionTest, GetAttention) {
Expand All @@ -88,18 +90,18 @@ TEST(ExtensionActionTest, GetAttention) {
ExtensionAction script_badge(
std::string(), ActionInfo::TYPE_SCRIPT_BADGE, ActionInfo());
EXPECT_FALSE(script_badge.GetIsVisible(1));
EXPECT_FALSE(script_badge.GetIconAnimation(1));
EXPECT_FALSE(script_badge.GetIconAnimation(1).get());
script_badge.SetAppearance(1, ExtensionAction::WANTS_ATTENTION);
EXPECT_TRUE(script_badge.GetIsVisible(1));
EXPECT_TRUE(script_badge.GetIconAnimation(1));
EXPECT_TRUE(script_badge.GetIconAnimation(1).get());

// Simulate waiting long enough for the animation to end.
message_loop.reset(); // Can't have 2 MessageLoops alive at once.
message_loop.reset(new base::MessageLoop);
EXPECT_FALSE(script_badge.GetIconAnimation(1)); // Sanity check.
EXPECT_FALSE(script_badge.GetIconAnimation(1).get()); // Sanity check.

script_badge.SetAppearance(1, ExtensionAction::ACTIVE);
EXPECT_FALSE(script_badge.GetIconAnimation(1))
EXPECT_FALSE(script_badge.GetIconAnimation(1).get())
<< "The animation should not play again if the icon was already visible.";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class SyncSharedChangeProcessorTest : public testing::Test {
&sync_service_,
&error_handler_,
syncer::AUTOFILL,
base::WeakPtr<syncer::SyncMergeResult>()));
base::WeakPtr<syncer::SyncMergeResult>()).get());
}

base::MessageLoopForUI ui_loop_;
Expand Down
17 changes: 15 additions & 2 deletions gpu/command_buffer/service/context_group.cc
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,19 @@ bool IsNull(const base::WeakPtr<gles2::GLES2Decoder>& decoder) {
return !decoder.get();
}

template <typename T>
class WeakPtrEquals {
public:
explicit WeakPtrEquals(T* t) : t_(t) {}

bool operator()(const base::WeakPtr<T>& t) {
return t.get() == t_;
}

private:
T* const t_;
};

} // namespace anonymous

bool ContextGroup::HaveContexts() {
Expand All @@ -251,8 +264,8 @@ bool ContextGroup::HaveContexts() {
}

void ContextGroup::Destroy(GLES2Decoder* decoder, bool have_context) {
decoders_.erase(std::remove(decoders_.begin(), decoders_.end(),
decoder->AsWeakPtr()),
decoders_.erase(std::remove_if(decoders_.begin(), decoders_.end(),
WeakPtrEquals<gles2::GLES2Decoder>(decoder)),
decoders_.end());
// If we still have contexts do nothing.
if (HaveContexts()) {
Expand Down
4 changes: 2 additions & 2 deletions net/http/http_cache_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5925,7 +5925,7 @@ TEST(HttpCache, SetPriority) {
// Shouldn't crash, but doesn't do anything either.
trans->SetPriority(net::LOW);

EXPECT_FALSE(cache.network_layer()->last_transaction());
EXPECT_FALSE(cache.network_layer()->last_transaction().get());
EXPECT_EQ(net::DEFAULT_PRIORITY,
cache.network_layer()->last_create_transaction_priority());

Expand All @@ -5935,7 +5935,7 @@ TEST(HttpCache, SetPriority) {
EXPECT_EQ(net::ERR_IO_PENDING,
trans->Start(&info, callback.callback(), net::BoundNetLog()));

ASSERT_TRUE(cache.network_layer()->last_transaction());
ASSERT_TRUE(cache.network_layer()->last_transaction().get());
EXPECT_EQ(net::LOW,
cache.network_layer()->last_create_transaction_priority());
EXPECT_EQ(net::LOW,
Expand Down
2 changes: 1 addition & 1 deletion net/spdy/spdy_write_queue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ void SpdyWriteQueue::RemovePendingWritesForStream(
continue;
for (std::deque<PendingWrite>::const_iterator it = queue_[i].begin();
it != queue_[i].end(); ++it) {
DCHECK_NE(it->stream, stream);
DCHECK_NE(it->stream.get(), stream.get());
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions net/url_request/url_request_ftp_job_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,11 @@ TEST_F(URLRequestFtpJobPriorityTest, SetTransactionPriorityOnStart) {
&req_, &ftp_factory_, &ftp_auth_cache_));
job->SetPriority(LOW);

EXPECT_FALSE(network_layer_.last_transaction());
EXPECT_FALSE(network_layer_.last_transaction().get());

job->Start();

ASSERT_TRUE(network_layer_.last_transaction());
ASSERT_TRUE(network_layer_.last_transaction().get());
EXPECT_EQ(LOW, network_layer_.last_transaction()->priority());
}

Expand All @@ -177,7 +177,7 @@ TEST_F(URLRequestFtpJobPriorityTest, SetTransactionPriority) {
&req_, &ftp_factory_, &ftp_auth_cache_));
job->SetPriority(LOW);
job->Start();
ASSERT_TRUE(network_layer_.last_transaction());
ASSERT_TRUE(network_layer_.last_transaction().get());
EXPECT_EQ(LOW, network_layer_.last_transaction()->priority());

job->SetPriority(HIGHEST);
Expand All @@ -192,15 +192,15 @@ TEST_F(URLRequestFtpJobPriorityTest, SetSubsequentTransactionPriority) {
job->Start();

job->SetPriority(LOW);
ASSERT_TRUE(network_layer_.last_transaction());
ASSERT_TRUE(network_layer_.last_transaction().get());
EXPECT_EQ(LOW, network_layer_.last_transaction()->priority());

job->Kill();
network_layer_.ClearLastTransaction();

// Creates a second transaction.
job->Start();
ASSERT_TRUE(network_layer_.last_transaction());
ASSERT_TRUE(network_layer_.last_transaction().get());
EXPECT_EQ(LOW, network_layer_.last_transaction()->priority());
}

Expand Down
10 changes: 5 additions & 5 deletions net/url_request/url_request_http_job_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,11 @@ TEST_F(URLRequestHttpJobTest, SetTransactionPriorityOnStart) {
scoped_refptr<TestURLRequestHttpJob> job(new TestURLRequestHttpJob(&req_));
job->SetPriority(LOW);

EXPECT_FALSE(network_layer_.last_transaction());
EXPECT_FALSE(network_layer_.last_transaction().get());

job->Start();

ASSERT_TRUE(network_layer_.last_transaction());
ASSERT_TRUE(network_layer_.last_transaction().get());
EXPECT_EQ(LOW, network_layer_.last_transaction()->priority());
}

Expand All @@ -89,7 +89,7 @@ TEST_F(URLRequestHttpJobTest, SetTransactionPriority) {
scoped_refptr<TestURLRequestHttpJob> job(new TestURLRequestHttpJob(&req_));
job->SetPriority(LOW);
job->Start();
ASSERT_TRUE(network_layer_.last_transaction());
ASSERT_TRUE(network_layer_.last_transaction().get());
EXPECT_EQ(LOW, network_layer_.last_transaction()->priority());

job->SetPriority(HIGHEST);
Expand All @@ -103,15 +103,15 @@ TEST_F(URLRequestHttpJobTest, SetSubsequentTransactionPriority) {
job->Start();

job->SetPriority(LOW);
ASSERT_TRUE(network_layer_.last_transaction());
ASSERT_TRUE(network_layer_.last_transaction().get());
EXPECT_EQ(LOW, network_layer_.last_transaction()->priority());

job->Kill();
network_layer_.ClearLastTransaction();

// Creates a second transaction.
job->Start();
ASSERT_TRUE(network_layer_.last_transaction());
ASSERT_TRUE(network_layer_.last_transaction().get());
EXPECT_EQ(LOW, network_layer_.last_transaction()->priority());
}

Expand Down
10 changes: 5 additions & 5 deletions sync/internal_api/public/util/weak_handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ class WeakHandleCore
template <typename U>
void DoCall0(void (U::*fn)(void)) const {
CHECK(IsOnOwnerThread());
if (!Get()) {
if (!Get().get()) {
return;
}
(Get().get()->*fn)();
Expand All @@ -218,7 +218,7 @@ class WeakHandleCore
void DoCall1(void (U::*fn)(A1),
typename ParamTraits<A1>::ForwardType a1) const {
CHECK(IsOnOwnerThread());
if (!Get()) {
if (!Get().get()) {
return;
}
(Get().get()->*fn)(a1);
Expand All @@ -229,7 +229,7 @@ class WeakHandleCore
typename ParamTraits<A1>::ForwardType a1,
typename ParamTraits<A2>::ForwardType a2) const {
CHECK(IsOnOwnerThread());
if (!Get()) {
if (!Get().get()) {
return;
}
(Get().get()->*fn)(a1, a2);
Expand All @@ -241,7 +241,7 @@ class WeakHandleCore
typename ParamTraits<A2>::ForwardType a2,
typename ParamTraits<A3>::ForwardType a3) const {
CHECK(IsOnOwnerThread());
if (!Get()) {
if (!Get().get()) {
return;
}
(Get().get()->*fn)(a1, a2, a3);
Expand All @@ -254,7 +254,7 @@ class WeakHandleCore
typename ParamTraits<A3>::ForwardType a3,
typename ParamTraits<A4>::ForwardType a4) const {
CHECK(IsOnOwnerThread());
if (!Get()) {
if (!Get().get()) {
return;
}
(Get().get()->*fn)(a1, a2, a3, a4);
Expand Down
4 changes: 2 additions & 2 deletions sync/internal_api/public/util/weak_handle_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,15 @@ TEST_F(WeakHandleTest, InitializedAfterDestroy) {
h = b.AsWeakHandle();
}
EXPECT_TRUE(h.IsInitialized());
EXPECT_FALSE(h.Get());
EXPECT_FALSE(h.Get().get());
}

TEST_F(WeakHandleTest, InitializedAfterInvalidate) {
StrictMock<Base> b;
WeakHandle<Base> h = b.AsWeakHandle();
b.Kill();
EXPECT_TRUE(h.IsInitialized());
EXPECT_FALSE(h.Get());
EXPECT_FALSE(h.Get().get());
}

TEST_F(WeakHandleTest, Call) {
Expand Down
6 changes: 3 additions & 3 deletions webkit/browser/quota/quota_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,7 @@ class QuotaManager::GetModifiedSinceHelper {
const GetOriginsCallback& callback,
StorageType type,
bool success) {
if (!manager) {
if (!manager.get()) {
// The operation was aborted.
callback.Run(std::set<GURL>(), type);
return;
Expand All @@ -734,7 +734,7 @@ class QuotaManager::DumpQuotaTableHelper {
void DidDumpQuotaTable(const base::WeakPtr<QuotaManager>& manager,
const DumpQuotaTableCallback& callback,
bool success) {
if (!manager) {
if (!manager.get()) {
// The operation was aborted.
callback.Run(QuotaTableEntries());
return;
Expand Down Expand Up @@ -766,7 +766,7 @@ class QuotaManager::DumpOriginInfoTableHelper {
void DidDumpOriginInfoTable(const base::WeakPtr<QuotaManager>& manager,
const DumpOriginInfoTableCallback& callback,
bool success) {
if (!manager) {
if (!manager.get()) {
// The operation was aborted.
callback.Run(OriginInfoTableEntries());
return;
Expand Down

0 comments on commit eab9d01

Please sign in to comment.