Kill URLRequestJobTracker.

Kill URLRequestJobTracker.

BUG=81160
TEST=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=86241

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

I'm don't see the code that calls SystemMonitor::AddObserver

http://codereview.chromium.org/7043007/diff/5002/chrome/browser/net/chrome_ne...
File chrome/browser/net/chrome_network_delegate.cc (right):

http://codereview.chromium.org/7043007/diff/5002/chrome/browser/net/chrome_ne...
chrome/browser/net/chrome_network_delegate.cc:105:
TaskManager::GetInstance()->model()->NotifyBytesRead(request, bytes_read);
Should I presume that TaskManager::GetInstance can be safely called from the IO
thread?... that is not clear from task_manager.h

http://codereview.chromium.org/7043007/diff/5002/net/base/network_delegate.h
File net/base/network_delegate.h (right):

http://codereview.chromium.org/7043007/diff/5002/net/base/network_delegate.h#...
net/base/network_delegate.h:52: void NotifyBytesRead(const URLRequest& request,
int bytes_read);
It is interesting that bytes_read is always the raw_bytes coming from the
network, as opposed to the post-filtered byte count. That looks correct for the
(currently only) consumer of this data, but it is not clear that this is the
generic form, given that the overall interface of URLRequest hides this value.

I guess we need to at least document that...

I was actually thinking that it would be better to observe at the URLRequest
level instead of the URLRequestJob level, but if we want the raw bytes it has to
be done at the current level.

http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
File net/proxy/network_delegate_error_observer_unittest.cc (right):

http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
net/proxy/network_delegate_error_observer_unittest.cc:44: virtual void
OnBytesRead(const URLRequest& /* request */,
nit: I would follow the current style of not commenting out the argument name.

http://codereview.chromium.org/7043007/diff/5002/net/url_request/url_request_...
File net/url_request/url_request_job.cc (right):

http://codereview.chromium.org/7043007/diff/5002/net/url_request/url_request_...
net/url_request/url_request_job.cc:34:
ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {}
nit: Could you move the closing bracket to the next line? I think we use that
pattern more often for methods that are not being declared at the same time. You
do the same at line 39, but line 36 has the more common pattern.

http://codereview.chromium.org/7043007/diff/5002/net/url_request/url_request_...
File net/url_request/url_request_test_util.cc (right):

http://codereview.chromium.org/7043007/diff/5002/net/url_request/url_request_...
net/url_request/url_request_test_util.cc:283: int /* bytes_read */) {}
nit: Same comment here about the argument name. And I guess that what looks bad
to me are multi-line definitions that end up with {}. See the rest of this file.

[rvargas (doing something else), 2011-05-18 21:46:49]

I forgot to mention that ChromeOS *may* be using URLRequestJobTracker.

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

Thanks for catching the AddObserver/RemoveObserver bugs. I forgot about those.

And where is ChromeOS using this? I'll kick off a linux_chromeos run to make
sure I flush it out.

http://codereview.chromium.org/7043007/diff/5002/chrome/browser/net/chrome_ne...
File chrome/browser/net/chrome_network_delegate.cc (right):

http://codereview.chromium.org/7043007/diff/5002/chrome/browser/net/chrome_ne...
chrome/browser/net/chrome_network_delegate.cc:105:
TaskManager::GetInstance()->model()->NotifyBytesRead(request, bytes_read);
On 2011/05/18 21:45:18, rvargas wrote:
> Should I presume that TaskManager::GetInstance can be safely called from the
IO
> thread?... that is not clear from task_manager.h

It's actually only correct to be called on the IO thread. Yeah, non-obvious from
task_manager.h.

http://codereview.chromium.org/7043007/diff/5002/net/base/network_delegate.h
File net/base/network_delegate.h (right):

http://codereview.chromium.org/7043007/diff/5002/net/base/network_delegate.h#...
net/base/network_delegate.h:52: void NotifyBytesRead(const URLRequest& request,
int bytes_read);
On 2011/05/18 21:45:18, rvargas wrote:
> It is interesting that bytes_read is always the raw_bytes coming from the
> network, as opposed to the post-filtered byte count. That looks correct for
the
> (currently only) consumer of this data, but it is not clear that this is the
> generic form, given that the overall interface of URLRequest hides this value.
> 
> I guess we need to at least document that...
> 
> I was actually thinking that it would be better to observe at the URLRequest
> level instead of the URLRequestJob level, but if we want the raw bytes it has
to
> be done at the current level.

I've renamed to NotifyRawBytesRead().

http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
File net/proxy/network_delegate_error_observer_unittest.cc (right):

http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
net/proxy/network_delegate_error_observer_unittest.cc:44: virtual void
OnBytesRead(const URLRequest& /* request */,
On 2011/05/18 21:45:18, rvargas wrote:
> nit: I would follow the current style of not commenting out the argument name.

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"

http://codereview.chromium.org/7043007/diff/5002/net/url_request/url_request_...
File net/url_request/url_request_job.cc (right):

http://codereview.chromium.org/7043007/diff/5002/net/url_request/url_request_...
net/url_request/url_request_job.cc:34:
ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {}
On 2011/05/18 21:45:18, rvargas wrote:
> nit: Could you move the closing bracket to the next line? I think we use that
> pattern more often for methods that are not being declared at the same time.
You
> do the same at line 39, but line 36 has the more common pattern.

Done.

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

I don't know if ChromeOS actually uses this code... I just know that there is
some code that displays some UI based on network activity.

http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
File net/proxy/network_delegate_error_observer_unittest.cc (right):

http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
net/proxy/network_delegate_error_observer_unittest.cc:44: virtual void
OnBytesRead(const URLRequest& /* request */,
On 2011/05/18 23:55:55, willchan wrote:
> On 2011/05/18 21:45:18, rvargas wrote:
> > nit: I would follow the current style of not commenting out the argument
name.
> 
> 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"

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]

http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
File net/proxy/network_delegate_error_observer_unittest.cc (right):

http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
net/proxy/network_delegate_error_observer_unittest.cc:44: virtual void
OnBytesRead(const URLRequest& /* request */,
On 2011/05/19 00:40:33, rvargas wrote:
> On 2011/05/18 23:55:55, willchan wrote:
> > On 2011/05/18 21:45:18, rvargas wrote:
> > > nit: I would follow the current style of not commenting out the argument
> name.
> > 
> > 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"
> 
> 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.

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]

http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
File net/proxy/network_delegate_error_observer_unittest.cc (right):

http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
net/proxy/network_delegate_error_observer_unittest.cc:44: virtual void
OnBytesRead(const URLRequest& /* request */,
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:
> On 2011/05/19 00:40:33, rvargas wrote:
> > On 2011/05/18 23:55:55, willchan wrote:
> > > On 2011/05/18 21:45:18, rvargas wrote:
> > > > nit: I would follow the current style of not commenting out the argument
> > name.
> > > 
> > > 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"
> > 
> > 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.
> 
> 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?

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

http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
File net/proxy/network_delegate_error_observer_unittest.cc (right):

http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
net/proxy/network_delegate_error_observer_unittest.cc:44: virtual void
OnBytesRead(const URLRequest& /* request */,
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:
> 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:
> > On 2011/05/19 00:40:33, rvargas wrote:
> > > On 2011/05/18 23:55:55, willchan wrote:
> > > > On 2011/05/18 21:45:18, rvargas wrote:
> > > > > nit: I would follow the current style of not commenting out the
argument
> > > name.
> > > > 
> > > > 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"
> > > 
> > > 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.
> > 
> > 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 02:50:00]

http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
File net/proxy/network_delegate_error_observer_unittest.cc (right):

http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
net/proxy/network_delegate_error_observer_unittest.cc:44: virtual void
OnBytesRead(const URLRequest& /* request */,
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:
> 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:
> > 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:
> > > On 2011/05/19 00:40:33, rvargas wrote:
> > > > On 2011/05/18 23:55:55, willchan wrote:
> > > > > On 2011/05/18 21:45:18, rvargas wrote:
> > > > > > nit: I would follow the current style of not commenting out the
> argument
> > > > name.
> > > > > 
> > > > > 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"
> > > > 
> > > > 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.
> > > 
> > > 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?
> > 
>

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

http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
File net/proxy/network_delegate_error_observer_unittest.cc (right):

http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
net/proxy/network_delegate_error_observer_unittest.cc:44: virtual void
OnBytesRead(const URLRequest& /* request */,
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:
> 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:
> > 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:
> > > 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:
> > > > On 2011/05/19 00:40:33, rvargas wrote:
> > > > > On 2011/05/18 23:55:55, willchan wrote:
> > > > > > On 2011/05/18 21:45:18, rvargas wrote:
> > > > > > > nit: I would follow the current style of not commenting out the
> > argument
> > > > > name.
> > > > > > 
> > > > > > 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"
> > > > > 
> > > > > 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.
> > > > 
> > > > 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?
> > > 
> > 
>

[willchan no longer on Chromium, 2011-05-20 23:46:51]

Oh, I don't have time to get to this today before you take a look again,
since I'm busy with code reviews. I think I saw you were on vacation next
week? If you're going to be gone for awhile, can I get a LGTM? I'll take
care of undoing the ignored parameters. I'd prefer not to be blocked
long-term on this. If you'll be back on Monday, I'm fine waiting until then
for a review.

On Fri, May 20, 2011 at 3:57 PM, <willchan@chromium.org> wrote:

>
>
>
http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
> File net/proxy/network_delegate_error_observer_unittest.cc (right):
>
>
>
http://codereview.chromium.org/7043007/diff/5002/net/proxy/network_delegate_e...
> net/proxy/network_delegate_error_observer_unittest.cc:44: virtual void
> OnBytesRead(const URLRequest& /* request */,
> 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:
>
>> 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:
>> > 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:
>> > > 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:
>> > > > On 2011/05/19 00:40:33, rvargas wrote:
>> > > > > On 2011/05/18 23:55:55, willchan wrote:
>> > > > > > On 2011/05/18 21:45:18, rvargas wrote:
>> > > > > > > nit: I would follow the current style of not commenting
>>
> out the
>
>> > argument
>> > > > > name.
>> > > > > >
>> > > > > > 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"
>> > > > >
>> > > > > 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.
>> > > >
>> > > > 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?
>> > >
>> >
>>
>
>
> http://codereview.chromium.org/7043007/
>

[rvargas (doing something else), 2011-05-21 01:00:44]

Sorry I missed that you updated the CL.

LGTM (I can take another look over the weekend if you want me to. Are you flying
this weekend?)

http://codereview.chromium.org/7043007/diff/4003/net/url_request/url_request_...
File net/url_request/url_request_job.cc (right):

http://codereview.chromium.org/7043007/diff/4003/net/url_request/url_request_...
net/url_request/url_request_job.cc:35:
base::SystemMonitor::Get()->AddObserver(this);
We probably should allow the case of Get() returning NULL.

[willchan no longer on Chromium, 2011-05-21 01:58:19]

Very possibly;) I'll let you know.
On May 20, 2011 6:00 PM, <rvargas@chromium.org> wrote:
>
> Sorry I missed that you updated the CL.
>
> LGTM (I can take another look over the weekend if you want me to. Are you
flying
> this weekend?)
>
>
>
>
>
>
>
http://codereview.chromium.org/7043007/diff/4003/net/url_request/url_request_...
> File net/url_request/url_request_job.cc (right):
>
>
http://codereview.chromium.org/7043007/diff/4003/net/url_request/url_request_...
> net/url_request/url_request_job.cc:35:
> base::SystemMonitor::Get()->AddObserver(this);
> We probably should allow the case of Get() returning NULL.
>
>
> http://codereview.chromium.org/7043007/

[willchan no longer on Chromium, 2011-05-21 23:33:23]

OK, updated! I'll probably send it to the commit bots tomorrow.

[commit-bot: I haz the power, 2011-05-22 19:01:01]

Change committed as 86241