Do not reload pengingItem to open Reading List offline page.

Reload will delete Pending Item so altering it and reload will reload
the lastCommittedItem.
If there is a pending item, stop loading and trigger a new navigation.

BUG=678293

Review-Url: https://codereview.chromium.org/2610953002
Cr-Commit-Position: refs/heads/master@{#441926}

ios/chrome/browser/reading_list/reading_list_web_state_observer.h

@@ -49,7 +49,7 @@ class ReadingListWebStateObserver : public web::WebStateObserver,
   void StopCheckingProgress();
 
   // Loads the offline version of the URL in place of the current page.
-  void LoadOfflineReadingListEntry(web::NavigationItem* item);
+  void LoadOfflineReadingListEntry();
 
   // Returns if the current page with |url| has an offline version that can be
   // displayed if the normal loading fails.

ios/chrome/browser/reading_list/reading_list_web_state_observer.mm

@@ -125,7 +125,7 @@ void ReadingListWebStateObserver::ReadingListModelLoaded(
   const GURL& currentURL = item->GetVirtualURL();
   if (IsUrlAvailableOffline(currentURL)) {
     pending_url_ = currentURL;
-    LoadOfflineReadingListEntry(item);
+    LoadOfflineReadingListEntry();
     StopCheckingProgress();
   }
 }
@@ -203,7 +203,7 @@ void ReadingListWebStateObserver::PageLoaded(
     reading_list_model_->SetReadStatus(pending_url_, true);
     UMA_HISTOGRAM_BOOLEAN("ReadingList.OfflineVersionDisplayed", false);
   } else {
-    LoadOfflineReadingListEntry(item);
+    LoadOfflineReadingListEntry();
   }
   StopCheckingProgress();
 }
@@ -245,7 +245,7 @@ void ReadingListWebStateObserver::VerifyIfReadingListEntryStartedLoading() {
   double progress = web_state()->GetLoadingProgress();
   const double kMinimumExpectedProgressPerStep = 0.25;
   if (progress < try_number_ * kMinimumExpectedProgressPerStep) {
-    LoadOfflineReadingListEntry(item);
+    LoadOfflineReadingListEntry();
     StopCheckingProgress();
     return;
   }
@@ -258,9 +258,7 @@ void ReadingListWebStateObserver::VerifyIfReadingListEntryStartedLoading() {
   }
 }
 
-void ReadingListWebStateObserver::LoadOfflineReadingListEntry(
-    web::NavigationItem* item) {
-  DCHECK(item);
+void ReadingListWebStateObserver::LoadOfflineReadingListEntry() {
   DCHECK(reading_list_model_);
   if (!pending_url_.is_valid() || !IsUrlAvailableOffline(pending_url_)) {
     return;
@@ -270,9 +268,20 @@ void ReadingListWebStateObserver::LoadOfflineReadingListEntry(
   DCHECK(entry->DistilledState() == ReadingListEntry::PROCESSED);
   GURL url =
       reading_list::DistilledURLForPath(entry->DistilledPath(), entry->URL());
-  item->SetURL(url);
-  item->SetVirtualURL(pending_url_);
-  web_state()->GetNavigationManager()->Reload(false);
+  web::NavigationManager* manager = web_state()->GetNavigationManager();
+  web::NavigationItem* item = manager->GetPendingItem();
+  if (item) {
+    web_state()->Stop();
+    web::WebState::OpenURLParams params(url, item->GetReferrer(),
+                                        WindowOpenDisposition::CURRENT_TAB,
+                                        item->GetTransitionType(), NO);
+    web_state()->OpenURL(params);
+  } else {
+    item = manager->GetLastCommittedItem();
+    item->SetURL(url);
+    item->SetVirtualURL(pending_url_);
+    web_state()->GetNavigationManager()->Reload(false);
+  }
   reading_list_model_->SetReadStatus(entry->URL(), true);
   UMA_HISTOGRAM_BOOLEAN("ReadingList.OfflineVersionDisplayed", true);
 }

ios/chrome/browser/reading_list/reading_list_web_state_observer_unittest.mm

@@ -31,6 +31,18 @@ class TestNavigationManager : public web::TestNavigationManager {
   bool reload_called_ = false;
 };
 
+// A Test navigation manager that remembers the last opened parameters.
+class TestWebState : public web::TestWebState {
+ public:
+  void OpenURL(const web::WebState::OpenURLParams& params) override {
+    last_opened_url_ = params.url;
+  }
+  const GURL& LastOpenedUrl() { return last_opened_url_; }
+
+ private:
+  GURL last_opened_url_;
+};
+
 // Test fixture to test loading of Reading list entries.
 class ReadingListWebStateObserverTest : public web::WebTest {
  public:
@@ -54,7 +66,7 @@ class ReadingListWebStateObserverTest : public web::WebTest {
   std::unique_ptr<web::NavigationItem> last_committed_item_;
   std::unique_ptr<ReadingListModelImpl> reading_list_model_;
   TestNavigationManager* test_navigation_manager_;
-  web::TestWebState test_web_state_;
+  TestWebState test_web_state_;
 };
 
 // Tests that failing loading an online version does not mark it read.
@@ -97,8 +109,8 @@ TEST_F(ReadingListWebStateObserverTest, TestLoadReadingListOnline) {
   EXPECT_TRUE(entry->IsRead());
 }
 
-// Tests that loading a distilled version of an entry.
-TEST_F(ReadingListWebStateObserverTest, TestLoadReadingListDistilled) {
+// Tests that loading a distilled version of an entry from a commited entry.
+TEST_F(ReadingListWebStateObserverTest, TestLoadReadingListDistilledCommitted) {
   GURL url(kTestURL);
   std::string distilled_path = kTestDistilledPath;
   reading_list_model_->SetEntryDistilledPath(url,
@@ -112,10 +124,32 @@ TEST_F(ReadingListWebStateObserverTest, TestLoadReadingListDistilled) {
   test_web_state_.OnPageLoaded(web::PageLoadCompletionStatus::FAILURE);
   test_web_state_.SetLoading(false);
 
+  EXPECT_FALSE(test_navigation_manager_->ReloadCalled());
+  EXPECT_EQ(test_web_state_.LastOpenedUrl(), distilled_url);
+  EXPECT_TRUE(entry->IsRead());
+}
+
+// Tests that loading a distilled version of an entry.
+TEST_F(ReadingListWebStateObserverTest, TestLoadReadingListDistilledPending) {
+  GURL url(kTestURL);
+  std::string distilled_path = kTestDistilledPath;
+  reading_list_model_->SetEntryDistilledPath(url,
+                                             base::FilePath(distilled_path));
+  const ReadingListEntry* entry = reading_list_model_->GetEntryByURL(url);
+  GURL distilled_url =
+      reading_list::DistilledURLForPath(entry->DistilledPath(), entry->URL());
+
+  test_navigation_manager_->SetPendingItem(nil);
+  test_navigation_manager_->GetLastCommittedItem()->SetURL(url);
+  test_web_state_.SetLoading(true);
+  test_web_state_.OnPageLoaded(web::PageLoadCompletionStatus::FAILURE);
+  test_web_state_.SetLoading(false);
+
   EXPECT_TRUE(test_navigation_manager_->ReloadCalled());
   EXPECT_EQ(test_navigation_manager_->GetLastCommittedItem()->GetVirtualURL(),
             url);
   EXPECT_EQ(test_navigation_manager_->GetLastCommittedItem()->GetURL(),
             distilled_url);
+
   EXPECT_TRUE(entry->IsRead());
 }