Improve testing for SDCH.

net/base/sdch_dictionary_fetcher.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Support modularity by calling to load a new SDCH filter dictionary.
 // Note that this sort of calling can't be done in the /net directory, as it has
 // no concept of the HTTP cache (which is only visible at the browser level).
 
 #ifndef NET_BASE_SDCH_DICTIONARY_FETCHER_H_
 #define NET_BASE_SDCH_DICTIONARY_FETCHER_H_
@@ skipping 11 matching lines @@
 namespace net {
 
 class URLFetcher;
 class URLRequestContextGetter;
 
 class NET_EXPORT SdchDictionaryFetcher
     : public URLFetcherDelegate,
       public SdchFetcher,
       public base::NonThreadSafe {
  public:
-  // Consumer must guarantee that the SdchManager pointer outlives
+  // The consumer must guarantee that |*manager| outlives
   // this object.  The current implementation guarantees this by
   // the SdchManager owning this object.
+  // SdchDictionaryFetcher takes a reference to |*context|.

[jar (doing other things), 2014/08/13 01:28:59]

nit: I had to read this several times, and look at code (members) to understand
the comment.  The use of the word "reference" was confusing.  Could you say:

// The constructor takes ownership of |*context| and will destroy it when done.

.or...

...destroy it when this is destroyed.

or simply....

// The constructor takes ownership of |*context|.

[Randy Smith (Not in Mondays), 2014/08/13 02:05:26]

So I'll say that if you ask me again, but it because URLRequestContextGetter is
refcounted, it's not true.  Ownership could be shared between many objects.  Do
you still want me to say "takes ownership of"?

If it was as simple as taking ownership, this would be a scoped_ptr<> and I
wouldn't have bothered with a comment :-J.

[jar (doing other things), 2014/08/14 22:33:25]

Ah... ok... with that context, I see how I misread/confused the comment.  When I
was "take a reference" I somehow misread it to be a "C++ reference" as in stuff
like

int &foo = goo;

or similar passed in as a "reference argument."

Would you be willing to write:

"...adds a reference to ..."

Would that work for you?  ...or is this comment sooooo standard, it is only
being misread by me, perhaps due to caffeine??

I'll leave it to you to decide... I can now live with either, but would (I
think) prefer the suggested change.

Alternatively... any chance we can pass in a refptr so that it is not necessary
to make this comment?

[Randy Smith (Not in Mondays), 2014/08/18 19:11:06]

There was some reason that was a hassle when I first started this, I think
because of how complex the instantiation logic calling the constructor was, but
with the earlier rewrite, this looks doable, and as such, definitely the right
idea.  Done.

   SdchDictionaryFetcher(SdchManager* manager,
                         URLRequestContextGetter* context);
   virtual ~SdchDictionaryFetcher();
 
   // Implementation of SdchFetcher class.
   virtual void Schedule(const GURL& dictionary_url) OVERRIDE;
   virtual void Cancel() OVERRIDE;
 
  private:
   // Delay in ms between Schedule and actual download.
@@ skipping 45 matching lines @@
   // Store the system_url_request_context_getter to use it when we start
   // fetching.
   scoped_refptr<URLRequestContextGetter> context_;
 
   DISALLOW_COPY_AND_ASSIGN(SdchDictionaryFetcher);
 };
 
 }  // namespace net
 
 #endif  // NET_BASE_SDCH_DICTIONARY_FETCHER_H_