Closed Bug 813438 Opened 7 years ago Closed 7 years ago

Add NS_SniffContent

Categories

(Core :: Audio/Video, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(4 files, 1 obsolete file)

This will prevent code duplication when implementing deocodeAudioData.
Attached patch Patch (v1) (obsolete) — Splinter Review
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #683449 - Flags: review?(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?
Attachment #683449 - Flags: review?(bzbarsky) → review-
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?
I guess I'll put it in this other dumpster known as nsNetUtil.h.  ;-)
Depends on: 813787
Attachment #683449 - Attachment is obsolete: true
Attachment #683799 - Flags: review?(bzbarsky)
Attachment #683800 - Flags: review?(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.
Attachment #683799 - Flags: review?(bzbarsky) → review+
Comment on attachment 683799 [details] [diff] [review]
Part 1: Implement NS_SniffContent

Actually, maybe NS_DATA_SNIFFER_CATEGORY for the new category name?
Comment on attachment 683800 [details] [diff] [review]
Part 2: Remove the I/O service content sniffer cache

r=me
Attachment #683800 - Flags: review?(bzbarsky) → review+
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 on attachment 683801 [details] [diff] [review]
Part 3: Rewrite the unknown decoder to use NS_SniffContent

r=me
Attachment #683801 - Flags: review?(bzbarsky) → review+
Comment on attachment 683802 [details] [diff] [review]
Part 4: Rewrite AsyncFaviconHelper to use NS_SniffContent

r=me.

Sorry for the lag here...
Attachment #683802 - Flags: review?(bzbarsky) → review+
(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.
Summary: Add nsContentUtils::SniffContent → Add NS_SniffContent
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.
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
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.
Interesting.  Also, it's not clear to me why this compiles elsewhere... Perhaps because something somewhere #includes nsNetUtil.h in time?
You need to log in before you can comment on or make changes to this bug.