Closed Bug 757882 Opened 8 years ago Closed 8 years ago

remove a 200+ redundant lines from nsAHttpConnection implementations

Categories

(Core :: Networking: HTTP, enhancement)

x86_64
Linux
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: mcmanus, Assigned: mcmanus)

References

Details

Attachments

(1 file)

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.
Attached patch patch 0Splinter Review
Assignee: nobody → mcmanus
Status: NEW → ASSIGNED
Attachment #626483 - Flags: review?(jduell.mcbugs)
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 :)
Attachment #626483 - Flags: review?(jduell.mcbugs) → review+
(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!
https://hg.mozilla.org/mozilla-central/rev/2cfcbe649545
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.