Re: The worst piece of Mozilla code

2014-10-20 Thread Gabriele Svelto
On 17/10/2014 00:32, Nicholas Nethercote wrote:
 Thanks for the replies so far! I deliberately left this question vague
 to see what kind of responses people would give. But mostly I'm
 interested in code whose awfulness impacts users in a serious way.
 Ones where refactoring/rewriting efforts would be valuable. That
 excludes code that no longer exists :)

A lot of early FxOS code both in gecko and gaia was hacked together in
no time during caffeine-fueled work weeks by jet-lagged developers. Some
of it was cleaned up but some is still with us, mostly in gaia I'd say,
where it often does have an impact on end users. A few things that come
to mind:

- In some applications transitions between a panel or a similar UI state
are not atomic so if you tap fast enough funny things happen.

- We often store application data in the DOM again making for funny bugs
when we update the DOM asynchronously while the user is interacting with
the application.

- I'm not really sure all of our IndexedDB uses are doing transactions
properly. We had (and possibly still have) multiple places where we were
doing things that should have been atomic in different transactions
(because of async_storage.js for example) and we assumed that a
transaction succeeded if the last request succeeded (but not the
transaction itself).

- Dual-SIM support is still partially done. Some things work, some
things kinda work, some things don't work at all (e.g. use SIMs from two
different countries and only the first one's country prefix will be
recognized correctly when matching numbers).

- Last but not least issues in horribly buggy Android binary blobs were
worked around in gecko, some of those workarounds ended up being
duplicated in different pieces of code as refactorings came and went, e.g.:

http://dxr.mozilla.org/mozilla-central/source/gfx/layers/RotatedBuffer.cpp#403
http://dxr.mozilla.org/mozilla-central/source/gfx/layers/client/TextureClient.cpp#226

I know we blacklist drivers on desktops too; what makes this worse is
that we can't ask users to update their drivers on their phones and
these horrors keep creeping into our codebase and we don't know when
we'll be able to get rid of them (or if we'll remember all the places we
put them in).

 Gabriele



signature.asc
Description: OpenPGP digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-20 Thread Thomas Zimmermann
Hi,

this mail made me laugh, because I can tell very similar stories with
different examples. (Not sure if :) or :( )

On the positive side, I think we got a lot better since then by both,
fixing broken designs and code, and also applying learned lessons to new
sub-systems.

Best regards
Thomas

Am 20.10.2014 12:54, schrieb Gabriele Svelto:
 On 17/10/2014 00:32, Nicholas Nethercote wrote:
 Thanks for the replies so far! I deliberately left this question vague
 to see what kind of responses people would give. But mostly I'm
 interested in code whose awfulness impacts users in a serious way.
 Ones where refactoring/rewriting efforts would be valuable. That
 excludes code that no longer exists :)
 
 A lot of early FxOS code both in gecko and gaia was hacked together in
 no time during caffeine-fueled work weeks by jet-lagged developers. Some
 of it was cleaned up but some is still with us, mostly in gaia I'd say,
 where it often does have an impact on end users. A few things that come
 to mind:
 
 - In some applications transitions between a panel or a similar UI state
 are not atomic so if you tap fast enough funny things happen.
 
 - We often store application data in the DOM again making for funny bugs
 when we update the DOM asynchronously while the user is interacting with
 the application.
 
 - I'm not really sure all of our IndexedDB uses are doing transactions
 properly. We had (and possibly still have) multiple places where we were
 doing things that should have been atomic in different transactions
 (because of async_storage.js for example) and we assumed that a
 transaction succeeded if the last request succeeded (but not the
 transaction itself).
 
 - Dual-SIM support is still partially done. Some things work, some
 things kinda work, some things don't work at all (e.g. use SIMs from two
 different countries and only the first one's country prefix will be
 recognized correctly when matching numbers).
 
 - Last but not least issues in horribly buggy Android binary blobs were
 worked around in gecko, some of those workarounds ended up being
 duplicated in different pieces of code as refactorings came and went, e.g.:
 
 http://dxr.mozilla.org/mozilla-central/source/gfx/layers/RotatedBuffer.cpp#403
 http://dxr.mozilla.org/mozilla-central/source/gfx/layers/client/TextureClient.cpp#226
 
 I know we blacklist drivers on desktops too; what makes this worse is
 that we can't ask users to update their drivers on their phones and
 these horrors keep creeping into our codebase and we don't know when
 we'll be able to get rid of them (or if we'll remember all the places we
 put them in).
 
  Gabriele
 
 
 
 ___
 dev-platform mailing list
 dev-platform@lists.mozilla.org
 https://lists.mozilla.org/listinfo/dev-platform
 

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-17 Thread Henri Sivonen
On Fri, Oct 17, 2014 at 1:32 AM, Nicholas Nethercote
n.netherc...@gmail.com wrote:
 On Thu, Oct 16, 2014 at 11:32 PM, Nicholas Nethercote
 n.netherc...@gmail.com wrote:

 I was wondering what people think is the worst piece of code in the
 entire Mozilla codebase. I'll leave the exact meanings of worst and
 piece of code unspecified...

 Thanks for the replies so far! I deliberately left this question vague
 to see what kind of responses people would give. But mostly I'm
 interested in code whose awfulness impacts users in a serious way.

Well, most of the time the user-facing correctness problems are not
serious, but nsDocLoader::mIsLoadingDocument is very bogus, causes
correctness problems and has consequences that include us still having
the ghost of the old HTML parser around for about:blank.

(nsDocLoader in general isn't nice code.)

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-17 Thread Seth Fowler
Thank you Bobby and Josh for all your work to improve ImageLib!

I’m pushing hard on making it better still (with a lot of help from folks like 
Timothy Nikkel and Michael Wu). Hopefully next time we try to decide what the 
worst piece of Mozilla code is, ImageLib won’t be a candidate. =)

- Seth

On Oct 17, 2014, at 12:25 AM, Bobby Holley bobbyhol...@gmail.com wrote:

 On Fri, Oct 17, 2014 at 1:03 AM, Josh Matthews j...@joshmatthews.net
 wrote:
 
 I'm not certain that the image/src/ code is as bad as you make out any
 more. bholley certainly is no longer the expert there; I took over a bunch
 of his work to clean it up a year or two ago, and Seth is the benevolent
 dictator now and has done some good cleanup work on it as well.
 
 
 Yes, I stepped down as an ImageLib peer a little under three years ago. And
 when comparing the initial states of ImageLib and XPConnect when I
 inherited them, ImageLib was a dream to work with. ;-)
 ___
 dev-platform mailing list
 dev-platform@lists.mozilla.org
 https://lists.mozilla.org/listinfo/dev-platform

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-17 Thread Neil

Nicholas Nethercote wrote:


I was wondering what people think is the worst piece of code in the entire Mozilla codebase. I'll 
leave the exact meanings of worst and piece of code unspecified...

When you get time, find someone to tell you about Morse code. (No, I 
don't mean Samuel.)


--
Warning: May contain traces of nuts.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-17 Thread Neil

Ehsan Akhgari wrote:

Speaking about code that causes correctness bugs that actually affect 
our end users, the best example that I know is editor/.


Not just correctness, but the unique way it use pointers to nsCOMPtr all 
over...


--
Warning: May contain traces of nuts.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-17 Thread Neil

Mike Hoye wrote:

I mean, if you find somebody in their office today curled up in a 
ball, rocking back and forth and muttering mork, mork, mork over and 
over again, that person's having a bad flashback. Call for help, stay 
with them. Tell them we've got sqlite now and it's going to be OK.


Those are different use cases; mork did synchronous C++ arbitrary key 
value store very well 15 years ago, as long as you were prepared to 
flush the file on the main thread. (I guess OS.writeAtomic and JSON get 
you there for JS these days.)


--
Warning: May contain traces of nuts.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread Andy Wingo
On Thu 16 Oct 2014 14:32, Nicholas Nethercote n.netherc...@gmail.com writes:

 I was wondering what people think is the worst piece of code in the
 entire Mozilla codebase. I'll leave the exact meanings of worst and
 piece of code unspecified...

The LegacyCompExprTransplanter.

https://hg.mozilla.org/integration/mozilla-inbound/file/ce796ac8901b/js/src/frontend/Parser.cpp#l6110
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread Nicolas B. Pierron

On 10/16/2014 02:32 PM, Nicholas Nethercote wrote:

I was wondering what people think is the worst piece of code in the
entire Mozilla codebase. I'll leave the exact meanings of worst and
piece of code unspecified...


Simple, any file named configure.in in the code base, because deprecated 
tools, and because this is the thing that I always have to run when a 
build-directory.


“configure does not scale—never!—” [1]

[1] http://hubble.gforge.inria.fr/parallel-builds.html

--
Nicolas B. Pierron
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread Mike Hoye

On 2014-10-16 8:32 AM, Nicholas Nethercote wrote:

Hi,

I was wondering what people think is the worst piece of code in the
entire Mozilla codebase. I'll leave the exact meanings of worst and
piece of code unspecified...

Currently or ever?

I mean, if you find somebody in their office today curled up in a ball, 
rocking back and forth and muttering mork, mork, mork over and over 
again, that person's having a bad flashback. Call for help, stay with 
them. Tell them we've got sqlite now and it's going to be OK.


Don't mention Thunderbird.

- mhoye
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread Nicolas B. Pierron
On 10/16/2014 03:08 PM, Nicolas B. Pierron wrote: On 10/16/2014 02:32 PM, 
Nicholas Nethercote wrote:

 I was wondering what people think is the worst piece of code in the
 entire Mozilla codebase. I'll leave the exact meanings of worst and
 piece of code unspecified...

 Simple, any file named configure.in in the code base, because (1) deprecated
 tools, and because this is the thing that I always have to run when a (2) 
build-directory.


(1) we use deprecated tools
(2) always have to run it when we make a new build-directory.


 “configure does not scale—never!—” [1]

 [1] http://hubble.gforge.inria.fr/parallel-builds.html



--
Nicolas B. Pierron

One day I would have to figure out why thunderbird crash when it fails to 
save drafts of mailing list replies.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread Joshua Cranmer 

On 10/16/2014 7:32 AM, Nicholas Nethercote wrote:

Hi,

I was wondering what people think is the worst piece of code in the
entire Mozilla codebase. I'll leave the exact meanings of worst and
piece of code unspecified...


http://dxr.mozilla.org/comm-central/source/mailnews/mime/src/mimedrft.cpp. 
C code masquerading as C++ that use XPCOM classes directly. Manual 
memory allocation up the wazoo. Cleans temporary files on error but not 
success.


--
Joshua Cranmer
Thunderbird and DXR developer
Source code archæologist

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread Andy Wingo
On Thu 16 Oct 2014 15:24, Joshua Cranmer  pidgeo...@gmail.com writes:

 http://dxr.mozilla.org/comm-central/source/mailnews/mime/src/mimedrft.cpp. C
 code masquerading as C++ that use XPCOM classes directly. Manual memory
 allocation up the wazoo. Cleans temporary files on error but not
 success.

Hahaha CreateCompositionFields has 15 char* arguments, one after the
other, and one of them isn't const.  Good times :)

Andy
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread Robert O'Callahan
On Fri, Oct 17, 2014 at 1:32 AM, Nicholas Nethercote n.netherc...@gmail.com
 wrote:

 I was wondering what people think is the worst piece of code in the
 entire Mozilla codebase. I'll leave the exact meanings of worst and
 piece of code unspecified...


Probably not the worst, but always deserves a mention:
http://dxr.mozilla.org/mozilla-central/source/layout/xul/nsSprocketLayout.cpp#632

  // That's it!  If you made it this far without having a nervous
breakdown,   // congratulations!  Go get yourself a beer.

... which ties into the XUL thread :-)

Rob
-- 
oIo otoeololo oyooouo otohoaoto oaonoyooonoeo owohooo oioso oaonogoroyo
owoiotoho oao oboroootohoeoro oooro osoiosotoeoro owoiololo oboeo
osouobojoeocoto otooo ojouodogomoeonoto.o oAogoaoiono,o oaonoyooonoeo
owohooo
osoaoyoso otooo oao oboroootohoeoro oooro osoiosotoeoro,o o‘oRoaocoao,o’o
oioso
oaonosowoeoroaoboloeo otooo otohoeo ocooouoroto.o oAonodo oaonoyooonoeo
owohooo
osoaoyoso,o o‘oYooouo ofolo!o’o owoiololo oboeo oiono odoaonogoeoro
ooofo
otohoeo ofoioroeo ooofo ohoeololo.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread Marcio Galli
only code? Long time ago I found a PSD file in the Netscape source.
About 1MB with a few layers.

m

On Thu, Oct 16, 2014 at 4:52 PM, Robert O'Callahan rob...@ocallahan.org wrote:
 On Fri, Oct 17, 2014 at 1:32 AM, Nicholas Nethercote n.netherc...@gmail.com
 wrote:

 I was wondering what people think is the worst piece of code in the
 entire Mozilla codebase. I'll leave the exact meanings of worst and
 piece of code unspecified...


 Probably not the worst, but always deserves a mention:
 http://dxr.mozilla.org/mozilla-central/source/layout/xul/nsSprocketLayout.cpp#632

   // That's it!  If you made it this far without having a nervous
 breakdown,   // congratulations!  Go get yourself a beer.

 ... which ties into the XUL thread :-)

 Rob
 --
 oIo otoeololo oyooouo otohoaoto oaonoyooonoeo owohooo oioso oaonogoroyo
 owoiotoho oao oboroootohoeoro oooro osoiosotoeoro owoiololo oboeo
 osouobojoeocoto otooo ojouodogomoeonoto.o oAogoaoiono,o oaonoyooonoeo
 owohooo
 osoaoyoso otooo oao oboroootohoeoro oooro osoiosotoeoro,o o‘oRoaocoao,o’o
 oioso
 oaonosowoeoroaoboloeo otooo otohoeo ocooouoroto.o oAonodo oaonoyooonoeo
 owohooo
 osoaoyoso,o o‘oYooouo ofolo!o’o owoiololo oboeo oiono odoaonogoeoro
 ooofo
 otohoeo ofoioroeo ooofo ohoeololo.
 ___
 dev-platform mailing list
 dev-platform@lists.mozilla.org
 https://lists.mozilla.org/listinfo/dev-platform



-- 
www.telasocial.com
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread Justin Dolske

On 10/16/14 5:32 AM, Nicholas Nethercote wrote:


I was wondering what people think is the worst piece of code in the
entire Mozilla codebase. I'll leave the exact meanings of worst and
piece of code unspecified...


It's gone now, but I always held a special hate for nsIDialogParamBlock.

http://mxr.mozilla.org/seamonkey/source/embedding/components/windowwatcher/public/nsIDialogParamBlock.idl

It was a horrid XPCOM thing to pass strings around for creating a 
prompt, by way of magic numbers. Want to set the title? Set string #12! 
Or maybe it was #3. They're listed in the completely separate 
nsPIPromptService.idl, but they're confusing (#12 is eDialogTitle, #3 is 
eTitleMessage), and IIRC some are complete lies.


Most of the other original prompting code was similarly bad.

Justin
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread Martin Thomson
roc said:
 Probably not the worst, but always deserves a mention:
 http://dxr.mozilla.org/mozilla-central/source/layout/xul/nsSprocketLayout.cpp#632

That's relatively short.  This is 800 lines, complete with several layers of 
goto:
http://dxr.mozilla.org/mozilla-central/source/security/nss/lib/ssl/ssl3con.c?from=ssl3_HandleClientHellocase=true#7618
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread Randell Jesup
On Fri, Oct 17, 2014 at 1:32 AM, Nicholas Nethercote n.netherc...@gmail.com
 wrote:

 I was wondering what people think is the worst piece of code in the
 entire Mozilla codebase. I'll leave the exact meanings of worst and
 piece of code unspecified...


Probably not the worst, but always deserves a mention:
http://dxr.mozilla.org/mozilla-central/source/layout/xul/nsSprocketLayout.cpp#632

  // That's it!  If you made it this far without having a nervous
  // breakdown, congratulations!  Go get yourself a beer.

... which ties into the XUL thread :-)

Reminds me of the total rewrite I did of the table-layout code for
SpyGlass eons ago, to actually work correctly (I think I even had it
handle bug 10212 correct - height 100% when nesting tables).  Quite
brain-warping, especially how changes can ripple through multiple times.

-- 
Randell Jesup, Mozilla Corp
remove news for personal email
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread L. David Baron
On Thursday 2014-10-16 05:32 -0700, Nicholas Nethercote wrote:
 I was wondering what people think is the worst piece of code in the
 entire Mozilla codebase. I'll leave the exact meanings of worst and
 piece of code unspecified...

I'd probably pick the table and row height computation code in the
table layout code.  There's row height code in
nsTableRowGroupFrame::ReflowChildren,
nsTableRowGroupFrame::CalculateRowHeights,
nsTableRowGroupFrame::GetHeightBasis, and
nsTableRowFrame::CalculateCellActualHeight, which then has
interesting interactions with table/cell height code via things like
the mPercentHeightObserver member of nsHTMLReflowState and
nsTableCellFrame::NotifyPercentHeight, and the SpecialHeightReflow
concept in nsTableFrame.  (The worst bit is that we could have
massively simplified it and sped it up by matching IE6's much
cleaner model, but now it's probably too late for that since other
browsers have copied us.)

-David

-- 
턞   L. David Baron http://dbaron.org/   턂
턢   Mozilla  https://www.mozilla.org/   턂
 Before I built a wall I'd ask to know
 What I was walling in or walling out,
 And to whom I was like to give offense.
   - Robert Frost, Mending Wall (1914)


signature.asc
Description: Digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread Nicholas Nethercote
On Thu, Oct 16, 2014 at 11:32 PM, Nicholas Nethercote
n.netherc...@gmail.com wrote:

 I was wondering what people think is the worst piece of code in the
 entire Mozilla codebase. I'll leave the exact meanings of worst and
 piece of code unspecified...

Thanks for the replies so far! I deliberately left this question vague
to see what kind of responses people would give. But mostly I'm
interested in code whose awfulness impacts users in a serious way.
Ones where refactoring/rewriting efforts would be valuable. That
excludes code that no longer exists :)

With this in mind, a function that is hideous but non-buggy and
doesn't cause maintenance problems (e.g. because the hideousness
doesn't leak too far) wouldn't qualify as bad. In comparison, a
sub-system with frequent subtle correctness issues would be very bad.

So I'd be interested in suggestions along these lines. Feel free to
email me privately if you prefer.

Nick
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread Nicholas Nethercote
On Fri, Oct 17, 2014 at 8:55 AM, Andreas Gal andr...@mozilla.com wrote:

 I would like to nominate image/src/* and in particular its class hierarchy 
 which completely doesn’t make any sense what so ever. imgRequest, 
 imgIRequest, we got it all.

Does this cause correctness problems, or is it just hard to read and
thus modify? Is there a path that could be taken to gradually improve
it?

Nick
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread Andreas Gal

The code is really bizarre, needlessly complex and impossible to understand and 
maintain. We could use a lot of improvements in this area to better decide what 
images to load when and how and when to retain or purge them. There is a lot of 
state machinery and multi-threading at work. I wouldn’t be surprised if we find 
a couple nasty correctness bugs if we ever decide to clean up this mess. 
bholley is the expert for this code I think. He can give you a better overview 
(full disclosure: this code used to be much worse before he went to town on it).

Andreas

On Oct 16, 2014, at 7:33 PM, Nicholas Nethercote n.netherc...@gmail.com wrote:

 On Fri, Oct 17, 2014 at 8:55 AM, Andreas Gal andr...@mozilla.com wrote:
 
 I would like to nominate image/src/* and in particular its class hierarchy 
 which completely doesn’t make any sense what so ever. imgRequest, 
 imgIRequest, we got it all.
 
 Does this cause correctness problems, or is it just hard to read and
 thus modify? Is there a path that could be taken to gradually improve
 it?
 
 Nick

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread L. David Baron
On Thursday 2014-10-16 19:45 -0300, Andreas Gal wrote:
 The code is really bizarre, needlessly complex and impossible to understand 
 and maintain. We could use a lot of improvements in this area to better 
 decide what images to load when and how and when to retain or purge them.

Seth has been doing a lot of work on cleaning up the image code
lately, most recently on rewriting the caching behavior (including
adding the ability to store more than one decoded form of an image,
which enables things like downscale-on-decode), which is currently
in-progress.

-David

-- 
턞   L. David Baron http://dbaron.org/   턂
턢   Mozilla  https://www.mozilla.org/   턂
 Before I built a wall I'd ask to know
 What I was walling in or walling out,
 And to whom I was like to give offense.
   - Robert Frost, Mending Wall (1914)


signature.asc
Description: Digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread Josh Matthews
I'm not certain that the image/src/ code is as bad as you make out any 
more. bholley certainly is no longer the expert there; I took over a 
bunch of his work to clean it up a year or two ago, and Seth is the 
benevolent dictator now and has done some good cleanup work on it as well.


Cheers,
Josh

On 2014-10-16 6:45 PM, Andreas Gal wrote:


The code is really bizarre, needlessly complex and impossible to understand and 
maintain. We could use a lot of improvements in this area to better decide what 
images to load when and how and when to retain or purge them. There is a lot of 
state machinery and multi-threading at work. I wouldn’t be surprised if we find 
a couple nasty correctness bugs if we ever decide to clean up this mess. 
bholley is the expert for this code I think. He can give you a better overview 
(full disclosure: this code used to be much worse before he went to town on it).

Andreas

On Oct 16, 2014, at 7:33 PM, Nicholas Nethercote n.netherc...@gmail.com wrote:


On Fri, Oct 17, 2014 at 8:55 AM, Andreas Gal andr...@mozilla.com wrote:


I would like to nominate image/src/* and in particular its class hierarchy 
which completely doesn’t make any sense what so ever. imgRequest, imgIRequest, 
we got it all.


Does this cause correctness problems, or is it just hard to read and
thus modify? Is there a path that could be taken to gradually improve
it?

Nick




___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread Andreas Gal

I am glad to hear there is so much activity happening there. Kind of makes my 
point though: that code needed it :)

Andreas

On Oct 16, 2014, at 8:03 PM, Josh Matthews j...@joshmatthews.net wrote:

 I'm not certain that the image/src/ code is as bad as you make out any more. 
 bholley certainly is no longer the expert there; I took over a bunch of his 
 work to clean it up a year or two ago, and Seth is the benevolent dictator 
 now and has done some good cleanup work on it as well.
 
 Cheers,
 Josh
 
 On 2014-10-16 6:45 PM, Andreas Gal wrote:
 
 The code is really bizarre, needlessly complex and impossible to understand 
 and maintain. We could use a lot of improvements in this area to better 
 decide what images to load when and how and when to retain or purge them. 
 There is a lot of state machinery and multi-threading at work. I wouldn’t be 
 surprised if we find a couple nasty correctness bugs if we ever decide to 
 clean up this mess. bholley is the expert for this code I think. He can give 
 you a better overview (full disclosure: this code used to be much worse 
 before he went to town on it).
 
 Andreas
 
 On Oct 16, 2014, at 7:33 PM, Nicholas Nethercote n.netherc...@gmail.com 
 wrote:
 
 On Fri, Oct 17, 2014 at 8:55 AM, Andreas Gal andr...@mozilla.com wrote:
 
 I would like to nominate image/src/* and in particular its class hierarchy 
 which completely doesn’t make any sense what so ever. imgRequest, 
 imgIRequest, we got it all.
 
 Does this cause correctness problems, or is it just hard to read and
 thus modify? Is there a path that could be taken to gradually improve
 it?
 
 Nick
 
 
 ___
 dev-platform mailing list
 dev-platform@lists.mozilla.org
 https://lists.mozilla.org/listinfo/dev-platform

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: The worst piece of Mozilla code

2014-10-16 Thread Trevor Saunders
On Fri, Oct 17, 2014 at 09:32:20AM +1100, Nicholas Nethercote wrote:
 On Thu, Oct 16, 2014 at 11:32 PM, Nicholas Nethercote
 n.netherc...@gmail.com wrote:
 
  I was wondering what people think is the worst piece of code in the
  entire Mozilla codebase. I'll leave the exact meanings of worst and
  piece of code unspecified...
 
 Thanks for the replies so far! I deliberately left this question vague
 to see what kind of responses people would give. But mostly I'm
 interested in code whose awfulness impacts users in a serious way.
 Ones where refactoring/rewriting efforts would be valuable. That
 excludes code that no longer exists :)
 
 With this in mind, a function that is hideous but non-buggy and
 doesn't cause maintenance problems (e.g. because the hideousness
 doesn't leak too far) wouldn't qualify as bad. In comparison, a
 sub-system with frequent subtle correctness issues would be very bad.

For that I'd tend to agree with ehsan editor/ and the selection bits in
layout/.

RDF is more terrible, but probably less important and its more a problem
of removal than of refactoring.

Trev

 
 So I'd be interested in suggestions along these lines. Feel free to
 email me privately if you prefer.
 
 Nick
 ___
 dev-platform mailing list
 dev-platform@lists.mozilla.org
 https://lists.mozilla.org/listinfo/dev-platform


signature.asc
Description: Digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform