[android_webview] onPageStarted should happen after shouldIgnoreNavigation.

The AwContentsClient.onPageStarted callback should never be issued before
the shouldIgnoreNavigation callback.


BUG=154292


Review URL: https://chromiumcodereview.appspot.com/11269042

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@167938 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 101 insertions, 23 deletions

diff --git a/content/components/navigation_interception/intercept_navigation_delegate.cc b/content/components/navigation_interception/intercept_navigation_delegate.cc
index 6609a36..0eb25f78 100644
--- a/content/components/navigation_interception/intercept_navigation_delegate.cc
+++ b/content/components/navigation_interception/intercept_navigation_delegate.cc
@@ -31,6 +31,7 @@ const void* kInterceptNavigationDelegateUserDataKey =
 bool CheckIfShouldIgnoreNavigationOnUIThread(RenderViewHost* source,
                                              const GURL& url,
                                              const content::Referrer& referrer,
+                                             bool is_post,
                                              bool has_user_gesture) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(source);
@@ -44,7 +45,7 @@ bool CheckIfShouldIgnoreNavigationOnUIThread(RenderViewHost* source,
     return false;
 
   return intercept_navigation_delegate->ShouldIgnoreNavigation(
-      url, has_user_gesture);
+      url, is_post, has_user_gesture);
 }
 
 } // namespace
@@ -81,6 +82,7 @@ InterceptNavigationDelegate::~InterceptNavigationDelegate() {
 
 bool InterceptNavigationDelegate::ShouldIgnoreNavigation(
     const GURL& url,
+    bool is_post,
     bool has_user_gesture) {
   if (!url.is_valid())
     return false;
@@ -97,6 +99,7 @@ bool InterceptNavigationDelegate::ShouldIgnoreNavigation(
       env,
       jdelegate.obj(),
       jstring_url.obj(),
+      is_post,
       has_user_gesture);
 }
 
diff --git a/content/components/navigation_interception/intercept_navigation_delegate.h b/content/components/navigation_interception/intercept_navigation_delegate.h
index 94f6582..a80efa3 100644
--- a/content/components/navigation_interception/intercept_navigation_delegate.h
+++ b/content/components/navigation_interception/intercept_navigation_delegate.h
@@ -54,6 +54,7 @@ class InterceptNavigationDelegate : public base::SupportsUserData::Data {
       net::URLRequest* request);
 
   virtual bool ShouldIgnoreNavigation(const GURL& url,
+                                      bool is_post,
                                       bool has_user_gesture);
  private:
   JavaObjectWeakGlobalRef weak_jdelegate_;
diff --git a/content/components/navigation_interception/intercept_navigation_resource_throttle.cc b/content/components/navigation_interception/intercept_navigation_resource_throttle.cc
index 579d373..894fb0c 100644
--- a/content/components/navigation_interception/intercept_navigation_resource_throttle.cc
+++ b/content/components/navigation_interception/intercept_navigation_resource_throttle.cc
@@ -23,26 +23,35 @@ namespace content {
 
 namespace {
 
+struct ShouldIgnoreCallbackParams {
+    int render_process_id;
+    int render_view_id;
+    GURL url;
+    Referrer referrer;
+    bool has_user_gesture;
+    bool is_post;
+};
+
 void CheckIfShouldIgnoreNavigationOnUIThread(
-    int render_process_id,
-    int render_view_id,
-    const GURL& url,
-    const Referrer& referrer,
-    bool has_user_gesture,
+    const ShouldIgnoreCallbackParams& params,
     InterceptNavigationResourceThrottle::CheckOnUIThreadCallback
-        should_ignore_callback,
+    should_ignore_callback,
     base::Callback<void(bool)> callback) {
 
   bool should_ignore_navigation = false;
   RenderViewHost* rvh =
-      RenderViewHost::FromID(render_process_id, render_view_id);
+      RenderViewHost::FromID(params.render_process_id, params.render_view_id);
 
   if (rvh) {
-    GURL validated_url(url);
+    GURL validated_url(params.url);
     RenderViewHost::FilterURL(rvh->GetProcess(), false, &validated_url);
 
     should_ignore_navigation = should_ignore_callback.Run(
-        rvh, validated_url, referrer, has_user_gesture);
+        rvh,
+        validated_url,
+        params.referrer,
+        params.is_post,
+        params.has_user_gesture);
   }
 
   BrowserThread::PostTask(
@@ -88,16 +97,21 @@ bool InterceptNavigationResourceThrottle::CheckIfShouldIgnoreNavigation(
   // This class should only be instantiated for top level frame requests.
   DCHECK(info->IsMainFrame());
 
+  ShouldIgnoreCallbackParams params;
+  params.render_process_id = render_process_id;
+  params.render_view_id = render_view_id;
+  params.url = url;
+  params.referrer = Referrer(GURL(request_->referrer()),
+                             info->GetReferrerPolicy());
+  params.has_user_gesture = info->HasUserGesture();
+  params.is_post = request_->method() == "POST";
+
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
       base::Bind(
           &CheckIfShouldIgnoreNavigationOnUIThread,
-          render_process_id,
-          render_view_id,
-          url,
-          Referrer(GURL(request_->referrer()), info->GetReferrerPolicy()),
-          info->HasUserGesture(),
+          params,
           should_ignore_callback_,
           base::Bind(
               &InterceptNavigationResourceThrottle::OnResultObtained,
diff --git a/content/components/navigation_interception/intercept_navigation_resource_throttle.h b/content/components/navigation_interception/intercept_navigation_resource_throttle.h
index 742f696..90d7883 100644
--- a/content/components/navigation_interception/intercept_navigation_resource_throttle.h
+++ b/content/components/navigation_interception/intercept_navigation_resource_throttle.h
@@ -31,6 +31,7 @@ class InterceptNavigationResourceThrottle : public content::ResourceThrottle {
   typedef base::Callback<bool(content::RenderViewHost* /* source */,
                               const GURL& /*url*/,
                               const content::Referrer& /*referrer*/,
+                              bool /*is_post*/,
                               bool /*has_user_gesture*/)>
       CheckOnUIThreadCallback;
 
diff --git a/content/components/navigation_interception/intercept_navigation_resource_throttle_unittest.cc b/content/components/navigation_interception/intercept_navigation_resource_throttle_unittest.cc
index ccbb3a6..d7cf936 100644
--- a/content/components/navigation_interception/intercept_navigation_resource_throttle_unittest.cc
+++ b/content/components/navigation_interception/intercept_navigation_resource_throttle_unittest.cc
@@ -45,9 +45,10 @@ void ContinueTestCase() {
 
 class MockInterceptCallbackReceiver {
  public:
-  MOCK_METHOD4(ShouldIgnoreNavigation, bool(RenderViewHost* source,
+  MOCK_METHOD5(ShouldIgnoreNavigation, bool(RenderViewHost* source,
                                             const GURL& url,
                                             const content::Referrer& referrer,
+                                            bool is_post,
                                             bool has_user_gesture));
 };
 
@@ -93,6 +94,7 @@ class MockResourceController
 class TestIOThreadState {
  public:
   TestIOThreadState(const GURL& url, int render_process_id, int render_view_id,
+                    const std::string& request_method,
                     MockInterceptCallbackReceiver* callback_receiver)
       : request_(url, NULL, resource_context_.GetRequestContext()),
         throttle_(NULL) {
@@ -110,6 +112,7 @@ class TestIOThreadState {
           base::Bind(&MockInterceptCallbackReceiver::ShouldIgnoreNavigation,
                      base::Unretained(callback_receiver))));
       throttle_->set_controller_for_testing(&throttle_controller_);
+      request_.set_method(request_method);
   }
 
   void ThrottleWillStartRequest(bool* defer) {
@@ -170,13 +173,14 @@ class InterceptNavigationResourceThrottleTest
 
   void RunThrottleWillStartRequestOnIOThread(
       const GURL& url,
+      const std::string& request_method,
       int render_process_id,
       int render_view_id,
       bool* defer) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
     TestIOThreadState* io_thread_state =
         new TestIOThreadState(url, render_process_id, render_view_id,
-                              mock_callback_receiver_.get());
+                              request_method, mock_callback_receiver_.get());
 
     SetIOThreadState(io_thread_state);
     io_thread_state->ThrottleWillStartRequest(defer);
@@ -197,10 +201,10 @@ class InterceptNavigationResourceThrottleTest
       bool* defer) {
 
     ON_CALL(*mock_callback_receiver_,
-            ShouldIgnoreNavigation(_, _, _, _))
+            ShouldIgnoreNavigation(_, _, _, _, _))
       .WillByDefault(Return(callback_action == IgnoreNavigation));
     EXPECT_CALL(*mock_callback_receiver_,
-                ShouldIgnoreNavigation(rvh(), Eq(GURL(kTestUrl)), _, _))
+                ShouldIgnoreNavigation(rvh(), Eq(GURL(kTestUrl)), _, _, _))
       .Times(1);
 
     BrowserThread::PostTask(
@@ -211,6 +215,7 @@ class InterceptNavigationResourceThrottleTest
                 RunThrottleWillStartRequestOnIOThread,
             base::Unretained(this),
             GURL(kTestUrl),
+            "GET",
             web_contents()->GetRenderViewHost()->GetProcess()->GetID(),
             web_contents()->GetRenderViewHost()->GetRoutingID(),
             base::Unretained(defer)));
@@ -271,7 +276,7 @@ TEST_F(InterceptNavigationResourceThrottleTest,
           base::Unretained(this)));
 
   EXPECT_CALL(*mock_callback_receiver_,
-              ShouldIgnoreNavigation(_, _, _, _))
+              ShouldIgnoreNavigation(_, _, _, _, _))
       .Times(0);
 
   BrowserThread::PostTask(
@@ -282,6 +287,7 @@ TEST_F(InterceptNavigationResourceThrottleTest,
           RunThrottleWillStartRequestOnIOThread,
           base::Unretained(this),
           GURL(kTestUrl),
+          "GET",
           web_contents()->GetRenderViewHost()->GetProcess()->GetID(),
           web_contents()->GetRenderViewHost()->GetRoutingID(),
           base::Unretained(&defer)));
@@ -309,6 +315,7 @@ TEST_F(InterceptNavigationResourceThrottleTest,
               RunThrottleWillStartRequestOnIOThread,
           base::Unretained(this),
           GURL(kTestUrl),
+          "GET",
           MSG_ROUTING_NONE,
           MSG_ROUTING_NONE,
           base::Unretained(&defer)));
@@ -324,10 +331,10 @@ TEST_F(InterceptNavigationResourceThrottleTest,
   bool defer = false;
 
   ON_CALL(*mock_callback_receiver_,
-          ShouldIgnoreNavigation(_, Ne(GURL(kUnsafeTestUrl)), _, _))
+          ShouldIgnoreNavigation(_, Ne(GURL(kUnsafeTestUrl)), _, _, _))
       .WillByDefault(Return(false));
   EXPECT_CALL(*mock_callback_receiver_,
-              ShouldIgnoreNavigation(_, Ne(GURL(kUnsafeTestUrl)), _, _))
+              ShouldIgnoreNavigation(_, Ne(GURL(kUnsafeTestUrl)), _, _, _))
       .Times(1);
 
   BrowserThread::PostTask(
@@ -338,6 +345,57 @@ TEST_F(InterceptNavigationResourceThrottleTest,
               RunThrottleWillStartRequestOnIOThread,
           base::Unretained(this),
           GURL(kUnsafeTestUrl),
+          "GET",
+          web_contents()->GetRenderViewHost()->GetProcess()->GetID(),
+          web_contents()->GetRenderViewHost()->GetRoutingID(),
+          base::Unretained(&defer)));
+
+  // Wait for the request to finish processing.
+  message_loop_.Run();
+}
+
+TEST_F(InterceptNavigationResourceThrottleTest,
+       CallbackIsPostFalseForGet) {
+  bool defer = false;
+
+  EXPECT_CALL(*mock_callback_receiver_,
+              ShouldIgnoreNavigation(_, Ne(GURL(kUnsafeTestUrl)), _, false, _))
+      .WillOnce(Return(false));
+
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(
+          &InterceptNavigationResourceThrottleTest::
+              RunThrottleWillStartRequestOnIOThread,
+          base::Unretained(this),
+          GURL(kTestUrl),
+          "GET",
+          web_contents()->GetRenderViewHost()->GetProcess()->GetID(),
+          web_contents()->GetRenderViewHost()->GetRoutingID(),
+          base::Unretained(&defer)));
+
+  // Wait for the request to finish processing.
+  message_loop_.Run();
+}
+
+TEST_F(InterceptNavigationResourceThrottleTest,
+       CallbackIsPostTrueForPost) {
+  bool defer = false;
+
+  EXPECT_CALL(*mock_callback_receiver_,
+              ShouldIgnoreNavigation(_, Ne(GURL(kUnsafeTestUrl)), _, true, _))
+      .WillOnce(Return(false));
+
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(
+          &InterceptNavigationResourceThrottleTest::
+              RunThrottleWillStartRequestOnIOThread,
+          base::Unretained(this),
+          GURL(kTestUrl),
+          "POST",
           web_contents()->GetRenderViewHost()->GetProcess()->GetID(),
           web_contents()->GetRenderViewHost()->GetRoutingID(),
           base::Unretained(&defer)));
diff --git a/content/components/navigation_interception/java/src/org/chromium/content/components/navigation_interception/InterceptNavigationDelegate.java b/content/components/navigation_interception/java/src/org/chromium/content/components/navigation_interception/InterceptNavigationDelegate.java
index a14281f..c95e72c 100644
--- a/content/components/navigation_interception/java/src/org/chromium/content/components/navigation_interception/InterceptNavigationDelegate.java
+++ b/content/components/navigation_interception/java/src/org/chromium/content/components/navigation_interception/InterceptNavigationDelegate.java
@@ -13,9 +13,10 @@ public interface InterceptNavigationDelegate {
      * certain navigations to Intents to 3rd party applications.
      *
      * @param url the target url of the navigation.
+     * @param isPost true if the the navigation method is "POST".
      * @param isUserGestrue true if the navigation was initiated by the user.
      * @return true if the navigation should be ignored.
      */
     @CalledByNative
-    boolean shouldIgnoreNavigation(String url, boolean isUserGestrue);
+    boolean shouldIgnoreNavigation(String url, boolean isPost, boolean isUserGestrue);
 }