Closed Bug 1377923 Opened 2 years ago Closed 2 years ago

Large volume of warnings on nightly - WARNING: 'obs', file ... image/RasterImage.cpp, line 1402 (VectorImage.cpp / ImageFactory.cpp)

Categories

(Core :: ImageLib, defect)

Unspecified
macOS
defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 1377920
Tracking Status
firefox56 --- affected

People

(Reporter: Alex_Gaynor, Assigned: johannh)

References

Details

Attachments

(1 file)

STR:

1) Use debug build from central (a3b192dc8344)
2) Browse to basically any website (e.g. google.com)
3) Observe the parade of:

[Child 47418] WARNING: 'obs', file /Users/agaynor/projects/mozilla-central/image/RasterImage.cpp, line 1402

They spew out at several per second for me.

(Hopefully this is the correct component)
I bet this code really wanted to warn if the observer service _wasn't_ found...
Blocks: 1363059
Flags: needinfo?(jhofmann)
Just noticed this same code pattern occurs at |image/VectorImage.cpp| line 1032.
Argh, sorry. Technically I'm on PTO, but I can see this is really annoying. I'll attach a patch. I'll rely on try to let me know if this compiles.
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Flags: needinfo?(jhofmann)
Comment on attachment 8883187 [details]
Bug 1377923 - Don't check if observer service is found in image loading/showing reporting code.

https://reviewboard.mozilla.org/r/154114/#review159292

Doh! I should have caught that in review.

::: image/ImageFactory.cpp:99
(Diff revision 1)
>  
>  #ifdef DEBUG
>    // Record the image load for startup performance testing.
>    if (NS_IsMainThread()) {
>      nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
> -    if (NS_WARN_IF(obs)) {
> +    if (!NS_WARN_IF(!obs)) {

The double inversion punctuated by a warning is pretty awkward. Given that this is debug only code and that we know that GetObserverService() will not return null at any time we could be creating images (it needs to be one of the first things to start up, and last to shut down), I don't think we need to null check at all. Either MOZ_ASSERT instead or else do nothing and allow us to crash doing a null-deref a couple lines down. Either option would be much more visible than a warning, and in this case that would be a good thing since we'd really like to know why on earth the observer service isn't available here.

The same change would apply at the other places touched by this patch.
Attachment #8883187 - Flags: review?(jwatt)
Duplicate of this bug: 1378164
Adding the actual warning to the summary to avoid further duplicates.
Summary: Large volume of warnings on nightly → Large volume of warnings on nightly - WARNING: 'obs', file ... image/RasterImage.cpp, line 1402
Comment on attachment 8883187 [details]
Bug 1377923 - Don't check if observer service is found in image loading/showing reporting code.

https://reviewboard.mozilla.org/r/154114/#review159502
Attachment #8883187 - Flags: review?(jwatt) → review+
Summary: Large volume of warnings on nightly - WARNING: 'obs', file ... image/RasterImage.cpp, line 1402 → Large volume of warnings on nightly - WARNING: 'obs', file ... image/RasterImage.cpp, line 1402 (RasterImage.cpp / ImageFactory.cpp)
Summary: Large volume of warnings on nightly - WARNING: 'obs', file ... image/RasterImage.cpp, line 1402 (RasterImage.cpp / ImageFactory.cpp) → Large volume of warnings on nightly - WARNING: 'obs', file ... image/RasterImage.cpp, line 1402 (VectorImage.cpp / ImageFactory.cpp)
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 7e5638fc75cc -d 62a566f7ab69: rebasing 405284:7e5638fc75cc "Bug 1377923 - Don't check if observer service is found in image loading/showing reporting code. r=jwatt" (tip)
merging image/ImageFactory.cpp
merging image/RasterImage.cpp
merging image/VectorImage.cpp
warning: conflicts while merging image/ImageFactory.cpp! (edit, then use 'hg resolve --mark')
warning: conflicts while merging image/RasterImage.cpp! (edit, then use 'hg resolve --mark')
warning: conflicts while merging image/VectorImage.cpp! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1377920
You need to log in before you can comment on or make changes to this bug.