Distinguish STS observation times from PKP observation times.

net/http/transport_security_state.cc

Index: net/http/transport_security_state.cc
diff --git a/net/http/transport_security_state.cc b/net/http/transport_security_state.cc
index 2515a4bec53b5f055e8e32bd1b5179cbcb2b6649..6269bdb3e515c1fc80ffa3e7edbb6308754de2e3 100644
--- a/net/http/transport_security_state.cc
+++ b/net/http/transport_security_state.cc
@@ -202,9 +202,17 @@ void TransportSecurityState::DeleteAllDynamicDataSince(const base::Time& time) {
 
   DomainStateMap::iterator i = enabled_hosts_.begin();
   while (i != enabled_hosts_.end()) {
-    if (i->second.created >= time) {
+    if (i->second.sts_observed >= time && i->second.pkp_observed >= time) {
       dirtied = true;
       enabled_hosts_.erase(i++);
+    } else if (i->second.sts_observed >= time) {
+      dirtied = true;
+      i->second.upgrade_mode = DomainState::MODE_DEFAULT;
+      i++;
+    } else if (i->second.pkp_observed >= time) {
+      dirtied = true;
+      i->second.dynamic_spki_hashes.clear();
+      i++;
     } else {
       i++;
     }

[wtc, 2013/12/12 23:20:44]

Since we do |i++| in all cases, this can be written as a for loop.

[palmer, 2013/12/13 01:42:42]

Actually, that exposed a bug: when we do enabled_hosts_.erase, we were
invalidating |i|. New version avoids that.

[wtc, 2013/12/13 02:35:04]

Ah, I remember this issue, and your original code is a trick to avoid this
problem. When we do
    enabled_hosts_.erase(i++);

three things happen in this order:
    1. |i| is incremented.
    2. A copy of the old value of |i| is created and
       passed to enabled_hosts_.erase.
    3. enabled_hosts_.erase invalidates the old value
       of |i|.

I'd prefer that we go back to the old code. We can simplify it as follows:

  while (i != enabled_hosts_.end()) {
    if (i->second.sts_observed >= time && i->second.pkp_observed >= time) {
      dirtied = true;
      enabled_hosts_.erase(i++);
      continue;
    }

    if (i->second.sts_observed >= time) {
      dirtied = true;
      i->second.upgrade_mode = DomainState::MODE_DEFAULT;
    } else if (i->second.pkp_observed >= time) {
      dirtied = true;
      i->second.dynamic_spki_hashes.clear();
    }
    ++i;  // Note the pre-increment operator for efficiency.
  }

@@ -614,7 +622,7 @@ bool TransportSecurityState::AddHSTSHeader(const std::string& host,
       domain_state.upgrade_mode = DomainState::MODE_DEFAULT;
     else
       domain_state.upgrade_mode = DomainState::MODE_FORCE_HTTPS;
-    domain_state.created = now;
+    domain_state.sts_observed = now;
     domain_state.upgrade_expiry = now + max_age;
     EnableHost(host, domain_state);
     return true;
@@ -635,7 +643,7 @@ bool TransportSecurityState::AddHPKPHeader(const std::string& host,
                       &max_age, &domain_state.pkp_include_subdomains,
                       &domain_state.dynamic_spki_hashes)) {
     // TODO(palmer): http://crbug.com/243865 handle max-age == 0.
-    domain_state.created = now;
+    domain_state.pkp_observed = now;
     domain_state.dynamic_spki_hashes_expiry = now + max_age;
     EnableHost(host, domain_state);
     return true;
@@ -657,7 +665,7 @@ bool TransportSecurityState::AddHSTS(const std::string& host,
   if (i != enabled_hosts_.end())
     domain_state = i->second;
 
-  domain_state.created = base::Time::Now();
+  domain_state.sts_observed = base::Time::Now();
   domain_state.sts_include_subdomains = include_subdomains;
   domain_state.upgrade_expiry = expiry;
   domain_state.upgrade_mode = DomainState::MODE_FORCE_HTTPS;
@@ -680,7 +688,7 @@ bool TransportSecurityState::AddHPKP(const std::string& host,
   if (i != enabled_hosts_.end())
     domain_state = i->second;
 
-  domain_state.created = base::Time::Now();
+  domain_state.pkp_observed = base::Time::Now();
   domain_state.pkp_include_subdomains = include_subdomains;
   domain_state.dynamic_spki_hashes_expiry = expiry;
   domain_state.dynamic_spki_hashes = hashes;
@@ -825,9 +833,11 @@ void TransportSecurityState::AddOrUpdateEnabledHosts(
 
 TransportSecurityState::DomainState::DomainState()
     : upgrade_mode(MODE_DEFAULT),
-      created(base::Time::Now()),
       sts_include_subdomains(false),
       pkp_include_subdomains(false) {
+  base::Time now(base::Time::Now());
+  sts_observed = now;
+  pkp_observed = now;
 }
 
 TransportSecurityState::DomainState::~DomainState() {