"Is new" attribute looks wrong on some code review issues (before/after SA)

[jankeromnes]

MozReview URL: https://reviewboard.mozilla.org/r/241468/diff/10/

Problem:

• Production has 2 publishable mozlint issues
• Staging has 0

More details in the email reports:

[production]

mozlint - flake8

    Path: testing/raptor/raptor/results.py
    Level: error
    Line: 36
    Third Party: no
    Disabled rule: no
    Publishable: yes

expected 2 blank lines, found 1

mozlint - flake8

    Path: testing/raptor/raptor/raptor.py
    Level: error
    Line: 183
    Third Party: no
    Disabled rule: no
    Publishable: yes

expected 2 blank lines after class or function definition, found 1

but Staging says these issues aren't "new":

[staging]

mozlint - flake8

    Path: testing/raptor/raptor/results.py
    Level: error
    Line: 36
    Third Party: no
    Disabled rule: no
    Publishable: no
    Is new: no

expected 2 blank lines, found 1

mozlint - flake8

    Path: testing/raptor/raptor/raptor.py
    Level: error
    Line: 183
    Third Party: no
    Disabled rule: no
    Publishable: no
    Is new: no

expected 2 blank lines after class or function definition, found 1

Both are bugs (the issues should be detected as "new"):

1. The file testing/raptor/raptor/results.py is new, so any issues detected in it should be "new"
2. The line testing/raptor/raptor/raptor.py:183 wasn't actually modified by the developer, but the developer forgot to leave two blank lines above it, while there were 2 blank lines before, so the issue is technically "new" too

[sylvestre]

The bugzilla comment: https://bugzilla.mozilla.org/show_bug.cgi?id=1449195#c29

The diffing algo should indeed catch it!

[jankeromnes]

Additionally, the only occurrences of Is new: true that I found are a bit weird:

https://reviewboard.mozilla.org/r/231228/diff/1/

clang-format

    Path: media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.h
    Mode: replace
    Lines: from 499, on 51 lines
    Is new: yes

Old lines:

{
  public:
    WebrtcVideoDecoderProxy() :
      mDecoderImpl(new WebrtcGmpVideoDecoder)
    {}

    virtual ~WebrtcVideoDecoderProxy()
    {
      RegisterDecodeCompleteCallback(nullptr);
    }

    uint64_t PluginID() const override
    {
      return mDecoderImpl->PluginID();
    }

    int32_t InitDecode(const webrtc::VideoCodec* aCodecSettings,
                       int32_t aNumberOfCores) override
    {
      return mDecoderImpl->InitDecode(aCodecSettings, aNumberOfCores);
    }

    int32_t Decode(
        const webrtc::EncodedImage& aInputImage,
        bool aMissingFrames,
        const webrtc::RTPFragmentationHeader* aFragmentation,
        const webrtc::CodecSpecificInfo* aCodecSpecificInfo,
        int64_t aRenderTimeMs) override
    {
      return mDecoderImpl->Decode(aInputImage,
                                  aMissingFrames,
                                  aFragmentation,
                                  aCodecSpecificInfo,
                                  aRenderTimeMs);
    }

    int32_t RegisterDecodeCompleteCallback(
      webrtc::DecodedImageCallback* aCallback) override
    {
      return mDecoderImpl->RegisterDecodeCompleteCallback(aCallback);
    }

    int32_t Release() override
    {
      return mDecoderImpl->ReleaseGmp();
    }

  private:
    RefPtr<WebrtcGmpVideoDecoder> mDecoderImpl;
};

New lines:

{
public:
  WebrtcVideoDecoderProxy()
    : mDecoderImpl(new WebrtcGmpVideoDecoder)
  {
  }

  virtual ~WebrtcVideoDecoderProxy()
  {
    RegisterDecodeCompleteCallback(nullptr);
  }

  uint64_t PluginID() const override { return mDecoderImpl->PluginID(); }

  int32_t InitDecode(const webrtc::VideoCodec* aCodecSettings,
                     int32_t aNumberOfCores) override
  {
    return mDecoderImpl->InitDecode(aCodecSettings, aNumberOfCores);
  }

  int32_t Decode(const webrtc::EncodedImage& aInputImage,
                 bool aMissingFrames,
                 const webrtc::RTPFragmentationHeader* aFragmentation,
                 const webrtc::CodecSpecificInfo* aCodecSpecificInfo,
                 int64_t aRenderTimeMs) override
  {
    return mDecoderImpl->Decode(aInputImage,
                                aMissingFrames,
                                aFragmentation,
                                aCodecSpecificInfo,
                                aRenderTimeMs);
  }

  int32_t RegisterDecodeCompleteCallback(
    webrtc::DecodedImageCallback* aCallback) override
  {
    return mDecoderImpl->RegisterDecodeCompleteCallback(aCallback);
  }

  int32_t Release() override { return mDecoderImpl->ReleaseGmp(); }

private:
  RefPtr<WebrtcGmpVideoDecoder> mDecoderImpl;
};

clang-format

    Path: media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.h
    Mode: insert
    Lines: from 551, on 1 lines
    Is new: yes

Old lines:

#endif

New lines:

#endif

and:

clang-format

    Path: media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp
    Mode: replace
    Lines: from 797, on 6 lines
    Is new: yes

Old lines:

int32_t
WebrtcGmpVideoDecoder::Decode(const webrtc::EncodedImage& aInputImage,
                              bool aMissingFrames,
                              const webrtc::RTPFragmentationHeader* aFragmentation,
                              const webrtc::CodecSpecificInfo* aCodecSpecificInfo,
                              int64_t aRenderTimeMs)

New lines:

int32_t
WebrtcGmpVideoDecoder::Decode(
  const webrtc::EncodedImage& aInputImage,
  bool aMissingFrames,
  const webrtc::RTPFragmentationHeader* aFragmentation,
  const webrtc::CodecSpecificInfo* aCodecSpecificInfo,
  int64_t aRenderTimeMs)

clang-format

    Path: media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp
    Mode: replace
    Lines: from 829, on 4 lines
    Is new: yes

Old lines:

      // InitDone hasn't been called yet (race)
      GMPDecodeData *data = new GMPDecodeData(aInputImage,
                                              aMissingFrames,
                                              aRenderTimeMs);

New lines:

      // InitDone hasn't been called yet (race)
      GMPDecodeData* data =
        new GMPDecodeData(aInputImage, aMissingFrames, aRenderTimeMs);

Most of these lines weren't actually modified by the patch, except WebrtcGmpVideoCodec.cpp:829.