757882 - remove a 200+ redundant lines from nsAHttpConnection implementations

Status: RESOLVED FIXED

(Core :: Networking: HTTP, enhancement)

Description

[Patrick McManus [:mcmanus]]

use macros to delete 200+ lines of code. yea!

depends on spdy3 (737470) because that patch uses some of the members impacted by this patch.

Comment 1

[Patrick McManus [:mcmanus]]

Attachment: patch 0

Comment 2

[Jason Duell]

Comment on attachment 626483 [details] [diff] [review]
patch 0

Review of attachment 626483 [details] [diff] [review]:
-----------------------------------------------------------------

A bit gross to do this all with the preprocessor (an alternative could be to have all these classes implement some sort of "nsAHttpConnectionForwarder<class Cxn>" class, with methods that call "Cxn->ResumeSend()", etc.  But that's just an aesthetic, newfangled C++ for newfangled C++'s sake preference.  And you're piggybacking on the existing preprocessor stuff, so I guess it's fine...

::: netwerk/protocol/http/nsAHttpConnection.h
@@ +173,4 @@
>      PRUint32 CancelPipeline(nsresult originalReason);   \
> +    nsAHttpTransaction::Classifier Classification();      \
> +    /*                                                    \
> +       Thes methods below have automatic definitions that just forward the \

s/Thes/These/

@@ +192,5 @@
> +      }                                                   \
> +      return (fwdObject)->GetSecurityInfo(result);        \
> +    }                                                     \
> +    nsresult ResumeSend()                  \
> +    {                                      \

If you're going to use fixed-column position backslashes, I suggest you always go with putting them in column 80, so if a change to the function makes one line longer, you don't have to either reindent everything (and lose hg blame), or leave it looking ugly.  I believe the 80-column rule is the convention in the JS engine, FWIW.   OTOH, the rest of this macro just uses even uglier "put the slash just after the last character", so really, anything's an improvement :)

Comment 3

[Patrick McManus [:mcmanus]]

(In reply to Jason Duell (:jduell) from comment #2)
> Comment on attachment 626483 [details] [diff] [review]
> patch 0
> 
> Review of attachment 626483 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> A bit gross to do this all with the preprocessor (an alternative could be to

Man, I really thought you were the perfect audience for a remove code patch :) !

Anyhow I put those back slashes all in line. They won't hurt anybody now.

Thanks!

Comment 4

[Patrick McManus [:mcmanus]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2cfcbe649545

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2cfcbe649545