PlzNavigate: Get StreamPrivate API to work.

extensions/browser/guest_view/mime_handler_view/mime_handler_stream_manager.cc

Index: extensions/browser/guest_view/mime_handler_view/mime_handler_stream_manager.cc
diff --git a/extensions/browser/guest_view/mime_handler_view/mime_handler_stream_manager.cc b/extensions/browser/guest_view/mime_handler_view/mime_handler_stream_manager.cc
index 78b76340f07594e71f6b6e7d9303300e5cf9cdd8..164d216a0d0c01772ac9e7034465ea3f15fa3fdf 100644
--- a/extensions/browser/guest_view/mime_handler_view/mime_handler_stream_manager.cc
+++ b/extensions/browser/guest_view/mime_handler_view/mime_handler_stream_manager.cc
@@ -8,8 +8,10 @@
 #include "base/memory/singleton.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
+#include "content/public/common/browser_side_navigation_policy.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extensions_browser_client.h"
 #include "extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h"
@@ -72,19 +74,18 @@ class MimeHandlerStreamManager::EmbedderObserver
     : public content::WebContentsObserver {
  public:
   EmbedderObserver(MimeHandlerStreamManager* stream_manager,
+                   const std::string& view_id,
+                   content::WebContents* web_contents,
+                   int frame_tree_node_id,
                    int render_process_id,
-                   int render_frame_id,
-                   const std::string& view_id);
+                   int render_frame_id);
 
  private:
   // WebContentsObserver overrides.
   void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
   void RenderProcessGone(base::TerminationStatus status) override;
-  void DidStartProvisionalLoadForFrame(
-      content::RenderFrameHost* render_frame_host,
-      const GURL& validated_url,
-      bool is_error_page,
-      bool is_iframe_srcdoc) override;
+  void DidStartNavigation(
+      content::NavigationHandle* navigation_handle) override;
   void WebContentsDestroyed() override;
 
   void AbortStream();
@@ -92,9 +93,10 @@ class MimeHandlerStreamManager::EmbedderObserver
   bool IsTrackedRenderFrameHost(content::RenderFrameHost* render_frame_host);
 
   MimeHandlerStreamManager* const stream_manager_;
+  const std::string view_id_;
+  const int frame_tree_node_id_;
   const int render_process_id_;
   const int render_frame_id_;
-  const std::string view_id_;
 };
 
 MimeHandlerStreamManager::MimeHandlerStreamManager()
@@ -113,13 +115,16 @@ MimeHandlerStreamManager* MimeHandlerStreamManager::Get(
 void MimeHandlerStreamManager::AddStream(
     const std::string& view_id,
     std::unique_ptr<StreamContainer> stream,
+    content::WebContents* web_contents,
+    int frame_tree_node_id,
     int render_process_id,
     int render_frame_id) {
   streams_by_extension_id_[stream->extension_id()].insert(view_id);
   auto result = streams_.insert(std::make_pair(view_id, std::move(stream)));
   DCHECK(result.second);
   embedder_observers_[view_id] = base::MakeUnique<EmbedderObserver>(
-      this, render_process_id, render_frame_id, view_id);
+      this, view_id, web_contents, frame_tree_node_id, render_process_id,
+      render_frame_id);
 }
 
 std::unique_ptr<StreamContainer> MimeHandlerStreamManager::ReleaseStream(
@@ -153,23 +158,26 @@ void MimeHandlerStreamManager::OnExtensionUnloaded(
 
 MimeHandlerStreamManager::EmbedderObserver::EmbedderObserver(
     MimeHandlerStreamManager* stream_manager,
+    const std::string& view_id,
+    content::WebContents* web_contents,
+    int frame_tree_node_id,
     int render_process_id,
-    int render_frame_id,
-    const std::string& view_id)
-    : WebContentsObserver(content::WebContents::FromRenderFrameHost(
-          content::RenderFrameHost::FromID(render_process_id,
-                                           render_frame_id))),
+    int render_frame_id)
+    : WebContentsObserver(web_contents),
       stream_manager_(stream_manager),
+      view_id_(view_id),
+      frame_tree_node_id_(frame_tree_node_id),
       render_process_id_(render_process_id),
-      render_frame_id_(render_frame_id),
-      view_id_(view_id) {
-}
+      render_frame_id_(render_frame_id) {}
 
 void MimeHandlerStreamManager::EmbedderObserver::RenderFrameDeleted(
     content::RenderFrameHost* render_frame_host) {
-  if (!IsTrackedRenderFrameHost(render_frame_host))
+  // For PlzNavigate, we only cleanup when the renderer process goes away or
+  // if the main frame or the child frame initiates a new navigation.
+  if (content::IsBrowserSideNavigationEnabled() ||

[lazyboy, 2016/09/17 00:59:01]

Q: Isn't it better to keep this as is? That way,
node.removeChild(iframe_containing_pdf) will continue to work gracefully.

[ananta, 2016/09/17 02:06:07]

Thanks. Done. I made a change to RenderFrameHostImpl to not return the
FrameTreeNode's id if the RenderFrameHost got swapped with another host. This
causes us to inadvarently clean up the stream which causes the first load of PDF
to fail.

Scenario is as below:
1. A RenderFrameHostImpl instance is created for the PDF navigation.
2. We setup the stream with the frame tree node id etc. Assume it is 1
3. Another RenderFrameHostImpl instance is created and the instance created in
step 1 gets swapped out. Now the FrameTreeNode is associated with a different
instance. We want the stream to remain valid in this case.

+      !IsTrackedRenderFrameHost(render_frame_host)) {
     return;
-
+  }
   AbortStream();
 }
 
@@ -177,15 +185,15 @@ void MimeHandlerStreamManager::EmbedderObserver::RenderProcessGone(
     base::TerminationStatus status) {
   AbortStream();
 }
-void MimeHandlerStreamManager::EmbedderObserver::
-    DidStartProvisionalLoadForFrame(content::RenderFrameHost* render_frame_host,
-                                    const GURL& validated_url,
-                                    bool is_error_page,
-                                    bool is_iframe_srcdoc) {
-  if (!IsTrackedRenderFrameHost(render_frame_host))
-    return;
 
-  AbortStream();
+void MimeHandlerStreamManager::EmbedderObserver::DidStartNavigation(
+    content::NavigationHandle* navigation_handle) {
+  // If the top level frame or the associated child frame is navigating away
+  // cleanup the stream.
+  if (navigation_handle->IsInMainFrame() ||
+      IsTrackedRenderFrameHost(navigation_handle->GetRenderFrameHost())) {
+    AbortStream();
+  }
 }
 
 void MimeHandlerStreamManager::EmbedderObserver::WebContentsDestroyed() {
@@ -200,8 +208,13 @@ void MimeHandlerStreamManager::EmbedderObserver::AbortStream() {
 
 bool MimeHandlerStreamManager::EmbedderObserver::IsTrackedRenderFrameHost(
     content::RenderFrameHost* render_frame_host) {
-  return render_frame_host->GetRoutingID() == render_frame_id_ &&
-         render_frame_host->GetProcess()->GetID() == render_process_id_;
+  if (frame_tree_node_id_ != -1) {

[lazyboy, 2016/09/17 00:59:01]

s/-1/FrameTreeNode::FrameTreeNodeInvalidId

[ananta, 2016/09/17 02:06:07]

Can't do that as that constant is defined in the FrameTreeNode class which is
not in content public

+    return render_frame_host->GetFrameTreeNodeId() == frame_tree_node_id_;
+  } else {
+    DCHECK((render_frame_id_ != -1) && (render_process_id_ != -1));
+    return render_frame_host->GetRoutingID() == render_frame_id_ &&
+           render_frame_host->GetProcess()->GetID() == render_process_id_;
+  }
 }
 
 }  // namespace extensions