Remove CHROMIUM_MOZILLA_BUILD ifdefs, since always defined and #ifndef codepaths broken anyway

RESOLVED FIXED in mozilla9

Status

()

Core
Build Config
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: emorley, Assigned: emorley)

Tracking

Trunk
mozilla9
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
(2011-08-22 12:03:46) ejpbruel: Ms2ger: in child_process_host.cc, there is some code that is not enclosed in CHROMIUM_MOZILLA_BUILD, but can only work if that macro is enabled, because it relies on a function that is inherited from a class that is only included if that macro is there
(2011-08-22 12:04:10) ejpbruel: Ms2ger: is it possible that the use of this macro is currently unchecked, and therefore even broken in places?
(2011-08-22 12:04:12) Ms2ger: ejpbruel, yeah, it's been fiction for a while already
(2011-08-22 12:04:39) ejpbruel: Ms2ger: we might wanna get rid of it then, its only purpose now is to confuse me when studying code
(2011-08-22 12:05:08) Ms2ger: Go for it :)
Note that it's always defined *on Mozilla builds*. The code comes from chromium, and there, it's not defined. I don't know if the original chromium code now contains some of these CHROMIUM_MOZILLA_BUILD ifdefs, nor if we care about keeping them. Benjamin would know, I guess.

Comment 2

6 years ago
The ifdefs were added to indicate the places that we modified the chromium code. Since we don't expect to stay synced with that code, we can probably remove them, but it's cjones' call.
We haven't upstreamed any of our local modifications.  There are a small number that might be generally applicable (one helper and one new file come to mind), but most really are Mozilla specific.  FWIW, we tried to upstream a very general performance fix for STLport and it was not taken.  The chromium IPC impl for linux diverged from us in an incompatible way since we pulled, and we don't stand to gain anything but work by updating (yet).  The long-term plan is to extract the code from the 4 or 5 nontrivial files we pulled and integrate it more tightly into Gecko, so we can do away with the needless duplication of the 20 or more files of "framework" code that we pulled, for which analogues mostly already exist in Gecko.  That isn't a high priority for anyone though.

I personally don't care much about ifdef CHROMIUM_MOZILLA_BUILD, but if it's confusing people who are trying to understand the code, then I'm all for nuking it.
(Assignee)

Comment 4

6 years ago
Created attachment 554977 [details] [diff] [review]
Remove CHROMIUM_MOZILLA_BUILD

Removes the CHROMIUM_MOZILLA_BUILD ifdefs from the tree.

http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=6f5233f2fe68
Attachment #554977 - Flags: review?(jones.chris.g)
(Assignee)

Comment 5

6 years ago
Created attachment 554979 [details] [diff] [review]
Additional cleanup

I've kept this additional cleanup separate to make it easier to review, since it's everything that wasn't just mechanical CHROMIUM_MOZILLA_BUILD ifdef/ifndef removal.

IPC_MESSAGE_ENABLE_RPC is now always defined, so can be removed:
http://mxr.mozilla.org/mozilla-central/search?string=IPC_MESSAGE_ENABLE_RPC

The BASE_API prefix defines is now always empty, so can be removed:
http://mxr.mozilla.org/mozilla-central/search?string=BASE_API

MessageLoop::EnableHistogrammer(), MessageLoop::StartHistogrammer() and MessageLoop::HistogramEvent() no longer do anything, so have been taken out. 

This leaves the VALUE_TO_NUMBER_AND_NAME define unused, so has been taken out:
http://mxr.mozilla.org/mozilla-central/search?string=VALUE_TO_NUMBER_AND_NAME
Attachment #554979 - Flags: review?(jones.chris.g)
Attachment #554977 - Flags: review?(jones.chris.g) → review+
Attachment #554979 - Flags: review?(jones.chris.g) → review+
(Assignee)

Comment 6

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/0f3983da53fb
http://hg.mozilla.org/integration/mozilla-inbound/rev/08a3ae5c564a
Flags: in-testsuite-
Target Milestone: --- → mozilla9
http://hg.mozilla.org/mozilla-central/rev/0f3983da53fb
http://hg.mozilla.org/mozilla-central/rev/08a3ae5c564a
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.