Bug 96649

Summary: [EFL] Assertion reached on RenderThemeEFL when setting a theme to an invalid path
Product: WebKit Reporter: Thiago Marcos P. Santos <tmpsantos>
Component: WebKit EFLAssignee: Thiago Marcos P. Santos <tmpsantos>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, gyuyoung.kim, kenneth, lucas.de.marchi, mikhail.pozdnyakov, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Thiago Marcos P. Santos 2012-09-13 07:36:00 PDT
As reported by EWK2UnitTestBase.ewk_view_theme_set:

ASSERTION FAILED: m_themePath == path
/home/buildslave-1/webkit-buildslave/efl-linux-64-debug/build/Source/WebCore/platform/efl/RenderThemeEfl.cpp(502) : WTF::String WebCore::RenderThemeEfl::themePath() const
1   0x2b92b47ad899 WebCore::RenderThemeEfl::themePath() const
2   0x2b92b47ac93c WebCore::RenderThemeEfl::getThemePartFromCache(WebCore::FormType, WebCore::IntSize const&)
3   0x2b92b47ace32 WebCore::RenderThemeEfl::paintThemePart(WebCore::RenderObject*, WebCore::FormType, WebCore::PaintInfo const&, WebCore::IntRect const&)
4   0x2b92b47af28b WebCore::RenderThemeEfl::paintButton(WebCore::RenderObject*, WebCore::PaintInfo const&, WebCore::IntRect const&)
5   0x2b92b40e5451 WebCore::RenderTheme::paint(WebCore::RenderObject*, WebCore::PaintInfo const&, WebCore::IntRect const&)
6   0x2b92b3fc6326 WebCore::RenderBox::paintBoxDecorations(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&)
7   0x2b92b3f6a01c WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&)
8   0x2b92b3f67c36 WebCore::RenderBlock::paint(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&)
9   0x2b92b3f3f00b WebCore::InlineBox::paint(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&, WebCore::FractionalLayoutUnit, WebCore::FractionalLayoutUnit)
10  0x2b92b3f45117 WebCore::InlineFlowBox::paint(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&, WebCore::FractionalLayoutUnit, WebCore::FractionalLayoutUnit)
11  0x2b92b410268f WebCore::RootInlineBox::paint(WebCore::PaintInfo&, WebCore::FractionalLayoutPoint const&, WebCore::FractionalLayoutUnit, WebCore::FractionalLayoutUnit)
Comment 1 Thiago Marcos P. Santos 2012-09-13 07:40:21 PDT
Created attachment 163870 [details]
Patch
Comment 2 Mikhail Pozdnyakov 2012-09-13 07:46:50 PDT
Comment on attachment 163870 [details]
Patch

Looks sane
Comment 3 Chris Dumez 2012-09-13 07:48:20 PDT
Comment on attachment 163870 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=163870&action=review

> Source/WebCore/platform/efl/RenderThemeEfl.cpp:492
> +    if (!loadTheme())

Shouldn't we update m_themePath in loadTheme() instead? This would avoid this extra code.
Comment 4 Thiago Marcos P. Santos 2012-09-13 08:03:20 PDT
Comment on attachment 163870 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=163870&action=review

>> Source/WebCore/platform/efl/RenderThemeEfl.cpp:492
>> +    if (!loadTheme())
> 
> Shouldn't we update m_themePath in loadTheme() instead? This would avoid this extra code.

I tried first changing loadTheme() to loadTheme(const String&) and later merging it with setThemePath(const String&) but it was getting more complicated because of this:

ALWAYS_INLINE bool loadThemeIfNeeded() const
{   
    return m_edje || (!m_themePath.isEmpty() && const_cast<RenderThemeEfl*>(this)->loadTheme());
}

You have to handle the use case when you are loading the theme without changing the path. Since loadTheme is called often and setTheme (hopefully) is not, I choose to keep this check there.
Comment 5 WebKit Review Bot 2012-09-13 11:33:19 PDT
Comment on attachment 163870 [details]
Patch

Clearing flags on attachment: 163870

Committed r128485: <http://trac.webkit.org/changeset/128485>
Comment 6 WebKit Review Bot 2012-09-13 11:33:23 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Chris Dumez 2012-09-13 22:37:01 PDT
I'm using r128552 and I still hit the same assertion:

$ WebKitBuild/Debug/bin/EWebLauncher -b=tiled
ASSERTION FAILED: m_themePath == path
/home/chris/unencrypted/WebKit/Source/WebCore/platform/efl/RenderThemeEfl.cpp(496) : WTF::String WebCore::RenderThemeEfl::themePath() const
1   0x7fd2bc2ae53d WebCore::RenderThemeEfl::themePath() const
2   0x7fd2bc2b4218 WebCore::ScrollbarEfl::setParent(WebCore::ScrollView*)
3   0x7fd2bb95928c WebCore::ScrollView::addChild(WTF::PassRefPtr<WebCore::Widget>)
4   0x7fd2bb9597a0 WebCore::ScrollView::setHasVerticalScrollbar(bool)
5   0x7fd2bb95aec6 WebCore::ScrollView::updateScrollbars(WebCore::IntSize const&)
6   0x7fd2bb959a53 WebCore::ScrollView::setScrollbarModes(WebCore::ScrollbarMode, WebCore::ScrollbarMode, bool, bool)
7   0x7fd2bb8ca993 WebCore::ScrollView::setVerticalScrollbarMode(WebCore::ScrollbarMode, bool)
8   0x7fd2bb8c079d WebCore::FrameView::layout(bool)
9   0x7fd2bb8c872d WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive()
10  0x7fd2bb8afecc WebCore::FocusController::setActive(bool)
11  0x7fd2becb70c5
12  0x7fd2becb7667
13  0x7fd2be920ce2 evas_object_event_callback_call
14  0x7fd2be92b669 evas_object_focus_set
15  0x405fdf
16  0x4065ab main
17  0x7fd2be54c76d __libc_start_main
18  0x403739
Segmentation fault (core dumped)
Comment 8 Chris Dumez 2012-09-14 00:31:48 PDT
Marking as closed again since the crash was caused by r128565. The crash is the same so I thought this fix did not work.