Remove useless else blocks in order to reduce complexity

RESOLVED FIXED in Firefox 54

Status

()

Core
Rewriting and Analysis
RESOLVED FIXED
10 months ago
10 months ago

People

(Reporter: sylvestre, Assigned: sylvestre)

Tracking

(Blocks: 1 bug)

Trunk
mozilla54
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(37 attachments)

59 bytes, text/x-review-board-request
davidb
: review+
Details | Review
59 bytes, text/x-review-board-request
fitzgen
: review+
Details | Review
59 bytes, text/x-review-board-request
Away for a while
: review+
Details | Review
59 bytes, text/x-review-board-request
baku
: review+
Details | Review
59 bytes, text/x-review-board-request
baku
: review+
Details | Review
59 bytes, text/x-review-board-request
baku
: review+
Details | Review
59 bytes, text/x-review-board-request
cpearce
: review+
Details | Review
59 bytes, text/x-review-board-request
jya
: review+
Details | Review
59 bytes, text/x-review-board-request
nical
: review+
Details | Review
59 bytes, text/x-review-board-request
jya
: review+
Details | Review
59 bytes, text/x-review-board-request
jimm
: review+
Details | Review
59 bytes, text/x-review-board-request
Dolske
: review+
Details | Review
59 bytes, text/x-review-board-request
dholbert
: review+
Details | Review
59 bytes, text/x-review-board-request
bbouvier
: review+
Details | Review
59 bytes, text/x-review-board-request
heycam
: review+
Details | Review
59 bytes, text/x-review-board-request
jesup
: review+
Details | Review
59 bytes, text/x-review-board-request
njn
: review+
Details | Review
59 bytes, text/x-review-board-request
dragana
: review+
Details | Review
59 bytes, text/x-review-board-request
tnikkel
: review+
Details | Review
59 bytes, text/x-review-board-request
froydnj
: review+
Details | Review
59 bytes, text/x-review-board-request
drno
: review+
Details | Review
59 bytes, text/x-review-board-request
Dexter
: review+
Details | Review
59 bytes, text/x-review-board-request
jmaher
: review+
Details | Review
59 bytes, text/x-review-board-request
mhowell
: review+
Details | Review
59 bytes, text/x-review-board-request
froydnj
: review+
Details | Review
59 bytes, text/x-review-board-request
froydnj
: review+
Details | Review
59 bytes, text/x-review-board-request
froydnj
: review+
Details | Review
59 bytes, text/x-review-board-request
Pike
: review+
Details | Review
59 bytes, text/x-review-board-request
gcp
: review+
Details | Review
59 bytes, text/x-review-board-request
Away for a while
: review+
Details | Review
59 bytes, text/x-review-board-request
mstange
: review+
Details | Review
59 bytes, text/x-review-board-request
francois
: review+
Details | Review
59 bytes, text/x-review-board-request
Away for a while
: review+
Details | Review
59 bytes, text/x-review-board-request
jimm
: review+
Details | Review
59 bytes, text/x-review-board-request
froydnj
: review+
Details | Review
59 bytes, text/x-review-board-request
mstange
: review+
Details | Review
59 bytes, text/x-review-board-request
nical
: review+
Details | Review
(Assignee)

Description

10 months ago
Trying this checkers (with automatic rewrite):
http://clang.llvm.org/extra/clang-tidy/checks/readability-else-after-return.html

It can transform such code:
bool nsViewManager::IsViewInserted(nsView *aView)
{
  if (mRootView == aView) {
    return true;
  } else if (aView->GetParent() == nullptr) {
    return false;
  } else {
    nsView* view = aView->GetParent()->GetFirstChild();
    while (view != nullptr) {
      if (view == aView) {
        return true;
      }
      view = view->GetNextSibling();
    }
    return false;
  }
}
into
bool nsViewManager::IsViewInserted(nsView *aView)
{
  if (mRootView == aView) {
    return true;
  }
  if (aView->GetParent() == nullptr) {
    return false;
  }
  nsView* view = aView->GetParent()->GetFirstChild();
  while (view != nullptr) {
    if (view == aView) {
      return true;
    }
    view = view->GetNextSibling();
  }
  return false;
}
(Assignee)

Comment 1

10 months ago
This is part of the evaluation of the reliability of checkers to integrate them more into the workflow.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 39

10 months ago
mozreview-review
Comment on attachment 8835412 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in layout/

https://reviewboard.mozilla.org/r/111134/#review112308

::: layout/style/nsCSSRuleProcessor.cpp:2187
(Diff revision 1)
>        else if (attr->mFunction == NS_ATTR_FUNC_EQUALS) {
>          result =
>            aElement->
>            AttrValueIs(attr->mNameSpace, matchAttribute, attr->mValue,
>                        attr->IsValueCaseSensitive(isHTML) ? eCaseMatters
>                        : eIgnoreCase);

Nit: Please preserve the indentation here.
Attachment #8835412 - Flags: review?(cam) → review+

Comment 40

10 months ago
mozreview-review
Comment on attachment 8835416 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in view/

https://reviewboard.mozilla.org/r/111142/#review112310
Attachment #8835416 - Flags: review?(tnikkel) → review+

Comment 41

10 months ago
mozreview-review
Comment on attachment 8835419 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/components/telemetry/

https://reviewboard.mozilla.org/r/111148/#review112312
Attachment #8835419 - Flags: review?(alessio.placitelli) → review+
(Assignee)

Comment 42

10 months ago
(In reply to Cameron McCormack (:heycam) from comment #39)
> Nit: Please preserve the indentation here.
Yeah, sorry, I run clang-format on the diff.
I will fix that.

Comment 43

10 months ago
mozreview-review
Comment on attachment 8835420 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity testing/mochitest/ssltunnel/

https://reviewboard.mozilla.org/r/111150/#review112316

great stuff
Attachment #8835420 - Flags: review?(jmaher) → review+

Comment 44

10 months ago
mozreview-review
Comment on attachment 8835411 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in js/

https://reviewboard.mozilla.org/r/111132/#review112318

I've stopped reviewing at some point: JS code style allows us to have code up to 99 columns, comments up to 79 columns. This patch contains a lot of unnecessary line-wrapping that I'd like to see removed before r+ing this.

::: js/xpconnect/loader/mozJSSubScriptLoader.cpp:165
(Diff revision 1)
> -                return false;
> +            return false;
> -            }
> +        }
> -            return JS::CompileFunction(cx, envChain, options, nullptr, 0, nullptr,
> +        return JS::CompileFunction(cx, envChain, options, nullptr, 0, nullptr,
> -                                       srcBuf, function);
> +                                   srcBuf, function);
> -        }
>      } else {

Heh, still an else-after-return here :) Guess your tool should be re-run until you reach a fixed-point.

::: js/xpconnect/wrappers/XrayWrapper.cpp:561
(Diff revision 1)
> +        if (IsTypedArrayKey(key)) {
> -            if (IsArrayIndex(GetArrayIndexFromId(cx, id))) {
> +          if (IsArrayIndex(GetArrayIndexFromId(cx, id))) {
> -                // WebExtensions can't use cloneInto(), so we just let them do
> +            // WebExtensions can't use cloneInto(), so we just let them do
> -                // the slow thing to maximize compatibility.
> +            // the slow thing to maximize compatibility.
> -                if (CompartmentPrivate::Get(CurrentGlobalOrNull(cx))->isWebExtensionContentScript) {
> +            if (CompartmentPrivate::Get(CurrentGlobalOrNull(cx))
> +                  ->isWebExtensionContentScript) {

nit: keep all of this one one line please.

::: js/xpconnect/wrappers/XrayWrapper.cpp:567
(Diff revision 1)
> -                    Rooted<PropertyDescriptor> innerDesc(cx);
> +              Rooted<PropertyDescriptor> innerDesc(cx);
> -                    {
> +              {
> -                        JSAutoCompartment ac(cx, target);
> +                JSAutoCompartment ac(cx, target);
> -                        JS_MarkCrossZoneId(cx, id);
> +                JS_MarkCrossZoneId(cx, id);
> -                        if (!JS_GetOwnPropertyDescriptorById(cx, target, id, &innerDesc))
> +                if (!JS_GetOwnPropertyDescriptorById(
> +                      cx, target, id, &innerDesc))

ditto

::: js/xpconnect/wrappers/XrayWrapper.cpp:571
(Diff revision 1)
> -                        if (!JS_GetOwnPropertyDescriptorById(cx, target, id, &innerDesc))
> +                if (!JS_GetOwnPropertyDescriptorById(
> +                      cx, target, id, &innerDesc))
> -                            return false;
> +                  return false;
> -                    }
> +              }
> -                    if (innerDesc.isDataDescriptor() && innerDesc.value().isNumber()) {
> +              if (innerDesc.isDataDescriptor() &&
> +                  innerDesc.value().isNumber()) {

ditto

::: js/xpconnect/wrappers/XrayWrapper.cpp:582
(Diff revision 1)
> -                    JS_ReportErrorASCII(cx, "Accessing TypedArray data over Xrays is slow, and forbidden "
> +            JS_ReportErrorASCII(
> +              cx,
> +              "Accessing TypedArray data over Xrays is slow, and forbidden "
> -                                        "in order to encourage performant code. To copy TypedArrays "
> +              "in order to encourage performant code. To copy TypedArrays "
> -                                        "across origin boundaries, consider using Components.utils.cloneInto().");
> +              "across origin boundaries, consider using "
> +              "Components.utils.cloneInto().");

Keep the original formatting here too, please.

::: js/xpconnect/wrappers/XrayWrapper.cpp:590
(Diff revision 1)
>          } else if (key == JSProto_Function) {
> -            if (id == GetJSIDByIndex(cx, XPCJSContext::IDX_LENGTH)) {
> +          if (id == GetJSIDByIndex(cx, XPCJSContext::IDX_LENGTH)) {
> -                FillPropertyDescriptor(desc, wrapper, JSPROP_PERMANENT | JSPROP_READONLY,
> +            FillPropertyDescriptor(
> +              desc,
> +              wrapper,
> +              JSPROP_PERMANENT | JSPROP_READONLY,

ditto

::: js/xpconnect/wrappers/XrayWrapper.cpp:596
(Diff revision 1)
> -                                       NumberValue(JS_GetFunctionArity(JS_GetObjectFunction(target))));
> +              NumberValue(JS_GetFunctionArity(JS_GetObjectFunction(target))));
> -                return true;
> +            return true;
> -            } else if (id == GetJSIDByIndex(cx, XPCJSContext::IDX_NAME)) {
> -                RootedString fname(cx, JS_GetFunctionId(JS_GetObjectFunction(target)));
> +          }
> +          if (id == GetJSIDByIndex(cx, XPCJSContext::IDX_NAME)) {
> +            RootedString fname(cx,
> +                               JS_GetFunctionId(JS_GetObjectFunction(target)));

Stopped here.
Attachment #8835411 - Flags: review?(bbouvier)

Comment 45

10 months ago
mozreview-review
Comment on attachment 8835415 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in netwerk/

https://reviewboard.mozilla.org/r/111140/#review112320

::: netwerk/dns/nsEffectiveTLDService.cpp:290
(Diff revision 1)
>        if (entry->IsWild() && prevDomain) {
>          // wildcard rules imply an eTLD one level inferior to the match.
>          eTLD = prevDomain;
>          break;
>  
> -      } else if (entry->IsNormal() || !nextDot) {
> +      } if (entry->IsNormal() || !nextDot) {

move 'if (...' into a new line.
Attachment #8835415 - Flags: review?(dd.mozilla) → review+

Comment 46

10 months ago
mozreview-review-reply
Comment on attachment 8835411 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in js/

https://reviewboard.mozilla.org/r/111132/#review112318

> Heh, still an else-after-return here :) Guess your tool should be re-run until you reach a fixed-point.

note that our coding style requires the use of brackets, even for single line operation.
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Control_Structures

Comment 47

10 months ago
mozreview-review-reply
Comment on attachment 8835411 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in js/

https://reviewboard.mozilla.org/r/111132/#review112318

> note that our coding style requires the use of brackets, even for single line operation.
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Control_Structures

Also note Spidermonkey uses its own style: https://wiki.mozilla.org/JavaScript:SpiderMonkey:C_Coding_Style
Not sure if xpconnect's style is the same as Spidermonkey. A good rule of thumb would be to reuse the same style as in this file to keep it consistent.

Comment 48

10 months ago
mozreview-review
Comment on attachment 8835401 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/browser-element/

https://reviewboard.mozilla.org/r/111112/#review112334

::: dom/browser-element/BrowserElementParent.cpp:173
(Diff revision 1)
>                             val, &status);
>  
>    if (dispatchSucceeded) {
>      if (aPopupFrameElement->IsInUncomposedDoc()) {
>        return BrowserElementParent::OPEN_WINDOW_ADDED;
> -    } else if (status == nsEventStatus_eConsumeNoDefault) {
> +    } if (status == nsEventStatus_eConsumeNoDefault) {

if() in a new line.
Attachment #8835401 - Flags: review?(amarchesini) → review+

Comment 49

10 months ago
mozreview-review
Comment on attachment 8835402 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/canvas

https://reviewboard.mozilla.org/r/111114/#review112336
Attachment #8835402 - Flags: review?(amarchesini) → review+

Comment 50

10 months ago
mozreview-review
Comment on attachment 8835403 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/ipc

https://reviewboard.mozilla.org/r/111116/#review112338
Attachment #8835403 - Flags: review?(amarchesini) → review+

Comment 51

10 months ago
mozreview-review
Comment on attachment 8835407 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/media/

https://reviewboard.mozilla.org/r/111124/#review112340

::: dom/media/fmp4/MP4Demuxer.cpp:401
(Diff revision 1)
>      aNumSamples--;
>    }
>  
>    if (samples->mSamples.IsEmpty()) {
> -    return SamplesPromise::CreateAndReject(NS_ERROR_DOM_MEDIA_END_OF_STREAM, __func__);
> -  } else {
> +    return SamplesPromise::CreateAndReject(NS_ERROR_DOM_MEDIA_END_OF_STREAM,
> +                                           __func__);

fairly certain that this is no longer applicable following bug 1325707

::: dom/media/fmp4/MP4Demuxer.cpp:413
(Diff revision 1)
> -        mNeedSPSForTelemetry = AccumulateSPSTelemetry(extradata);
> +      mNeedSPSForTelemetry = AccumulateSPSTelemetry(extradata);
> -      }
> +    }
> -    }
> +  }
>  
> -    if (mNextKeyframeTime.isNothing() ||
> +  if (mNextKeyframeTime.isNothing() ||
> -        samples->mSamples.LastElement()->mTime >= mNextKeyframeTime.value().ToMicroseconds()) {
> +      samples->mSamples.LastElement()->mTime >=

operator >= is to be on the next line and aligned with the first operand.

Comment 52

10 months ago
mozreview-review
Comment on attachment 8835426 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in security/sandbox/linux/

https://reviewboard.mozilla.org/r/111162/#review112348
Attachment #8835426 - Flags: review?(gpascutto) → review+

Comment 53

10 months ago
mozreview-review
Comment on attachment 8835406 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in gfx/

https://reviewboard.mozilla.org/r/111122/#review112350

::: gfx/2d/image_operations.cpp:196
(Diff revision 1)
>  SkBitmap ImageOperations::Resize(const SkBitmap& source,
>                                   ResizeMethod method,
>                                   int dest_width, int dest_height,
>                                   const SkIRect& dest_subset,
>                                   void* dest_pixels /* = nullptr */) {
>    if (method == ImageOperations::RESIZE_SUBPIXEL)

Please use this as an occasion to brace this if block.

::: gfx/layers/LayerTreeInvalidation.cpp:796
(Diff revision 1)
>        *aGeometryChanged = true;
>      }
>      return result;
> -  } else {
> -    bool geometryChanged = (aGeometryChanged != nullptr) ? *aGeometryChanged : false;
> +  }
> +  bool geometryChanged =
> +    (aGeometryChanged != nullptr) ? *aGeometryChanged : false;

nit: I'd rather keep this assignment in one line.

::: gfx/layers/Layers.cpp:673
(Diff revision 1)
>  }
>  
>  Matrix4x4
>  Layer::GetLocalTransform()
>  {
>    if (HostLayer* shadow = AsHostLayer())

Please brace the if here as well (I realize I'm hijacking this patch but I care more about this than the extra else blocks).
Attachment #8835406 - Flags: review?(nical.bugzilla) → review+

Comment 54

10 months ago
mozreview-review
Comment on attachment 8835434 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in widget/xremoteclient/

https://reviewboard.mozilla.org/r/111178/#review112354

::: widget/xremoteclient/XRemoteClient.cpp:154
(Diff revision 1)
>  {
>    if (event->error_code == BadWindow) {
>      sGotBadWindow = true;
>      return 0; // ignored
>    }
> -  else {
> +  

Please remove the trailin spaces here and two lines below. Also, it looks like the statement below has an extra indentation now.
Attachment #8835434 - Flags: review?(nical.bugzilla) → review+

Comment 55

10 months ago
mozreview-review
Comment on attachment 8835429 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/components/url-classifier/

https://reviewboard.mozilla.org/r/111168/#review112358
Attachment #8835429 - Flags: review?(francois) → review+
Comment on attachment 8835422 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in modules/libjar/

https://reviewboard.mozilla.org/r/111154/#review112378
Attachment #8835422 - Flags: review?(nfroyd) → review+
Comment on attachment 8835423 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in modules/libpref/

https://reviewboard.mozilla.org/r/111156/#review112380
Attachment #8835423 - Flags: review?(nfroyd) → review+
Comment on attachment 8835424 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in mozglue/misc/

https://reviewboard.mozilla.org/r/111158/#review112382
Attachment #8835424 - Flags: review?(nfroyd) → review+
Comment on attachment 8835432 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in uriloader/exthandler/unix/

https://reviewboard.mozilla.org/r/111174/#review112384

::: uriloader/exthandler/unix/nsOSHelperAppService.cpp:1117
(Diff revision 1)
>  
>              if (match) { // we did not fail any test clauses; all is good
>                // get out of here
>                mailcapFile->Close();
>                return NS_OK;
> -            } else { // pretend that this match never happened
> +            } // pretend that this match never happened

Please move the comment to the next line, on its own line.
Attachment #8835432 - Flags: review?(nfroyd) → review+
Comment on attachment 8835417 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in xpcom/

https://reviewboard.mozilla.org/r/111144/#review112374

No objection in principle to this change, but r- for all the unrelated formatting changes (wrapping, indentation, changing the spaces and whatnot in individual lines).

::: xpcom/build/FileLocation.cpp:197
(Diff revision 1)
> -      int32_t read = PR_Read(mFd, aBuf + totalRead,
> +      int32_t read = PR_Read(mFd,
> +                             aBuf + totalRead,

Please revert this change.

::: xpcom/build/FileLocation.cpp:208
(Diff revision 1)
> -    nsZipCursor cursor(mItem, mZip, reinterpret_cast<uint8_t*>(aBuf),
> -                       aLen, true);
> +    nsZipCursor cursor(
> +      mItem, mZip, reinterpret_cast<uint8_t*>(aBuf), aLen, true);

Please revert this change.

::: xpcom/build/FileLocation.cpp:213
(Diff revision 1)
> -      nsZipArchive::sFileCorruptedReason = "FileLocation::Data: insufficient data";
> +      nsZipArchive::sFileCorruptedReason =
> +        "FileLocation::Data: insufficient data";

Please revert this change.

::: xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:74
(Diff revision 1)
> -            if (nr_fpr < FPR_COUNT)
> +          if (nr_fpr < FPR_COUNT)
> -                dp->val.d = fpregs[nr_fpr++];
> +            dp->val.d = fpregs[nr_fpr++];
> -            else
> +          else
> -                dp->val.d = *(double*) ap++;
> +            dp->val.d = *(double*)ap++;

Some of the indentation changes in this file are appropriate...and some are not.  Please review this file (or turn the clang-format pass off, or...) before the next review.
Attachment #8835417 - Flags: review?(nfroyd) → review-

Comment 61

10 months ago
mozreview-review
Comment on attachment 8835425 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in rdf/util/

https://reviewboard.mozilla.org/r/111160/#review112420
Attachment #8835425 - Flags: review?(l10n) → review+
(Assignee)

Updated

10 months ago
Depends on: 1338171

Comment 62

10 months ago
mozreview-review
Comment on attachment 8835413 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/

https://reviewboard.mozilla.org/r/111136/#review112428

r- for style changes and for changing a directory of imported code (I'll ping bwc to review though since he handles that imported code, and JSEP)

::: media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp:2503
(Diff revision 1)
> +  if (mCurrentLocalDescription) {
>      return mCurrentLocalDescription.get();
>    }
>  
>    return nullptr;

Since this is a UniquePtr, wouldn't this be just "return mCurrentLocalDescription.get()"?

::: media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp:2516
(Diff revision 1)
> +  if (mCurrentRemoteDescription) {
>      return mCurrentRemoteDescription.get();
>    }
>  
>    return nullptr;

ditto

::: media/webrtc/signaling/src/media-conduit/AudioConduit.cpp:553
(Diff revision 1)
> -  if(!success)
> +  if (!success) {
> -  {

Is this supposed to be a style-rewrite of these files as well?  While I wouldn't be against a style-fixing pass (especially for files with mixed styles), I'd suggest keeping them separate.

::: media/webrtc/signaling/src/media-conduit/AudioConduit.cpp:1003
(Diff revision 1)
> -  if(GetNum10msSamplesForFrequency(freq))
> +   if (GetNum10msSamplesForFrequency(freq)) {
> -  {
> -    return true;
> +     return true;
> -  } else {
> -    return false;
> -  }
> +   }
> +   return false;

return !!GetNum10msSamplesForFrequency(freq);
Or != 0 if that's clearer than !!

::: media/webrtc/signaling/src/media-conduit/AudioConduit.cpp:1013
(Diff revision 1)
> -  switch(samplingFreqHz)
> -  {
> -    case 16000: return 160; //160 samples
> -    case 32000: return 320; //320 samples
> -    case 44100: return 441; //441 samples
> -    case 48000: return 480; //480 samples
> -    default:    return 0; // invalid or unsupported
> +   switch (samplingFreqHz) {
> +     case 16000:
> +       return 160; //160 samples
> +     case 32000:
> +       return 320; //320 samples
> +     case 44100:
> +       return 441; //441 samples
> +     case 48000:
> +       return 480; //480 samples
> +     default:
> +       return 0; // invalid or unsupported
> -  }
> +   }

This is all style

::: media/webrtc/signaling/src/media-conduit/VideoConduit.cpp:314
(Diff revision 1)
>    if ("VP8" == name) {
>      return webrtc::VideoEncoder::EncoderType::kVp8;
> -  } else if ("VP9" == name) {
> +  }
> +  if ("VP9" == name) {
>      return webrtc::VideoEncoder::EncoderType::kVp9;
> -  } else if ("H264" == name) {
> +  }
> +  if ("H264" == name) {
>      return webrtc::VideoEncoder::EncoderType::kH264;
>    }

I actually find the original more obvious here, and I believe there's no performance penalty for this.

::: media/webrtc/signaling/src/media-conduit/VideoConduit.cpp:385
(Diff revision 1)
>    if ("VP8" == name) {
>      return webrtc::VideoDecoder::DecoderType::kVp8;
> -  } else if ("VP9" == name) {
> +  }
> +  if ("VP9" == name) {
>      return webrtc::VideoDecoder::DecoderType::kVp9;
> -  } else if ("H264" == name) {
> +  }
> +  if ("H264" == name) {
>      return webrtc::VideoDecoder::DecoderType::kH264;
>    }
>  
>    return webrtc::VideoDecoder::DecoderType::kUnsupportedCodec;

ditto

::: media/webrtc/signaling/src/sdp/sipcc/sdp_access.c:72
(Diff revision 1)
>  {
> -    if (sdp_p->version == SDP_INVALID_VALUE) {
> +  if (sdp_p->version == SDP_INVALID_VALUE) {
> -        return (FALSE);
> +    return (FALSE);
> -    } else {
> -        return (TRUE);
> -    }
> +  }
> +  return (TRUE);

This is imported code.  That said, we basically maintain it, but I don't suggest running style checks on imported code.

If we do want to change this, I'd make it "return sdp_t->version != SDP_INVALID_VALUE;"
Attachment #8835413 - Flags: review?(rjesup) → review-

Comment 63

10 months ago
Comment on attachment 8835413 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/

Byron - see my review comments before looking
Attachment #8835413 - Flags: review?(docfaraday)

Comment 64

10 months ago
mozreview-review
Comment on attachment 8835413 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/

https://reviewboard.mozilla.org/r/111136/#review112456

Please don't mess with the sipcc code like this. This invites merge hell into our lives. The rest of this looks basically fine to me.
Attachment #8835413 - Flags: review?(docfaraday) → review-

Comment 65

10 months ago
mozreview-review
Comment on attachment 8835421 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity modules/libmar/

https://reviewboard.mozilla.org/r/111152/#review112464
Attachment #8835421 - Flags: review?(mhowell) → review+
(Assignee)

Comment 66

10 months ago
(In reply to Byron Campen [:bwc] from comment #64)
> Please don't mess with the sipcc code like this.
Wasn't aware it was thirdparty code, added it to the list: bug 1338193

Comment 67

10 months ago
mozreview-review
Comment on attachment 8835399 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity devtools/shared/

https://reviewboard.mozilla.org/r/111108/#review112502
Attachment #8835399 - Flags: review?(nfitzgerald) → review+

Comment 68

10 months ago
mozreview-review
Comment on attachment 8835410 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity image/

https://reviewboard.mozilla.org/r/111130/#review112504

Thanks for doing this cleanup!

One general nit, mostly for future reference -- so right now, it looks like every single one of these commit messages is:
> Bug 1338086 - Remove useless else blocks in order to reduce complexity r=[somebody]
...and that makes it pretty much impossible for sheriffs, hg archeologists, & other interested parties to differentiate between these 37 commits in our pushlog.

Ideally it would be nice if these commit messages including the patch's affected top-level directories, or something like that to indicate what differentiates them -- e.g.
 Bug 1338086 - Remove useless else blocks in order to reduce complexity, in /image/ r=dholbert
...or...
 Bug 1338086 - Remove useless else blocks in order to reduce complexity, in imagelib code r=dholbert

If you feel up to doing that, gold star! But it's also probably not a big enough deal to worry much about at this point - mostly noting for future bugs of this sort.

::: image/imgLoader.cpp:1705
(Diff revision 1)
> -    return true;
> +  return true;
> +
> -  }
> +}

Please drop this blank line

(between the now-final return statement and the function's ending curly-brace)
Attachment #8835410 - Flags: review?(dholbert) → review+
(Assignee)

Comment 69

10 months ago
> If you feel up to doing that, gold star! 
Yeah, i am planning to do it ;)

Comment 70

10 months ago
mozreview-review
Comment on attachment 8835400 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/base/

https://reviewboard.mozilla.org/r/111110/#review112536
Attachment #8835400 - Flags: review?(ehsan) → review+

Comment 71

10 months ago
mozreview-review
Comment on attachment 8835427 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/components/remote/

https://reviewboard.mozilla.org/r/111164/#review112538
Attachment #8835427 - Flags: review?(ehsan) → review+

Comment 72

10 months ago
mozreview-review
Comment on attachment 8835430 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/mozapps/update/updater/

https://reviewboard.mozilla.org/r/111170/#review112540
Attachment #8835430 - Flags: review?(ehsan) → review+

Comment 73

10 months ago
mozreview-review
Comment on attachment 8835428 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/components/typeaheadfind

https://reviewboard.mozilla.org/r/111166/#review112566
Attachment #8835428 - Flags: review?(mstange) → review+

Comment 74

10 months ago
mozreview-review
Comment on attachment 8835433 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity widget/

https://reviewboard.mozilla.org/r/111176/#review112568
Attachment #8835433 - Flags: review?(mstange) → review+

Comment 75

10 months ago
mozreview-review
Comment on attachment 8835404 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/media/gmp*

https://reviewboard.mozilla.org/r/111118/#review112582
Attachment #8835404 - Flags: review?(cpearce) → review+
Comment on attachment 8835398 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity accessible/atk

https://reviewboard.mozilla.org/r/111106/#review112616

r+ with a couple of nits to fix:

::: accessible/atk/nsMaiInterfaceTable.cpp:359
(Diff revision 1)
>  {
>    AccessibleWrap* accWrap = GetAccessibleWrap(ATK_OBJECT(aTable));
>    if (accWrap) {
>      return static_cast<gboolean>(accWrap->AsTable()->
>        IsCellSelected(aRowIdx, aColIdx));
> -  } else if (ProxyAccessible* proxy = GetProxy(ATK_OBJECT(aTable))) {
> +  } if (ProxyAccessible* proxy = GetProxy(ATK_OBJECT(aTable))) {

Please put 'if' on new line.

::: accessible/atk/nsMaiInterfaceText.cpp:530
(Diff revision 1)
>      if (!text || !text->IsTextRole()) {
>        return FALSE;
>      }
>  
>      return text->AddToSelection(aStartOffset, aEndOffset);
> -  } else if (ProxyAccessible* proxy = GetProxy(ATK_OBJECT(aText))) {
> +  } if (ProxyAccessible* proxy = GetProxy(ATK_OBJECT(aText))) {

Please put 'if' on new line.
Attachment #8835398 - Flags: review?(dbolter) → review+

Comment 77

10 months ago
mozreview-review
Comment on attachment 8835405 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/media

https://reviewboard.mozilla.org/r/111120/#review112632
Attachment #8835405 - Flags: review?(jyavenard) → review+

Comment 78

10 months ago
mozreview-review
Comment on attachment 8835407 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/media/

https://reviewboard.mozilla.org/r/111124/#review112634
Attachment #8835407 - Flags: review?(jyavenard) → review+

Comment 79

10 months ago
mozreview-review
Comment on attachment 8835414 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in memory/

https://reviewboard.mozilla.org/r/111138/#review112666

::: memory/replace/logalloc/replay/Replay.cpp:523
(Diff revision 1)
>  
>      /* jemalloc_stats and free are functions with no result. */
>      if (func == Buffer("jemalloc_stats")) {
>        replay.jemalloc_stats(args);
>        continue;
> -    } else if (func == Buffer("free")) {
> +    } if (func == Buffer("free")) {

The 'if' needs to be on a separate line! Without that, this change makes the code less readable.
Attachment #8835414 - Flags: review?(n.nethercote) → review-
(Assignee)

Comment 80

10 months ago
mozreview-review
Comment on attachment 8835413 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/

https://reviewboard.mozilla.org/r/111136/#review112840

::: media/webrtc/signaling/src/media-conduit/VideoConduit.cpp:314
(Diff revision 1)
>    if ("VP8" == name) {
>      return webrtc::VideoEncoder::EncoderType::kVp8;
> -  } else if ("VP9" == name) {
> +  }
> +  if ("VP9" == name) {
>      return webrtc::VideoEncoder::EncoderType::kVp9;
> -  } else if ("H264" == name) {
> +  }
> +  if ("H264" == name) {
>      return webrtc::VideoEncoder::EncoderType::kH264;
>    }

I can fix that but are you sure? This is going to make the tool trigger this all the time at beginning (until we have the feature to ignore)

Comment 81

10 months ago
mozreview-review
Comment on attachment 8835408 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in dom/plugins/

https://reviewboard.mozilla.org/r/111126/#review112870

::: dom/plugins/base/nsPluginHost.cpp:2847
(Diff revision 1)
>      // There is no profile yet, this will tell us if there is going to be one
>      // in the future.
>      directoryService->Get(NS_APP_PROFILE_DIR_STARTUP, NS_GET_IID(nsIFile),
>                            getter_AddRefs(mPluginRegFile));
>      if (!mPluginRegFile)
>        return NS_ERROR_FAILURE;

nit, add braces here
Attachment #8835408 - Flags: review?(jmathies) → review+

Comment 82

10 months ago
mozreview-review
Comment on attachment 8835431 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in toolkit/xre/

https://reviewboard.mozilla.org/r/111172/#review112872
Attachment #8835431 - Flags: review?(jmathies) → review+
(Before landing, it would probably be worth grepping the whole patch-set (via "hg out -p | grep" or similar) for any lines that include "} if", to hopefully catch any instances that reviewers might not have already pointed out. It seems like that's an antipattern that the semi-automated process here produced repeatedly, so it'd be nice to be sure you've caught all of them.)
(Assignee)

Comment 84

10 months ago
(In reply to Daniel Holbert [:dholbert] from comment #83)
> (Before landing, it would probably be worth grepping the whole patch-set
> (via "hg out -p | grep" or similar) for any lines that include "} if", to
> hopefully catch any instances that reviewers might not have already pointed
> out. 
I fixed everything yesterday but I cannot push to mozreview because of bug 1338530 :( sorry about that

I also reported a bug upstream about that https://llvm.org/bugs/show_bug.cgi?id=31915
even if upstream is already aware of that:
https://github.com/llvm-mirror/clang-tools-extra/blob/master/clang-tidy/readability/ElseAfterReturnCheck.cpp#L48

Comment 85

10 months ago
mozreview-review
Comment on attachment 8835418 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/mtransport/

https://reviewboard.mozilla.org/r/111146/#review112946

::: media/mtransport/test_nr_socket.cpp:561
(Diff revision 1)
> -          port_mappings_.front()->external_socket_->my_addr().as_string,
> +        port_mappings_.front()->external_socket_->my_addr().as_string,
> -          port_mappings_.front()->remote_address_.as_string);
> +        port_mappings_.front()->remote_address_.as_string);
> -
> -    port_mappings_.front()->last_used_ = PR_IntervalNow();
> +  port_mappings_.front()->last_used_ = PR_IntervalNow();
> -    return port_mappings_.front()->external_socket_->write(msg, len, written);
> +  return port_mappings_.front()->external_socket_->write(msg, len, written);
> +

Please remove this added empty line.

::: media/mtransport/test_nr_socket.cpp:908
(Diff revision 1)
>  
>    if (r == R_WOULDBLOCK) {
>      r_log(LOG_GENERIC, LOG_DEBUG, "Enqueueing UDP packet to %s", to.as_string);
>      send_queue_.push_back(RefPtr<UdpPacket>(new UdpPacket(msg, len, to)));
>      return 0;
> -  } else if (r) {
> +  } if (r) {

This should go onto a new line.
Attachment #8835418 - Flags: review?(drno) → review+

Comment 86

10 months ago
mozreview-review-reply
Comment on attachment 8835413 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/

https://reviewboard.mozilla.org/r/111136/#review112840

> I can fix that but are you sure? This is going to make the tool trigger this all the time at beginning (until we have the feature to ignore)

That's the downside of using automated tools like this: you can ignore them, but then they'll bug you to the end of time.  So either we need a "I know what I'm doing" block for the tool, or you give in.

While I find the 'else if ("foo" == bar) {' form clearer, It's not worth manually ignoring it all the time.  If we had a suppression mechanism (or rather if the tool did), that'd work.

Go ahead and change it

Comment 87

10 months ago
(In reply to Randell Jesup [:jesup] from comment #86)
> Comment on attachment 8835413 [details]
> Bug 1338086 - Remove useless else blocks in order to reduce complexity
> 
> https://reviewboard.mozilla.org/r/111136/#review112840
> 
> > I can fix that but are you sure? This is going to make the tool trigger this all the time at beginning (until we have the feature to ignore)
> 
> That's the downside of using automated tools like this: you can ignore them,
> but then they'll bug you to the end of time.  So either we need a "I know
> what I'm doing" block for the tool, or you give in.

Can't you add a // NOLINT comment on the line you want clang-tidy to ignore?

Comment 88

10 months ago
mozreview-review
Comment on attachment 8835409 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in extensions/gio/

https://reviewboard.mozilla.org/r/111128/#review113532
Attachment #8835409 - Flags: review?(dolske) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment on attachment 8835417 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in xpcom/

https://reviewboard.mozilla.org/r/111144/#review113704

Thanks.
Attachment #8835417 - Flags: review?(nfroyd) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 155

10 months ago
mozreview-review
Comment on attachment 8835411 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in js/

https://reviewboard.mozilla.org/r/111132/#review113708

Indentation consists of 4 spaces in Spidermonkey, not 2. There are many unwanted space changes in this file, so can't r+ yet, sorry.

::: js/xpconnect/loader/mozJSSubScriptLoader.cpp
(Diff revision 2)
> -                return false;
> +            return false;
> -            }
> +        }
> -            return JS::CompileFunction(cx, envChain, options, nullptr, 0, nullptr,
> +        return JS::CompileFunction(cx, envChain, options, nullptr, 0, nullptr,
> -                                       srcBuf, function);
> +                                   srcBuf, function);
> -        }
> +    }
> -    } else {

nit: please add a \n before the multi-line comment

::: js/xpconnect/wrappers/XrayWrapper.cpp:582
(Diff revision 2)
> -                }
> +          }
> -            }
>          } else if (key == JSProto_Function) {
> -            if (id == GetJSIDByIndex(cx, XPCJSContext::IDX_LENGTH)) {
> +          if (id == GetJSIDByIndex(cx, XPCJSContext::IDX_LENGTH)) {
> -                FillPropertyDescriptor(desc, wrapper, JSPROP_PERMANENT | JSPROP_READONLY,
> +              FillPropertyDescriptor(desc, wrapper, JSPROP_PERMANENT | JSPROP_READONLY,
> -                                       NumberValue(JS_GetFunctionArity(JS_GetObjectFunction(target))));
> +              NumberValue(JS_GetFunctionArity(JS_GetObjectFunction(target))));

nit: align `NumberValue` with `desc` above.
Attachment #8835411 - Flags: review?(bbouvier)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 161

10 months ago
mozreview-review
Comment on attachment 8835411 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in js/

https://reviewboard.mozilla.org/r/111132/#review113730

Thanks!
Attachment #8835411 - Flags: review?(bbouvier) → review+
Comment hidden (mozreview-request)

Comment 163

10 months ago
mozreview-review
Comment on attachment 8835414 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in memory/

https://reviewboard.mozilla.org/r/111138/#review113846
Attachment #8835414 - Flags: review?(n.nethercote) → review+

Comment 164

10 months ago
mozreview-review
Comment on attachment 8835413 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/

https://reviewboard.mozilla.org/r/111136/#review113868

::: media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp:1814
(Diff revisions 1 - 5)
> -  for (auto & mRemoteTrack : mRemoteTracks) {
> -    mRemoteTrack.mAssignedMLine.reset();
> +  for (auto i = mRemoteTracks.begin(); i != mRemoteTracks.end(); ++i) {
> +    i->mAssignedMLine.reset();
>    }

this patch is undoing a bunch of fixes from the for() loops patch...
Attachment #8835413 - Flags: review?(rjesup) → review-
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 203

10 months ago
mozreview-review
Comment on attachment 8835413 [details]
Bug 1338086 - Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/

https://reviewboard.mozilla.org/r/111136/#review114156

Make the one mod I indicate, and this can land!

::: media/webrtc/signaling/src/media-conduit/AudioConduit.cpp:542
(Diff revision 7)
> -      //copy this to local database
> +    //copy this to local database
> -      if(CopyCodecToDB(codec))
> +    if(CopyCodecToDB(codec))
> -      {
> +    {
> -        success = true;
> +      success = true;
> -      } else {
> +    } else {
> -        CSFLogError(logTag,"%s Unable to updated Codec Database", __FUNCTION__);
> +      CSFLogError(logTag,"%s Unable to updated Codec Database", __FUNCTION__);
> -        return kMediaConduitUnknownError;
> +      return kMediaConduitUnknownError;
> -      }
> +    }
> +    success = true;

if(!CopyCodecToDB(codec)) {
  CFSLogError(...);
  return kMediaConduitUnknownError;
}
success = true;

::: media/webrtc/signaling/src/media-conduit/AudioConduit.cpp:930
(Diff revision 7)
>    // XXX(pkerr) - the PacketOptions are being ignored. This parameter was added along
>    // with the Call API update in the webrtc.org codebase.
>    // The only field in it is the packet_id, which is used when the header
>    // extension for TransportSequenceNumber is being used, which we don't.
> -  (void) options;
> -  if(mTransmitterTransport &&
> +  (void)options;
> +  if (mTransmitterTransport &&

this file is an unfortunate mix of "if (" and "if(", so I have no problem moving it towards standard moz style ("if (").  Normally I'm loathe to change stuff for no solid reason, even if mixed-usage cases like this, because it messes over Blame.
Attachment #8835413 - Flags: review?(rjesup) → review+
Comment hidden (mozreview-request)

Comment 205

10 months ago
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/145447470634
Remove useless else blocks in order to reduce complexity in extensions/gio/ r=Dolske
https://hg.mozilla.org/integration/autoland/rev/f2690dfca9be
Remove useless else blocks in order to reduce complexity in dom/plugins/ r=jimm
https://hg.mozilla.org/integration/autoland/rev/9fef6905c065
Remove useless else blocks in order to reduce complexity in gfx/ r=nical
https://hg.mozilla.org/integration/autoland/rev/e6ce7125efcb
Remove useless else blocks in order to reduce complexity in dom/media r=jya
https://hg.mozilla.org/integration/autoland/rev/ce59e8f3aaf1
Remove useless else blocks in order to reduce complexity in dom/media/gmp* r=cpearce
https://hg.mozilla.org/integration/autoland/rev/81e69418454e
Remove useless else blocks in order to reduce complexity in dom/ipc r=baku
https://hg.mozilla.org/integration/autoland/rev/f7d85c7f485f
Remove useless else blocks in order to reduce complexity in dom/canvas r=baku
https://hg.mozilla.org/integration/autoland/rev/d687959b0d9a
Remove useless else blocks in order to reduce complexity in dom/browser-element/ r=baku
https://hg.mozilla.org/integration/autoland/rev/1d8304f5e83a
Remove useless else blocks in order to reduce complexity devtools/shared/ r=fitzgen
https://hg.mozilla.org/integration/autoland/rev/fc5dd8085a28
Remove useless else blocks in order to reduce complexity accessible/atk r=davidb
https://hg.mozilla.org/integration/autoland/rev/e5080c5fd186
Remove useless else blocks in order to reduce complexity image/ r=dholbert
https://hg.mozilla.org/integration/autoland/rev/22a9503bcd34
Remove useless else blocks in order to reduce complexity widget/ r=mstange
https://hg.mozilla.org/integration/autoland/rev/fbaa986c8e67
Remove useless else blocks in order to reduce complexity in uriloader/exthandler/unix/ r=froydnj
https://hg.mozilla.org/integration/autoland/rev/d4a5c7295bbd
Remove useless else blocks in order to reduce complexity in toolkit/xre/ r=jimm
https://hg.mozilla.org/integration/autoland/rev/d897e71f8818
Remove useless else blocks in order to reduce complexity in toolkit/mozapps/update/updater/ r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/8d7006b65a2c
Remove useless else blocks in order to reduce complexity in toolkit/components/url-classifier/ r=francois
https://hg.mozilla.org/integration/autoland/rev/e7fcd290e89e
Remove useless else blocks in order to reduce complexity in toolkit/components/typeaheadfind r=mstange
https://hg.mozilla.org/integration/autoland/rev/cd8319b71afd
Remove useless else blocks in order to reduce complexity in toolkit/components/remote/ r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/3096b29c3a5b
Remove useless else blocks in order to reduce complexity in security/sandbox/linux/ r=gcp
https://hg.mozilla.org/integration/autoland/rev/aea7a26ac8fc
Remove useless else blocks in order to reduce complexity in rdf/util/ r=Pike
https://hg.mozilla.org/integration/autoland/rev/a0a07671fdb1
Remove useless else blocks in order to reduce complexity in mozglue/misc/ r=froydnj
https://hg.mozilla.org/integration/autoland/rev/dbc0c3eb16b4
Remove useless else blocks in order to reduce complexity in modules/libpref/ r=froydnj
https://hg.mozilla.org/integration/autoland/rev/5151fc89c210
Remove useless else blocks in order to reduce complexity in modules/libjar/ r=froydnj
https://hg.mozilla.org/integration/autoland/rev/8a6881a79dad
Remove useless else blocks in order to reduce complexity modules/libmar/ r=mhowell
https://hg.mozilla.org/integration/autoland/rev/1c91ef325bc9
Remove useless else blocks in order to reduce complexity testing/mochitest/ssltunnel/ r=jmaher
https://hg.mozilla.org/integration/autoland/rev/be741314456b
Remove useless else blocks in order to reduce complexity in toolkit/components/telemetry/ r=Dexter
https://hg.mozilla.org/integration/autoland/rev/a6f7b959e857
Remove useless else blocks in order to reduce complexity in media/mtransport/ r=drno
https://hg.mozilla.org/integration/autoland/rev/a1411b3c461e
Remove useless else blocks in order to reduce complexity in xpcom/ r=froydnj
https://hg.mozilla.org/integration/autoland/rev/0775748c6e49
Remove useless else blocks in order to reduce complexity in view/ r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/5bf8f4e866ef
Remove useless else blocks in order to reduce complexity in netwerk/ r=dragana
https://hg.mozilla.org/integration/autoland/rev/2e9d9af6bd9a
Remove useless else blocks in order to reduce complexity in memory/ r=njn
https://hg.mozilla.org/integration/autoland/rev/9e7ec782abbc
Remove useless else blocks in order to reduce complexity in layout/ r=heycam
https://hg.mozilla.org/integration/autoland/rev/36a9f59888a8
Remove useless else blocks in order to reduce complexity in js/ r=bbouvier
https://hg.mozilla.org/integration/autoland/rev/498ce28ac33e
Remove useless else blocks in order to reduce complexity in widget/xremoteclient/ r=nical
https://hg.mozilla.org/integration/autoland/rev/c37a59aa9e9d
Remove useless else blocks in order to reduce complexity in dom/media/ r=jya
https://hg.mozilla.org/integration/autoland/rev/11cfe50731f8
Remove useless else blocks in order to reduce complexity in dom/base/ r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/713d7f119dc0
Remove useless else blocks in order to reduce complexity in media/webrtc/signaling/ r=jesup

Comment 206

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/145447470634
https://hg.mozilla.org/mozilla-central/rev/f2690dfca9be
https://hg.mozilla.org/mozilla-central/rev/9fef6905c065
https://hg.mozilla.org/mozilla-central/rev/e6ce7125efcb
https://hg.mozilla.org/mozilla-central/rev/ce59e8f3aaf1
https://hg.mozilla.org/mozilla-central/rev/81e69418454e
https://hg.mozilla.org/mozilla-central/rev/f7d85c7f485f
https://hg.mozilla.org/mozilla-central/rev/d687959b0d9a
https://hg.mozilla.org/mozilla-central/rev/1d8304f5e83a
https://hg.mozilla.org/mozilla-central/rev/fc5dd8085a28
https://hg.mozilla.org/mozilla-central/rev/e5080c5fd186
https://hg.mozilla.org/mozilla-central/rev/22a9503bcd34
https://hg.mozilla.org/mozilla-central/rev/fbaa986c8e67
https://hg.mozilla.org/mozilla-central/rev/d4a5c7295bbd
https://hg.mozilla.org/mozilla-central/rev/d897e71f8818
https://hg.mozilla.org/mozilla-central/rev/8d7006b65a2c
https://hg.mozilla.org/mozilla-central/rev/e7fcd290e89e
https://hg.mozilla.org/mozilla-central/rev/cd8319b71afd
https://hg.mozilla.org/mozilla-central/rev/3096b29c3a5b
https://hg.mozilla.org/mozilla-central/rev/aea7a26ac8fc
https://hg.mozilla.org/mozilla-central/rev/a0a07671fdb1
https://hg.mozilla.org/mozilla-central/rev/dbc0c3eb16b4
https://hg.mozilla.org/mozilla-central/rev/5151fc89c210
https://hg.mozilla.org/mozilla-central/rev/8a6881a79dad
https://hg.mozilla.org/mozilla-central/rev/1c91ef325bc9
https://hg.mozilla.org/mozilla-central/rev/be741314456b
https://hg.mozilla.org/mozilla-central/rev/a6f7b959e857
https://hg.mozilla.org/mozilla-central/rev/a1411b3c461e
https://hg.mozilla.org/mozilla-central/rev/0775748c6e49
https://hg.mozilla.org/mozilla-central/rev/5bf8f4e866ef
https://hg.mozilla.org/mozilla-central/rev/2e9d9af6bd9a
https://hg.mozilla.org/mozilla-central/rev/9e7ec782abbc
https://hg.mozilla.org/mozilla-central/rev/36a9f59888a8
https://hg.mozilla.org/mozilla-central/rev/498ce28ac33e
https://hg.mozilla.org/mozilla-central/rev/c37a59aa9e9d
https://hg.mozilla.org/mozilla-central/rev/11cfe50731f8
https://hg.mozilla.org/mozilla-central/rev/713d7f119dc0
Status: NEW → RESOLVED
Last Resolved: 10 months ago
status-firefox54: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.