813438 - Add NS_SniffContent

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[(no longer active)]

This will prevent code duplication when implementing deocodeAudioData.

Comment 1

[(no longer active)]

Attachment: Patch (v1)

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 683449 [details] [diff] [review]
Patch (v1)

This is changing the behavior of the favicon code: "content-sniffing-services" and "net-content-sniffers" are not the same category...

Which of those two sorts of sniffing are you after?  Presumably the "content-sniffing-services" kind, yes?

Comment 3

[(no longer active)]

Hmm, no I'm actually interested in net-content-sniffers (that's what toolkit/components/contentsniffer is, which is the thing that I really care about here.)

So there's a bunch of places in netwerk/ which use this, but they can't depend on content/.  And the toolkit/ callsite uses content-sniffing-services.  So I wonder if there is any real value in putting this in nsContentUtils.

What do you think?

Comment 4

[(no longer active)]

I guess I'll put it in this other dumpster known as nsNetUtil.h.  ;-)

Comment 5

[(no longer active)]

Attachment: Part 1: Implement NS_SniffContent

Comment 6

[(no longer active)]

Attachment: Part 2: Remove the I/O service content sniffer cache

Comment 7

[(no longer active)]

Attachment: Part 3: Rewrite the unknown decoder to use NS_SniffContent

Comment 8

[(no longer active)]

Attachment: Part 4: Rewrite AsyncFaviconHelper to use NS_SniffContent

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 683799 [details] [diff] [review]
Part 1: Implement NS_SniffContent

Please document the valid values of aSnifferType.

And perhaps s/aSniffedContent/aSniffedType/?

Also document that aSniffedType will not be modified if no type was sniffed?

r=me with that.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 683799 [details] [diff] [review]
Part 1: Implement NS_SniffContent

Actually, maybe NS_DATA_SNIFFER_CATEGORY for the new category name?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 683800 [details] [diff] [review]
Part 2: Remove the I/O service content sniffer cache

r=me

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 683799 [details] [diff] [review]
Part 1: Implement NS_SniffContent

Actually, one more question.  Won't this cause shutdown leaks because those static objects hold an nsISupports pointer to the category observer?

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 683801 [details] [diff] [review]
Part 3: Rewrite the unknown decoder to use NS_SniffContent

r=me

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 683802 [details] [diff] [review]
Part 4: Rewrite AsyncFaviconHelper to use NS_SniffContent

r=me.

Sorry for the lag here...

Comment 15

[(no longer active)]

(In reply to Boris Zbarsky (:bz) from comment #12)
> Comment on attachment 683799 [details] [diff] [review]
> Part 1: Implement NS_SniffContent
> 
> Actually, one more question.  Won't this cause shutdown leaks because those
> static objects hold an nsISupports pointer to the category observer?

Hmm, yeah it does.  I'll have to put those in an extern variable and clear them when the net module shuts down, I guess.

Comment 16

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4f55f76e34ee
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a518d6c8b60
https://hg.mozilla.org/integration/mozilla-inbound/rev/383b6031c417
https://hg.mozilla.org/integration/mozilla-inbound/rev/231cce402e46

Comment 17

[Ed Morley [:emorley]]

Unfortunately backed out for various timeouts:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=231cce402e46

https://hg.mozilla.org/integration/mozilla-inbound/rev/607fa073ae8e

Comment 18

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/704cd2fef9e8
https://hg.mozilla.org/integration/mozilla-inbound/rev/a10521472bdf
https://hg.mozilla.org/integration/mozilla-inbound/rev/883c4223dd20
https://hg.mozilla.org/integration/mozilla-inbound/rev/1002aa671831
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e3dfbc9d798

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/704cd2fef9e8
https://hg.mozilla.org/mozilla-central/rev/a10521472bdf
https://hg.mozilla.org/mozilla-central/rev/883c4223dd20
https://hg.mozilla.org/mozilla-central/rev/1002aa671831
https://hg.mozilla.org/mozilla-central/rev/2e3dfbc9d798

Comment 20

[Axel Hecht [:Pike]]

I'm failing to compile the desktop b2g debug build on osx now, with similar errors to http://logbot.glob.com.au/?c=mozilla%23developers&s=27+Nov+2012&e=27+Nov+2012&h=nsNetUtil#c444593, or

/usr/local/bin/ccache /usr/bin/clang++ -o nsNetModule.o -c  -fvisibility=hidden -DXPCOM_TRANSLATE_NSGM_ENTRY_POINT=1 -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  -DSTATIC_EXPORTABLE_JS_API -DNO_NSPR_10_SUPPORT -DEXCLUDE_SKIA_DEPENDENCIES  -DOS_POSIX=1 -DOS_MACOSX=1  -DIMPL_NS_NET -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../base/src -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../dns -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../socket -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../streamconv/src -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../streamconv/converters -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../mime -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../cache -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../protocol/about -I../dns  -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../protocol/data  -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../protocol/file  -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../protocol/ftp  -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../protocol/http  -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../protocol/res  -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../protocol/viewsource  -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../protocol/websocket  -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../protocol/wyciwyg  -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../protocol/device  -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../system/mac -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../cookie -I/Users/axelhecht/src/central/mozilla-central/netwerk/build/../wifi -I/Users/axelhecht/src/central/mozilla-central/ipc/chromium/src -I/Users/axelhecht/src/central/mozilla-central/ipc/glue -I../../ipc/ipdl/_ipdlheaders  -I/Users/axelhecht/src/central/mozilla-central/netwerk/build -I. -I../../dist/include  -I/Users/axelhecht/src/central/mozilla-central/obj-b2g-debug/dist/include/nspr -I/Users/axelhecht/src/central/mozilla-central/obj-b2g-debug/dist/include/nss      -fPIC -Qunused-arguments  -Qunused-arguments -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Wtype-limits -Wempty-body -Wno-ctor-dtor-privacy -Wno-overlength-strings -Wno-invalid-offsetof -Wno-variadic-macros -Wno-c++0x-extensions -Wno-extended-offsetof -Wno-unknown-warning-option -Wno-return-type-c-linkage -Wno-mismatched-tags -DMOZ_ENABLE_JS_DUMP -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -std=gnu++0x -pthread -DNO_X11 -pipe  -DDEBUG -D_DEBUG -DTRACING -g -fno-omit-frame-pointer  -Werror -Wno-error=uninitialized  -Qunused-arguments  -DMOZILLA_CLIENT -include ../../mozilla-config.h -MD -MF .deps/nsNetModule.o.pp  /Users/axelhecht/src/central/mozilla-central/netwerk/build/nsNetModule.cpp
In file included from /Users/axelhecht/src/central/mozilla-central/netwerk/build/nsNetModule.cpp:85:
In file included from /Users/axelhecht/src/central/mozilla-central/netwerk/build/../base/src/nsInputStreamChannel.h:9:
In file included from /Users/axelhecht/src/central/mozilla-central/netwerk/build/../base/src/nsBaseChannel.h:26:
../../dist/include/nsNetUtil.h:2173:43: error: attribute declaration must precede definition [-Werror]
  extern NS_HIDDEN_(ContentSnifferCache*) gNetSniffers;

wokbok:mozilla-central axelhecht$ hg diff netwerk/
diff --git a/netwerk/build/nsNetModule.cpp b/netwerk/build/nsNetModule.cpp
--- a/netwerk/build/nsNetModule.cpp
+++ b/netwerk/build/nsNetModule.cpp
@@ -36,6 +36,7 @@
 #include "nsXULAppAPI.h"
 #include "nsCategoryCache.h"
 #include "nsIContentSniffer.h"
+#include "nsNetUtil.h"
 
 #include "nsNetCID.h"
 
helps, though.

Comment 21

[(no longer active)]

This error does not make a lot of sense to me; it seems like gcc doesn't like to see  a declaration of a variable after its definition (which I thought would be a no-op).

Anyways, I pushed the fix: https://hg.mozilla.org/integration/mozilla-inbound/rev/38324b54e6d6

Comment 22

[Jeff Walden [:Waldo]]

The issue is the code looks basically like this:

  __attribute__((visibility("hidden"))) int* gPtr = nullptr; // in the .cpp
  inline void foo() // in a header #included in the .cpp
  {
    extern __attribute__((visibility("hidden"))) int* gPtr;
  }

The first line here declares gPtr not as an extern.  Meanwhile the later line *does* declare it as an extern.  The fix is to make sure gPtr is declared (through the header) before the definition is seen.

That said, I'm not sure why it's complaining about *attribute declaration*, rather than about an extern being seen after the symbol was first defined without an extern.

Comment 23

[(no longer active)]

Interesting.  Also, it's not clear to me why this compiles elsewhere... Perhaps because something somewhere #includes nsNetUtil.h in time?

Comment 24

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/38324b54e6d6