Kill URLRequestJobTracker.

net/proxy/network_delegate_error_observer_unittest.cc

 // Copyright (c) 2011 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.
 
 #include "net/proxy/network_delegate_error_observer.h"
 
 #include "base/message_loop_proxy.h"
 #include "base/threading/thread.h"
 #include "net/base/net_errors.h"
 #include "net/base/network_delegate.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 DISABLE_RUNNABLE_METHOD_REFCOUNT(net::NetworkDelegateErrorObserver);
 
 namespace net {
 
 namespace {
+
 class TestNetworkDelegate : public net::NetworkDelegate {
  public:
   TestNetworkDelegate() : got_pac_error_(false) {}
   virtual ~TestNetworkDelegate() {}
 
   bool got_pac_error() const { return got_pac_error_; }
 
  private:
   // net::NetworkDelegate:
   virtual int OnBeforeURLRequest(URLRequest* request,
                                  CompletionCallback* callback,
                                  GURL* new_url) {
     return net::OK;
   }
   virtual int OnBeforeSendHeaders(uint64 request_id,
                                   CompletionCallback* callback,
                                   HttpRequestHeaders* headers) {
     return net::OK;
   }
   virtual void OnRequestSent(uint64 request_id,
                              const HostPortPair& socket_address,
                              const HttpRequestHeaders& headers) {}
   virtual void OnBeforeRedirect(URLRequest* request,
                                 const GURL& new_location) {}
   virtual void OnResponseStarted(URLRequest* request) {}
+  virtual void OnBytesRead(const URLRequest& /* request */,

[rvargas (doing something else), 2011/05/18 21:45:18]

nit: I would follow the current style of not commenting out the argument name.

[willchan no longer on Chromium, 2011/05/18 23:55:55]

I'm going to fix the rest. The style guide explicitly says to comment them out
in
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...:
"If some parameters are unused, comment out the variable name in the function
definition"

[rvargas (doing something else), 2011/05/19 00:40:33]

Sure, I know about the style guide, but that is not common at all in Chrome
code, and in the past we've followed the current convention over changing the
code to follow Google style guide.

[willchan no longer on Chromium, 2011/05/19 23:39:18]

I think we've documented all our differences/quirks from the official Google
style guide in the Chromium style guide. If it's not in there, then I'd rather
keep the code Google style guide conformant so it's easier for us to get Google
readability reviews.

Do you feel strongly here?

[rvargas (doing something else), 2011/05/20 00:37:31]

I just don't see the reason for sometimes saying "the code already looks like
this, let's document the exception to the rule" and sometimes saying "let's
change old code to the new style". I mostly go with the final words of the style
guide, about observing the current style of the file that I'm modifying and just
going with it.

I know you are not doing that, but that's one reason of why I generally don't
like changes that just go everywhere changing the style (unless it is something
clearly broken).

In this particular case, yeah, somewhat strongly because it is one of those
things that I personally dislike (I know that's not a valid reason), don't see
the benefit, and it is mostly extra noise to me... and breaks commenting out a
big part of a file whenever I have to do that (almost never, but still).

And that comes from somebody that was a fan of saying UNUSED_ARGUMENT(foo) to
make the compiler happy with a high warning level :P

On 2011/05/19 23:39:18, willchan wrote:

[willchan no longer on Chromium, 2011/05/20 01:08:40]

OK, I think most of this seems to be your personal stylistic preferences. I know
that wtc and I have both respected the Google C++ style guide in this regard. Do
you care about this particular instance or do you care more globally? If you
care globally and think others should change, then I think elevating to
chromium-dev is appropriate. If you care more about these particular instances,
then I'm fine undoing my style changes here.

On 2011/05/20 00:37:31, rvargas wrote:

[rvargas (doing something else), 2011/05/20 02:50:00]

I did a search under /chrome/ and I found very few examples of people actually
doing this, but it may be that the arguments are always used.

More seriously, I'm less interested in forcing people to use one way or the
other (I would not have said anything if this was a new file), and I have the
feeling that if we consult chromium-dev the few people that care about it will
say let's change the current code.

I hope you're not feeling that we're wasting our time discussing this.

On 2011/05/20 01:08:40, willchan wrote:

[willchan no longer on Chromium, 2011/05/20 22:57:38]

Fair enough. I'll try to be consistent with existing code and only do a
different way in new files. That's perfectly reasonable. I'll undo what I have
here.

On 2011/05/20 02:50:00, rvargas wrote:

+                           int /* bytes_read */) {}
   virtual void OnCompleted(URLRequest* request) {}
   virtual void OnURLRequestDestroyed(URLRequest* request) {}
   virtual void OnHttpTransactionDestroyed(uint64 request_id) {}
   virtual URLRequestJob* OnMaybeCreateURLRequestJob(URLRequest* request) {
     return NULL;
   }
   virtual void OnPACScriptError(int line_number, const string16& error) {
     got_pac_error_ = true;
   }
 
@@ skipping 31 matching lines @@
       FROM_HERE,
       NewRunnableMethod(&observer,
                         &NetworkDelegateErrorObserver::OnPACScriptError,
                         42, string16()));
   thread.Stop();
   MessageLoop::current()->RunAllPending();
   // Shouldn't have crashed until here...
 }
 
 }  // namespace net