Fix content settings's cookie code to work with PlzNavigate.

There are two main fixes:
-use GetWebContentsGetter instead of the RPH/RFH IDs since the latter don't exist for browser side navigation
-switching TabSpecificContentSettings to use the new navigation callbacks. This is needed because with PlzNavigate, the old didstartprovisionalload callback (from the new renderer) would run after the cookie request (from the browser), and so we would wipe out the settings map. Using the new callbacks preserves the order of these callbacks with PlzNavigate.

This fixes the following browser tests with PlzNavigate:
ContentSettingsTest.RedirectCrossOrigin
ContentSettingsTest.RedirectLoopCookies

BUG=504347

Review-Url: https://codereview.chromium.org/2374443003
Cr-Commit-Position: refs/heads/master@{#421224}

chrome/browser/chrome_content_browser_client.cc

@@ -778,6 +778,12 @@ class ClearSiteDataObserver : public BrowsingDataRemover::Observer {
   int count_;
 };
 
+WebContents* GetWebContents(int render_process_id, int render_frame_id) {
+  RenderFrameHost* rfh =
+      RenderFrameHost::FromID(render_process_id, render_frame_id);
+  return WebContents::FromRenderFrameHost(rfh);
+}
+
 }  // namespace
 
 ChromeContentBrowserClient::ChromeContentBrowserClient()
@@ -1811,10 +1817,12 @@ bool ChromeContentBrowserClient::AllowGetCookie(
   bool allow = io_data->GetCookieSettings()->
       IsReadingCookieAllowed(url, first_party);
 
+  base::Callback<content::WebContents*(void)> wc_getter =
+      base::Bind(&GetWebContents, render_process_id, render_frame_id);
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
-      base::Bind(&TabSpecificContentSettings::CookiesRead, render_process_id,
-                 render_frame_id, url, first_party, cookie_list, !allow));
+      base::Bind(&TabSpecificContentSettings::CookiesRead, wc_getter, url,
+                 first_party, cookie_list, !allow));
   return allow;
 }
 
@@ -1832,11 +1840,12 @@ bool ChromeContentBrowserClient::AllowSetCookie(
       io_data->GetCookieSettings();
   bool allow = cookie_settings->IsSettingCookieAllowed(url, first_party);
 
+  base::Callback<content::WebContents*(void)> wc_getter =
+      base::Bind(&GetWebContents, render_process_id, render_frame_id);
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
-      base::Bind(&TabSpecificContentSettings::CookieChanged, render_process_id,
-                 render_frame_id, url, first_party, cookie_line, options,
-                 !allow));
+      base::Bind(&TabSpecificContentSettings::CookieChanged, wc_getter, url,
+                 first_party, cookie_line, options, !allow));
   return allow;
 }
 

chrome/browser/content_settings/content_settings_usages_state_unittest.cc

@@ -21,7 +21,6 @@ namespace {
 ContentSettingsUsagesState::CommittedDetails CreateDetailsWithURL(
     const GURL& url) {
   ContentSettingsUsagesState::CommittedDetails details;
-  details.current_url_valid = true;
   details.current_url = url;
   return details;
 }

chrome/browser/content_settings/tab_specific_content_settings.cc

@@ -38,6 +38,7 @@
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_frame_host.h"
@@ -53,22 +54,21 @@ using content::NavigationController;
 using content::NavigationEntry;
 using content::WebContents;
 
-DEFINE_WEB_CONTENTS_USER_DATA_KEY(TabSpecificContentSettings);
-
 namespace {
 
-ContentSettingsUsagesState::CommittedDetails GetCommittedDetails(
-    const content::LoadCommittedDetails& details) {
-  ContentSettingsUsagesState::CommittedDetails committed_details;
-  committed_details.current_url_valid = !!details.entry;
-  if (details.entry)
-    committed_details.current_url = details.entry->GetURL();
-  committed_details.previous_url = details.previous_url;
-  return committed_details;
+static TabSpecificContentSettings* GetForWCGetter(
+    const base::Callback<content::WebContents*(void)>& wc_getter) {
+  WebContents* web_contents = wc_getter.Run();
+  if (!web_contents)
+    return nullptr;
+
+  return TabSpecificContentSettings::FromWebContents(web_contents);
 }
 
 }  // namespace
 
+DEFINE_WEB_CONTENTS_USER_DATA_KEY(TabSpecificContentSettings);
+
 TabSpecificContentSettings::SiteDataObserver::SiteDataObserver(
     TabSpecificContentSettings* tab_specific_content_settings)
     : tab_specific_content_settings_(tab_specific_content_settings) {
@@ -133,15 +133,14 @@ TabSpecificContentSettings* TabSpecificContentSettings::GetForFrame(
 }
 
 // static
-void TabSpecificContentSettings::CookiesRead(int render_process_id,
-                                             int render_frame_id,
-                                             const GURL& url,
-                                             const GURL& frame_url,
-                                             const net::CookieList& cookie_list,
-                                             bool blocked_by_policy) {
+void TabSpecificContentSettings::CookiesRead(
+    const base::Callback<content::WebContents*(void)>& wc_getter,
+    const GURL& url,
+    const GURL& frame_url,
+    const net::CookieList& cookie_list,
+    bool blocked_by_policy) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  TabSpecificContentSettings* settings =
-      GetForFrame(render_process_id, render_frame_id);
+  TabSpecificContentSettings* settings = GetForWCGetter(wc_getter);
   if (settings) {
     settings->OnCookiesRead(url, frame_url, cookie_list,
                             blocked_by_policy);
@@ -150,16 +149,14 @@ void TabSpecificContentSettings::CookiesRead(int render_process_id,
 
 // static
 void TabSpecificContentSettings::CookieChanged(
-    int render_process_id,
-    int render_frame_id,
+    const base::Callback<WebContents*(void)>& wc_getter,
     const GURL& url,
     const GURL& frame_url,
     const std::string& cookie_line,
     const net::CookieOptions& options,
     bool blocked_by_policy) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  TabSpecificContentSettings* settings =
-      GetForFrame(render_process_id, render_frame_id);
+  TabSpecificContentSettings* settings = GetForWCGetter(wc_getter);
   if (settings)
     settings->OnCookieChanged(url, frame_url, cookie_line, options,
                               blocked_by_policy);
@@ -778,41 +775,48 @@ bool TabSpecificContentSettings::OnMessageReceived(
   return handled;
 }
 
-void TabSpecificContentSettings::DidNavigateMainFrame(
-    const content::LoadCommittedDetails& details,
-    const content::FrameNavigateParams& params) {
-  if (!details.is_in_page) {
-    // Clear "blocked" flags.
-    ClearBlockedContentSettingsExceptForCookies();
-    blocked_plugin_names_.clear();
-    GeolocationDidNavigate(details);
-    MidiDidNavigate(details);
-
-    if (web_contents()->GetVisibleURL().SchemeIsHTTPOrHTTPS()) {
-      content_settings::RecordPluginsAction(
-          content_settings::PLUGINS_ACTION_TOTAL_NAVIGATIONS);
-    }
-  }
-}
-
-void TabSpecificContentSettings::DidStartProvisionalLoadForFrame(
-    content::RenderFrameHost* render_frame_host,
-    const GURL& validated_url,
-    bool is_error_page,
-    bool is_iframe_srcdoc) {
-  if (render_frame_host->GetParent())
+void TabSpecificContentSettings::DidStartNavigation(
+    content::NavigationHandle* navigation_handle) {
+  if (!navigation_handle->IsInMainFrame())
     return;
 
+  const content::NavigationController& controller =
+      web_contents()->GetController();
+  content::NavigationEntry* last_committed_entry =
+      controller.GetLastCommittedEntry();
+  if (last_committed_entry)
+    previous_url_ = last_committed_entry->GetURL();
+
   // If we're displaying a network error page do not reset the content
   // settings delegate's cookies so the user has a chance to modify cookie
   // settings.
-  if (!is_error_page)
+  if (!navigation_handle->IsErrorPage())
     ClearCookieSpecificContentSettings();
   ClearGeolocationContentSettings();
   ClearMidiContentSettings();
   ClearPendingProtocolHandler();
 }
 
+void TabSpecificContentSettings::DidFinishNavigation(
+      content::NavigationHandle* navigation_handle) {
+  if (!navigation_handle->IsInMainFrame() ||
+      !navigation_handle->HasCommitted() ||
+      navigation_handle->IsSamePage()) {
+    return;
+  }
+
+  // Clear "blocked" flags.
+  ClearBlockedContentSettingsExceptForCookies();
+  blocked_plugin_names_.clear();
+  GeolocationDidNavigate(navigation_handle);
+  MidiDidNavigate(navigation_handle);
+
+  if (web_contents()->GetVisibleURL().SchemeIsHTTPOrHTTPS()) {
+    content_settings::RecordPluginsAction(
+        content_settings::PLUGINS_ACTION_TOTAL_NAVIGATIONS);
+  }
+}
+
 void TabSpecificContentSettings::AppCacheAccessed(const GURL& manifest_url,
                                                   bool blocked_by_policy) {
   if (blocked_by_policy) {
@@ -847,13 +851,20 @@ void TabSpecificContentSettings::ClearMidiContentSettings() {
 }
 
 void TabSpecificContentSettings::GeolocationDidNavigate(
-    const content::LoadCommittedDetails& details) {
-  geolocation_usages_state_.DidNavigate(GetCommittedDetails(details));
+    content::NavigationHandle* navigation_handle) {
+  ContentSettingsUsagesState::CommittedDetails committed_details;
+  committed_details.current_url = navigation_handle->GetURL();
+  committed_details.previous_url = previous_url_;
+
+  geolocation_usages_state_.DidNavigate(committed_details);
 }
 
 void TabSpecificContentSettings::MidiDidNavigate(
-    const content::LoadCommittedDetails& details) {
-  midi_usages_state_.DidNavigate(GetCommittedDetails(details));
+    content::NavigationHandle* navigation_handle) {
+  ContentSettingsUsagesState::CommittedDetails committed_details;
+  committed_details.current_url = navigation_handle->GetURL();
+  committed_details.previous_url = previous_url_;
+  midi_usages_state_.DidNavigate(committed_details);
 }
 
 void TabSpecificContentSettings::BlockAllContentForTesting() {

chrome/browser/content_settings/tab_specific_content_settings.h

@@ -102,24 +102,24 @@ class TabSpecificContentSettings
   // current page or while loading it. |blocked_by_policy| should be true, if
   // reading cookies was blocked due to the user's content settings. In that
   // case, this function should invoke OnContentBlocked.
-  static void CookiesRead(int render_process_id,
-                          int render_frame_id,
-                          const GURL& url,
-                          const GURL& first_party_url,
-                          const net::CookieList& cookie_list,
-                          bool blocked_by_policy);
+  static void CookiesRead(
+      const base::Callback<content::WebContents*(void)>& wc_getter,
+      const GURL& url,
+      const GURL& first_party_url,
+      const net::CookieList& cookie_list,
+      bool blocked_by_policy);
 
   // Called when a specific cookie in the current page was changed.
   // |blocked_by_policy| should be true, if the cookie was blocked due to the
   // user's content settings. In that case, this function should invoke
   // OnContentBlocked.
-  static void CookieChanged(int render_process_id,
-                            int render_frame_id,
-                            const GURL& url,
-                            const GURL& first_party_url,
-                            const std::string& cookie_line,
-                            const net::CookieOptions& options,
-                            bool blocked_by_policy);
+  static void CookieChanged(
+      const base::Callback<content::WebContents*(void)>& wc_getter,
+      const GURL& url,
+      const GURL& first_party_url,
+      const std::string& cookie_line,
+      const net::CookieOptions& options,
+      bool blocked_by_policy);
 
   // Called when a specific Web database in the current page was accessed. If
   // access was blocked due to the user's content settings,
@@ -398,14 +398,10 @@ class TabSpecificContentSettings
       content::RenderFrameHost* render_frame_host) override;
   bool OnMessageReceived(const IPC::Message& message,
                          content::RenderFrameHost* render_frame_host) override;
-  void DidNavigateMainFrame(
-      const content::LoadCommittedDetails& details,
-      const content::FrameNavigateParams& params) 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 DidFinishNavigation(
+      content::NavigationHandle* navigation_handle) override;
   void AppCacheAccessed(const GURL& manifest_url,
                         bool blocked_by_policy) override;
 
@@ -425,11 +421,10 @@ class TabSpecificContentSettings
   void ClearMidiContentSettings();
 
   // Updates Geolocation settings on navigation.
-  void GeolocationDidNavigate(
-      const content::LoadCommittedDetails& details);
+  void GeolocationDidNavigate(content::NavigationHandle* navigation_handle);
 
   // Updates MIDI settings on navigation.
-  void MidiDidNavigate(const content::LoadCommittedDetails& details);
+  void MidiDidNavigate(content::NavigationHandle* navigation_handle);
 
   // All currently registered |SiteDataObserver|s.
   base::ObserverList<SiteDataObserver> observer_list_;
@@ -493,6 +488,9 @@ class TabSpecificContentSettings
   bool subresource_filter_enabled_;
   bool subresource_filter_blockage_indicated_;
 
+  // Holds the previous committed url during a navigation.
+  GURL previous_url_;
+
   // Observer to watch for content settings changed.
   ScopedObserver<HostContentSettingsMap, content_settings::Observer> observer_;
 

chrome/browser/net/chrome_network_delegate.cc

@@ -427,14 +427,12 @@ bool ChromeNetworkDelegate::OnCanGetCookies(
   bool allow = cookie_settings_->IsReadingCookieAllowed(
       request.url(), request.first_party_for_cookies());
 
-  int render_process_id = -1;
-  int render_frame_id = -1;
-  if (content::ResourceRequestInfo::GetRenderFrameForRequest(
-          &request, &render_process_id, &render_frame_id)) {
+  const ResourceRequestInfo* info = ResourceRequestInfo::ForRequest(&request);
+  if (info) {
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
         base::Bind(&TabSpecificContentSettings::CookiesRead,
-                   render_process_id, render_frame_id,
+                   info->GetWebContentsGetterForRequest(),
                    request.url(), request.first_party_for_cookies(),
                    cookie_list, !allow));
   }
@@ -452,14 +450,12 @@ bool ChromeNetworkDelegate::OnCanSetCookie(const net::URLRequest& request,
   bool allow = cookie_settings_->IsSettingCookieAllowed(
       request.url(), request.first_party_for_cookies());
 
-  int render_process_id = -1;
-  int render_frame_id = -1;
-  if (content::ResourceRequestInfo::GetRenderFrameForRequest(
-          &request, &render_process_id, &render_frame_id)) {
+  const ResourceRequestInfo* info = ResourceRequestInfo::ForRequest(&request);
+  if (info) {
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
         base::Bind(&TabSpecificContentSettings::CookieChanged,
-                   render_process_id, render_frame_id,
+                   info->GetWebContentsGetterForRequest(),
                    request.url(), request.first_party_for_cookies(),
                    cookie_line, *options, !allow));
   }

components/content_settings/core/browser/content_settings_usages_state.cc

@@ -10,12 +10,6 @@
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/url_formatter/url_formatter.h"
 
-ContentSettingsUsagesState::CommittedDetails::CommittedDetails()
-    : current_url_valid(false) {
-}
-
-ContentSettingsUsagesState::CommittedDetails::~CommittedDetails() {}
-
 ContentSettingsUsagesState::ContentSettingsUsagesState(
     HostContentSettingsMap* host_content_settings_map,
     ContentSettingsType type)
@@ -33,12 +27,10 @@ void ContentSettingsUsagesState::OnPermissionSet(
 }
 
 void ContentSettingsUsagesState::DidNavigate(const CommittedDetails& details) {
-  if (details.current_url_valid)
-    embedder_url_ = details.current_url;
+  embedder_url_ = details.current_url;
   if (state_map_.empty())
     return;
-  if (!details.current_url_valid ||
-      details.previous_url.GetOrigin() != details.current_url.GetOrigin()) {
+  if (details.previous_url.GetOrigin() != details.current_url.GetOrigin()) {
     state_map_.clear();
     return;
   }

components/content_settings/core/browser/content_settings_usages_state.h

@@ -22,10 +22,6 @@ class ContentSettingsUsagesState {
  public:
   // Information about navigation.
   struct CommittedDetails {
-    CommittedDetails();
-    ~CommittedDetails();
-
-    bool current_url_valid;
     GURL current_url;
     GURL previous_url;
   };