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

RESOLVED FIXED in Firefox 50

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: bmo, Assigned: u459114)

Tracking

Trunk
mozilla50
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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().
(Reporter)

Updated

2 years ago
Assignee: nobody → cku
Blocks: 1251161
Status: NEW → ASSIGNED
(Assignee)

Comment 1

2 years ago
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.
(Assignee)

Comment 2

2 years ago
Created attachment 8764971 [details]
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/
(Assignee)

Comment 3

2 years ago
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/
(Assignee)

Updated

2 years ago
Attachment #8764971 - Flags: review?(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
Attachment #8764971 - Flags: review?(dholbert) → review+

Comment 5

2 years ago
Pushed by cku@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d2050362e8a
Fix null pointer access in TryToStartImageLoadOnValue. r=dholbert

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0d2050362e8a
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
See Also: → bug 1290650
(Assignee)

Comment 7

2 years ago
(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
You need to log in before you can comment on or make changes to this bug.