Closed Bug 639754 Opened 13 years ago Closed 13 years ago

Remove MOZ_IPC checks since IPC is always built now

Categories

(Core :: IPC, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla5

People

(Reporter: matjk7, Assigned: matjk7)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 6 obsolete files)

User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b13pre) Gecko/20110308 Firefox/4.0b13pre
Build Identifier: 

Bug 638755 made --disable-ipc obsolete, so all MOZ_IPC checks can be safely removed.

Reproducible: Always
Depends on: 638755
Attached patch patch (obsolete) — Splinter Review
Attachment #517663 - Flags: review?(benjamin)
Assignee: nobody → matjk7
Status: UNCONFIRMED → NEW
Ever confirmed: true
Component: Build Config → IPC
QA Contact: build-config → ipc
Blocks: 644093
Attached patch patch (obsolete) — Splinter Review
Updated to tip.
Attachment #517663 - Attachment is obsolete: true
Attachment #517663 - Flags: review?(benjamin)
Attachment #522260 - Flags: review?(benjamin)
Attached patch patch (obsolete) — Splinter Review
Updated again.
Attachment #522260 - Attachment is obsolete: true
Attachment #522260 - Flags: review?(benjamin)
Attachment #522315 - Flags: review?(benjamin)
Attached patch patch (obsolete) — Splinter Review
Updated again.
Attachment #522315 - Attachment is obsolete: true
Attachment #522315 - Flags: review?(benjamin)
Attachment #522545 - Flags: review?(benjamin)
Comment on attachment 522545 [details] [diff] [review]
patch

>diff --git a/content/base/src/nsFrameLoader.cpp b/content/base/src/nsFrameLoader.cpp

>-#ifdef MOZ_IPC
> #  include "base/basictypes.h"
>-#endif

unindent the nested #include

>diff --git a/docshell/base/IHistory.h b/docshell/base/IHistory.h

>      * @pre aURI must not be null.
>-     * @pre aLink may be null only in the MOZ_IPC parent process.
>+     * @pre aLink may be null only in the parent process.

I think this should say "only in the parent (chrome) process."

>diff --git a/gfx/layers/Layers.cpp b/gfx/layers/Layers.cpp

>-#ifdef MOZ_IPC
> # include "mozilla/layers/ShadowLayers.h"
>-#endif  // MOZ_IPC

unindent

>-#ifdef MOZ_IPC
> // NB: eventually these methods will be defined unconditionally, and
> // can be moved into Layers.h

Either fix this, or file a followup.


>diff --git a/gfx/layers/basic/BasicLayers.cpp b/gfx/layers/basic/BasicLayers.cpp

>-#ifdef MOZ_IPC
> #  include "gfxSharedImageSurface.h"

unindent

>diff --git a/gfx/layers/opengl/ThebesLayerOGL.h b/gfx/layers/opengl/ThebesLayerOGL.h

>-#ifdef MOZ_IPC
> # include "mozilla/layers/PLayers.h"
> # include "mozilla/layers/ShadowLayers.h"
>-#endif

unindent

>diff --git a/ipc/chromium/chromium-config.mk b/ipc/chromium/chromium-config.mk

> endif # }
>\ No newline at end of file

fix this while you're here?

>diff --git a/layout/build/Makefile.in b/layout/build/Makefile.in

>-ifdef MOZ_IPC
> GKIPCLIB=../ipc/$(LIB_PREFIX)gkipc_s.$(LIB_SUFFIX)
>-else
>-GKIPCLIB=$(NULL)
>-endif
> 
> SHARED_LIBRARY_LIBS = \
> 	../base/$(LIB_PREFIX)gkbase_s.$(LIB_SUFFIX) \
> 	../forms/$(LIB_PREFIX)gkforms_s.$(LIB_SUFFIX) \
> 	../generic/$(LIB_PREFIX)gkgeneric_s.$(LIB_SUFFIX) \
> 	$(GKIPCLIB) \
> 	../style/$(LIB_PREFIX)gkstyle_s.$(LIB_SUFFIX) \

Remove GKIPCLIB altogether and just hardcode gkipc_s into SHARED_LIBRARY LIBS.

>diff --git a/toolkit/crashreporter/nsExceptionHandler.cpp b/toolkit/crashreporter/nsExceptionHandler.cpp

>-#if defined(MOZ_IPC)
> #  include "client/windows/crash_generation/crash_generation_server.h"
>-#endif
> #include "client/windows/handler/exception_handler.h"
> #include <DbgHelp.h>
> #include <string.h>
> #elif defined(XP_MACOSX)
>-#if defined(MOZ_IPC)
> #  include "client/mac/crash_generation/client_info.h"
> #  include "client/mac/crash_generation/crash_generation_server.h"
>-#endif

unindent both

>-#if defined(MOZ_IPC)
> #  include "client/linux/crash_generation/client_info.h"
> #  include "client/linux/crash_generation/crash_generation_server.h"
>-#endif

and here

>diff --git a/toolkit/xre/nsSigHandlers.cpp b/toolkit/xre/nsSigHandlers.cpp

>-#ifdef MOZ_IPC
> #  include "nsXULAppAPI.h"
>-#endif

unindent

>diff --git a/widget/src/android/nsWidgetFactory.cpp b/widget/src/android/nsWidgetFactory.cpp

>@@ -90,21 +88,19 @@ nsFilePickerConstructor(nsISupports *aOu
>                         void **aResult)
> {
>   *aResult = nsnull;
>   if (aOuter != nsnull) {
>       return NS_ERROR_NO_AGGREGATION;
>   }
>   nsCOMPtr<nsIFilePicker> picker;
>   
>-#ifdef MOZ_IPC
>     if (XRE_GetProcessType() == GeckoProcessType_Content)
>         picker = new nsFilePickerProxy();
>     else 
>-#endif
>         picker = new nsFilePicker;

fix indentation while you're here, this method has a bizarre mix of two-space and four-space that should be made consistent.

>diff --git a/widget/src/gtk2/nsWindow.h b/widget/src/gtk2/nsWindow.h

>-#ifdef MOZ_IPC
> #  include "mozilla/ipc/SharedMemorySysV.h"
>-#endif

unindent

>diff --git a/widget/src/shared/nsShmImage.h b/widget/src/shared/nsShmImage.h

>-#ifdef MOZ_IPC
> #  include "mozilla/ipc/SharedMemorySysV.h"
>-#endif

unindent

r=me with nits picked
Attachment #522545 - Flags: review?(benjamin) → review+
Attached patch patch (obsolete) — Splinter Review
Nits addressed, except for mercurial whining about no newline at the end of chromium-config.mk (It seemed to ignore any changes made by me so I just gave up after a few tries). Will file a followup bug on moving those methods to Layers.h as well.
Attachment #522545 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/a5dbece71e4a

Thanks for the patch!
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Attached patch patch (obsolete) — Splinter Review
This patch _should_ fix the test failures but I don't have time to test it actually works right now.
Attachment #523169 - Attachment is obsolete: true
Can you just attach the change on top of the last one, instead of a whole new patch, so we can see what changed?
(In reply to comment #10)
> Can you just attach the change on top of the last one, instead of a whole new
> patch, so we can see what changed?

The only difference between the latest patch and the patch that burned the tree are the changes in History.cpp. Those changes were actually in the first patch posted in this bug, but were eaten by Mercurial when updating the patch for some reason. I'll update the patch again and double-check if nothing else got lost when rebasing.
Attached patch patchSplinter Review
Here's the updated version of the last patch. It should work just fine, but it wouldn't hurt running this through the try server just to be sure. However I don't have commit access, so is there anyone that can push this patch for me?
Attachment #523307 - Attachment is obsolete: true
Keywords: checkin-needed
Attached patch bustage fixSplinter Review
Apparently I missed several uses of MOZ_IPC. Not asking for review since this is technically a bustage fix even tho it didn't seem to trigger any test failures somehow.
Attached patch mobile patchSplinter Review
This broke Fennec because neither mobile-browser nor the new mozilla-central/mobile subdirectory was included.
Attachment #524349 - Flags: review?(mark.finkle)
Attachment #524349 - Flags: review?(mark.finkle) → review+
Pushed bustage fix and mobile patch:
http://hg.mozilla.org/mozilla-central/rev/681420e25df6
http://hg.mozilla.org/mozilla-central/rev/f3b02ea8cbf1
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
http://hg.mozilla.org/mozilla-central/rev/70cedf3a3327
Whiteboard: fixed-in-cedar
Target Milestone: --- → mozilla2.2
Up to now it was possible to build for PPC but IPC is not compiling there so Firefox cannot be built for ppc any longer.
I think that's bug 579757. That's intentional, we are making IPC support a required platform feature. Non-tier-1 platforms can add support for it, but we're not going to support ifdefs to work without it.
Blocks: 659944
Shouldn't run-if.config = ipc lines be removed from testing/xpcshell/xpcshell.ini?
You need to log in before you can comment on or make changes to this bug.