Remove unused constructors in Color

[fedejeanne]

They were added a while ago but they are not used and their visibility makes them unusable outside the same package.

What confuses me is the comment (JavaDoc) saying Prevents uninitialized instances from being created outside the package. Maybe @jonahgraham / @akurtakov remember why they were needed or what the JavaDoc means?

Additional info

• Introduced in 6f25fa5
• Fix for https://bugs.eclipse.org/bugs/show_bug.cgi?id=563560
• Related to https://bugs.eclipse.org/bugs/show_bug.cgi?id=563018

[fedejeanne]

I'm keeping this PR as a draft to prevent that someone merges it before I can fully understand the reason for the constructors in the first place. This is more of an "exploratory" PR.

[github-actions]

Test Results

   545 files  ±0     545 suites  ±0   28m 12s ⏱️ - 3m 50s
 4 376 tests ±0   4 358 ✅ ±0   18 💤 ±0  0 ❌ ±0 
16 643 runs  ±0  16 503 ✅ ±0  140 💤 ±0  0 ❌ ±0 

Results for commit f47a1e9. ± Comparison against base commit 44f0d56.

[jonahgraham]

TL;DR:

I don't have an objection to this patch as written, if someone wants to propagate the removal of device in Color.gtk_new/win32_new/cocoa_new at some point in the future it is easy to add back in.

---

I have pulled together much of the previous discussion here.

The short version is that if we could break API compatibility we wouldn't have Color extend Device anymore. Since that is too much of a breaking change, we have to workaround partially living devices. Below isn't new information, but with the loss of data from gerrit being gone I am restating them here.

There was some discussion when we removed the use of device from Color about what backwards compatibility, especially related to operation of getDevice. You can see that getDevice has a special case just for Color.

I bring this up, because I believe the correct fix is to actually use the new device-less constructor rather than remove them from their call sites (Color.gtk_new/win32_new/cocoa_new). But doing so may resurrect the earlier conversation as to whether anyone relies on Color.getDevice() to return the device passed in. In addition, you have to decide how far back through the call hierarchy to propagate the removal, all calls to Color.gtk_new/win32_new/cocoa_new probably shouldn't have device passed in either.

Much of the above happened on the now gone gerrit instance, but some of the comments in Bugzilla still exist:

• https://bugs.eclipse.org/bugs/show_bug.cgi?id=563018#c17
• https://bugs.eclipse.org/bugs/show_bug.cgi?id=563560#c9
• https://bugs.eclipse.org/bugs/show_bug.cgi?id=565208#c0

IIRC I achieved my primary goal of removing Colors being mis-reported in S-Leak and decided not to propagate the removal of Device further as that would have involved touching too many files to warrant the value.

As for the comment (Prevents uninitialized instances from being created outside the package.) it was just copied from the existing constructor when I wrote the new one. It isn't quite right and you can update it - I suspect the comment was to explain why the constructor isn't public, but that long predates my involvement.

[fedejeanne]

Thank you @jonahgraham for the clarification and for approving the PR. Since the constructor is non-API and it's not used internally, it is safe to be removed so I'm merging it.

I briefly thought that maybe there were some code using reflection which relied on the existence of such constructors (kind of like JUnit 3 needing constructors without any parameters) but since this does not seem to be the case, It's all good 👍