Remove redundant boolean from unit test code. (issue 10386171)

0 views
Skip to first unread message

domi...@chromium.org

unread,
May 16, 2012, 1:06:57 PM5/16/12
to mme...@chromium.org, chromium...@chromium.org, tburkar...@chromium.org, cbentze...@chromium.org, dominic...@chromium.org, mme...@chromium.org
Reviewers: Matt Menke,

Description:
Remove redundant boolean from unit test code.

BUG=None
TEST=None


Please review this at https://chromiumcodereview.appspot.com/10386171/

SVN Base: svn://svn.chromium.org/chrome/trunk/src

Affected files:
M chrome/browser/prerender/prerender_contents.h
M chrome/browser/prerender/prerender_contents.cc
M chrome/browser/prerender/prerender_manager_unittest.cc


Index: chrome/browser/prerender/prerender_contents.cc
diff --git a/chrome/browser/prerender/prerender_contents.cc
b/chrome/browser/prerender/prerender_contents.cc
index
098a67a74cb737c1920b8e06b86768543ec1dc75..425befdee92a4ecd7837c724686098b09022bd35
100644
--- a/chrome/browser/prerender/prerender_contents.cc
+++ b/chrome/browser/prerender/prerender_contents.cc
@@ -237,7 +237,8 @@ PrerenderContents::PrerenderContents(
const content::Referrer& referrer,
Origin origin,
uint8 experiment_id)
- : prerender_manager_(prerender_manager),
+ : prerendering_has_started_(false),
+ prerender_manager_(prerender_manager),
prerender_tracker_(prerender_tracker),
prerender_url_(url),
referrer_(referrer),
@@ -246,7 +247,6 @@ PrerenderContents::PrerenderContents(
has_stopped_loading_(false),
has_finished_loading_(false),
final_status_(FINAL_STATUS_MAX),
- prerendering_has_started_(false),
match_complete_status_(MATCH_COMPLETE_DEFAULT),
prerendering_has_been_cancelled_(false),
child_id_(-1),
Index: chrome/browser/prerender/prerender_contents.h
diff --git a/chrome/browser/prerender/prerender_contents.h
b/chrome/browser/prerender/prerender_contents.h
index
854c55d2600fc24cfca3635990acc9a9509357f2..199bfa2ce83876de5397c399ad81f3bd63b9a70e
100644
--- a/chrome/browser/prerender/prerender_contents.h
+++ b/chrome/browser/prerender/prerender_contents.h
@@ -242,6 +242,8 @@ class PrerenderContents : public
content::NotificationObserver,
virtual content::WebContents* CreateWebContents(
content::SessionStorageNamespace* session_storage_namespace);

+ bool prerendering_has_started_;
+
private:
class TabContentsDelegateImpl;

@@ -299,8 +301,6 @@ class PrerenderContents : public
content::NotificationObserver,
// |this|, when |this| has a RenderView.
FinalStatus final_status_;

- bool prerendering_has_started_;
-
// The MatchComplete status of the prerender, indicating how it relates
// to being a MatchComplete dummy (see definition of MatchCompleteStatus
// above).
Index: chrome/browser/prerender/prerender_manager_unittest.cc
diff --git a/chrome/browser/prerender/prerender_manager_unittest.cc
b/chrome/browser/prerender/prerender_manager_unittest.cc
index
dd023b194ff6157878d2fa35b627fb7077921393..f977810fb019bcd3b23ac259085038f55fd92030
100644
--- a/chrome/browser/prerender/prerender_manager_unittest.cc
+++ b/chrome/browser/prerender/prerender_manager_unittest.cc
@@ -18,6 +18,7 @@
#include "testing/gtest/include/gtest/gtest.h"

using content::BrowserThread;
+using content::Referrer;

namespace prerender {

@@ -28,12 +29,12 @@ class DummyPrerenderContents : public PrerenderContents
{
DummyPrerenderContents(PrerenderManager* prerender_manager,
PrerenderTracker* prerender_tracker,
const GURL& url,
+ const Referrer& referrer,
Origin origin,
FinalStatus expected_final_status)
: PrerenderContents(prerender_manager, prerender_tracker,
- NULL, url, content::Referrer(),
- origin, PrerenderManager::kNoExperiment),
- has_started_(false),
+ NULL, url, referrer, origin,
+ PrerenderManager::kNoExperiment),
expected_final_status_(expected_final_status) {
}

@@ -44,7 +45,7 @@ class DummyPrerenderContents : public PrerenderContents {
virtual void StartPrerendering(
const content::RenderViewHost* source_render_view_host,
content::SessionStorageNamespace* session_storage_namespace)
OVERRIDE {
- has_started_ = true;
+ prerendering_has_started_ = true;
}

virtual bool GetChildId(int* child_id) const OVERRIDE {
@@ -57,12 +58,9 @@ class DummyPrerenderContents : public PrerenderContents {
return true;
}

- bool has_started() const { return has_started_; }
-
FinalStatus expected_final_status() const { return
expected_final_status_; }

private:
- bool has_started_;
FinalStatus expected_final_status_;
};

@@ -107,6 +105,7 @@ class TestPrerenderManager : public PrerenderManager {
FinalStatus expected_final_status) {
DummyPrerenderContents* prerender_contents =
new DummyPrerenderContents(this, prerender_tracker_, url,
+ Referrer(),
ORIGIN_LINK_REL_PRERENDER,
expected_final_status);
SetNextPrerenderContents(prerender_contents);
@@ -119,7 +118,8 @@ class TestPrerenderManager : public PrerenderManager {
FinalStatus expected_final_status) {
DummyPrerenderContents* prerender_contents =
new DummyPrerenderContents(this, prerender_tracker_, url,
- origin, expected_final_status);
+ Referrer(), origin,
+ expected_final_status);
SetNextPrerenderContents(prerender_contents);
return prerender_contents;
}
@@ -130,6 +130,7 @@ class TestPrerenderManager : public PrerenderManager {
FinalStatus expected_final_status) {
DummyPrerenderContents* prerender_contents =
new DummyPrerenderContents(this, prerender_tracker_, url,
+ Referrer(),
ORIGIN_LINK_REL_PRERENDER,
expected_final_status);
for (std::vector<GURL>::const_iterator it = alias_urls.begin();
@@ -143,9 +144,7 @@ class TestPrerenderManager : public PrerenderManager {

// Shorthand to add a simple preload with a reasonable source.
bool AddSimplePrerender(const GURL& url) {
- return AddPrerenderFromLinkRelPrerender(-1, -1,
- url,
- content::Referrer());
+ return AddPrerenderFromLinkRelPrerender(-1, -1, url, Referrer());
}

void set_rate_limit_enabled(bool enabled) {
@@ -174,7 +173,7 @@ class TestPrerenderManager : public PrerenderManager {

virtual PrerenderContents* CreatePrerenderContents(
const GURL& url,
- const content::Referrer& referrer,
+ const Referrer& referrer,
Origin origin,
uint8 experiment_id) OVERRIDE {
DCHECK(next_prerender_contents_.get());
@@ -246,7 +245,7 @@ TEST_F(PrerenderManagerTest, FoundTest) {
url,
FINAL_STATUS_USED);
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(url));
- EXPECT_TRUE(prerender_contents->has_started());
+ EXPECT_TRUE(prerender_contents->prerendering_has_started());
ASSERT_EQ(prerender_contents, prerender_manager()->GetEntry(url));
}

@@ -261,7 +260,7 @@ TEST_F(PrerenderManagerTest, DropSecondRequestTest) {
DummyPrerenderContents* null = NULL;
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(url));
EXPECT_EQ(null, prerender_manager()->next_prerender_contents());
- EXPECT_TRUE(prerender_contents->has_started());
+ EXPECT_TRUE(prerender_contents->prerendering_has_started());

DummyPrerenderContents* prerender_contents1 =
prerender_manager()->CreateNextPrerenderContents(
@@ -270,7 +269,7 @@ TEST_F(PrerenderManagerTest, DropSecondRequestTest) {
EXPECT_FALSE(prerender_manager()->AddSimplePrerender(url));
EXPECT_EQ(prerender_contents1,
prerender_manager()->next_prerender_contents());
- EXPECT_FALSE(prerender_contents1->has_started());
+ EXPECT_FALSE(prerender_contents1->prerendering_has_started());

ASSERT_EQ(prerender_contents, prerender_manager()->GetEntry(url));
}
@@ -285,7 +284,7 @@ TEST_F(PrerenderManagerTest, ExpireTest) {
DummyPrerenderContents* null = NULL;
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(url));
EXPECT_EQ(null, prerender_manager()->next_prerender_contents());
- EXPECT_TRUE(prerender_contents->has_started());
+ EXPECT_TRUE(prerender_contents->prerendering_has_started());
prerender_manager()->AdvanceTime(prerender_manager()->GetMaxAge() +
base::TimeDelta::FromSeconds(1));
ASSERT_EQ(null, prerender_manager()->GetEntry(url));
@@ -302,7 +301,7 @@ TEST_F(PrerenderManagerTest, DropOldestRequestTest) {
DummyPrerenderContents* null = NULL;
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(url));
EXPECT_EQ(null, prerender_manager()->next_prerender_contents());
- EXPECT_TRUE(prerender_contents->has_started());
+ EXPECT_TRUE(prerender_contents->prerendering_has_started());

GURL url1("http://news.google.com/");
DummyPrerenderContents* prerender_contents1 =
@@ -311,7 +310,7 @@ TEST_F(PrerenderManagerTest, DropOldestRequestTest) {
FINAL_STATUS_USED);
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(url1));
EXPECT_EQ(null, prerender_manager()->next_prerender_contents());
- EXPECT_TRUE(prerender_contents1->has_started());
+ EXPECT_TRUE(prerender_contents1->prerendering_has_started());

ASSERT_EQ(null, prerender_manager()->GetEntry(url));
ASSERT_EQ(prerender_contents1, prerender_manager()->GetEntry(url1));
@@ -329,7 +328,7 @@ TEST_F(PrerenderManagerTest, TwoElementPrerenderTest) {
DummyPrerenderContents* null = NULL;
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(url));
EXPECT_EQ(null, prerender_manager()->next_prerender_contents());
- EXPECT_TRUE(prerender_contents->has_started());
+ EXPECT_TRUE(prerender_contents->prerendering_has_started());

GURL url1("http://news.google.com/");
DummyPrerenderContents* prerender_contents1 =
@@ -338,7 +337,7 @@ TEST_F(PrerenderManagerTest, TwoElementPrerenderTest) {
FINAL_STATUS_USED);
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(url1));
EXPECT_EQ(null, prerender_manager()->next_prerender_contents());
- EXPECT_TRUE(prerender_contents1->has_started());
+ EXPECT_TRUE(prerender_contents1->prerendering_has_started());

GURL url2("http://images.google.com/");
DummyPrerenderContents* prerender_contents2 =
@@ -347,7 +346,7 @@ TEST_F(PrerenderManagerTest, TwoElementPrerenderTest) {
FINAL_STATUS_USED);
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(url2));
EXPECT_EQ(null, prerender_manager()->next_prerender_contents());
- EXPECT_TRUE(prerender_contents2->has_started());
+ EXPECT_TRUE(prerender_contents2->prerendering_has_started());

ASSERT_EQ(null, prerender_manager()->GetEntry(url));
ASSERT_EQ(prerender_contents1, prerender_manager()->GetEntry(url1));
@@ -399,7 +398,7 @@ TEST_F(PrerenderManagerTest, RateLimitInWindowTest) {
DummyPrerenderContents* null = NULL;
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(url));
EXPECT_EQ(null, prerender_manager()->next_prerender_contents());
- EXPECT_TRUE(prerender_contents->has_started());
+ EXPECT_TRUE(prerender_contents->prerendering_has_started());

prerender_manager()->set_rate_limit_enabled(true);

prerender_manager()->AdvanceTimeTicks(base::TimeDelta::FromMilliseconds(1));
@@ -422,7 +421,7 @@ TEST_F(PrerenderManagerTest,
RateLimitOutsideWindowTest) {
DummyPrerenderContents* null = NULL;
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(url));
EXPECT_EQ(null, prerender_manager()->next_prerender_contents());
- EXPECT_TRUE(prerender_contents->has_started());
+ EXPECT_TRUE(prerender_contents->prerendering_has_started());

prerender_manager()->set_rate_limit_enabled(true);
prerender_manager()->AdvanceTimeTicks(
@@ -435,7 +434,7 @@ TEST_F(PrerenderManagerTest,
RateLimitOutsideWindowTest) {
FINAL_STATUS_MANAGER_SHUTDOWN);
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(url1));
EXPECT_EQ(null, prerender_manager()->next_prerender_contents());
- EXPECT_TRUE(rate_limit_prerender_contents->has_started());
+ EXPECT_TRUE(rate_limit_prerender_contents->prerendering_has_started());
prerender_manager()->set_rate_limit_enabled(false);
}

@@ -456,10 +455,10 @@ TEST_F(PrerenderManagerTest, PendingPrerenderTest) {

EXPECT_TRUE(prerender_manager()->AddPrerenderFromLinkRelPrerender(
child_id, route_id,
- pending_url, content::Referrer(url,
WebKit::WebReferrerPolicyDefault)));
+ pending_url, Referrer(url, WebKit::WebReferrerPolicyDefault)));

EXPECT_TRUE(prerender_manager()->IsPendingEntry(pending_url));
- EXPECT_TRUE(prerender_contents->has_started());
+ EXPECT_TRUE(prerender_contents->prerendering_has_started());
ASSERT_EQ(prerender_contents, prerender_manager()->GetEntry(url));
}

@@ -475,7 +474,7 @@ TEST_F(PrerenderManagerTest, ControlGroup) {
url,
FINAL_STATUS_MANAGER_SHUTDOWN);
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(url));
- EXPECT_FALSE(prerender_contents->has_started());
+ EXPECT_FALSE(prerender_contents->prerendering_has_started());
}

// Tests that prerendering is cancelled when the source render view does
not
@@ -487,7 +486,7 @@ TEST_F(PrerenderManagerTest, SourceRenderViewClosed) {
url,
FINAL_STATUS_MANAGER_SHUTDOWN);
EXPECT_FALSE(prerender_manager()->AddPrerenderFromLinkRelPrerender(
- 100, 100, url, content::Referrer()));
+ 100, 100, url, Referrer()));
}

// Tests that the prerender manager ignores fragment references when
matching
@@ -500,7 +499,7 @@ TEST_F(PrerenderManagerTest, PageMatchesFragmentTest) {
prerender_manager()->CreateNextPrerenderContents(url,
FINAL_STATUS_USED);
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(url));
- EXPECT_TRUE(prerender_contents->has_started());
+ EXPECT_TRUE(prerender_contents->prerendering_has_started());
ASSERT_EQ(prerender_contents,
prerender_manager()->GetEntry(fragment_url));
}

@@ -514,7 +513,7 @@ TEST_F(PrerenderManagerTest, FragmentMatchesPageTest) {
prerender_manager()->CreateNextPrerenderContents(fragment_url,
FINAL_STATUS_USED);
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(fragment_url));
- EXPECT_TRUE(prerender_contents->has_started());
+ EXPECT_TRUE(prerender_contents->prerendering_has_started());
ASSERT_EQ(prerender_contents, prerender_manager()->GetEntry(url));
}

@@ -528,7 +527,7 @@ TEST_F(PrerenderManagerTest,
FragmentMatchesFragmentTest) {
prerender_manager()->CreateNextPrerenderContents(fragment_url,
FINAL_STATUS_USED);
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(fragment_url));
- EXPECT_TRUE(prerender_contents->has_started());
+ EXPECT_TRUE(prerender_contents->prerendering_has_started());
ASSERT_EQ(prerender_contents,
prerender_manager()->GetEntry(other_fragment_url));
}
@@ -541,7 +540,7 @@ TEST_F(PrerenderManagerTest, ClearTest) {
url,
FINAL_STATUS_CACHE_OR_HISTORY_CLEARED);
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(url));
- EXPECT_TRUE(prerender_contents->has_started());
+ EXPECT_TRUE(prerender_contents->prerendering_has_started());

prerender_manager()->ClearData(PrerenderManager::CLEAR_PRERENDER_CONTENTS);
DummyPrerenderContents* null = NULL;
EXPECT_EQ(null, prerender_manager()->FindEntry(url));
@@ -554,7 +553,7 @@ TEST_F(PrerenderManagerTest, CancelAllTest) {
prerender_manager()->CreateNextPrerenderContents(
url, FINAL_STATUS_CANCELLED);
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(url));
- EXPECT_TRUE(prerender_contents->has_started());
+ EXPECT_TRUE(prerender_contents->prerendering_has_started());
prerender_manager()->CancelAllPrerenders();
const DummyPrerenderContents* null = NULL;
EXPECT_EQ(null, prerender_manager()->FindEntry(url));
@@ -568,7 +567,7 @@ TEST_F(PrerenderManagerTest,
CancelOmniboxRemovesOmniboxTest) {
prerender_manager()->CreateNextPrerenderContents(
url, ORIGIN_OMNIBOX, FINAL_STATUS_CANCELLED);
EXPECT_TRUE(prerender_manager()->AddPrerenderFromOmnibox(url, NULL));
- EXPECT_TRUE(prerender_contents->has_started());
+ EXPECT_TRUE(prerender_contents->prerendering_has_started());
prerender_manager()->CancelOmniboxPrerenders();
const DummyPrerenderContents* null = NULL;
EXPECT_EQ(null, prerender_manager()->FindEntry(url));
@@ -580,7 +579,7 @@ TEST_F(PrerenderManagerTest,
CancelOmniboxDoesNotRemoveLinkTest) {
prerender_manager()->CreateNextPrerenderContents(
url, ORIGIN_LINK_REL_PRERENDER, FINAL_STATUS_MANAGER_SHUTDOWN);
EXPECT_TRUE(prerender_manager()->AddSimplePrerender(url));
- EXPECT_TRUE(prerender_contents->has_started());
+ EXPECT_TRUE(prerender_contents->prerendering_has_started());
prerender_manager()->CancelOmniboxPrerenders();
const DummyPrerenderContents* null = NULL;
EXPECT_NE(null, prerender_manager()->FindEntry(url));


mme...@chromium.org

unread,
May 16, 2012, 1:09:40 PM5/16/12
to domi...@chromium.org, chromium...@chromium.org, tburkar...@chromium.org, cbentze...@chromium.org, dominic...@chromium.org
LG, other than the whole merge failure thing...

https://chromiumcodereview.appspot.com/10386171/

mme...@chromium.org

unread,
May 16, 2012, 2:12:17 PM5/16/12
to domi...@chromium.org, chromium...@chromium.org, tburkar...@chromium.org, cbentze...@chromium.org, dominic...@chromium.org

commi...@chromium.org

unread,
May 16, 2012, 2:53:39 PM5/16/12
to domi...@chromium.org, mme...@chromium.org, chromium...@chromium.org, tburkar...@chromium.org, cbentze...@chromium.org, dominic...@chromium.org

commi...@chromium.org

unread,
May 16, 2012, 4:26:07 PM5/16/12
to domi...@chromium.org, mme...@chromium.org, chromium...@chromium.org, tburkar...@chromium.org, cbentze...@chromium.org, dominic...@chromium.org
Reply all
Reply to author
Forward
0 new messages