[Webkit-unassigned] [Bug 56393] Without checking existence of the renderer of the element, tries to access the enclosing layer.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=56393





--- Comment #24 from Alexis Menard <alexis.menard at openbossa.org>  2011-06-14 12:35:52 PST ---
(In reply to comment #23)
> (In reply to comment #22)
> Now I understand your question.
> 
> I added this ASSERT so that this situation would fail on all platforms
> so I could write the test.  If we had Symbian layout tests, that would
> be a much better alternative.
> 
> Orignally, I put the ASSERT at the site of
> the call as you suggest, but that did not make any sense once I applied
> the patch for checking the null renderer.
> 
> IntRect FrameView::windowClipRect()
>   ...
>   ASSERT(elt && elt->renderer());
> 
>   // Patch for null renderer
>   RenderLayer* layer = 0; 
>   if (elt && elt->renderer()) 
>     layer = elt->renderer()->enclosingLayer(); 
> 
> So I thought the best place to include the ASSERT was in 
> enclosingLayer().

The first idea was better than the current one.

One thing bother me. If the test is good enough you don't need the ASSERT, it should just crash properly therefore it will be catch at a LayoutTest failure. A crashing test is good enough to be treated as a regression/failure.

> 
> I agree that the ASSERT is a bit odd.  However, I could not think
> of a better way to expose this problem in the Layout tests.  I am 
> open to other suggestions.  While this currently only fails on Symbian,
> I assume we don't want WebKit depending on undefined C++ behavior.

That's why I'm annoying you with my questions. We can't put such code.

> 
> I hope this makes sense.  If not, please ask more questions.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.