Closed Bug 698263 Opened 13 years ago Closed 12 years ago

Rename mozilla::imagelib namespaces to mozilla::image

Categories

(Core :: Graphics: ImageLib, defect)

defect
Not set
trivial

Tracking

()

RESOLVED FIXED
mozilla12

People

(Reporter: emorley, Assigned: atulagrwl, Mentored)

References

Details

(Whiteboard: [good first bug])

Attachments

(1 file, 3 obsolete files)

Bug 66984 moved ImageLib to image/ to get rid of the controversial name. As part of the discussion on tree location, it was suggested the namespace should match...

Bobby Holley (:bholley), bug 66984 comment #26:
> Just out of curiosity, why not imagelib? We've been namespacing new files as
> mozilla::imagelib, so it might be nice if they matched.
> 
> 'image' does have aesthetic appeal though, so I'm happy sticking with that
> too. ;-)

Joe Drew (:JOEDREW!), bug 66984 comment #27:
> We should fix that by changing mozilla::imagelib to mozilla::image :)


For anyone wanting to do this as a first bug:

1) Get mozilla-central:
   https://developer.mozilla.org/En/Developer_Guide/Source_Code/Mercurial

2) Rename the imagelib namespaces to image:
   http://mxr.mozilla.org/mozilla-central/search?string=imagelib&case=on

3) Check everything still builds ok:
   https://developer.mozilla.org/En/Simple_Firefox_build

4) Create a patch to attach here:
   https://developer.mozilla.org/En/Developer_Guide/How_to_Submit_a_Patch
   https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3f

5) Set the r? flag to request review from Joe Drew when you attach the patch (type in :JOEDREW and it will auto-complete).

If you have any questions (or would like to take on this bug and don't have the needed bugzilla permissions to assign it to yourself), just ask! :-)
Flags: in-testsuite-
Assignee: nobody → abhishekkumarsingh.cse
Attachment #572187 - Flags: review?(joe)
Attachment #572187 - Flags: review?(joe) → review?(bmo)
Comment on attachment 572187 [details] [diff] [review]
imagelib namespaces is changed to image

Joe was the correct person to ask review from here, I'm not an imagelib peer. The review was requested over the weekend, so Joe will not have seen it yet. (The average time for reviews varies, but a few days-a week is probably the rough ballpark to expect).
Attachment #572187 - Flags: review?(bmo) → review?(joe)
Comment on attachment 572187 [details] [diff] [review]
imagelib namespaces is changed to image

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

I've marked below (with "here") places you should *not* s/imagelib/image/; mostly these are in comments. I think it's helpful to be able to refer to "imagelib" itself, which is shorthand for "mozilla::image" from now on.

::: content/base/src/nsObjectLoadingContent.h
@@ +362,5 @@
>        UpdateFallbackState(nsIContent* aContent, AutoFallback& fallback,
>                            const nsCString& aTypeHint);
>  
>      /**
> +     * The final listener to ship the data to (image, uriloader, etc)

here

::: image/decoders/nsPNGDecoder.cpp
@@ +602,5 @@
>    png_read_update_info(png_ptr, info_ptr);
>    decoder->mChannels = channels = png_get_channels(png_ptr, info_ptr);
>  
>    /*---------------------------------------------------------------*/
> +  /* copy PNG info into image structs (formerly png_set_dims()) */

here

::: image/public/ImageErrors.h
@@ +39,5 @@
>  
>  #include "nsError.h"
>  
>  /**
> + * image specific nsresult success and error codes

here

::: image/public/imgILoader.idl
@@ +109,5 @@
>     * @param aObserver the observer (may be null)
>     * @param cx some random data
>     * @param aListener [out]
>     *        A listener that you must send the channel's notifications and data to.
> +   *        Can be null, in which case image has found a cached image and is

here

::: image/src/SVGDocumentWrapper.h
@@ +184,5 @@
>    bool                        mIgnoreInvalidation;
>    bool                        mRegisteredForXPCOMShutdown;
>  
>    // Lazily-initialized pointer to nsGkAtoms::svg, to make life easier in
> +  // non-libxul builds, which don't let us reference nsGkAtoms from image.

here

::: image/src/imgLoader.cpp
@@ +1782,5 @@
>  
>      // If we're loading off the network, explicitly don't notify our proxy,
>      // because necko (or things called from necko, such as imgCacheValidator)
>      // are going to call our notifications asynchronously, and we can't make it
> +    // further asynchronous because observers might rely on image completing

here

@@ +1913,5 @@
>      // Explicitly don't notify our proxy, because we're loading off the
>      // network, and necko (or things called from necko, such as
>      // imgCacheValidator) are going to call our notifications asynchronously,
>      // and we can't make it further asynchronous because observers might rely
> +    // on image completing its work between the channel's OnStartRequest and

here

::: image/src/imgRequest.cpp
@@ +1121,5 @@
>                rv = nsMemory::HeapMinimize(true);
>                rv |= rasterImage->SetSourceSizeHint(sizeHint);
>                // If we've still failed at this point, things are going downhill
>                if (NS_FAILED(rv)) {
> +                NS_WARNING("About to hit OOM in image!");

here

::: layout/base/nsDocumentViewer.cpp
@@ +1017,5 @@
>    mLoaded = true;
>  
>    // Now, fire either an OnLoad or OnError event to the document...
>    bool restoring = false;
> +  // XXXbz image kills off the document load for a full-page image with

here

::: layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp
@@ +3338,5 @@
>        if (isRTL)
>          twistyRect.x = rightEdge - twistyRect.width;
>        // yeah, I know it says we're drawing a background, but a twisty is really a fg
>        // object since it doesn't have anything that gecko would want to draw over it. Besides,
> +      // we have to prevent image from drawing it.

here

::: netwerk/protocol/http/nsHttpChannel.cpp
@@ +1215,5 @@
>  
>      ClearBogusContentEncodingIfNeeded();
>  
>      // this must be called before firing OnStartRequest, since http clients,
> +    // such as image, expect our cache entry to already have the correct

here

::: xulrunner/config/mozconfig
@@ +1,4 @@
>  # This file specifies the build flags for XULRunner.  You can use it by adding:
>  #  . $topsrcdir/xulrunner/config/mozconfig
>  # to the top of your mozconfig file.
> +ac_add_options --enable-debug

and drop this.
Attachment #572187 - Flags: review?(joe) → review+
Keyword:checkin-needed
Keywords: checkin-needed
Hi Abhishek :-)

The changes mentioned in comment 3 need to be made before this can be checked in.

It's common for reviewers to give r+ (ie grant review), but still list a few minor things that need to be changed first. These changes should still be made, the r+ just means they are happy for the updated patch to be attached (and then checkin-needed added), without another review.

Whilst making those changes, please can you also see here for how to add author/commit message:
http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed
Status: NEW → ASSIGNED
Keywords: checkin-needed
Abhishek if you need any help doing the things in comment 5 - just ask :-)
Attached patch Patch v1 (obsolete) — Splinter Review
Done using two shell commands and finally manually verifying the changes
grep -lr -r '::imagelib' *| xargs sed -i 's/::imagelib/::image/g'
grep -lr -r 'namespace imagelib' *| xargs sed -i 's/namespace imagelib/namespace image/g'

https://tbpl.mozilla.org/?tree=Try&rev=62632a303039
Assignee: abhishekkumarsingh.cse → atulagrwl
Attachment #572187 - Attachment is obsolete: true
Attachment #575454 - Flags: review?(joe)
Attachment #575454 - Attachment is obsolete: true
Attachment #575454 - Flags: review?(joe)
Abhishek, Sorry for taking the bug. I am assigning the bug back to you. Please make changes as suggested by reviewer and edmorley and let us know if you need any help.
Abhishek, Sorry for taking the bug. I am assigning the bug back to you. Please make changes as suggested by reviewer and edmorley and let us know if you need any help.
Assignee: atulagrwl → abhishekkumarsingh.cse
Attachment #572187 - Attachment is obsolete: false
Atul, all yours. (I think enough time has passed now)
Assignee: abhishekkumarsingh.cse → atulagrwl
Attached patch Patch v1.10 (obsolete) — Splinter Review
grep -lr -r '::imagelib' *| xargs sed -i 's/::imagelib/::image/g'
grep -lr -r 'namespace imagelib' *| xargs sed -i 's/namespace imagelib/namespace image/g'
https://tbpl.mozilla.org/?tree=Try&rev=38bd9c96e184
Attachment #572187 - Attachment is obsolete: true
Attachment #583843 - Flags: review?(joe)
Comment on attachment 583843 [details] [diff] [review]
Patch v1.10

Lovely! I'll push this to try; once it's come back, we'll get someone to check it in.
Attachment #583843 - Flags: review?(joe) → review+
Try run for 48e95de65ce6 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=48e95de65ce6
Results (out of 14 total builds):
    success: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jdrew@mozilla.com-48e95de65ce6
applying https://bug698263.bugzilla.mozilla.org/attachment.cgi?id=583843
patching file image/encoders/ico/nsICOEncoder.cpp
Hunk #1 FAILED at 40
1 out of 1 hunks FAILED -- saving rejects to file image/encoders/ico/nsICOEncoder.cpp.rej
patching file image/src/Decoder.cpp
Hunk #1 FAILED at 36
1 out of 2 hunks FAILED -- saving rejects to file image/src/Decoder.cpp.rej
patching file image/src/SVGDocumentWrapper.h
Hunk #1 FAILED at 54
1 out of 2 hunks FAILED -- saving rejects to file image/src/SVGDocumentWrapper.h.rej
patching file image/src/imgLoader.cpp
Hunk #1 FAILED at 89
1 out of 1 hunks FAILED -- saving rejects to file image/src/imgLoader.cpp.rej
patching file image/src/imgRequest.cpp
Hunk #1 FAILED at 86
Hunk #2 FAILED at 132
2 out of 2 hunks FAILED -- saving rejects to file image/src/imgRequest.cpp.rej
abort: patch failed to apply
Keywords: checkin-needed
Attached patch Patch v1.50Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=b35b1d258237
Attachment #583843 - Attachment is obsolete: true
Applying https://bug698263.bugzilla.mozilla.org/attachment.cgi?id=583843:

patching file image/src/RasterImage.cpp
Hunk #1 FAILED at 65
Hunk #2 succeeded at 169 (offset 2 lines).
Hunk #3 succeeded at 2949 (offset 4 lines).
1 out of 3 hunks FAILED -- saving rejects to file image/src/RasterImage.cpp.rej

Atul, please update and push to try. And be sure to mark this checkin-needed once the try results are in, so it doesn't get bit-rotted again! Thanks!
The patch applies fine with fuzz 6; probably no need for Atul upload a new patch.  I've got the fuzz-applied patch locally, & assuming it builds OK locally, I'll push to try & then m-i today.
(FWIW, the chunk that doesn't apply without fuzz is just for some #includes changing in contextual code, a few lines up from a "using namespace mozilla::image[lib];" line.)
Re-pushed to Try for good measure:
  https://tbpl.mozilla.org/?tree=Try&rev=66512cdeb0e9

I also verified that the diffs between the r+'d patch & latest patch are all from bitrot in contextual code (good).
Landed:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/9cf740464cd0

Thanks again, Atul & Abhishek!
Target Milestone: --- → mozilla12
Thanks for the patch! :-)

https://hg.mozilla.org/mozilla-central/rev/9cf740464cd0
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Mentor: emorley
Whiteboard: [good first bug] [mentor=edmorley] → [good first bug]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: