To fix the cross-site post submission bug.

content/browser/web_contents/navigation_entry_impl.h

 // Copyright (c) 2012 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.
 
 #ifndef CONTENT_BROWSER_WEB_CONTENTS_NAVIGATION_ENTRY_IMPL_H_
 #define CONTENT_BROWSER_WEB_CONTENTS_NAVIGATION_ENTRY_IMPL_H_
 
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/public/browser/favicon_status.h"
 #include "content/public/browser/global_request_id.h"
 #include "content/public/browser/navigation_entry.h"
+#include "content/public/common/frame_navigate_params.h"
 #include "content/public/common/ssl_status.h"
 
 namespace content {
+struct ViewMsg_Request;

[michaeln, 2012/11/02 00:08:23]

since this struct is defined in frame_navigate_params.h, is this forward decl
needed?

[irobert, 2012/11/02 17:22:55]

Done.

 
 class CONTENT_EXPORT NavigationEntryImpl
     : public NON_EXPORTED_BASE(NavigationEntry) {
  public:
   static NavigationEntryImpl* FromNavigationEntry(NavigationEntry* entry);
 
   NavigationEntryImpl();
   NavigationEntryImpl(SiteInstanceImpl* instance,
                       int page_id,
                       const GURL& url,
@@ skipping 132 matching lines @@
 
   // Whether this (pending) navigation is reload across site instances.
   // Resets to false after commit.
   void set_is_cross_site_reload(bool is_cross_site_reload) {
     is_cross_site_reload_ = is_cross_site_reload;
   }
   bool is_cross_site_reload() const {
     return is_cross_site_reload_;
   }
 
+  // request
+  ViewMsg_Request request;

[michaeln, 2012/11/02 00:08:23]

nits: we put trailing underbars on data members_ and for classes prefer data
members to be private

[michaeln, 2012/11/02 00:08:23]

Also, ditto comments about overlap with the existing structure.

[irobert, 2012/11/02 17:22:55]

Done.

+
  private:
   // WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
   // Session/Tab restore save portions of this class so that it can be recreated
   // later. If you add a new field that needs to be persisted you'll have to
   // update SessionService/TabRestoreService appropriately.
   // WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
 
   // See the accessors above for descriptions.
   int unique_id_;
   scoped_refptr<SiteInstanceImpl> site_instance_;
@@ skipping 13 matching lines @@
   int64 post_id_;
   RestoreType restore_type_;
   GURL original_request_url_;
   bool is_overriding_user_agent_;
   base::Time timestamp_;
 
   // This member is not persisted with session restore because it is transient.
   // If the post request succeeds, this field is cleared since the same
   // information is stored in |content_state_| above. It is also only shallow
   // copied with compiler provided copy constructor.
   scoped_refptr<const base::RefCountedMemory> browser_initiated_post_data_;

[michaeln, 2012/11/02 00:08:23]

I think you may want to harmonize your new more expressive POST handling with
this older more basic POST data handling. Having two means of handling POSTs in
parallel with one another will leave folks wondering which one to use and why?
Maybe...

scoped_refptr<const base::RefCountedMemory> browser_initiated_post_data_;
  vs
scoped_refptr<const T> content_initiated_cross_site_post_data_;

Also, how closely have you reviewed the browser_initiated_post_data handling?
The details about when this field is cleared and how it interacts with session
restore seem relevant to your new crosssite form posting case.

[irobert, 2012/11/02 17:22:55]

We also reviewed the browser_initiated_post_data, but this field only exist in
the params structs which pass data from browser to render process. For the
structs which pass data from render process to browser (e.g OpenURLParams,
NavigateParams), there is no such a field. I don't know which one,
scoped_refptr<const base::RefCountedMemory> or scoped_refptr<const T>, is
faster. If scoped_refptr<const base::RefCountedMemory> is faster, you are right,
we need to stick to this field to pass requestbody from render process to
browser process.

On 2012/11/02 00:08:23, michaeln wrote:

[Charlie Reis, 2012/11/05 16:21:40]

The browser_initiated_post_data_ stuff is for Android Webviews, allowing the
embedder app to generate POST submissions.

I agree with the idea of unifying the two approaches, but since Robert's
approach must handle more types of submissions, it probably makes sense to
switch the existing code to the new approach (whatever it ends up being).

 
   // This member is not persisted with session restore.
   std::string extra_headers_;
 
   // Used for specifying base URL for pages loaded via data URLs. Not persisted.
   GURL base_url_for_data_url_;
 
   // Whether the entry, while loading, was created for a renderer-initiated
   // navigation.  This dictates whether the URL should be displayed before the
   // navigation commits.  It is cleared on commit and not persisted.
@@ skipping 24 matching lines @@
   // Set when this entry should be able to access local file:// resources. This
   // value is not needed after the entry commits and is not persisted.
   bool can_load_local_resources_;
 
   // Copy and assignment is explicitly allowed for this class.
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_WEB_CONTENTS_NAVIGATION_ENTRY_IMPL_H_