NullPointerException in BorderLayout when Layout Data is null

[laeubi]

It is assumed that a null layout data implies the center region, but BorderLayout.getTotal is not aware of this and throws:

Caused by: java.lang.NullPointerException
	at org.eclipse.swt.layout.BorderLayout.lambda$4(BorderLayout.java:233)
	at java.base/java.util.stream.ReferencePipeline$4$1.accept(ReferencePipeline.java:212)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1655)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.IntPipeline.reduce(IntPipeline.java:491)
	at java.base/java.util.stream.IntPipeline.sum(IntPipeline.java:449)
	at org.eclipse.swt.layout.BorderLayout.getTotal(BorderLayout.java:233)
	at org.eclipse.swt.layout.BorderLayout.computeSize(BorderLayout.java:147)
	at org.eclipse.swt.widgets.Composite.computeSizeInPixels(Composite.java:269)
	at org.eclipse.swt.widgets.Control.computeSize(Control.java:837)
	at org.eclipse.swt.widgets.Control.pack(Control.java:1609)
	at org.eclipse.swt.widgets.Control.pack(Control.java:1584)

[niraj-modi]

Similarly with null Layout Data and layout.type = SWT.VERTICAL; BorderLayout.getMax() also throws NPE as below:
Exception in thread "main" java.lang.NullPointerException
at org.eclipse.swt.layout.BorderLayout.lambda$5(BorderLayout.java:244)
at java.base/java.util.stream.ReferencePipeline$4$1.accept(ReferencePipeline.java:212)
at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1654)
at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.base/java.util.stream.IntPipeline.reduce(IntPipeline.java:496)
at java.base/java.util.stream.IntPipeline.max(IntPipeline.java:459)
at org.eclipse.swt.layout.BorderLayout.getMax(BorderLayout.java:244)
at org.eclipse.swt.layout.BorderLayout.getMax(BorderLayout.java:240)
at org.eclipse.swt.layout.BorderLayout.computeSize(BorderLayout.java:149)
at org.eclipse.swt.widgets.Composite.computeSizeInPixels(Composite.java:217)
at org.eclipse.swt.widgets.Control.computeSize(Control.java:663)
at org.eclipse.swt.widgets.Control.pack(Control.java:2198)
at org.eclipse.swt.widgets.Control.pack(Control.java:2168)
at org.eclipse.swt.snippets.Snippet379.main(Snippet379.java:50)

[joel-majano]

I can take a look at this issue and begin working on a fix. Just want to confirm, @laeubi do you have a patch in progress already?

[laeubi]

@joel-majano I haven't yet prepared a patch and contributions are always welcome, so please go ahead
Just add me as a reviewer then, otherwise I'll be looking into this next week. I think the first step would be to enhance the test-case to reproduce this issue.

[joel-majano]

I was able to recreate the NPE that Niraj commented on, but to recreate the original issue I have to set SWT.HORIZONTAL and call shell.pack() explicitly (in both cases). Are these the expected failure conditions?

[laeubi]

At least the stacktrace indicates that there is a pack() involved:

at org.eclipse.swt.widgets.Control.pack(Control.java:2198)

[joel-majano]

Okay, in that case, I get this issue even if LayoutData is not null. Just taking Snippet 379, adding a shell.pack() call right before shell.open() creates the NPE. SWT.VERTICAL creates an NPE in BorderLayout.getMax() and SWT.HORIZONTAL creates an NPE in BorderLayout.getTotal().

Also, what would be the expected behaviour of pack()? I know BorderLayout has some spacing/margin dimensions specified, so is pack() a valid call with BorderLayout?

[laeubi]

I don't see a reason why pack() should not be supported.

[joel-majano]

Alright, still looking into this, no patch yet.