289515 - Remove nsImageWin::CreateDDB

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Win32, defect)

Description

[ArronM (:paper)]

nsImageWin::CreateDDB is a private function and isn't being called.

Comment 1

[ArronM (:paper)]

Attachment: Remove CreateDDB

I'll take a leap and assume this doesn't need a seperate r/sr.

Proof CreateDDB isn't being used:
http://lxr.mozilla.org/seamonkey/search?string=CreateDDB

Comment 2

[Justin Wood (:Callek)]

Might want to remove the OS2 reference to this (even though its not compiled of
course)

Also does not touching the win32.order file still compile? (I cannot test at
this environment) [and I do not know enough about its use to know from memory]

If we remove it though, why not remove its reference everywhere?

Comment 3

[Justin Wood (:Callek)]

For the record, biesi told me that win32.order is not used anywhere, but you may
want to remove the line anyway there (imho) but that would be far from me to decide.

Comment 4

[ArronM (:paper)]

I'd rather leave the OS2 code to the OS2 coders.  While I can speculate that the
function will never be used for OS2, I could also speculate about a lot of
functions that will never be created/used in the OS2 code.  OS2 coders usually
do their own thing.

As for win32.order, I did a bit of digging today just to find out what the file
was, and I agree with you, it would also be far from me to decide whether to
remove the line from win32.order, or the whole file.

Just so that my log digging doesn't go to waste, here's what I learned:
Apparently win32.order was used to "reorder symbols in the DLLs/executables so
that they can be loaded faster" (quote from biesi, 09:34 on Sept 19, 2003).  

Also, from Aug 1, 2003:
11:25 < timeless> tehre's code that generates win32.order files
11:26 < timeless> it isn't in the build path, i need to poke it the next time
i'm in windows

Now, I don't know if one can safely go into the win32.order file and edit it, or
run the win32.order generator.

And finally, on June 23, 2003, the longest conversation on win32.order that I
could find
11:34 < tor> how about the win32.order files?
11:34 < bsmedberg> are those still used?
11:35 < tor> given they haven't been updated for years, probably not heavily
11:35 < d33p> i think thats the only dependecy on the mozilla precompiled binaries
11:35 < d33p> otherwise it could be a slew of things 
11:36 < biesi> bsmedberg: they aren't used
11:36 < biesi> bsmedberg: but when I asked someone (cathleen?) about them, the
answer was "let's keep them just in case"
11:37 < bsmedberg> heh, that's silly
11:37 <@cls> welcome to Mozilla
11:39 < biesi> I think we also have some leftovers from the nmake build system


So, yeah, patch remains as is :)

Comment 5

[Justin Wood (:Callek)]

Just to benefit you, this from when Biesi talked to me on IRC; as this log
shows, the OS2 stuff should be removed, if not anything about the win32.order
comment:

(13:30:35) biesi: Callek, fwiw win32.order is not used by anything
(13:30:46) Callek: biesi: ahh, ok
(13:30:55) Callek: biesi: thank you (as I did not know if it was, as stated in
the bug)
(13:33:25) biesi: Callek, also, what did you mean with "(even though its not
compiled of course)"?
(13:33:40) biesi: Callek, it is compiled on OS/2, is it not?
(13:34:21) Callek: biesi: |#ifdef OS2TODO| makes me assume it is not compiled in
OS/2 especially since there is no impl of it in OS2 according to lxr
(13:36:13) biesi: ah, I didn't check the file
(13:36:23) biesi: missing definitions of nonvirtual functions aren't fatal though
(13:36:28) biesi: (unless they are called...)
(13:36:40) mkaplyMeeting is now known as mkaply
(13:37:05) ***mkaply wonders what is being discussed
(13:37:51) biesi: mkaply, https://bugzilla.mozilla.org/show_bug.cgi?id=289515#c2
(13:38:03) Callek: biesi: as I said, probably wouldnt hurt, and might actually
be beneficial to remove, just to help prevent confusion... (/me cannot recall
his initial wording at the moment though)
(13:38:13) biesi: Callek, yes, I agree it should be removed
(13:39:01) mkaply: In our port to OS/2, we simply took the XXXWin files and used
OS2TODO or ndef XP_OS2 to get rid of stuff. 
(13:39:08) mkaply: So please remove it from ours if it isn't in Win anymore

Comment 6

[ArronM (:paper)]

Attachment: Remove CreateDDB

Fair enough, it's now gone from OS2. :)

I've also created Bug 289663 for Win32.order

Comment 7

[Asa Dotzler [:asa]]

Comment on attachment 180144 [details] [diff] [review]
Remove CreateDDB

a=asa

Comment 8

[ArronM (:paper)]

Checking in gfx/src/os2/nsImageOS2.h;
/cvsroot/mozilla/gfx/src/os2/nsImageOS2.h,v  <--  nsImageOS2.h
new revision: 1.23; previous revision: 1.22
done
Checking in gfx/src/windows/nsImageWin.cpp;
/cvsroot/mozilla/gfx/src/windows/nsImageWin.cpp,v  <--  nsImageWin.cpp
new revision: 3.146; previous revision: 3.145
done
Checking in gfx/src/windows/nsImageWin.h;
/cvsroot/mozilla/gfx/src/windows/nsImageWin.h,v  <--  nsImageWin.h
new revision: 3.63; previous revision: 3.62
done