Reload offline version on load failure

ios/chrome/browser/reading_list/reading_list_web_state_observer.mm

Index: ios/chrome/browser/reading_list/reading_list_web_state_observer.mm
diff --git a/ios/chrome/browser/reading_list/reading_list_web_state_observer.mm b/ios/chrome/browser/reading_list/reading_list_web_state_observer.mm
index b4aa9c0983e0990ab5f7f49a9064cd8bef3b5f2b..cde7073b2e62138b7c82a9dbe68a0603c8867566 100644
--- a/ios/chrome/browser/reading_list/reading_list_web_state_observer.mm
+++ b/ios/chrome/browser/reading_list/reading_list_web_state_observer.mm
@@ -9,7 +9,8 @@
 #include "base/memory/ptr_util.h"
 #include "components/reading_list/ios/reading_list_model.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
-#include "ios/chrome/browser/reading_list/reading_list_entry_loading_util.h"
+#include "ios/chrome/browser/chrome_url_constants.h"
+#include "ios/chrome/browser/reading_list/offline_url_utils.h"
 #include "ios/chrome/browser/reading_list/reading_list_model_factory.h"
 #include "ios/web/public/navigation_item.h"
 #include "ios/web/public/navigation_manager.h"
@@ -83,15 +84,60 @@ ReadingListWebStateObserver::ReadingListWebStateObserver(
   DCHECK(reading_list_model_);
 }
 
-void ReadingListWebStateObserver::DidStopLoading() {
-  StopCheckingProgress();
+void ReadingListWebStateObserver::DidStartLoading() {
+  if (!reading_list_model_->loaded() || !web_state()) {

[Eugene But (OOO till 7-30), 2016/12/19 18:47:04]

nit: Do you want to create ShouldStopCheckingProgress method to simplify this
method?

if (ShouldStopCheckingProgress()) {
  StopCheckingProgress();
} else {
  GURL virtual_url = item->GetVirtualURL();
  StartCheckingProgress(virtual_url.empty() ? item->GetURL() : virtual_url);
}

You will be able to reuse that method inside LoadReadingListDistilled as well.

[Olivier, 2016/12/20 12:29:47]

Created some methods to simplify checks

+    StopCheckingProgress();
+    return;
+  }
+  web::NavigationItem* item =
+      web_state()->GetNavigationManager()->GetVisibleItem();
+
+  if (!item) {
+    StopCheckingProgress();
+    return;
+  }
+
+  GURL virtual_url = item->GetVirtualURL();
+  GURL loading_url = item->GetURL();
+  if (virtual_url.is_empty()) {

[Eugene But (OOO till 7-30), 2016/12/19 18:47:04]

nit: Do you want to move |virtual_url| creation closer to the same place?

[kkhorimoto, 2016/12/19 21:18:50]

NavigationItemImpl::GetVirtualURL() will return GetURL() if the virtual URL is
empty.  This codepath is never excuted.

[Olivier, 2016/12/20 12:29:47]

Done.

+    virtual_url = loading_url;
+  }
+  if (loading_url.SchemeIs(kChromeUIScheme) &&
+      loading_url.host() == kChromeUIOfflineHost) {
+    StopCheckingProgress();
+    return;
+  }
+  const ReadingListEntry* entry =
+      reading_list_model_->GetEntryByURL(virtual_url);
+  if (!entry || (entry->DistilledState() != ReadingListEntry::PROCESSED)) {
+    StopCheckingProgress();
+    return;
+  }
+  StartCheckingProgress(virtual_url);
 }
 
 void ReadingListWebStateObserver::PageLoaded(
     web::PageLoadCompletionStatus load_completion_status) {
-  if (load_completion_status == web::PageLoadCompletionStatus::SUCCESS &&
-      pending_url_.is_valid()) {
+  if (!reading_list_model_->loaded() || !web_state() ||
+      !pending_url_.is_valid()) {
+    StopCheckingProgress();
+    return;
+  }
+  web::NavigationItem* item =
+      web_state()->GetNavigationManager()->GetVisibleItem();
+  if (!item) {
+    StopCheckingProgress();
+    return;
+  }
+
+  if (load_completion_status == web::PageLoadCompletionStatus::SUCCESS) {
     reading_list_model_->SetReadStatus(pending_url_, true);
+  } else {
+    const ReadingListEntry* entry =
+        reading_list_model_->GetEntryByURL(pending_url_);
+    if (entry) {
+      LoadReadingListDistilled();
+    }
   }
   StopCheckingProgress();
 }
@@ -133,11 +179,31 @@ void ReadingListWebStateObserver::VerifyIfReadingListEntryStartedLoading() {
   double progress = web_state()->GetLoadingProgress();
   const double kMinimumExpectedProgressPerStep = 0.25;
   if (progress < try_number_ * kMinimumExpectedProgressPerStep) {
-    reading_list::LoadReadingListDistilled(*entry, reading_list_model_,
-                                           web_state());
+    LoadReadingListDistilled();
   }
   if (try_number_ >= 3) {
     // Loading reached 75%, let the page finish normal loading.
     timer_->Stop();
   }
 }
+
+void ReadingListWebStateObserver::LoadReadingListDistilled() {

[Eugene But (OOO till 7-30), 2016/12/19 18:47:05]

How about LoadDistilledReadingListEntry or LoadOfflineReadingListEntry (not sure
which is more important for this name offline or distilled)?

[Olivier, 2016/12/20 12:29:47]

Done.

+  const ReadingListEntry* entry =
+      reading_list_model_->GetEntryByURL(pending_url_);
+  DCHECK(entry->DistilledState() == ReadingListEntry::PROCESSED);
+  GURL url =
+      reading_list::DistilledURLForPath(entry->DistilledPath(), entry->URL());
+  web::NavigationItem* item =
+      web_state()->GetNavigationManager()->GetVisibleItem();
+  if (!item) {
+    return;
+  }
+  if (pending_url_.SchemeIs(kChromeUIScheme) &&
+      pending_url_.host() == kChromeUIOfflineHost) {
+    return;
+  }
+  item->SetURL(url);
+  item->SetVirtualURL(pending_url_);
+  web_state()->GetNavigationManager()->Reload(NO);

[Eugene But (OOO till 7-30), 2016/12/19 18:47:05]

s/NO/false

[Olivier, 2016/12/20 12:29:47]

Done.

+  reading_list_model_->SetReadStatus(entry->URL(), true);
+}