Remove superfluous isNull checks

Remove superfluous isNull checks

The !isEmpty check already implies !isNull.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=166123

[rwlbuis, 2014-01-30 00:22:36]

Straightforward patch.

[Inactive, 2014-01-30 00:45:28]

lgtm

[commit-bot: I haz the power, 2014-01-30 00:45:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rob.buis@samsung.com/149873002/1

[dominicc (has gone to gerrit), 2014-01-30 03:14:39]

https://codereview.chromium.org/149873002/diff/1/Source/core/dom/Document.cpp
File Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/149873002/diff/1/Source/core/dom/Document.cpp...
Source/core/dom/Document.cpp:763: if (!typeExtension.isEmpty())
What about CustomElementConstructorBuilder.cpp/constructCustomElement?

[rwlbuis, 2014-01-30 03:26:33]

On 2014/01/30 03:14:39, dominicc wrote:
> https://codereview.chromium.org/149873002/diff/1/Source/core/dom/Document.cpp
> File Source/core/dom/Document.cpp (right):
> 
>
https://codereview.chromium.org/149873002/diff/1/Source/core/dom/Document.cpp...
> Source/core/dom/Document.cpp:763: if (!typeExtension.isEmpty())
> What about CustomElementConstructorBuilder.cpp/constructCustomElement?

I am not familiar with this code. However I'll stop the commit for now and see
if I can include that change in a next patch set.

[commit-bot: I haz the power, 2014-01-30 03:26:55]

CQ bit was unchecked on CL. Ignoring.

[dominicc (has gone to gerrit), 2014-01-30 03:31:20]

I could be misremembering, but I think that helper may pass a null type
extension to createElementNS.

If you remove the null tests, I think it makes sense to assert that the string
is not null. Because you seem to be relying on the IDL's handling of null, but
other callers (now or later) might not pick up on that subtlety.

[Inactive, 2014-01-30 03:36:28]

On 2014/01/30 03:31:20, dominicc wrote:
> I could be misremembering, but I think that helper may pass a null type
> extension to createElementNS.
> 
> If you remove the null tests, I think it makes sense to assert that the string
> is not null. Because you seem to be relying on the IDL's handling of null, but
> other callers (now or later) might not pick up on that subtlety.

He did *not* remove the null checks. Those are already included in the isEmpty()
checks.
This patch does not have any behavior change. It merely removes superfluous
checks.

If isNull() would return true, then isEmpty() would also return true.

[dominicc (has gone to gerrit), 2014-01-30 03:40:43]

Ah! That sounds fine.
On Jan 30, 2014 12:36 PM, <ch.dumez@samsung.com> wrote:

> On 2014/01/30 03:31:20, dominicc wrote:
>
>> I could be misremembering, but I think that helper may pass a null type
>> extension to createElementNS.
>>
>
>  If you remove the null tests, I think it makes sense to assert that the
>> string
>> is not null. Because you seem to be relying on the IDL's handling of
>> null, but
>> other callers (now or later) might not pick up on that subtlety.
>>
>
> He did *not* remove the null checks. Those are already included in the
> isEmpty()
> checks.
> This patch does not have any behavior change. It merely removes superfluous
> checks.
>
> If isNull() would return true, then isEmpty() would also return true.
>
> https://codereview.chromium.org/149873002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2014-01-30 12:12:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rob.buis@samsung.com/149873002/1

[commit-bot: I haz the power, 2014-01-30 12:12:46]

Change committed as 166123

[commit-bot: I haz the power, 2014-01-30 12:13:03]

CQ bit was unchecked on CL. Ignoring.