Last Comment Bug 698263 - Rename mozilla::imagelib namespaces to mozilla::image
: Rename mozilla::imagelib namespaces to mozilla::image
Status: RESOLVED FIXED
[good first bug]
:
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: Trunk
: All All
: -- trivial (vote)
: mozilla12
Assigned To: Atul Aggarwal
:
Mentors: Ed Morley [:emorley]
Depends on: 66984
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-30 05:43 PDT by Ed Morley [:emorley]
Modified: 2014-06-25 01:05 PDT (History)
8 users (show)
emorley: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
imagelib namespaces is changed to image (53.79 KB, patch)
2011-11-05 04:17 PDT, Abhishek Singh
joe: review+
Details | Diff | Splinter Review
Patch v1 (40.76 KB, patch)
2011-11-18 07:48 PST, Atul Aggarwal
no flags Details | Diff | Splinter Review
Patch v1.10 (40.68 KB, patch)
2011-12-22 10:09 PST, Atul Aggarwal
joe: review+
Details | Diff | Splinter Review
Patch v1.50 (40.07 KB, patch)
2012-01-06 09:08 PST, Atul Aggarwal
no flags Details | Diff | Splinter Review

Description Ed Morley [:emorley] 2011-10-30 05:43:21 PDT
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! :-)
Comment 1 Abhishek Singh 2011-11-05 04:17:44 PDT
Created attachment 572187 [details] [diff] [review]
imagelib namespaces is changed to image
Comment 2 Ed Morley [:emorley] 2011-11-07 05:40:05 PST
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).
Comment 3 Joe Drew (not getting mail) 2011-11-07 11:52:45 PST
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.
Comment 4 Abhishek Singh 2011-11-07 22:54:46 PST
Keyword:checkin-needed
Comment 5 Ed Morley [:emorley] 2011-11-07 23:25:52 PST
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
Comment 6 Ed Morley [:emorley] 2011-11-14 12:15:03 PST
Abhishek if you need any help doing the things in comment 5 - just ask :-)
Comment 7 Atul Aggarwal 2011-11-18 07:48:13 PST
Created attachment 575454 [details] [diff] [review]
Patch v1

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
Comment 8 Atul Aggarwal 2011-11-18 09:09:08 PST
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.
Comment 9 Atul Aggarwal 2011-11-18 09:09:37 PST
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.
Comment 10 Ed Morley [:emorley] 2011-12-21 17:44:21 PST
Atul, all yours. (I think enough time has passed now)
Comment 11 Atul Aggarwal 2011-12-22 10:09:00 PST
Created attachment 583843 [details] [diff] [review]
Patch v1.10

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
Comment 12 Joe Drew (not getting mail) 2012-01-03 13:58:28 PST
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.
Comment 13 Mozilla RelEng Bot 2012-01-03 18:02:52 PST
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
Comment 14 Dão Gottwald [:dao] 2012-01-04 03:35:02 PST
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
Comment 16 Ali Juma [:ajuma] 2012-01-20 10:39:37 PST
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!
Comment 17 Daniel Holbert [:dholbert] 2012-01-20 11:36:18 PST
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.
Comment 18 Daniel Holbert [:dholbert] 2012-01-20 11:42:42 PST
(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.)
Comment 19 Daniel Holbert [:dholbert] 2012-01-20 12:20:44 PST
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).
Comment 20 Daniel Holbert [:dholbert] 2012-01-20 14:44:45 PST
Landed:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/9cf740464cd0

Thanks again, Atul & Abhishek!
Comment 21 Ed Morley [:emorley] 2012-01-21 05:54:10 PST
Thanks for the patch! :-)

https://hg.mozilla.org/mozilla-central/rev/9cf740464cd0

Note You need to log in before you can comment on or make changes to this bug.