[iOS Reading List] Add distilling WKWebView to the view hierarchy.

ios/chrome/browser/reading_list/reading_list_distiller_page.mm

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
@@ skipping 54 matching lines @@
   if (old_web_state) {
     web_state_dispatcher_->ReturnWebState(std::move(old_web_state));
   }
   std::unique_ptr<web::WebState> new_web_state =
       web_state_dispatcher_->RequestWebState();
   AttachWebState(std::move(new_web_state));
   original_url_ = url;
   FetchFavicon(url);
 
   DistillerPageIOS::DistillPageImpl(url, script);
+
+  // Some pages require that the WKWebView is in the view hierarchy to be

[marq (ping after 24h), 2017/02/16 14:28:18]

I don't suppose there's any way to determine which pages these are, so we can
avoid this where possible?

[Olivier, 2017/02/16 14:43:41]

I don't think we can.
But we can do it only on second try?

[wychen, 2017/02/16 16:38:59]

You mean if it distills nothing, then distill it again with this trick again? If
the performance impact of putting WebView in the view hierarchy is not too high,
it might not worth the trouble. Distillation itself alone takes ~0.3 seconds.

[Olivier, 2017/02/16 20:02:27]

We always try multiple time to distill as some network issues can prevent
distillation on first try.
We could do this work around only starting on second try.

[wychen, 2017/02/17 00:59:55]

Makes sense. Thanks!

+  // rendered.
+  // Add the view and move it out of the screen.
+  CGRect frame = [[[UIApplication sharedApplication] keyWindow] frame];
+  frame.origin.x = 2 * std::max(frame.size.width, frame.size.height);

[stkhapugin, 2017/02/16 12:53:59]

2 can be too small for iPad multitasking: if you start in slideover portrait
(window width ~= 1/3 of screen width) and switch to full screen, it might be
visible? Not 100% sure if autolayout won't fix it for you by pushing it further
offscreen though. 

I don't have a preference of separate window vs offscreen: in a separate window
there might be a bug in the future where WKWebView doesn't render in it, and
same goes for offscreen. Since we've been using this trick for prerendering,
maybe it's logical to just keep using it. 

[Olivier, 2017/02/16 13:04:18]

I don't know if it prerendering is still done that way.
I will check that

[marq (ping after 24h), 2017/02/16 14:28:18]

It might be safer to position the offscreen view in the upper left quadrant of
the coordinate space, so even if the window expands, it will do so down and to
the right relative to the webview.

[Olivier, 2017/02/16 14:43:41]

Done.

+  [CurrentWebState()->GetView() setFrame:frame];

[marq (ping after 24h), 2017/02/16 14:28:18]

DCHECK that the webstate's view doesn't already have a superview.

[Olivier, 2017/02/16 14:43:41]

Done.

+  [[[UIApplication sharedApplication] keyWindow]
+      addSubview:CurrentWebState()->GetView()];

[marq (ping after 24h), 2017/02/16 14:28:18]

Maybe insert at index 0, so it's behind everything else?

[Olivier, 2017/02/16 14:43:40]

Done.

 }
 
 void ReadingListDistillerPage::FetchFavicon(const GURL& page_url) {
   if (!CurrentWebState() || !page_url.is_valid()) {
     return;
   }
   favicon::WebFaviconDriver* favicon_driver =
       favicon::WebFaviconDriver::FromWebState(CurrentWebState());
   DCHECK(favicon_driver);
   favicon_driver->FetchFavicon(page_url);
 }
 
 void ReadingListDistillerPage::OnDistillationDone(const GURL& page_url,
                                                   const base::Value* value) {
   std::unique_ptr<web::WebState> old_web_state = DetachWebState();
   if (old_web_state) {
+    [old_web_state->GetView() removeFromSuperview];
     web_state_dispatcher_->ReturnWebState(std::move(old_web_state));
   }
   DistillerPageIOS::OnDistillationDone(page_url, value);
 }
 
 bool ReadingListDistillerPage::IsLoadingSuccess(
     web::PageLoadCompletionStatus load_completion_status) {
   if (load_completion_status != web::PageLoadCompletionStatus::SUCCESS) {
     return false;
   }
@@ skipping 126 matching lines @@
   if (!new_gurl.is_valid()) {
     return false;
   }
   FetchFavicon(new_gurl);
   web::NavigationManager::WebLoadParams params(new_gurl);
   CurrentWebState()->GetNavigationManager()->LoadURLWithParams(params);
   return true;
 }
 
 }  // namespace reading_list