Patrick Brunschwig Assignee
	Description • 13 years ago

ipccode still uses the old way of mutex-locking which is no longer supported.

The attached patch also adds a readme.txt containing instructions for building.

	Alex Vincent [:WeirdAl]
	Comment 1 • 13 years ago

I want to think twice about this one.  It looks like you're completely removing support for FF3.x (pre-Gecko 2.0) builds.

Given the number of places you had to change the old way to the new one, may I suggest you branch (or tag, if you don't think we'll need to ever change it) the existing code for pre-2.0 platform consumers, and land this on tip.

Is there any way in the configure step that we can check the platform version we're building?  That would allow us to fail much faster than if someone accidentally checked out tip code against FF3.6, for example.

I'll finish my review later today.  I'm not up to speed on mozilla::Mutex, and I typically build using --enable-extensions (which is why I asked about checking in configure).

	Alex Vincent [:WeirdAl]
	Comment 2 • 13 years ago

Comment on attachment 537457 [details]
Patch to fix.

I'm sorry, but I really don't think I can honestly review this one.  I don't know how our mutexes work.

I'd recommend getting review from someone on this list:
http://hg.mozilla.org/mozilla-central/log/068197c2a88e/xpcom/glue/Mutex.h

feedback+ with your response to my previous comment about branching/tagging and configure-time failure:  nothing obviously broken here.

I'm hoping cjones can help us out here.

>--- /dev/null	2011-06-05 17:34:47.000000000 +0200
>+++ readme.txt	2011-06-05 18:08:42.000000000 +0200
>+makemake tries to read your .mozconfig file to determint @MOZ_OBJDIR@. You can
>+alternatively provide your own MOZ_OBJDIR using makemake -o

typo:  determine

	Patrick Brunschwig Assignee
	Comment 3 • 13 years ago

(In reply to comment #1)
> I want to think twice about this one.  It looks like you're completely
> removing support for FF3.x (pre-Gecko 2.0) builds.

Yes indeed. Those who still want the old code can get it under the tag "gecko-192."

> Is there any way in the configure step that we can check the platform
> version we're building?  That would allow us to fail much faster than if
> someone accidentally checked out tip code against FF3.6, for example.

I don't think this is really needed. IF the build fails, you checkout the new code and just rebuild ipccode, you don't need to re-build the rest.

> I'll finish my review later today.  I'm not up to speed on mozilla::Mutex,
> and I typically build using --enable-extensions (which is why I asked about
> checking in configure).

I found that --enable-extensions don't work on Thunderbird, that's why I wrote makemake.

(In reply to comment #2)
> Comment on attachment 537457 [details]
> Patch to fix.
> 
> I'm sorry, but I really don't think I can honestly review this one.  I don't
> know how our mutexes work.

No worries. It's relatively simple though. There's a page describing the changes you'll have to follow to move from Gecko 2.0 to Gecko 5.0:
https://developer.mozilla.org/En/XPCOM_Thread_Synchronization

	Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
	Comment 4 • 13 years ago

The PRLock->Mutex conversion looks fine.  I can r+ or feedback+ that if you want, not sure what you're looking for.

	Patrick Brunschwig Assignee
	Comment 5 • 13 years ago

Checked in: http://hg.mozilla.org/ipccode/rev/3aa8d8e7a70a