1237227 - [Static Analysis][Dereference null return value] In function nsView::DidCompositeWindow from nsView.cpp

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Andi [:andi]]

The Static Analysis tool Coverity added that context->GetDisplayRootPresContext() can be null thus having later on a null pointer dereference. 
The path that the analyzer takes in GetDisplayPresContext is:

>>nsPresContext::GetDisplayRootPresContext()
>>{
>>  nsPresContext* pc = this;
    	1. Condition true, taking true branch
>>  for (;;) {
>>    nsPresContext* parent = pc->GetParentPresContext();
    	2. Condition !parent, taking true branch
>>    if (!parent) {
>>      // Not sure if this is always strictly the parent, but it works for GetRootPresContext
>>      // where the current pres context has no frames.
>>      nsIDocument *doc = pc->Document();
    	3. Condition doc, taking true branch
>>      if (doc) {
>>        doc = doc->GetParentDocument();
    	4. Condition doc, taking true branch
>>        if (doc) {
>>          nsIPresShell* shell = doc->GetShell();
    	5. Condition shell, taking false branch
>>          if (shell) {
>>            parent = shell->GetPresContext();
>>          }
>>        }
>>      }
>>    }
    	6. Condition !parent, taking true branch
>>    if (!parent || parent == pc)
    	7. Breaking from loop
>>      break;
>>    pc = parent;
>>  }
    	8. Condition pc->IsRoot(), taking false branch
    	9. return_null: Explicitly returning null.
>>  return pc->IsRoot() ? static_cast<nsRootPresContext*>(pc) : nullptr;
>>}

And the dereference will occur:
>> context->GetDisplayRootPresContext()->GetRootPresContext()->NotifyDidPaintForSubtree(nsIPresShell::PAINT_COMPOSITE);

If we are 100% sure that this scenario is not possible we could wave the issue on Coverity side and mark it as a false positive, otherwise we could assert the result of this function, that will later trigger when automation tests are ran.
Looking on crash-stats i don't find any issues related with the above one but still, an assert doesn't hurt.

Comment 1

[Andi [:andi]]

Attachment: Bug 1237227.diff

Comment 2

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/577396a6287e

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/577396a6287e