Be slightly more precise in how we propagate mixed content state.

Mixed content can't really propagate from one process to another, so we don't need to break the lock icon for every tab in the same origin when we see insecure content.  We need only break the lock icon for all the tabs in the same process.

R=jcampan
TEST=SSLUITest.TestMixedContentsTwoTabs

Review URL: http://codereview.chromium.org/115230

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

8 files changed, 55 insertions, 31 deletions

diff --git a/chrome/browser/load_from_memory_cache_details.h b/chrome/browser/load_from_memory_cache_details.h
index f2a5c5e..27dc4ee 100644
--- a/chrome/browser/load_from_memory_cache_details.h
+++ b/chrome/browser/load_from_memory_cache_details.h
@@ -14,11 +14,13 @@ class LoadFromMemoryCacheDetails {
        const GURL& url,
        const std::string& frame_origin,
        const std::string& main_frame_origin,
+       int pid,
        int cert_id,
        int cert_status)
        : url_(url),
          frame_origin_(frame_origin),
          main_frame_origin_(main_frame_origin),
+         pid_(pid),
          cert_id_(cert_id),
          cert_status_(cert_status)
   { }
@@ -28,6 +30,7 @@ class LoadFromMemoryCacheDetails {
   const GURL& url() const { return url_; }
   const std::string& frame_origin() const { return frame_origin_; }
   const std::string& main_frame_origin() const { return main_frame_origin_; }
+  int pid() const { return pid_; }
   int ssl_cert_id() const { return cert_id_; }
   int ssl_cert_status() const { return cert_status_; }
 
@@ -35,6 +38,7 @@ private:
   GURL url_;
   std::string frame_origin_;
   std::string main_frame_origin_;
+  int pid_;
   int cert_id_;
   int cert_status_;
 
diff --git a/chrome/browser/ssl/ssl_host_state.cc b/chrome/browser/ssl/ssl_host_state.cc
index f8a063b..62bcb94 100644
--- a/chrome/browser/ssl/ssl_host_state.cc
+++ b/chrome/browser/ssl/ssl_host_state.cc
@@ -23,13 +23,13 @@ SSLHostState::SSLHostState() {
 SSLHostState::~SSLHostState() {
 }
 
-void SSLHostState::MarkHostAsBroken(const std::string& host) {
+void SSLHostState::MarkHostAsBroken(const std::string& host, int pid) {
   DCHECK(CalledOnValidThread());
 
-  broken_hosts_.insert(host);
+  broken_hosts_.insert(BrokenHostEntry(host, pid));
 }
 
-bool SSLHostState::DidMarkHostAsBroken(const std::string& host) {
+bool SSLHostState::DidMarkHostAsBroken(const std::string& host, int pid) {
   DCHECK(CalledOnValidThread());
 
   // CAs issue certificate for intranet hosts to everyone.  Therefore, we always
@@ -37,7 +37,8 @@ bool SSLHostState::DidMarkHostAsBroken(const std::string& host) {
   if (IsIntranetHost(host))
     return true;
 
-  return (broken_hosts_.find(host) != broken_hosts_.end());
+  return (broken_hosts_.find(
+      BrokenHostEntry(host, pid)) != broken_hosts_.end());
 }
 
 void SSLHostState::DenyCertForHost(net::X509Certificate* cert,
diff --git a/chrome/browser/ssl/ssl_host_state.h b/chrome/browser/ssl/ssl_host_state.h
index f66e9bc..a5018c7 100644
--- a/chrome/browser/ssl/ssl_host_state.h
+++ b/chrome/browser/ssl/ssl_host_state.h
@@ -27,13 +27,14 @@ class SSLHostState : public NonThreadSafe {
   SSLHostState();
   ~SSLHostState();
 
-  // Records that a host is "broken," that is, the origin for that host has been
-  // contaminated with insecure content, either via HTTP or via HTTPS with a
-  // bad certificate.
-  void MarkHostAsBroken(const std::string& host);
+  // Records that a host is "broken" in a particular render process.  That is,
+  // the origin for that host has been contaminated with insecure content,
+  // either via HTTP or via HTTPS with a bad certificate.
+  void MarkHostAsBroken(const std::string& host, int pid);
 
-  // Returns whether the specified host was marked as broken.
-  bool DidMarkHostAsBroken(const std::string& host);
+  // Returns whether the specified host was marked as broken in a particular
+  // render process.
+  bool DidMarkHostAsBroken(const std::string& host, int pid);
 
   // Records that |cert| is permitted to be used for |host| in the future.
   void DenyCertForHost(net::X509Certificate* cert, const std::string& host);
@@ -52,8 +53,9 @@ class SSLHostState : public NonThreadSafe {
   bool DidAllowMixedContentForHost(const std::string& host);
 
  private:
+  typedef std::pair<std::string, int> BrokenHostEntry;
   // Hosts which have been contaminated with unsafe content.
-  std::set<std::string> broken_hosts_;
+  std::set<BrokenHostEntry> broken_hosts_;
 
   // Certificate policies for each host.
   std::map<std::string, net::X509Certificate::Policy> cert_policy_for_host_;
diff --git a/chrome/browser/ssl/ssl_manager.cc b/chrome/browser/ssl/ssl_manager.cc
index 9d8da4b..0510810 100644
--- a/chrome/browser/ssl/ssl_manager.cc
+++ b/chrome/browser/ssl/ssl_manager.cc
@@ -182,14 +182,14 @@ void SSLManager::AddMessageToConsole(const string16& message,
 }
 
 // Delegate API method.
-void SSLManager::MarkHostAsBroken(const std::string& host) {
-  ssl_host_state_->MarkHostAsBroken(host);
+void SSLManager::MarkHostAsBroken(const std::string& host, int pid) {
+  ssl_host_state_->MarkHostAsBroken(host, pid);
   DispatchSSLInternalStateChanged();
 }
 
 // Delegate API method.
-bool SSLManager::DidMarkHostAsBroken(const std::string& host) const {
-  return ssl_host_state_->DidMarkHostAsBroken(host);
+bool SSLManager::DidMarkHostAsBroken(const std::string& host, int pid) const {
+  return ssl_host_state_->DidMarkHostAsBroken(host, pid);
 }
 
 // Delegate API method.
@@ -504,6 +504,7 @@ bool SSLManager::ShouldStartRequest(ResourceDispatcherHost* rdh,
                                                 info->resource_type,
                                                 info->frame_origin,
                                                 info->main_frame_origin,
+                                                info->process_id,
                                                 ui_loop),
                         &MixedContentHandler::Dispatch));
   return false;
@@ -588,6 +589,7 @@ void SSLManager::DidLoadFromMemoryCache(LoadFromMemoryCacheDetails* details) {
       details->frame_origin(),
       details->main_frame_origin(),
       FilterPolicy::DONT_FILTER,
+      details->pid(),
       details->ssl_cert_id(),
       details->ssl_cert_status());
 
@@ -651,6 +653,7 @@ void SSLManager::DidStartResourceResponse(ResourceRequestDetails* details) {
       details->frame_origin(),
       details->main_frame_origin(),
       details->filter_policy(),
+      details->origin_pid(),
       details->ssl_cert_id(),
       details->ssl_cert_status());
 
diff --git a/chrome/browser/ssl/ssl_manager.h b/chrome/browser/ssl/ssl_manager.h
index 57a6d63..5e6e77d 100644
--- a/chrome/browser/ssl/ssl_manager.h
+++ b/chrome/browser/ssl/ssl_manager.h
@@ -248,15 +248,21 @@ class SSLManager : public NotificationObserver {
                         ResourceType::Type resource_type,
                         const std::string& frame_origin,
                         const std::string& main_frame_origin,
+                        int pid,
                         MessageLoop* ui_loop)
         : ErrorHandler(rdh, request, resource_type, frame_origin,
-                       main_frame_origin, ui_loop) { }
+                       main_frame_origin, ui_loop),
+          pid_(pid) {}
+
+    int pid() const { return pid_; }
 
    protected:
     virtual void OnDispatchFailed() { TakeNoAction(); }
     virtual void OnDispatched() { manager()->OnMixedContent(this); }
 
    private:
+    int pid_;
+
     DISALLOW_COPY_AND_ASSIGN(MixedContentHandler);
   };
 
@@ -271,6 +277,7 @@ class SSLManager : public NotificationObserver {
                 const std::string& frame_origin,
                 const std::string& main_frame_origin,
                 FilterPolicy::Type filter_policy,
+                int pid,
                 int ssl_cert_id,
                 int ssl_cert_status)
         : manager_(manager),
@@ -279,6 +286,7 @@ class SSLManager : public NotificationObserver {
           frame_origin_(frame_origin),
           main_frame_origin_(main_frame_origin),
           filter_policy_(filter_policy),
+          pid_(pid),
           ssl_cert_id_(ssl_cert_id),
           ssl_cert_status_(ssl_cert_status) {
     }
@@ -289,6 +297,7 @@ class SSLManager : public NotificationObserver {
     const std::string& frame_origin() const { return frame_origin_; }
     const std::string& main_frame_origin() const { return main_frame_origin_; }
     FilterPolicy::Type filter_policy() const { return filter_policy_; }
+    int pid() const { return pid_; }
     int ssl_cert_id() const { return ssl_cert_id_; }
     int ssl_cert_status() const { return ssl_cert_status_; }
 
@@ -299,6 +308,7 @@ class SSLManager : public NotificationObserver {
     std::string frame_origin_;
     std::string main_frame_origin_;
     FilterPolicy::Type filter_policy_;
+    int pid_;
     int ssl_cert_id_;
     int ssl_cert_status_;
 
@@ -358,10 +368,10 @@ class SSLManager : public NotificationObserver {
   // Records that a host is "broken," that is, the origin for that host has been
   // contaminated with insecure content, either via HTTP or via HTTPS with a
   // bad certificate.
-  void MarkHostAsBroken(const std::string& host);
+  void MarkHostAsBroken(const std::string& host, int pid);
 
   // Returns whether the specified host was marked as broken.
-  bool DidMarkHostAsBroken(const std::string& host) const;
+  bool DidMarkHostAsBroken(const std::string& host, int pid) const;
 
   // Sets the maximum security style for the page.  If the current security
   // style is lower than |style|, this will not have an effect on the security
diff --git a/chrome/browser/ssl/ssl_policy.cc b/chrome/browser/ssl/ssl_policy.cc
index 829af05..6785eec 100644
--- a/chrome/browser/ssl/ssl_policy.cc
+++ b/chrome/browser/ssl/ssl_policy.cc
@@ -33,12 +33,14 @@ using WebKit::WebConsoleMessage;
 // Wrap all these helper classes in an anonymous namespace.
 namespace {
 
-static void MarkOriginAsBroken(SSLManager* manager, const std::string& origin) {
+static void MarkOriginAsBroken(SSLManager* manager,
+                               const std::string& origin,
+                               int pid) {
   GURL parsed_origin(origin);
   if (!parsed_origin.SchemeIsSecure())
     return;
 
-  manager->MarkHostAsBroken(parsed_origin.host());
+  manager->MarkHostAsBroken(parsed_origin.host(), pid);
 }
 
 static void AllowMixedContentForOrigin(SSLManager* manager,
@@ -54,19 +56,19 @@ static void UpdateStateForMixedContent(SSLManager::RequestInfo* info) {
   if (info->resource_type() != ResourceType::MAIN_FRAME ||
       info->resource_type() != ResourceType::SUB_FRAME) {
     // The frame's origin now contains mixed content and therefore is broken.
-    MarkOriginAsBroken(info->manager(), info->frame_origin());
+    MarkOriginAsBroken(info->manager(), info->frame_origin(), info->pid());
   }
 
   if (info->resource_type() != ResourceType::MAIN_FRAME) {
     // The main frame now contains a frame with mixed content.  Therefore, we
     // mark the main frame's origin as broken too.
-    MarkOriginAsBroken(info->manager(), info->main_frame_origin());
+    MarkOriginAsBroken(info->manager(), info->main_frame_origin(), info->pid());
   }
 }
 
 static void UpdateStateForUnsafeContent(SSLManager::RequestInfo* info) {
   // This request as a broken cert, which means its host is broken.
-  info->manager()->MarkHostAsBroken(info->url().host());
+  info->manager()->MarkHostAsBroken(info->url().host(), info->pid());
 
   UpdateStateForMixedContent(info);
 }
@@ -223,7 +225,7 @@ void SSLPolicy::OnMixedContent(SSLManager::MixedContentHandler* handler) {
   // If the user has added an exception, doctor the |filter_policy|.
   std::string host = GURL(handler->main_frame_origin()).host();
   if (handler->manager()->DidAllowMixedContentForHost(host) ||
-      handler->manager()->DidMarkHostAsBroken(host))
+      handler->manager()->DidMarkHostAsBroken(host, handler->pid()))
     filter_policy = FilterPolicy::DONT_FILTER;
 
   if (filter_policy != FilterPolicy::DONT_FILTER) {
@@ -269,7 +271,8 @@ void SSLPolicy::UpdateEntry(SSLManager* manager, NavigationEntry* entry) {
     return;
   }
 
-  if (manager->DidMarkHostAsBroken(entry->url().host()))
+  if (manager->DidMarkHostAsBroken(entry->url().host(),
+                                   entry->site_instance()->GetProcess()->pid()))
     entry->ssl().set_has_mixed_content();
 }
 
diff --git a/chrome/browser/ssl/ssl_uitest.cc b/chrome/browser/ssl/ssl_uitest.cc
index 5275b92..02d5d23 100644
--- a/chrome/browser/ssl/ssl_uitest.cc
+++ b/chrome/browser/ssl/ssl_uitest.cc
@@ -424,8 +424,9 @@ TEST_F(SSLUITest, TestMixedContentsLoadedFromJS) {
 }
 
 // Visits two pages from the same origin: one with mixed content and one
-// without.  The test checks that we propagate the mixed content state from one
-// to the other.
+// without.  The test checks that we don't propagate the mixed content state
+// from one process to another process.  This test is expected to fail in
+// single process mode.
 TEST_F(SSLUITest, TestMixedContentsTwoTabs) {
   scoped_refptr<HTTPSTestServer> https_server = GoodCertServer();
   scoped_refptr<HTTPTestServer> http_server = PlainServer();
@@ -465,13 +466,13 @@ TEST_F(SSLUITest, TestMixedContentsTwoTabs) {
   EXPECT_EQ(0, cert_status & net::CERT_STATUS_ALL_ERRORS);
   EXPECT_EQ(NavigationEntry::SSLStatus::MIXED_CONTENT, mixed_content_state);
 
-  // Which means the origin for the first tab has also been contaminated with
-  // mixed content.
+  // The origin for the first tab has not been contaminated with
+  // mixed content because its in a separate process.
   EXPECT_TRUE(tab1->GetSecurityState(&security_style, &cert_status,
                                      &mixed_content_state));
   EXPECT_EQ(SECURITY_STYLE_AUTHENTICATED, security_style);
   EXPECT_EQ(0, cert_status & net::CERT_STATUS_ALL_ERRORS);
-  EXPECT_EQ(NavigationEntry::SSLStatus::MIXED_CONTENT, mixed_content_state);
+  EXPECT_EQ(NavigationEntry::SSLStatus::NORMAL_CONTENT, mixed_content_state);
 }
 
 // Visits a page with an image over http.  Visits another page over https
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 9cea219..5bc2cacd 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -1836,7 +1836,7 @@ void TabContents::DidLoadResourceFromMemoryCache(
                                       &cert_id, &cert_status,
                                       &security_bits);
   LoadFromMemoryCacheDetails details(url, frame_origin, main_frame_origin,
-                                     cert_id, cert_status);
+                                     process()->pid(), cert_id, cert_status);
 
   NotificationService::current()->Notify(
       NotificationType::LOAD_FROM_MEMORY_CACHE,