ios/web/navigation/crw_session_controller_unittest.mm - Issue 2737203002: Remove CRWSessionEntry.

Unified Diff: ios/web/navigation/crw_session_controller_unittest.mm

Issue 2737203002: Remove CRWSessionEntry. (Closed)

Patch Set: self review Created 3 years, 9 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Index: ios/web/navigation/crw_session_controller_unittest.mm

diff --git a/ios/web/navigation/crw_session_controller_unittest.mm b/ios/web/navigation/crw_session_controller_unittest.mm

index 3a6ab60e2df053a40b94ac4bf866d69f5cbf506f..aad495b2cc966b67ab981aa37bd968ab6b942251 100644

--- a/ios/web/navigation/crw_session_controller_unittest.mm

+++ b/ios/web/navigation/crw_session_controller_unittest.mm

@@ -30,9 +30,8 @@ @interface CRWSessionController (Testing)

@implementation CRWSessionController (Testing)

- (const GURL&)URLForItemAtIndex:(size_t)index {

- web::NavigationItemList items = self.items;

- if (index < items.size())

- return items[index]->GetURL();

+ if (index < self.items.size())

+ return self.items[index]->GetURL();

return GURL::EmptyGURL();

}

@@ -78,7 +77,7 @@ void SetUp() override {

EXPECT_EQ(-1, [session_controller_ pendingItemIndex]);

[session_controller_ setPendingItemIndex:0];

EXPECT_EQ(0, [session_controller_ pendingItemIndex]);

- EXPECT_EQ([session_controller_ items].back(),

+ EXPECT_EQ([session_controller_ items].back().get(),

[session_controller_ pendingItem]);

}

@@ -118,7 +117,7 @@ void SetUp() override {

}

// Tests that adding a pending item resets pending item index.

-TEST_F(CRWSessionControllerTest, addPendingItemWithExistingPendingItemIndex) {

+TEST_F(CRWSessionControllerTest, addPendingItemWithExisingPendingItemIndex) {

Eugene But (OOO till 7-30) 2017/03/09 16:10:58 Revert back? Exising vs Existing?

kkhorimoto 2017/03/09 21:33:54 Done.

[session_controller_

addPendingItem:GURL("http://www.example.com")

referrer:web::Referrer()

@@ -178,7 +177,7 @@ void SetUp() override {

EXPECT_EQ(1U, [session_controller_ items].size());

EXPECT_EQ(GURL("http://www.url.com/"),

[session_controller_ URLForItemAtIndex:0U]);

- EXPECT_EQ([session_controller_ items].front(),

+ EXPECT_EQ([session_controller_ items].front().get(),

[session_controller_ currentItem]);

}

@@ -198,7 +197,7 @@ void SetUp() override {

EXPECT_EQ(1U, [session_controller_ items].size());

EXPECT_EQ(GURL("http://www.another.url.com/"),

[session_controller_ URLForItemAtIndex:0U]);

- EXPECT_EQ([session_controller_ items].front(),

+ EXPECT_EQ([session_controller_ items].front().get(),

[session_controller_ currentItem]);

}

@@ -273,7 +272,7 @@ void SetUp() override {

EXPECT_EQ(1U, [session_controller_ items].size());

EXPECT_EQ(GURL("http://www.another.url.com/"),

[session_controller_ URLForItemAtIndex:0U]);

- EXPECT_EQ([session_controller_ items].front(),

+ EXPECT_EQ([session_controller_ items].front().get(),

[session_controller_ currentItem]);

}

@@ -295,7 +294,7 @@ void SetUp() override {

EXPECT_EQ(1U, [session_controller_ items].size());

EXPECT_EQ(GURL("http://www.url.com/"),

[session_controller_ URLForItemAtIndex:0U]);

- EXPECT_EQ([session_controller_ items].front(),

+ EXPECT_EQ([session_controller_ items].front().get(),

[session_controller_ currentItem]);

}

@@ -309,25 +308,19 @@ void SetUp() override {

TEST_F(CRWSessionControllerTest,

commitPendingItemWithoutPendingItemWithCommittedItem) {

- // Setup committed item.

Eugene But (OOO till 7-30) 2017/03/09 16:10:58 Why? I think this test suppose to commit if there

kkhorimoto 2017/03/09 21:33:54 Yep you're right. I had fixed this, but must have

- [session_controller_

- addPendingItem:GURL("http://www.url.com")

- referrer:MakeReferrer("http://www.referer.com")

- transition:ui::PAGE_TRANSITION_TYPED

- initiationType:web::NavigationInitiationType::USER_INITIATED];

[session_controller_ commitPendingItem];

// Commit pending item when there is no such one

[session_controller_ commitPendingItem];

EXPECT_EQ(1U, [session_controller_ items].size());

- EXPECT_EQ([session_controller_ items].front(),

+ EXPECT_EQ([session_controller_ items].front().get(),

[session_controller_ currentItem]);

}

-// Tests that forward entries are discarded after navigation item is committed.

+// Tests that forward items are discarded after navigation item is committed.

TEST_F(CRWSessionControllerTest, commitPendingItemWithExistingForwardItems) {

- // Make 3 entries.

+ // Make 3 items.

[session_controller_

addPendingItem:GURL("http://www.example.com/0")

referrer:MakeReferrer("http://www.example.com/a")

@@ -358,7 +351,7 @@ void SetUp() override {

initiationType:web::NavigationInitiationType::RENDERER_INITIATED];

[session_controller_ commitPendingItem];

- // All forward entries should go away.

+ // All forward items should go away.

EXPECT_EQ(2U, [session_controller_ items].size());

EXPECT_EQ(0U, [session_controller_ forwardItems].size());

ASSERT_EQ(1, [session_controller_ currentNavigationIndex]);

@@ -391,15 +384,15 @@ void SetUp() override {

[session_controller_ goToItemAtIndex:1];

[session_controller_ setPendingItemIndex:0];

ASSERT_EQ(0, [session_controller_ pendingItemIndex]);

- web::NavigationItem* pendingItem = [session_controller_ pendingItem];

- ASSERT_TRUE(pendingItem);

+ web::NavigationItemImpl* pending_item = [session_controller_ pendingItem];

+ ASSERT_TRUE(pending_item);

ASSERT_EQ(1, [session_controller_ currentNavigationIndex]);

EXPECT_EQ(2, [session_controller_ previousNavigationIndex]);

[session_controller_ commitPendingItem];

// Verify that pending item has been committed and current and previous item

// indices updated.

- EXPECT_EQ([session_controller_ lastCommittedItem], pendingItem);

+ EXPECT_EQ(pending_item, [session_controller_ lastCommittedItem]);

EXPECT_EQ(-1, [session_controller_ pendingItemIndex]);

EXPECT_FALSE([session_controller_ pendingItem]);

EXPECT_EQ(0, [session_controller_ currentNavigationIndex]);

@@ -425,11 +418,11 @@ void SetUp() override {

initiationType:web::NavigationInitiationType::USER_INITIATED];

[session_controller_ commitPendingItem];

- // Discard noncommitted entries when there is no such one

+ // Discard noncommitted items when there is no such one

[session_controller_ discardNonCommittedItems];

EXPECT_EQ(1U, [session_controller_ items].size());

- EXPECT_EQ([session_controller_ items].front(),

+ EXPECT_EQ([session_controller_ items].front().get(),

[session_controller_ currentItem]);

}

@@ -468,7 +461,7 @@ void SetUp() override {

EXPECT_EQ(1U, [session_controller_ items].size());

EXPECT_EQ(GURL("http://www.url.com/"),

[session_controller_ URLForItemAtIndex:0U]);

- EXPECT_EQ([session_controller_ items].front(),

+ EXPECT_EQ([session_controller_ items].front().get(),

[session_controller_ currentItem]);

}

@@ -522,7 +515,7 @@ void SetUp() override {

// Tests inserting session controller state from empty session controller.

TEST_F(CRWSessionControllerTest, InsertStateFromEmptySessionController) {

- // Add 2 committed entries to target controller.

+ // Add 2 committed items to target controller.

[session_controller_

addPendingItem:GURL("http://www.url.com/0")

referrer:web::Referrer()

@@ -557,7 +550,7 @@ void SetUp() override {

// Tests inserting session controller state to empty session controller.

TEST_F(CRWSessionControllerTest, InsertStateToEmptySessionController) {

- // Create source session controller with 2 committed entries and one

+ // Create source session controller with 2 committed items and one

// pending item.

base::scoped_nsobject<CRWSessionController> other_session_controller(

[[CRWSessionController alloc] initWithBrowserState:&browser_state_

@@ -599,7 +592,7 @@ void SetUp() override {

// remains valid.

TEST_F(CRWSessionControllerTest,

InsertStateWithPendingItemIndexInTargetController) {

- // Add 2 committed entries and make the first item pending.

+ // Add 2 committed items and make the first item pending.

[session_controller_

addPendingItem:GURL("http://www.url.com/2")

referrer:web::Referrer()

@@ -696,10 +689,9 @@ void SetUp() override {

EXPECT_EQ(controller.get().items.size(), 3U);

EXPECT_EQ(controller.get().currentNavigationIndex, 1);

EXPECT_EQ(controller.get().previousNavigationIndex, -1);

- // Sanity check the current item, the CRWSessionItem unit test will ensure

+ // Sanity check the current item, the NavigationItem unit test will ensure

// the entire object is created properly.

- EXPECT_EQ(controller.get().currentItem->GetURL(),

- GURL("http://www.yahoo.com"));

+ EXPECT_EQ([controller currentItem]->GetURL(), GURL("http://www.yahoo.com"));

}

// Tests index of previous navigation item.

@@ -802,18 +794,12 @@ void SetUp() override {

[[CRWSessionController alloc] initWithBrowserState:&browser_state_

navigationItems:std::move(items)

currentIndex:0]);

- web::NavigationItemImpl* item0 =

- static_cast<web::NavigationItemImpl*>([controller items][0]);

- web::NavigationItemImpl* item1 =

- static_cast<web::NavigationItemImpl*>([controller items][1]);

- web::NavigationItemImpl* item2 =

- static_cast<web::NavigationItemImpl*>([controller items][2]);

- web::NavigationItemImpl* item3 =

- static_cast<web::NavigationItemImpl*>([controller items][3]);

- web::NavigationItemImpl* item4 =

- static_cast<web::NavigationItemImpl*>([controller items][4]);

- web::NavigationItemImpl* item5 =

- static_cast<web::NavigationItemImpl*>([controller items][5]);

+ web::NavigationItemImpl* item0 = [controller items][0].get();

+ web::NavigationItemImpl* item1 = [controller items][1].get();

+ web::NavigationItemImpl* item2 = [controller items][2].get();

+ web::NavigationItemImpl* item3 = [controller items][3].get();

+ web::NavigationItemImpl* item4 = [controller items][4].get();

+ web::NavigationItemImpl* item5 = [controller items][5].get();

item1->SetIsCreatedFromPushState(true);

item4->SetIsCreatedFromHashChange(true);

item5->SetIsCreatedFromPushState(true);

@@ -916,7 +902,7 @@ void SetUp() override {

EXPECT_EQ("http://www.example.com/2", forwardItems[1]->GetURL().spec());

}

-// Tests going to entries with existing and non-existing indices.

+// Tests going to items with existing and non-existing indices.

TEST_F(CRWSessionControllerTest, GoToItemAtIndex) {

[session_controller_

addPendingItem:GURL("http://www.example.com/0")

@@ -953,20 +939,20 @@ void SetUp() override {

EXPECT_TRUE([session_controller_ pendingItem]);

EXPECT_TRUE([session_controller_ transientItem]);

- // Going back should discard transient and pending entries.

+ // Going back should discard transient and pending items.

[session_controller_ goToItemAtIndex:1];

EXPECT_EQ(1, session_controller_.get().currentNavigationIndex);

EXPECT_EQ(3, session_controller_.get().previousNavigationIndex);

- EXPECT_FALSE([session_controller_ pendingItem]);

- EXPECT_FALSE([session_controller_ transientItem]);

+ EXPECT_FALSE(session_controller_.get().pendingItem);

+ EXPECT_FALSE(session_controller_.get().transientItem);

// Going forward should discard transient item.

[session_controller_ addTransientItemWithURL:GURL("http://www.example.com")];

- EXPECT_TRUE([session_controller_ transientItem]);

+ EXPECT_TRUE(session_controller_.get().transientItem);

[session_controller_ goToItemAtIndex:2];

EXPECT_EQ(2, session_controller_.get().currentNavigationIndex);

EXPECT_EQ(1, session_controller_.get().previousNavigationIndex);

- EXPECT_FALSE([session_controller_ transientItem]);

+ EXPECT_FALSE(session_controller_.get().transientItem);

// Out of bounds navigations should be no-op.

[session_controller_ goToItemAtIndex:-1];

@@ -983,10 +969,10 @@ void SetUp() override {

}

// Tests that visible URL is the same as transient URL if there are no committed

-// entries.

+// items.

TEST_F(CRWSessionControllerTest, VisibleItemWithSingleTransientItem) {

[session_controller_ addTransientItemWithURL:GURL("http://www.example.com")];

- web::NavigationItem* visible_item = [session_controller_ visibleItem];

+ web::NavigationItemImpl* visible_item = [session_controller_ visibleItem];

Eugene But (OOO till 7-30) 2017/03/09 16:10:58 Why? Using web::NavigationItem* seems like a suffi

kkhorimoto 2017/03/09 21:33:54 I've updated to use the more appropriate class. N

ASSERT_TRUE(visible_item);

EXPECT_EQ("http://www.example.com/", visible_item->GetURL().spec());

}

@@ -1001,7 +987,7 @@ void SetUp() override {

initiationType:web::NavigationInitiationType::USER_INITIATED];

[session_controller_ commitPendingItem];

[session_controller_ addTransientItemWithURL:GURL("http://www.example.com")];

- web::NavigationItem* visible_item = [session_controller_ visibleItem];

+ web::NavigationItemImpl* visible_item = [session_controller_ visibleItem];

ASSERT_TRUE(visible_item);

EXPECT_EQ("http://www.example.com/", visible_item->GetURL().spec());

}

@@ -1014,7 +1000,7 @@ void SetUp() override {

referrer:MakeReferrer("http://www.example.com/a")

transition:ui::PAGE_TRANSITION_LINK

initiationType:web::NavigationInitiationType::USER_INITIATED];

- web::NavigationItem* visible_item = [session_controller_ visibleItem];

+ web::NavigationItemImpl* visible_item = [session_controller_ visibleItem];

ASSERT_TRUE(visible_item);

EXPECT_EQ("http://www.example.com/0", visible_item->GetURL().spec());

}

@@ -1090,12 +1076,12 @@ void SetUp() override {

[session_controller_ setPendingItemIndex:0];

- web::NavigationItem* visible_item = [session_controller_ visibleItem];

+ web::NavigationItemImpl* visible_item = [session_controller_ visibleItem];

ASSERT_TRUE(visible_item);

EXPECT_EQ("http://www.example.com/0", visible_item->GetURL().spec());

}

-// Tests that |-backwardItems| is empty if all preceding entries are

+// Tests that |-backwardItems| is empty if all preceding items are

// redirects.

TEST_F(CRWSessionControllerTest, BackwardItemsForAllRedirects) {

[session_controller_

« ios/web/navigation/crw_session_controller.mm ('K') | « ios/web/navigation/crw_session_controller.mm ('k') | ios/web/navigation/crw_session_entry.h » ('j') | no next file with comments »