Andi [:andi] Assignee
	Description • 8 years ago

The Static Analysis tool Coverity detected that after |frame| is nullptr assigned, it could be null dereferenced.

>>        if (!frameKeeper.IsAlive()) {
>>          frame = nullptr;
>>        }
>>        // Implicit pointer capture for touch
>>        if (sPointerEventImplicitCapture &&
>>            pointerEvent->mMessage == ePointerDown &&
>>            pointerEvent->inputSource == nsIDOMMouseEvent::MOZ_SOURCE_TOUCH) {
>>          nsCOMPtr<nsIContent> targetContent;
>>          frame->GetContentForEvent(aEvent, getter_AddRefs(targetContent));
>>          while (targetContent && !targetContent->IsElement()) {
>>            targetContent = targetContent->GetParent();
>>          }
>>          if (targetContent) {
>>            SetPointerCapturingContent(pointerEvent->pointerId, targetContent);
>>          }
>>        }

	Daniel Holbert [:dholbert]
	Comment 2 • 8 years ago

This looks probably-good.  Slight nit: this could be simplified slightly to use "else if", instead of adding "frame" to the if-condition.  But maybe not a big deal.

In any case, I'm redirecting review to smaug, since this is in event-handling code (which he knows better than I) & since he reviewed the "frame = nullptr" assignment there (over in bug 1153130) and hence may be more likely to be aware of any subtleties here.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 3 • 8 years ago
mozreview-review

Comment on attachment 8799354 [details]
Bug 1308877 - prevent null pointer dereference in PresShell::HandleEvent.

https://reviewboard.mozilla.org/r/84552/#review83192

Thanks

	Pulsebot
	Comment 4 • 8 years ago

Pushed by bpostelnicu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2c3e14c84991
prevent null pointer dereference in PresShell::HandleEvent. r=smaug

	Phil Ringnalda (:philor)
	Comment 5 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/2c3e14c84991