remove a 200+ redundant lines from nsAHttpConnection implementations

RESOLVED FIXED in mozilla15

Status

()

Core
Networking: HTTP
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mcmanus, Assigned: mcmanus)

Tracking

unspecified
mozilla15
x86_64
Linux
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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.
Created attachment 626483 [details] [diff] [review]
patch 0
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/integration/mozilla-inbound/rev/2cfcbe649545
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/2cfcbe649545
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.