1281971 - PROCESS-CRASH | layout/style/test/test_value_cloning.html | application crashed [@ TryToStartImageLoadOnValue]

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Astley Chen (inactive)]

local try result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f1f3376cfb5&selectedJob=22529323

Root cause:
Nullptr dereference of local variable *imageURI* return from aValue.GetURLValue().

Comment 1

[u459114]

Before patch in bug 1245499 land, thing works fine even if aValue.GetURLValue() return nullptr, that's because ImageLoader will do null-check
ImageLoader::LoadImage(...)
{
  if (!aURI) {
    return;
  }
}

I think we may do the same thing in TryToStartImageLoadOnValue. But it's fishy that why caller pass aValue which does not carry valid URL.

Comment 2

[u459114]

Attachment: bug 1281971 - Fix null pointer access in TryToStartImageLoadOnValue.

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

Comment 3

[u459114]

Comment on attachment 8764971 [details]
bug 1281971 - Fix null pointer access in TryToStartImageLoadOnValue.

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

Comment 4

[Daniel Holbert [:dholbert]]

Comment on attachment 8764971 [details]
bug 1281971 - Fix null pointer access in TryToStartImageLoadOnValue.

https://reviewboard.mozilla.org/r/60540/#review57512

If this is reproducible locally, it'd be nice to debug this a bit and figure out why we're ending up with a null pointer here...

But in any case, as you note in comment 1, this is reinstating a check that we were already performing previously, so it seems fine. r=me

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d2050362e8a
Fix null pointer access in TryToStartImageLoadOnValue. r=dholbert

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0d2050362e8a

Comment 7

[u459114]

(In reply to Daniel Holbert [:dholbert] (mostly OOTO until Aug 9th) from comment #4)
> Comment on attachment 8764971 [details]
> bug 1281971 - Fix null pointer access in TryToStartImageLoadOnValue.
> 
> https://reviewboard.mozilla.org/r/60540/#review57512
> 
> If this is reproducible locally, it'd be nice to debug this a bit and figure
> out why we're ending up with a null pointer here...
> 
> But in any case, as you note in comment 1, this is reinstating a check that
> we were already performing previously, so it seems fine. r=me

The reason of null uri was written in Bug 1290650 comment 1