Change nsIChannel to support 64-bit content-length

RESOLVED FIXED in mozilla19

Status

()

Core
Networking: HTTP
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: jduell, Assigned: nwgh)

Tracking

(Blocks: 1 bug, {dev-doc-complete})

Other Branch
mozilla19
x86
Linux
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 6 obsolete attachments)

2.27 KB, patch
nwgh
: review+
Details | Diff | Splinter Review
35.68 KB, patch
nwgh
: review+
Details | Diff | Splinter Review
34.37 KB, patch
nwgh
: review+
Details | Diff | Splinter Review
2.67 KB, patch
nwgh
: review+
Details | Diff | Splinter Review
Another feature of necko HTTP channels that maps badly to e10s is the fact that necko channels are also nsHashPropertyBags, i.e. hashtables that clients can stuff various things into and then pull out later.

I would really like to avoid sending IPC traffic around for get/sets of these hashtables.   bz seems to think that in most cases, properties will only be retrieved by the same parent or child that set them, so we may not have to worry about this too much--just document that we don't support retrieval of properties set across a process boundary.

We have at least one known exception to this--we set a 'referer' property that may need to cross process boundaries.  There may be others.  So we should do a code or runtime audit of what gets set/retrieved to see what may need special handling.

Properties that need to be sent across the wire probably ought to be removed from the hashbag and instead put somewhere like nsIHttpChannelInternal.idl.  Actually, if we could move *all* uses to nsIHttpChannelInternal and/or something sensible like a context "void *" member variable, I'd be much happier.  Storing cruft in your objects in a big hashtable and doing lookups by name at runtime is the kind of programming that belongs in Javascript, not C++.
(Reporter)

Comment 1

7 years ago
Biesi tells me we also stuff in a 64-bit Content-length field into our PropertyBags, too (as NS_CHANNEL_PROP_CONTENT_LENGTH).  This is used by nsExternalHelperAppService and nsIncrementalDownload, at least, and possibly other things.  This may also need to be propagated across process boundaries (though it also might not, assuming uriloader lives in the chrome process, and so do nsExternalHelperAppService/nsIncrementalDownload).

It's also possible that we'll need to propagate NS_CHANNEL_PROP_CONTENT_DISPOSITION, which is used by the JAR channel (which may have an Http channel target, i.e. jar:http://www.foo.com/foo.jar) and URILoader.

While I still think it's a splendid idea to refactor nsHashBag out of our channels, realistically I think we just need to keep the API for now and propagate the the handful of properties that need it.
(Reporter)

Comment 2

7 years ago
We should feel free to change the API for necko to help get rid of these hashbag properties, particularly the ones that need to be propagated between child/chrome.

Bsmedberg suggests we change the APIs on mozilla-central first, and then propagate to the e10s branch.

Comment 3

7 years ago
I can take this. We should break this stuff sooner rather than later, if we're going to do it.
Assignee: nobody → dwitte

Comment 4

7 years ago
I have a patch that adds an nsIChannel2 with a contentLength64 property, and changes the dozen or so nsIChannel implementations in the tree to also implement nsIChannel2.

Frankly it's pretty gross, but we can do it if necessary. And that's just for that one property. There'll be other hashbag-related ones to come.

I'd rather wait to see if we're going to break binary compatibility on nsIChannel for other reasons. If so, the contentLength thing becomes much nicer.

Comment 5

7 years ago
We've decided that binary changes are OK now, right? (Is there a deadline for such changes? Last beta?)

The changes here will break script compat too, of course, but in a pretty visible way -- we'll throw when trying to QI to the property bag. Extensions should figure that out given enough beta time.

Comment 6

7 years ago
Getting this on blocker radar -- this needs to block b5 if we're going to do it. And we need to do something here, one way or the other. :)
blocking2.0: --- → ?

Comment 7

7 years ago
Created attachment 467183 [details] [diff] [review]
part 1: make nsIChannel.contentLength 64-bit

This breaks the interface. Looking for r & sr here. FWIW it looks like there isn't any script in our tree that uses the "content-length" hash property, so no script changes required as a result of this.
Attachment #467183 - Flags: superreview?(bzbarsky)
Attachment #467183 - Flags: review?(jduell.mcbugs)

Comment 8

7 years ago
Created attachment 467186 [details] [diff] [review]
part 2: implementors
Attachment #467186 - Flags: review?(jduell.mcbugs)

Comment 9

7 years ago
Created attachment 467187 [details] [diff] [review]
part 3: consumers
Attachment #467187 - Flags: review?(jduell.mcbugs)

Comment 10

7 years ago
After these three patches, there are no remaining references to the "content-length" hash property in the tree.

Comment 11

7 years ago
Comment on attachment 467183 [details] [diff] [review]
part 1: make nsIChannel.contentLength 64-bit

Actually, bz's on vacation, so I'll ping jst instead. ;)
Attachment #467183 - Flags: superreview?(bzbarsky) → superreview?(jst)

Updated

7 years ago
Attachment #467183 - Flags: superreview?(jst) → superreview+
(Reporter)

Updated

7 years ago
Attachment #467183 - Flags: review?(jduell.mcbugs) → review+
(Reporter)

Comment 12

7 years ago
Comment on attachment 467186 [details] [diff] [review]
part 2: implementors

+r with changes discuss on IRC.

Still makes me nervous to get rid of ENSURE_ARG macros, but dwitte makes good case we'll never get passed null.
Attachment #467186 - Flags: review?(jduell.mcbugs) → review+

Comment 13

7 years ago
I've built a list of all the in-tree uses of nsIPropertyBag/nsIWritablePropertyBag on channels.

Now the hard part: figuring out what to do with them. ;)

"docshell.previousURI":
http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#10332
http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#10367

"docshell.internalReferrer":
http://mxr.mozilla.org/mozilla-central/source/xpinstall/src/nsInstallTrigger.cpp#149
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/amContentHandler.js#73
http://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/nsExternalHelperAppService.cpp#1760
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/downloads/src/nsDownloadManager.cpp#2360
http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#9282
http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#8565

"docshell.newWindowTarget":
http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#8698
http://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/nsExternalHelperAppService.cpp#1548

"docshell.previousFlags":
http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#10369
http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#10346

"baseURI":
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp#1940
http://mxr.mozilla.org/mozilla-central/source/netwerk/test/unit/test_aboutblank.js#18
http://mxr.mozilla.org/mozilla-central/source/dom/src/jsurl/nsJSProtocolHandler.cpp#523
http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/about/nsAboutProtocolHandler.cpp#175

"channel-policy":
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsCSPService.cpp#221
http://mxr.mozilla.org/mozilla-central/source/netwerk/base/public/nsNetUtil.h#230
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsCSPService.cpp#287

"content-disposition":
http://mxr.mozilla.org/mozilla-central/source/modules/libjar/nsJARChannel.cpp#793
http://mxr.mozilla.org/mozilla-central/source/modules/libjar/nsJARChannel.cpp#804

Comment 14

7 years ago
My interpretations so far:

"content-disposition": not used in any meaningful way. http://mxr.mozilla.org/mozilla-central/source/netwerk/base/public/nsNetUtil.h#241 says it's the way of the future, but reality disagrees.

"channel-policy": we could add an interface and put this property on it, and have nsHttpChannel implement it. bsterne and dveditz say that'd be fine; we only really need to support it for httpchannels. Redirects from http --> non-http can just ditch the CSP info and fail.

"docshell.internalReferrer": we might be able to get equivalent info by getting the principal directly from the channel (via the notification callbacks).

"docshell.previousFlags": used for tracking redirects for history purposes.
(Reporter)

Comment 15

7 years ago
Comment on attachment 467187 [details] [diff] [review]
part 3: consumers

+r with fix to keep check for -1 in nsGnomeVFSInputStream::DoOpen.

We remove a lot of casts in this patch, which is great, unless we're adding compile-time warnings.  We have some cases of assigning 64 bit ints to 32 bit ones, which will probably want casts to avoid warnings.  Maybe grep make output before/after to compare warning count?

Otherwise looks great.  Thanks!   That was a a lot of code to chug through.
Attachment #467187 - Flags: review?(jduell.mcbugs) → review+
(Reporter)

Comment 16

7 years ago
Oh, and we should definitely run these patches through tryserver.
(Reporter)

Comment 17

7 years ago
> Redirects from http --> non-http can just ditch the CSP info and fail.

This doesn't mean all redirects from http to FTP would fail, does it?  That doesn't sound good.

Death to channel PropertyBagHashes!
(Reporter)

Comment 18

7 years ago
Should we split the Content-length patches into a separate bug and land once the tree is open for bizness?

Comment 19

7 years ago
Hmm, nevermind about "content-disposition", I missed a usage in nsURILoader. It was added in bug 474536.

It sounds like we could move that to a property on the channel.

I suppose the more useful thing to do at this point is figure out which of the properties in comment 13 only need to exist on either the parent or child but not both. Those can be left alone. The rest need to be dealt with.

Comment 20

7 years ago
(In reply to comment #17)
> This doesn't mean all redirects from http to FTP would fail, does it?  That
> doesn't sound good.

Only if the httpchannel had a CSP, in which case we can't redirect to FTP because the CSP info would be lost, allowing it to redirect back.

(In reply to comment #18)
> Should we split the Content-length patches into a separate bug and land once
> the tree is open for bizness?

I'll just land these here, and leave the bug open for more stuff.

Comment 21

7 years ago
bsterne's content policy tests (http://people.mozilla.org/~bsterne/content-security-policy/demo.cgi) pass in Fennec now that we have redirect stuff done, so changing "channel-policy" is definitely not a blocker.

Updated

7 years ago
Depends on: 589292
Blocking beta5 on this. The primary reasoning here is to make this API change before the release instead of after to minimize the API changes we'll need to do. We've already changed nsIChannel, so one more change now is effectively free for us and others who depend on this.
blocking2.0: ? → beta5+

Comment 23

7 years ago
Comment on attachment 467183 [details] [diff] [review]
part 1: make nsIChannel.contentLength 64-bit

http://hg.mozilla.org/projects/electrolysis/rev/8f84c33993c1

Comment 24

7 years ago
Comment on attachment 467186 [details] [diff] [review]
part 2: implementors

http://hg.mozilla.org/projects/electrolysis/rev/b69195520ef4

Comment 25

7 years ago
Comment on attachment 467187 [details] [diff] [review]
part 3: consumers

http://hg.mozilla.org/projects/electrolysis/rev/3c7ac02b55d3

Comment 26

7 years ago
Merged to m-c. I'll file separate bugs for the other bits as necessary.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
http://hg.mozilla.org/mozilla-central/rev/8f84c33993c1
http://hg.mozilla.org/mozilla-central/rev/b69195520ef4
http://hg.mozilla.org/mozilla-central/rev/3c7ac02b55d3

Updated

7 years ago
No longer depends on: 589292

Updated

7 years ago
Depends on: 589292
(Reporter)

Comment 28

7 years ago
Looks like we want to back this out? 

bsmedberg wrote on dev.platform:

The IID of nsIChannel was changed between beta4 and beta5, for bugs 589292 and
536324. While I agree that these are virtuous changes, I don't think we should
be changing the IIDs of base interfaces so late in the beta cycle. It's going
to be very difficult to distinguish between crashes caused by third-party
extensions which were compiled against beta4, and crashes caused by issues
within our own code.

This change *might* be related to the topcrash bugs 591880 and 591678, but I
really can't tell.

Unless these changes were absolutely necessary for e10s (and it doesn't look to
me as if they were), I think these should be backed out and postponed until
after branching.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Depends on: 591997

Comment 29

7 years ago
I'm not so sure we want to back this out. (The contentDisposition change isn't necessary, and we could back that out, but since we took the contentLength change we got that one for free.)

I don't have a complete understanding of what would break if we backed this out, but it worries me that people rely on the contentLength hash property and we have no easy way of remoting it. We could do a bunch of work inside nsHashPropertyBag itself, but that'd suck.

It might be that we get lucky and everything that depends on this lives in the child, but right now I'm not sure whether that's true or not.
We can rely on the contentLength hash property by implementing nsIHashPropertyBag and providing that key directly, can't we? That wouldn't involve interface changes.

Comment 31

7 years ago
Yep, I suppose we could specialcase it. Want me to roll that patch?
Yes please.
Being fixed in bug 591997?

Comment 34

7 years ago
Not blocking. -> Firefox 5
blocking2.0: beta5+ → ---
Target Milestone: --- → Future
> "docshell.internalReferrer": we might be able to get equivalent info by getting
> the principal directly from the channel (via the notification callbacks).

No, you can't.  The principal of the docshell is not immutable.  So if you want the principal it had when the channel was opened, you need to grab it when the channel is opened.

Comment 36

7 years ago
Would this make sense as a property (on nsIChannel?), i.e. is it something we're cool exposing to script? (Or can we expose the same information in a different way?)
We're cool exposing it to chrome script (heck, it already is!).
The other option is to make .referrer on nsIHttpChannel be the always-set thing and just have an mInternalReferrer member for what goes on the wire.  But then you might want people to know what goes on the wire...
If people want to know what goes on the wire, shouldn't they use getRequestHeader("referer")?
Hmm... Probably.  But that won't do the right thing before on-modify-request, right?
HttpBaseChannel::SetReferrer looks to me like it should work anytime.
Er... it doesn't.  It bails out in all sorts of cases, no?
Yes, in the ones where nothing goes out on the wire.

Comment 44

6 years ago
Reassigning to nobody. If anyone wants to work on this, feel free!
Assignee: dwitte → nobody
Status: REOPENED → NEW

Comment 45

6 years ago
Did these happy patches simply never land?
Assignee: nobody → crowderbt
(Reporter)

Comment 46

6 years ago
These patches just need to be relanded, but I want us to hold off until I figure out how much they conflict with those from bug 589292 and/or 215450.

renaming, since this is now much narrower than getting rid of nsHashBag entirely.
Summary: e10s HTTP: refactor nsHashPropertyBag → Change nsIChannel to support 64-bit content-length
(Reporter)

Comment 47

6 years ago
This will change nsIChannel to a 64 bit content-length (only binary addons should be affected). See "Part 1" patch.
Keywords: dev-doc-needed
-> me on Jason's request.
Assignee: crowderbt → honzab.moz
Status: NEW → ASSIGNED

Updated

5 years ago
Blocks: 740122
Taking on Josh's request
Assignee: honzab.moz → hurley
Ok, no problem.  But please ask next time, I just wanted to start with this today...  If the status is ASSIGNED, then I intend to work on this and sometimes I even already do so.  It is on my priority list too.
Created attachment 668560 [details] [diff] [review]
part 1: make nsIChannel.contentLength 64-bit

Un-bitrotted, carrying forward r+ and sr+ since it's the same bits, just updated for modern times.
Attachment #467183 - Attachment is obsolete: true
Attachment #668560 - Flags: superreview+
Attachment #668560 - Flags: review+
Created attachment 668561 [details] [diff] [review]
part 2: implementors

Un-bitrotted part 2. Re-requesting review, as things have changed enough in the last year to warrant it.
Attachment #467186 - Attachment is obsolete: true
Attachment #668561 - Flags: review?(jduell.mcbugs)
Created attachment 668562 [details] [diff] [review]
part 3: consumers

Un-bitrotted part 3. Re-requesting review for the same reason as part 2.
Attachment #467187 - Attachment is obsolete: true
Attachment #668562 - Flags: review?(jduell.mcbugs)
The three patches are running through try right now, https://tbpl.mozilla.org/?tree=Try&rev=7662e20a8996

We'll see just how much I broke :)
(Reporter)

Comment 55

5 years ago
Comment on attachment 668561 [details] [diff] [review]
part 2: implementors

I'm delegating this review to Steve.
Attachment #668561 - Flags: review?(jduell.mcbugs) → review?(sworkman)
(Reporter)

Updated

5 years ago
Attachment #668562 - Flags: review?(jduell.mcbugs) → review?(sworkman)
Created attachment 672064 [details] [diff] [review]
part 4: test

Now, with 100% more testing! r?sworkman since jduell already delegated the reviews for this bug to him.
Attachment #672064 - Flags: review?(sworkman)
Comment on attachment 668561 [details] [diff] [review]
part 2: implementors

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

Looks good r=me.
Attachment #668561 - Flags: review?(sworkman) → review+
Comment on attachment 668562 [details] [diff] [review]
part 3: consumers

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

Looks good - just a couple of minor things to look at.

::: extensions/gnomevfs/nsGnomeVFSProtocolHandler.cpp
@@ -432,5 @@
>        mBytesRemaining = info.size;
>  
>        // Update the content length attribute on the channel.  We do this
>        // synchronously without proxying.  This hack is not as bad as it looks!
> -      if (mBytesRemaining != UINT64_MAX) {

Discussed this with you in person: do you need to keep this UINT64_MAX check? And change NS_MAX(..., INT32_MAX) to NS_MAX(...,INT64_MAX)?

::: js/xpconnect/loader/mozJSSubScriptLoader.cpp
@@ +107,2 @@
>      nsCString buf;
>      rv = NS_ReadInputStreamToString(instream, buf, len);

Looks like len in NS_ReadInputStreamToString is uin32_t. Could your check above be '> UINT32_MAX' instead of '> INT32_MAX'?

::: netwerk/base/src/nsStreamLoader.cpp
@@ +72,5 @@
>      chan->GetContentLength(&contentLength);
>      if (contentLength >= 0) {
> +      if (contentLength > UINT32_MAX) {
> +        // Too big to fit into uint32, so let's bail.
> +        // XXX we should really make mAllocated and mLength 64-bit instead.

Discussed this with you. I understand it's a bit of a rathole to fix mAllocated and mLength and out of the original scope of this bug. So, no worries.

::: netwerk/protocol/ftp/FTPChannelParent.cpp
@@ +170,1 @@
>    chan->GetContentLength(&aContentLength);

Nitpick: Since you're already changing this line, can you rename aContentLength to contentLength to clarify that it's declared locally?

::: uriloader/exthandler/nsExternalHelperAppService.cpp
@@ +558,5 @@
>      channel->GetURI(getter_AddRefs(uri));
>  
> +  int64_t contentLength = -1;
> +  if (channel)
> +    channel->GetContentLength(&contentLength);

Is it ok to aggregate these two 'if (channel)' statements?

::: uriloader/prefetch/nsPrefetchService.cpp
@@ +810,5 @@
>      if (mChannel) {
> +        int64_t size64;
> +        nsresult rv = mChannel->GetContentLength(&size64);
> +        NS_ENSURE_SUCCESS(rv, rv);
> +        *aTotalSize = int32_t(size64); // XXX - loses precision

FYI: Looks like this truncates the bytes (I know we discussed it in person, but I had a little niggling thought, so I checked on my Mac and I'm getting truncated output, not smart casting). So, maybe you want to do something like the old nsBaseChannel::GetContentLength, and return (-1) if it's more than INT32_MAX. OR just truncate like HttpBaseChannel::GetContentLength. In any case, I'll let you decide how you want to proceed - leave it, or whatever :)
Attachment #668562 - Flags: review?(sworkman) → review+
Comment on attachment 672064 [details] [diff] [review]
part 4: test

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

::: netwerk/test/unit/test_bug536324_64bit_content_length.js
@@ +6,5 @@
> +const Cr = Components.results;
> +
> +Cu.import("resource://testing-common/httpd.js");
> +
> +const CONTENT_LENGTH = "1152921504606846975";

Can you add a comment to say what this is in reference to (U)INT32_MAX? :)

@@ +26,5 @@
> +    httpServer.stop(do_test_finished);
> +  }
> +};
> +
> +function hugeContentLength(metadata, response) {

'huge' should be in caps ;)
Attachment #672064 - Flags: review?(sworkman) → review+
Created attachment 673909 [details] [diff] [review]
part 2: implementors (version for checkin)

Updated part 2 patch for checkin. Carry forward r+
Attachment #668561 - Attachment is obsolete: true
Attachment #673909 - Flags: review+
Created attachment 673910 [details] [diff] [review]
part 3: consumers (version for checkin)

Updated part 3 for checkin. Carry forward r+
Attachment #668562 - Attachment is obsolete: true
Attachment #673910 - Flags: review+
Created attachment 673911 [details] [diff] [review]
part 4: test (version for checkin)

Updated part 4 for checkin. Carry forward r+
Attachment #672064 - Attachment is obsolete: true
Attachment #673911 - Flags: review+
Try looks good with the patch updates https://tbpl.mozilla.org/?tree=Try&rev=ca10b3e98d9b

Will land once the trees open.
Trees are open.
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2c998bad111
https://hg.mozilla.org/integration/mozilla-inbound/rev/90d8a63e061c
https://hg.mozilla.org/integration/mozilla-inbound/rev/af7e6c8e6d76
https://hg.mozilla.org/integration/mozilla-inbound/rev/4bb51e245a4e
https://hg.mozilla.org/mozilla-central/rev/c2c998bad111
https://hg.mozilla.org/mozilla-central/rev/90d8a63e061c
https://hg.mozilla.org/mozilla-central/rev/af7e6c8e6d76
https://hg.mozilla.org/mozilla-central/rev/4bb51e245a4e
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: Future → mozilla19

Comment 67

5 years ago
This broke Thunderbird:
/usr/bin/ccache /tools/gcc-4.5-0moz3/bin/g++ -o nsMsgQuickSearchDBView.o -c -I../../../mozilla/dist/stl_wrappers -I../../../mozilla/dist/system_wrappers -include /builds/slave/tb-c-cen-lnx/build/mozilla/config/gcc_hidden.h -DMOZ_LDAP_XPCOM -DMOZ_GLUE_IN_PROGRAM -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DZLIB_INTERNAL -DMOZ_THUNDERBIRD=1 -DOSTYPE=\"Linux2.6.32-220.el6\" -DOSARCH=Linux -DHAVE_MOVEMAIL  -I/builds/slave/tb-c-cen-lnx/build/mailnews/base/src -I. -I../../../mozilla/dist/include -I../../../mozilla/dist/include/nsprpub  `/builds/slave/tb-c-cen-lnx/build/objdir-tb/mozilla/dist/sdk/bin/nspr-config --prefix=/builds/slave/tb-c-cen-lnx/build/objdir-tb/mozilla/dist --includedir=/builds/slave/tb-c-cen-lnx/build/objdir-tb/mozilla/dist/include/nspr --cflags` -I/builds/slave/tb-c-cen-lnx/build/objdir-tb/mozilla/dist/include/nss      -fPIC  -pedantic -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Wtype-limits -Wempty-body -Werror=conversion-null -Wno-ctor-dtor-privacy -Wno-overlength-strings -Wno-invalid-offsetof -Wno-variadic-macros -Wcast-align -Wno-long-long -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -std=gnu++0x -pthread -pipe  -DNDEBUG -DTRIMMED -g -Os -freorder-blocks -finline-limit=50 -fno-omit-frame-pointer    -DMOZILLA_CLIENT -include ../../../comm-config.h -MD -MF .deps/nsMsgQuickSearchDBView.pp /builds/slave/tb-c-cen-lnx/build/mailnews/base/src/nsMsgQuickSearchDBView.cpp
nsMsgSearchDBView.cpp
../../../../mailnews/base/util/nsMsgProtocol.cpp:681:10: error: prototype for 'nsresult nsMsgProtocol::GetContentLength(int32_t*)' does not match any in class 'nsMsgProtocol'
../../../../mailnews/base/util/nsMsgProtocol.h:57:1350: error: candidate is: virtual nsresult nsMsgProtocol::GetContentLength(int64_t*)
../../../../mailnews/base/util/nsMsgProtocol.cpp:687:10: error: prototype for 'nsresult nsMsgProtocol::SetContentLength(int32_t)' does not match any in class 'nsMsgProtocol'
../../../../mailnews/base/util/nsMsgProtocol.h:57:1451: error: candidate is: virtual nsresult nsMsgProtocol::SetContentLength(int64_t)
make[7]: *** [nsMsgProtocol.o] Error 1

Comment 68

5 years ago
> This broke Thunderbird:
Standard8 pushed a bustage fix:
http://hg.mozilla.org/comm-central/rev/7a6093803e43
Added a note to https://developer.mozilla.org/en-US/docs/Firefox_19_for_developers and a call-out box to https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIChannel.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.