586566 - Move nsIconDecoder into libimg

Reporter

Description

•

14 years ago

bug 168048 moved most of the image-decoders into imagelib, but it left out libimgicon.  We should merge that in as well.  bholley has a patch.

(This has caused headaches in bug 584841 -- see bug 584841 comment 13, bug 584841 comment 15, bug 584841 comment 36, and bug 584841 comment 48.  Will be nice to have it merged in.)

Bobby Holley (:bholley)

Assignee

Comment 1

•

14 years ago

Adjusted the title to more accurately reflect what we're doing here. We can't easily move the icon channel stuff, as mentioned in bug 584841 comment 48. Luckily, just moving nsIconDecoder fixes our woes.

Summary: Merge libimgicon into imagelib → Move nsIconDecoder into libimg

Bobby Holley (:bholley)

Assignee

Comment 2

•

14 years ago

Attached patch patch v1 (obsolete) — Details — Splinter Review

Added a patch. This is meant to be applied on top of the roll-up patch in bug 584841, so I don't know if it applies cleanly on trunk. I'm hoping dholbert can figure this stuff out. ;)

Daniel Holbert [:dholbert]

Reporter

Comment 3

•

14 years ago

Attached patch patch v1a (patch by bholley, minor tweak by dholbert) — Details — Splinter Review

Here's bholley's patch again, tweaked to apply on trunk.

I just deleted this patch-chunk that modified libpr0n/decoders/icon/Makefile.in
>-# nsIconDecoder.cpp includes RasterImage.h
>-LOCAL_INCLUDES += -I$(topsrcdir)/modules/libpr0n/src/
>-

(That chunk removes some code added in bug 584841, and hence isn't applicable to stock mozilla-central. I'll modify the corresponding patch in bug 584841 so it layers on top of this patch here.)

Attachment #465125 - Attachment is obsolete: true

Daniel Holbert [:dholbert]

Reporter

Comment 4

•

14 years ago

Comment on attachment 465279 [details] [diff] [review]
patch v1a (patch by bholley, minor tweak by dholbert)

>From 42ece3d7166502eb251368ff54e9c23aca721b03 Mon Sep 17 00:00:00 2001
>From: Bobby Holley <bobbyholley@gmail.com>
>Date: Wed, 11 Aug 2010 19:03:17 -0400
>Subject: [PATCH] part 1 - v1 - Temporarily fold nsIconDecoder into decoders/bmp
>
>https://bugzilla.mozilla.org/show_bug.cgi?id=586566
>---
> modules/libpr0n/build/nsImageModule.cpp            |    6 ++++++
> modules/libpr0n/decoders/bmp/Makefile.in           |    2 +-
> .../decoders/{icon => bmp}/nsIconDecoder.cpp       |    0
> .../libpr0n/decoders/{icon => bmp}/nsIconDecoder.h |    0
> modules/libpr0n/decoders/icon/Makefile.in          |   15 ---------------
> modules/libpr0n/decoders/icon/nsIconModule.cpp     |   20 --------------------
> 6 files changed, 7 insertions(+), 36 deletions(-)
> rename modules/libpr0n/decoders/{icon => bmp}/nsIconDecoder.cpp (100%)
> rename modules/libpr0n/decoders/{icon => bmp}/nsIconDecoder.h (100%)
>
>diff --git a/modules/libpr0n/build/nsImageModule.cpp b/modules/libpr0n/build/nsImageModule.cpp
>index d63e1bc..302a359 100644
>--- a/modules/libpr0n/build/nsImageModule.cpp
>+++ b/modules/libpr0n/build/nsImageModule.cpp
>@@ -76,6 +76,7 @@
> // bmp/ico
> #include "nsBMPDecoder.h"
> #include "nsICODecoder.h"
>+#include "nsIconDecoder.h"
> #endif
> 
> #ifdef IMG_BUILD_DECODER_png
>@@ -127,6 +128,7 @@ NS_GENERIC_FACTORY_CONSTRUCTOR(nsJPEGEncoder)
> // bmp
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsICODecoder)
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsBMPDecoder)
>+NS_GENERIC_FACTORY_CONSTRUCTOR(nsIconDecoder)
> #endif
> 
> #ifdef IMG_BUILD_DECODER_png
>@@ -154,6 +156,7 @@ NS_DEFINE_NAMED_CID(NS_JPEGENCODER_CID);
> #ifdef IMG_BUILD_DECODER_bmp
> NS_DEFINE_NAMED_CID(NS_ICODECODER_CID);
> NS_DEFINE_NAMED_CID(NS_BMPDECODER_CID);
>+NS_DEFINE_NAMED_CID(NS_ICONDECODER_CID);
> #endif
> #ifdef IMG_BUILD_DECODER_png
> NS_DEFINE_NAMED_CID(NS_PNGDECODER_CID);
>@@ -180,6 +183,7 @@ static const mozilla::Module::CIDEntry kImageCIDs[] = {
> #ifdef IMG_BUILD_DECODER_bmp
>   { &kNS_ICODECODER_CID, false, NULL, nsICODecoderConstructor, },
>   { &kNS_BMPDECODER_CID, false, NULL, nsBMPDecoderConstructor, },
>+  { &kNS_ICONDECODER_CID, false, NULL, nsIconDecoderConstructor, },
> #endif
> #ifdef IMG_BUILD_DECODER_png
>   { &kNS_PNGDECODER_CID, false, NULL, nsPNGDecoderConstructor, },
>@@ -212,6 +216,7 @@ static const mozilla::Module::ContractIDEntry kImageContracts[] = {
>   { "@mozilla.org/image/decoder;3?type=image/vnd.microsoft.icon", &kNS_ICODECODER_CID },
>   { "@mozilla.org/image/decoder;3?type=image/bmp", &kNS_BMPDECODER_CID },
>   { "@mozilla.org/image/decoder;3?type=image/x-ms-bmp", &kNS_BMPDECODER_CID },
>+  { "@mozilla.org/image/decoder;3?type=image/icon", &kNS_ICONDECODER_CID },
> #endif
> #ifdef IMG_BUILD_DECODER_png
>   { "@mozilla.org/image/decoder;3?type=image/png", &kNS_PNGDECODER_CID },
>@@ -237,6 +242,7 @@ static const mozilla::Module::CategoryEntry kImageCategories[] = {
>   { "Gecko-Content-Viewers", "image/vnd.microsoft.icon", "@mozilla.org/content/document-loader-factory;1" },
>   { "Gecko-Content-Viewers", "image/bmp", "@mozilla.org/content/document-loader-factory;1" },
>   { "Gecko-Content-Viewers", "image/x-ms-bmp", "@mozilla.org/content/document-loader-factory;1" },
>+  { "Gecko-Content-Viewers", "image/icon", "@mozilla.org/content/document-loader-factory;1" },
> #endif
> #ifdef IMG_BUILD_DECODER_png
>   { "Gecko-Content-Viewers", "image/png", "@mozilla.org/content/document-loader-factory;1" },
>diff --git a/modules/libpr0n/decoders/bmp/Makefile.in b/modules/libpr0n/decoders/bmp/Makefile.in
>index 894e4d6..4fb3363 100644
>--- a/modules/libpr0n/decoders/bmp/Makefile.in
>+++ b/modules/libpr0n/decoders/bmp/Makefile.in
>@@ -50,7 +50,7 @@ MODULE_NAME	= nsBMPModule
> LIBXUL_LIBRARY  = 1
> 
> 
>-CPPSRCS        = nsBMPDecoder.cpp nsICODecoder.cpp
>+CPPSRCS        = nsBMPDecoder.cpp nsICODecoder.cpp nsIconDecoder.cpp
> 
> # nsBMPDecoder.cpp and nsICODecoder.cpp include RasterImage.h
> LOCAL_INCLUDES += -I$(topsrcdir)/modules/libpr0n/src/
>diff --git a/modules/libpr0n/decoders/icon/nsIconDecoder.cpp b/modules/libpr0n/decoders/bmp/nsIconDecoder.cpp
>similarity index 100%
>rename from modules/libpr0n/decoders/icon/nsIconDecoder.cpp
>rename to modules/libpr0n/decoders/bmp/nsIconDecoder.cpp
>diff --git a/modules/libpr0n/decoders/icon/nsIconDecoder.h b/modules/libpr0n/decoders/bmp/nsIconDecoder.h
>similarity index 100%
>rename from modules/libpr0n/decoders/icon/nsIconDecoder.h
>rename to modules/libpr0n/decoders/bmp/nsIconDecoder.h
>diff --git a/modules/libpr0n/decoders/icon/Makefile.in b/modules/libpr0n/decoders/icon/Makefile.in
>index 713db4f..32a2c5b 100644
>--- a/modules/libpr0n/decoders/icon/Makefile.in
>+++ b/modules/libpr0n/decoders/icon/Makefile.in
>@@ -90,18 +90,6 @@ CPPSRCS		= \
> 		nsIconProtocolHandler.cpp \
> 		$(NULL)
> 
>-ifneq (,$(filter qt gtk2,$(MOZ_WIDGET_TOOLKIT)))
>-USE_ICON_DECODER = 1
>-endif
>-ifeq (,$(filter-out Darwin OS2 BeOS,$(OS_ARCH)))
>-USE_ICON_DECODER = 1
>-endif
>-
>-ifdef USE_ICON_DECODER
>-CPPSRCS		+= nsIconDecoder.cpp
>-DEFINES		+= -DUSE_ICON_DECODER
>-endif
>-
> XPIDLSRCS	= nsIIconURI.idl
> 
> SHARED_LIBRARY_LIBS = $(PLATFORM)/$(LIB_PREFIX)imgicon$(PLATFORM)_s.$(LIB_SUFFIX)
>diff --git a/modules/libpr0n/decoders/icon/nsIconModule.cpp b/modules/libpr0n/decoders/icon/nsIconModule.cpp
>index 2538d08..10ba5f2 100644
>--- a/modules/libpr0n/decoders/icon/nsIconModule.cpp
>+++ b/modules/libpr0n/decoders/icon/nsIconModule.cpp
>@@ -40,9 +40,6 @@
> #include "mozilla/ModuleUtils.h"
> #include "nsServiceManagerUtils.h"
> 
>-#ifdef USE_ICON_DECODER
>-#include "nsIconDecoder.h"
>-#endif
> #include "nsIconProtocolHandler.h"
> #include "nsIconURI.h"
> #include "nsIconChannel.h"
>@@ -53,38 +50,21 @@
>  */
> #define NS_ICONPROTOCOL_CID   { 0xd0f9db12, 0x249c, 0x11d5, { 0x99, 0x5, 0x0, 0x10, 0x83, 0x1, 0xe, 0x9b } } 
> 
>-#ifdef USE_ICON_DECODER
>-NS_GENERIC_FACTORY_CONSTRUCTOR(nsIconDecoder)
>-#endif
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsIconProtocolHandler)
> 
>-#ifdef USE_ICON_DECODER
>-static const char gIconMimeType[] = "image/icon";
>-NS_DEFINE_NAMED_CID(NS_ICONDECODER_CID);
>-#endif
>-
> NS_DEFINE_NAMED_CID(NS_ICONPROTOCOL_CID);
> 
> static const mozilla::Module::CIDEntry kIconCIDs[] = {
>-#ifdef USE_ICON_DECODER
>-  { &kNS_ICONDECODER_CID, false, NULL, nsIconDecoderConstructor },
>-#endif
>   { &kNS_ICONPROTOCOL_CID, false, NULL, nsIconProtocolHandlerConstructor },
>   { NULL }
> };
> 
> static const mozilla::Module::ContractIDEntry kIconContracts[] = {
>-#ifdef USE_ICON_DECODER
>-  { "@mozilla.org/image/decoder;3?type=image/icon", &kNS_ICONDECODER_CID },
>-#endif
>   { NS_NETWORK_PROTOCOL_CONTRACTID_PREFIX "moz-icon", &kNS_ICONPROTOCOL_CID },
>   { NULL }
> };
> 
> static const mozilla::Module::CategoryEntry kIconCategories[] = {
>-#ifdef USE_ICON_DECODER
>-  { "Gecko-Content-Viewers", gIconMimeType, "@mozilla.org/content/document-loader-factory;1" },
>-#endif
>   { NULL }
> };
> 
>-- 
>1.7.1.1

Attachment #465279 - Attachment description: patch v1a → patch v1a (patch by bholley, minor tweak by dholbert)

Daniel Holbert [:dholbert]

Reporter

Comment 5

•

14 years ago

(d'oh, sorry for the spammy comment 4 -- I was updating patch name, and I guess I inadvertently clicked 'edit patch as comment' before clicking submit)

Bobby Holley (:bholley)

Assignee

Comment 6

•

14 years ago

Comment on attachment 465279 [details] [diff] [review]
patch v1a (patch by bholley, minor tweak by dholbert)

dholbert confirmed that this works. Flagging joe for review.

Attachment #465279 - Flags: review?(joe)

Joe Drew (not getting mail)

Comment 7

•

14 years ago

Comment on attachment 465279 [details] [diff] [review]
patch v1a (patch by bholley, minor tweak by dholbert)

Push this to try to make sure that we don't break the build on any of our Tier-1 platforms. Also, it'd be nice to see if Peter has any comments on whether this breaks the build on OS/2.

Attachment #465279 - Flags: review?(joe) → review+

Bobby Holley (:bholley)

Assignee

Comment 8

•

14 years ago

CCing peter to check it out for os/2. I'll leave dholbert to try/land it.

Daniel Holbert [:dholbert]

Reporter

Comment 9

•

14 years ago

Requesting blocking just to be on the safe side. (This is really a helper-patch for bug 584841, which is blocking-beta5+.)

blocking2.0: --- → ?

Joe Drew (not getting mail)

Updated

•

14 years ago

blocking2.0: ? → beta5+

Daniel Holbert [:dholbert]

Reporter

Comment 10

•

14 years ago

http://hg.mozilla.org/mozilla-central/rev/66b05b18e756

Status: ASSIGNED → RESOLVED

Closed: 14 years ago

OS: Linux → All

Hardware: x86 → All

Resolution: --- → FIXED

Walter Meinl

Comment 11

•

14 years ago

(In reply to comment #8)
> CCing peter to check it out for os/2. I'll leave dholbert to try/land it.
Thanks for the notification. It appears your patch did not break us ;-)
Mozilla/5.0 (OS/2; Warp 4.5; rv:2.0b4pre) Gecko/20100815 Minefield/4.0b4pre

patch v1 14 years ago Bobby Holley (:bholley) 6.85 KB, patch		Details \| Diff \| Splinter Review
patch v1a (patch by bholley, minor tweak by dholbert) 14 years ago Daniel Holbert [:dholbert] 6.58 KB, patch	joe : review+	Details \| Diff \| Splinter Review

Bugzilla

Quick Search

Move nsIconDecoder into libimg

Categories

(Core :: Graphics: ImageLib, defect)

Tracking

()

People

(Reporter: dholbert, Assigned: bholley)

References

Details

Crash Data

Security

(public)

User Story

Attachments

(1 file, 1 obsolete file)

Description

Comment 1

Comment 2

Comment 3

Comment 4

Comment 5

Comment 6

Comment 7

Comment 8

Comment 9

Updated

Comment 10

Comment 11

Attachment

General

Description

File Name

Content Type