Bug 1677194 Comment 19 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Revsied  patch.
(In reply to Emilio Cobos Álvarez (:emilio) from comment #15)
> Comment on attachment 9188712 [details] [diff] [review]
> 1677194.patch patch to fix the issue.
> 
> Review of attachment 9188712 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good, but I'd like to see the bits below fixed before granting r+,
> thanks!
> 
Thank you.

> ::: layout/xul/tree/nsTreeBodyFrame.cpp
> @@ +1597,5 @@
> >      FireRowCountChangedEvent(aIndex, aCount);
> >    }
> >  #endif  // #ifdef ACCESSIBILITY
> >  
> > +  // Validate pointer. Bug 1677194
> 
> I'd just remove the comment. Blame is for this.

Removed. I am from an older generation who needed to dealt with communication break down, or other problems often.
Having the comments IN THE SOURCE FILE was useful. But I removed the comment.
 
> @@ +1599,5 @@
> >  #endif  // #ifdef ACCESSIBILITY
> >  
> > +  // Validate pointer. Bug 1677194
> > +  AutoWeakFrame weakFrame(this);
> > +  nsCOMPtr<nsITreeView> view = mView;
> 
> I don't think you need this local fwiw. But anyhow no big deal.

I just followed the previous comment and so left this as is.
I did not know what I was doing. :-)
 
> @@ +1602,5 @@
> > +  AutoWeakFrame weakFrame(this);
> > +  nsCOMPtr<nsITreeView> view = mView;
> > +
> > +  // Adjust our selection. Note the use of |view| instead of |mView| 
> > +  nsCOMPtr<nsITreeSelection> sel = nullptr; 
> 
> Trailing space, and useless `= nullptr`.

Taken care of.
 
> @@ +1603,5 @@
> > +  nsCOMPtr<nsITreeView> view = mView;
> > +
> > +  // Adjust our selection. Note the use of |view| instead of |mView| 
> > +  nsCOMPtr<nsITreeSelection> sel = nullptr; 
> > +  nsresult rc = view->GetSelection(getter_AddRefs(sel));
> 
> no need to check `rc` (and also should be called `rv`). `sel` will never be
> non-null if it fails, so the code as it was was fine.
> @@ +1607,5 @@
> > +  nsresult rc = view->GetSelection(getter_AddRefs(sel));
> > +
> > +  if (NS_SUCCEEDED(rc) && sel) sel->AdjustSelection(aIndex, aCount);
> > +
Reverted to the original.
 
> > +  // Check
> 
> Useless comment.

Took out.

tryserver run:
try-comm-central:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=4a86f4fcf258f3eb6083e0bc76f34eff9f52381a
(I am not worried with windows issues. They are in comm-central runs as well.)

It does not introduce new bugs, and it certainly does NOT trigger the ASAN error anymore.

Back to Bug 1677194 Comment 19