OpenURL post data handling.

content/browser/frame_host/navigator_impl.cc

Index: content/browser/frame_host/navigator_impl.cc
diff --git a/content/browser/frame_host/navigator_impl.cc b/content/browser/frame_host/navigator_impl.cc
index 97067f9cae66bdd261c92eb6275b6e5aa9084799..42d13ec19c1638ee6bbd644155ff18036a6a6ceb 100644
--- a/content/browser/frame_host/navigator_impl.cc
+++ b/content/browser/frame_host/navigator_impl.cc
@@ -644,13 +644,16 @@ bool NavigatorImpl::ShouldAssignSiteForURL(const GURL& url) {
   return GetContentClient()->browser()->ShouldAssignSiteForURL(url);
 }
 
-void NavigatorImpl::RequestOpenURL(RenderFrameHostImpl* render_frame_host,
-                                   const GURL& url,
-                                   SiteInstance* source_site_instance,
-                                   const Referrer& referrer,
-                                   WindowOpenDisposition disposition,
-                                   bool should_replace_current_entry,
-                                   bool user_gesture) {
+void NavigatorImpl::RequestOpenURL(
+    RenderFrameHostImpl* render_frame_host,
+    const GURL& url,
+    const std::string& method,
+    const scoped_refptr<ResourceRequestBody>& body,
+    SiteInstance* source_site_instance,
+    const Referrer& referrer,
+    WindowOpenDisposition disposition,
+    bool should_replace_current_entry,
+    bool user_gesture) {
   // Note: This can be called for subframes (even when OOPIFs are not possible)
   // if the disposition calls for a different window.
 
@@ -713,6 +716,23 @@ void NavigatorImpl::RequestOpenURL(RenderFrameHostImpl* render_frame_host,
     params.is_renderer_initiated = false;
   }
 
+  // TODO / DO NOT SUBMIT: Need to plumb through |method| and |body|.
+  //
+  // Option #1:
+  // - Add |method| and |body| fields to OpenURLParams
+  //   - OpenURLParams is //content's public API
+  //   - ResourceRequestBody (the type of |body| variable here) is currently
+  //     internal to //content, but maybe we can expose a small part of it via
+  //     //content's public API:
+  //     - expose: ResourceRequestBody::CreateFromVectorOfBytes
+  //     - expose: thread-safe-ref-counting of ResourceRequestBody
+  //     - hide: ResourceRequestBody::impl_ (unique pointer to RRBImpl)
+  //
+  // Option #2:
+  // - |delegate_| below is still an internal-to-//content NavigatorDelegate.
+  //   So maybe we can keep things internal somehow?  I don't see how, since
+  //   ultimately we will always call into //chrome (or other embedder
+  //   of //content).

[Łukasz Anforowicz, 2016/05/20 23:24:22]

WDYT?  Does option #1 (exposing a minimal ResourceRequestBody out of //content's
public API) make sense?  I am not sure what is Chromium's stance toward classes
that only have an "impl" pointer.

Is there some other option here that I am missing?

[Łukasz Anforowicz, 2016/05/23 13:43:18]

Actually, feel free to ignore the question - after sleeping on the problem, I
now see that I should just do this in a similar way to what //content already
does for RFH/RFHI:

//content/public/common/resource_request_body.h (new file):

class CONTENT_EXPORT ResourceRequestBody
    : public base::RefCountedThreadSafe<ResourceRequestBody> {
 public:
  scoped_refptr<ResourceRequestBody> CreateFromBytes(
      const std::vector<char> bytes);

  // no public API, except CreateFromBytes + ref-counting.

 protected:
  ResourceRequestBody();

 private:
  friend class base::RefCountedThreadSafe<ResourceRequestBody>;
  DISALLOW_COPY_AND_ASSIGN(ResourceRequestBody);  
} 

//content/common/resource_request_body_impl.h (renamed from the old
//content/common/resource_request_body.h):
 
class CONTENT_EXPORT ResourceRequestBodyImpl
    : public ResourceRequestBody,
      public base::SupportsUserData {
 // All the old declarations and members...
}

   if (delegate_)
     delegate_->RequestOpenURL(render_frame_host, params);
 }