IOS Reading distillation: Handle Google AMP iFrame.

ios/chrome/browser/reading_list/reading_list_distiller_page.mm

Index: ios/chrome/browser/reading_list/reading_list_distiller_page.mm
diff --git a/ios/chrome/browser/reading_list/reading_list_distiller_page.mm b/ios/chrome/browser/reading_list/reading_list_distiller_page.mm
index 9384226ee501fa5a50e0d9b3cd893b7dd88437b5..2b3a231f1fe5539fda5447189b8c9cd397ba8b8a 100644
--- a/ios/chrome/browser/reading_list/reading_list_distiller_page.mm
+++ b/ios/chrome/browser/reading_list/reading_list_distiller_page.mm
@@ -5,14 +5,26 @@
 #include "ios/chrome/browser/reading_list/reading_list_distiller_page.h"
 
 #include "base/bind.h"
+#include "base/mac/foundation_util.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/favicon/ios/web_favicon_driver.h"
+#include "components/google/core/browser/google_util.h"
 #include "ios/chrome/browser/reading_list/favicon_web_state_dispatcher_impl.h"
+#import "ios/web/public/navigation_item.h"
+#import "ios/web/public/navigation_manager.h"
+#include "ios/web/public/ssl_status.h"
+#import "ios/web/public/web_state/js/crw_js_injection_receiver.h"
 #import "ios/web/public/web_state/web_state.h"
+#import "net/base/mac/url_conversions.h"
+#include "net/cert/cert_status_flags.h"
+#include "url/url_constants.h"
 
 namespace {
 // The delay between the page load and the distillation in seconds.
 const int64_t kDistillationDelayInSeconds = 2;
+const char* kGetIframeURLJavaScript =
+    "document.getElementsByTagName('iframe')[0].src;";
 }  // namespace
 
 namespace reading_list {
@@ -55,20 +67,101 @@ void ReadingListDistillerPage::OnDistillationDone(const GURL& page_url,
 
 void ReadingListDistillerPage::OnLoadURLDone(
     web::PageLoadCompletionStatus load_completion_status) {
-  if (load_completion_status == web::PageLoadCompletionStatus::FAILURE) {
+  if (load_completion_status != web::PageLoadCompletionStatus::SUCCESS) {

[noyau (Ping after 24h), 2017/01/18 16:19:37]

Can you explain this change a bit?

[Olivier, 2017/01/18 17:04:25]

At the moment, there is only two values, SUCCESS and FAILURE.
But jif pointed out this is more conservative if a third choice is added.
We only want to continue in case of a success anyway.

     DistillerPageIOS::OnLoadURLDone(load_completion_status);
     return;
   }
+  if (IsGoogleCachedAMPPage()) {
+    // Workaround for Google AMP pages.
+    HandleGoogleCachedAMPPage();
+  } else {
+    WaitForPageLoadCompletion();
+  }
+}
+
+void ReadingListDistillerPage::WaitForPageLoadCompletion() {
   base::WeakPtr<ReadingListDistillerPage> weak_this =
       weak_ptr_factory_.GetWeakPtr();
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE, base::Bind(&ReadingListDistillerPage::DelayedOnLoadURLDone,
-                            weak_this, load_completion_status),
+      FROM_HERE,
+      base::Bind(&ReadingListDistillerPage::DelayedOnLoadURLDone, weak_this),
       base::TimeDelta::FromSeconds(kDistillationDelayInSeconds));
 }
 
-void ReadingListDistillerPage::DelayedOnLoadURLDone(
-    web::PageLoadCompletionStatus load_completion_status) {
-  DistillerPageIOS::OnLoadURLDone(load_completion_status);
+void ReadingListDistillerPage::DelayedOnLoadURLDone() {
+  DistillerPageIOS::OnLoadURLDone(web::PageLoadCompletionStatus::SUCCESS);
+}
+
+bool ReadingListDistillerPage::IsGoogleCachedAMPPage() {
+  // All google AMP pages have URL in the form "https://google_domain/amp/..."
+  // and a valid certificate.
+  // This method checks that this is strictly the case.
+  const GURL& url = CurrentWebState()->GetLastCommittedURL();
+  if (!url.is_valid() || !url.SchemeIs(url::kHttpsScheme)) {
+    return false;
+  }
+  if (!google_util::IsGoogleDomainUrl(
+          url, google_util::DISALLOW_SUBDOMAIN,
+          google_util::DISALLOW_NON_STANDARD_PORTS) ||
+      !url.path().compare(0, 4, "amp/")) {
+    return false;
+  }
+  const web::SSLStatus& ssl_status = CurrentWebState()
+                                         ->GetNavigationManager()
+                                         ->GetLastCommittedItem()
+                                         ->GetSSL();
+  if (!ssl_status.certificate ||
+      (net::IsCertStatusError(ssl_status.cert_status) &&
+       !net::IsCertStatusMinorError(ssl_status.cert_status))) {
+    return false;
+  }
+
+  return true;
+}
+
+void ReadingListDistillerPage::HandleGoogleCachedAMPPage() {
+  base::WeakPtr<ReadingListDistillerPage> weak_this =
+      weak_ptr_factory_.GetWeakPtr();
+  [CurrentWebState()->GetJSInjectionReceiver()
+      executeJavaScript:@(kGetIframeURLJavaScript)
+      completionHandler:^(id result, NSError* error) {
+        if (weak_this &&
+            !weak_this->HandleGoogleCachedAMPPageJavaScriptResult(result,
+                                                                  error)) {
+          // If there is an error on navigation, continue normal distillation.
+          weak_this->WaitForPageLoadCompletion();

[noyau (Ping after 24h), 2017/01/18 16:19:37]

When I see a weak_this I always wonder if it could be deallocated between the
test if(weak_this) and here...

[Olivier, 2017/01/18 17:04:25]

Both this block and the code destroying this object are on main thread, so it
should not happen...

+        }
+        // If there is no error, the navigation completion will trigger a new
+        // |OnLoadURLDone| call that will resume the distillation.
+      }];
 }
+
+bool ReadingListDistillerPage::HandleGoogleCachedAMPPageJavaScriptResult(
+    id result,
+    NSError* error) {
+  if (error) {
+    return false;
+  }
+  NSString* result_string = base::mac::ObjCCast<NSString>(result);
+  NSURL* new_url = [NSURL URLWithString:result_string];
+  if (!new_url) {
+    return false;
+  }
+  bool is_cdn_ampproject =
+      [[new_url host] isEqualToString:@"cdn.ampproject.org"];
+  bool is_cdn_ampproject_subdomain =
+      [[new_url host] hasSuffix:@".cdn.ampproject.org"];
+
+  if (!is_cdn_ampproject && !is_cdn_ampproject_subdomain) {
+    return false;
+  }
+  GURL new_gurl = net::GURLWithNSURL(new_url);
+  if (!new_gurl.is_valid()) {
+    return false;
+  }
+  web::NavigationManager::WebLoadParams params(new_gurl);
+  CurrentWebState()->GetNavigationManager()->LoadURLWithParams(params);
+  return true;
+}
+
 }  // namespace reading_list