Pin dictionaries from being deleted while request is outstanding.

net/url_request/url_request_http_job.cc

Index: net/url_request/url_request_http_job.cc
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index 33a508849f4a7b8a35b9468de762cbb3d804d3ad..5daf6a07b0719fc27195a272954083586a0272e8 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -66,17 +66,13 @@ class URLRequestHttpJob::HttpFilterContext : public FilterContext {
   base::Time GetRequestTime() const override;
   bool IsCachedContent() const override;
   bool IsDownload() const override;
-  bool SdchResponseExpected() const override;
+  SdchManager::DictionarySet* SdchDictionariesAdvertised() const override;
   int64 GetByteReadCount() const override;
   int GetResponseCode() const override;
   const URLRequestContext* GetURLRequestContext() const override;
   void RecordPacketStats(StatisticSelector statistic) const override;
   const BoundNetLog& GetNetLog() const override;
 
-  // Method to allow us to reset filter context for a response that should have
-  // been SDCH encoded when there is an update due to an explicit HTTP header.
-  void ResetSdchResponseToFalse();
-
  private:
   URLRequestHttpJob* job_;
 
@@ -126,13 +122,9 @@ bool URLRequestHttpJob::HttpFilterContext::IsDownload() const {
   return (job_->request_info_.load_flags & LOAD_IS_DOWNLOAD) != 0;
 }
 
-void URLRequestHttpJob::HttpFilterContext::ResetSdchResponseToFalse() {
-  DCHECK(job_->sdch_dictionary_advertised_);
-  job_->sdch_dictionary_advertised_ = false;
-}
-
-bool URLRequestHttpJob::HttpFilterContext::SdchResponseExpected() const {
-  return job_->sdch_dictionary_advertised_;
+SdchManager::DictionarySet*
+URLRequestHttpJob::HttpFilterContext::SdchDictionariesAdvertised() const {
+  return job_->dictionaries_advertised_.get();
 }
 
 int64 URLRequestHttpJob::HttpFilterContext::GetByteReadCount() const {
@@ -200,7 +192,6 @@ URLRequestHttpJob::URLRequestHttpJob(
                      base::Unretained(this))),
       read_in_progress_(false),
       throttling_entry_(NULL),
-      sdch_dictionary_advertised_(false),
       sdch_test_activated_(false),
       sdch_test_control_(false),
       is_cached_content_(false),
@@ -330,6 +321,7 @@ void URLRequestHttpJob::NotifyHeadersComplete() {
   ProcessStrictTransportSecurityHeader();
   ProcessPublicKeyPinsHeader();
 
+  // Handle the server notification of a new SDCH dictionary.
   SdchManager* sdch_manager(request()->context()->sdch_manager());
   if (sdch_manager) {
     SdchProblemCode rv = sdch_manager->IsInSupportedDomain(request()->url());
@@ -371,6 +363,24 @@ void URLRequestHttpJob::NotifyHeadersComplete() {
     }
   }
 
+  // Handle the server signalling no SDCH encoding.
+  if (dictionaries_advertised_.get()) {

[Ryan Sleevi, 2014/11/20 22:50:30]

scoped_ptr implements Testable (it's only scoped_refptr<> that doesn't, and
dcheng is fixing that)

if (dictionaries_advertised) {

[Randy Smith (Not in Mondays), 2014/11/20 23:44:29]

Done.

+    // We are wary of proxies that discard or damage SDCH encoding.  If a server

[Ryan Sleevi, 2014/11/20 22:50:30]

*cough* s/.  If/. If/

[Randy Smith (Not in Mondays), 2014/11/20 23:44:29]

*embarrassed amused look* Done.  (Throughout CL.)

+    // explicitly states that this is not SDCH content, then we can correct our
+    // assumption that this is an SDCH response, and avoid the need to recover
+    // as though the content is corrupted (when we discover it is not SDCH
+    // encoded).
+    std::string sdch_response_status;
+    void* iter = NULL;
+    while (GetResponseHeaders()->EnumerateHeader(&iter, "X-Sdch-Encode",
+                                                 &sdch_response_status)) {
+      if (sdch_response_status == "0") {
+        dictionaries_advertised_.reset();
+        break;
+      }
+    }
+  }
+
   // The HTTP transaction may be restarted several times for the purposes
   // of sending authorization information. Each time it restarts, we get
   // notified of the headers completion so that we can update the cookie store.
@@ -532,27 +542,27 @@ void URLRequestHttpJob::AddExtraHeaders() {
         }
       }
     }
-    std::string avail_dictionaries;
     if (advertise_sdch) {
-      sdch_manager->GetAvailDictionaryList(request_->url(),
-                                           &avail_dictionaries);
-
-      // The AllowLatencyExperiment() is only true if we've successfully done a
-      // full SDCH compression recently in this browser session for this host.
-      // Note that for this path, there might be no applicable dictionaries,
-      // and hence we can't participate in the experiment.
-      if (!avail_dictionaries.empty() &&
-          sdch_manager->AllowLatencyExperiment(request_->url())) {
-        // We are participating in the test (or control), and hence we'll
-        // eventually record statistics via either SDCH_EXPERIMENT_DECODE or
-        // SDCH_EXPERIMENT_HOLDBACK, and we'll need some packet timing data.
-        packet_timing_enabled_ = true;
-        if (base::RandDouble() < .01) {
-          sdch_test_control_ = true;  // 1% probability.
-          advertise_sdch = false;
-        } else {
-          sdch_test_activated_ = true;
-        }
+      dictionaries_advertised_ =
+          sdch_manager->GetDictionarySet(request_->url());
+    }
+
+    // The AllowLatencyExperiment() is only true if we've successfully done a
+    // full SDCH compression recently in this browser session for this host.
+    // Note that for this path, there might be no applicable dictionaries,
+    // and hence we can't participate in the experiment.
+    if (dictionaries_advertised_.get() &&

[Ryan Sleevi, 2014/11/20 22:50:30]

ditto testability

[Randy Smith (Not in Mondays), 2014/11/20 23:44:29]

Done.

+        sdch_manager->AllowLatencyExperiment(request_->url())) {
+      // We are participating in the test (or control), and hence we'll
+      // eventually record statistics via either SDCH_EXPERIMENT_DECODE or
+      // SDCH_EXPERIMENT_HOLDBACK, and we'll need some packet timing data.
+      packet_timing_enabled_ = true;
+      if (base::RandDouble() < .01) {
+        sdch_test_control_ = true;  // 1% probability.
+        dictionaries_advertised_.reset();
+        advertise_sdch = false;
+      } else {
+        sdch_test_activated_ = true;
       }
     }
 
@@ -569,11 +579,10 @@ void URLRequestHttpJob::AddExtraHeaders() {
       // Include SDCH in acceptable list.
       request_info_.extra_headers.SetHeader(
           HttpRequestHeaders::kAcceptEncoding, "gzip, deflate, sdch");
-      if (!avail_dictionaries.empty()) {
+      if (dictionaries_advertised_.get()) {

[Ryan Sleevi, 2014/11/20 22:50:30]

ditto Testability

[Randy Smith (Not in Mondays), 2014/11/20 23:44:29]

You realize that by not just saying also "elsewhere in file" you're training me
to rely on my reviewers to catch all the instances of the problem?  :-}  Done. 
(And yes, I've searched for all uses of it.)

         request_info_.extra_headers.SetHeader(
             kAvailDictionaryHeader,
-            avail_dictionaries);
-        sdch_dictionary_advertised_ = true;
+            dictionaries_advertised_->GetDictionaryClientHashList());
         // Since we're tagging this transaction as advertising a dictionary,
         // we'll definitely employ an SDCH filter (or tentative sdch filter)
         // when we get a response.  When done, we'll record histograms via
@@ -1072,23 +1081,6 @@ Filter* URLRequestHttpJob::SetupFilter() const {
     encoding_types.push_back(Filter::ConvertEncodingToType(encoding_type));
   }
 
-  if (filter_context_->SdchResponseExpected()) {
-    // We are wary of proxies that discard or damage SDCH encoding.  If a server
-    // explicitly states that this is not SDCH content, then we can correct our
-    // assumption that this is an SDCH response, and avoid the need to recover
-    // as though the content is corrupted (when we discover it is not SDCH
-    // encoded).
-    std::string sdch_response_status;
-    iter = NULL;
-    while (headers->EnumerateHeader(&iter, "X-Sdch-Encode",
-                                    &sdch_response_status)) {
-      if (sdch_response_status == "0") {
-        filter_context_->ResetSdchResponseToFalse();
-        break;
-      }
-    }
-  }
-
   // Even if encoding types are empty, there is a chance that we need to add
   // some decoding, as some proxies strip encoding completely. In such cases,
   // we may need to add (for example) SDCH filtering (when the context suggests