1242436 - [Static Analysis][Logically dead code] In functions PushNotifier::NotifySubscriptionChangeWorkers and PushNotifier::NotifyPushWorkers

Status: RESOLVED FIXED

(Core :: DOM: Push Subscriptions, defect)

Description

[Andi [:andi]]

The Static Analysis tool Coverity added that variable ok is assign false an thus determining a logically dead code. 
For example, the below code from NotifySubscriptionChangeWorkers: 

>>  // Parent process, e10s enabled.
>>  bool ok = false;
>>  nsTArray<ContentParent*> contentActors;
>>  ContentParent::GetAll(contentActors);
>>  for (uint32_t i = 0; i < contentActors.Length(); ++i) {
>>    ok &= contentActors[i]->SendPushSubscriptionChange(
>>      PromiseFlatCString(aScope), IPC::Principal(aPrincipal));
>> }
>>  return ok ? NS_OK : NS_ERROR_FAILURE;

regardless of the value from contentActors[i]->SendPushSubscriptionChange, ok will always be false and in the end function will always return NS_ERROR_FAILURE.

I guess that default value for ok should have been true, and in this way if SendPushSubscriptionChange had failed value of ok would change to false, resulting that function would return  NS_ERROR_FAILURE otherwise returns NS_OK.

Comment 1

[Andi [:andi]]

Attachment: MozReview Request: Bug 1242436 - default value of ok = true in order to check the return of SendPush and SendPushSubscriptionChange. r?kitcambridge

Review commit: https://reviewboard.mozilla.org/r/32277/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32277/

Comment 2

[Andi [:andi]]

Comment on attachment 8711640 [details]
MozReview Request: Bug 1242436 - default value of ok = true in order to check the return of SendPush and SendPushSubscriptionChange. r?kitcambridge

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/32277/diff/1-2/

Comment 3

[Lina Butler [:lina]]

Comment on attachment 8711640 [details]
MozReview Request: Bug 1242436 - default value of ok = true in order to check the return of SendPush and SendPushSubscriptionChange. r?kitcambridge

https://reviewboard.mozilla.org/r/32277/#review28943

Thanks for catching that!

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fd1b82f7fbee

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fd1b82f7fbee