Bug 110145 - [harfbuzz] Crash in harfbuzz because direction is not set
Summary: [harfbuzz] Crash in harfbuzz because direction is not set
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 420+
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: WebKit Security Group
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-18 11:55 PST by Claudio Saavedra
Modified: 2013-02-19 09:58 PST (History)
8 users (show)

See Also:


Attachments
Patch (1.78 KB, patch)
2013-02-19 08:15 PST, Claudio Saavedra
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Claudio Saavedra 2013-02-18 11:55:22 PST
Pulling from hb master and with a wk build from today:

#0  0x0000003af8c35ba5 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:63
#1  0x0000003af8c37358 in __GI_abort () at abort.c:90
#2  0x0000003af8c2e972 in __assert_fail_base (fmt=0x3af8d793e8 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x7ffff5b00850 "props->direction != HB_DIRECTION_INVALID", file=file@entry=0x7ffff5b00829 "hb-shape-plan.cc", line=line@entry=93, 
    function=function@entry=0x7ffff5b00900 <hb_shape_plan_create::__PRETTY_FUNCTION__> "hb_shape_plan_t* hb_shape_plan_create(hb_face_t*, const hb_segment_properties_t*, const hb_feature_t*, unsigned int, const char* const*)") at assert.c:92
#3  0x0000003af8c2ea22 in __GI___assert_fail (assertion=0x7ffff5b00850 "props->direction != HB_DIRECTION_INVALID", file=0x7ffff5b00829 "hb-shape-plan.cc", line=93, 
    function=0x7ffff5b00900 <hb_shape_plan_create::__PRETTY_FUNCTION__> "hb_shape_plan_t* hb_shape_plan_create(hb_face_t*, const hb_segment_properties_t*, const hb_feature_t*, unsigned int, const char* const*)") at assert.c:101
#4  0x00007ffff5aa4669 in hb_shape_plan_create (face=face@entry=0x207fc10, props=props@entry=0x20b1698, user_features=user_features@entry=0x0, num_user_features=num_user_features@entry=0, shaper_list=shaper_list@entry=0x0) at hb-shape-plan.cc:93
#5  0x00007ffff5aa5145 in hb_shape_plan_create_cached (face=0x207fc10, props=0x20b1698, user_features=0x0, num_user_features=<optimized out>, shaper_list=0x0) at hb-shape-plan.cc:289
#6  0x00007ffff5aa3f42 in hb_shape_full (font=0x2081400, buffer=0x20b1620, features=0x0, num_features=0, shaper_list=<optimized out>) at hb-shape.cc:258
#7  0x00007ffff68e9780 in WebCore::HarfBuzzShaper::shapeHarfBuzzRuns(bool) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#8  0x00007ffff68eab11 in WebCore::HarfBuzzShaper::shape(WebCore::GlyphBuffer*) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#9  0x00007ffff68e4e53 in WebCore::Font::floatWidthForComplexText(WebCore::TextRun const&, WTF::HashSet<WebCore::SimpleFontData const*, WTF::PtrHash<WebCore::SimpleFontData const*>, WTF::HashTraits<WebCore::SimpleFontData const*> >*, WebCore::GlyphOverflow*) const ()
   from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#10 0x00007ffff702ebd2 in WebCore::Font::width(WebCore::TextRun const&, WTF::HashSet<WebCore::SimpleFontData const*, WTF::PtrHash<WebCore::SimpleFontData const*>, WTF::HashTraits<WebCore::SimpleFontData const*> >*, WebCore::GlyphOverflow*) const ()
   from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#11 0x00007ffff6a7ac8c in WebCore::RenderText::computePreferredLogicalWidths(float, WTF::HashSet<WebCore::SimpleFontData const*, WTF::PtrHash<WebCore::SimpleFontData const*>, WTF::HashTraits<WebCore::SimpleFontData const*> >&, WebCore::GlyphOverflow&) ()
   from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#12 0x00007ffff6a7cf76 in WebCore::RenderText::width(unsigned int, unsigned int, WebCore::Font const&, float, WTF::HashSet<WebCore::SimpleFontData const*, WTF::PtrHash<WebCore::SimpleFontData const*>, WTF::HashTraits<WebCore::SimpleFontData const*> >*, WebCore::GlyphOverflow*) const () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#13 0x00007ffff6968914 in WebCore::RenderBlock::LineBreaker::nextSegmentBreak(WebCore::BidiResolver<WebCore::InlineIterator, WebCore::BidiRun>&, WebCore::LineInfo&, WebCore::RenderBlock::RenderTextInfo&, WebCore::RenderBlock::FloatingObject*, unsigned int, WTF::Vector<WebCore::WordMeasurement, 64ul>&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#14 0x00007ffff6969577 in WebCore::RenderBlock::LineBreaker::nextLineBreak(WebCore::BidiResolver<WebCore::InlineIterator, WebCore::BidiRun>&, WebCore::LineInfo&, WebCore::RenderBlock::RenderTextInfo&, WebCore::RenderBlock::FloatingObject*, unsigned int, WTF::Vector<WebCore::WordMeasurement, 64ul>&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#15 0x00007ffff696b806 in WebCore::RenderBlock::layoutRunsAndFloatsInRange(WebCore::LineLayoutState&, WebCore::BidiResolver<WebCore::InlineIterator, WebCore::BidiRun>&, WebCore::InlineIterator const&, WebCore::BidiStatus const&, unsigned int) ()
   from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#16 0x00007ffff696ced9 in WebCore::RenderBlock::layoutRunsAndFloats(WebCore::LineLayoutState&, bool) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#17 0x00007ffff696d63d in WebCore::RenderBlock::layoutInlineChildren(bool, WebCore::LayoutUnit&, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#18 0x00007ffff694f6dc in WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#19 0x00007ffff693067b in WebCore::RenderBlock::layout() () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#20 0x00007ffff694de99 in WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#21 0x00007ffff694e65f in WebCore::RenderBlock::layoutBlockChildren(bool, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#22 0x00007ffff694f988 in WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#23 0x00007ffff693067b in WebCore::RenderBlock::layout() () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#24 0x00007ffff694de99 in WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#25 0x00007ffff694e65f in WebCore::RenderBlock::layoutBlockChildren(bool, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#26 0x00007ffff694f988 in WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#27 0x00007ffff693067b in WebCore::RenderBlock::layout() () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#28 0x00007ffff694de99 in WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#29 0x00007ffff694e65f in WebCore::RenderBlock::layoutBlockChildren(bool, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#30 0x00007ffff694f988 in WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#31 0x00007ffff693067b in WebCore::RenderBlock::layout() () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#32 0x00007ffff694ae3c in WebCore::RenderBlock::insertFloatingObject(WebCore::RenderBox*) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#33 0x00007ffff694b279 in WebCore::RenderBlock::handleFloatingChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo const&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#34 0x00007ffff694e63f in WebCore::RenderBlock::layoutBlockChildren(bool, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#35 0x00007ffff694f988 in WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#36 0x00007ffff693067b in WebCore::RenderBlock::layout() () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#37 0x00007ffff694de99 in WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#38 0x00007ffff694e65f in WebCore::RenderBlock::layoutBlockChildren(bool, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#39 0x00007ffff694f988 in WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#40 0x00007ffff693067b in WebCore::RenderBlock::layout() () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#41 0x00007ffff694de99 in WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#42 0x00007ffff694e65f in WebCore::RenderBlock::layoutBlockChildren(bool, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#43 0x00007ffff694f988 in WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#44 0x00007ffff693067b in WebCore::RenderBlock::layout() () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#45 0x00007ffff694de99 in WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#46 0x00007ffff694e65f in WebCore::RenderBlock::layoutBlockChildren(bool, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#47 0x00007ffff694f988 in WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#48 0x00007ffff693067b in WebCore::RenderBlock::layout() () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#49 0x00007ffff694de99 in WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#50 0x00007ffff694e65f in WebCore::RenderBlock::layoutBlockChildren(bool, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#51 0x00007ffff694f988 in WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#52 0x00007ffff693067b in WebCore::RenderBlock::layout() () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#53 0x00007ffff694de99 in WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#54 0x00007ffff694e65f in WebCore::RenderBlock::layoutBlockChildren(bool, WebCore::LayoutUnit&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#55 0x00007ffff694f988 in WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#56 0x00007ffff693067b in WebCore::RenderBlock::layout() () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#57 0x00007ffff6a8de19 in WebCore::RenderView::layoutContent(WebCore::LayoutState const&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#58 0x00007ffff6a8e797 in WebCore::RenderView::layout() () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#59 0x00007ffff68b1aae in WebCore::FrameView::layout(bool) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#60 0x00007ffff642d659 in WebCore::Document::updateLayoutIgnorePendingStylesheets() () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#61 0x00007ffff644d4c1 in WebCore::Element::offsetHeight() () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#62 0x00007ffff6ca5a4d in WebCore::jsElementOffsetHeight(JSC::ExecState*, JSC::JSValue, JSC::PropertyName) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#63 0x00007ffff4719c51 in llint_slow_path_get_by_id () from /opt/gnome-3.0/lib64/libjavascriptcoregtk-3.0.so.0
#64 0x00007ffff4724e4d in llint_op_get_by_id () from /opt/gnome-3.0/lib64/libjavascriptcoregtk-3.0.so.0
#65 0x00007fffe0140000 in ?? ()
#66 0x00007ffff47ca7b7 in JSC::FunctionExecutable::compileForCallInternal(JSC::ExecState*, JSC::JSScope*, JSC::JITCode::JITType, unsigned int) () from /opt/gnome-3.0/lib64/libjavascriptcoregtk-3.0.so.0
#67 0x00007ffff46bc019 in JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) () from /opt/gnome-3.0/lib64/libjavascriptcoregtk-3.0.so.0
#68 0x00007ffff47b320a in JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) () from /opt/gnome-3.0/lib64/libjavascriptcoregtk-3.0.so.0
#69 0x00007ffff6252566 in WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext*, WebCore::Event*) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#70 0x00007ffff646420b in WebCore::EventTarget::fireEventListeners(WebCore::Event*, WebCore::EventTargetData*, WTF::Vector<WebCore::RegisteredEventListener, 1ul>&) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#71 0x00007ffff64644f1 in WebCore::EventTarget::fireEventListeners(WebCore::Event*) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#72 0x00007ffff645cab1 in WebCore::EventContext::handleLocalEvents(WebCore::Event*) const () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#73 0x00007ffff645d90a in WebCore::EventDispatcher::dispatch() () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#74 0x00007ffff645d138 in WebCore::EventDispatcher::dispatchEvent(WebCore::Node*, WTF::PassRefPtr<WebCore::EventDispatchMediator>) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#75 0x00007ffff6477999 in WebCore::Node::dispatchEvent(WTF::PassRefPtr<WebCore::Event>) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#76 0x00007ffff642a5a6 in WebCore::Document::finishedParsing() () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#77 0x00007ffff6657a36 in WebCore::HTMLDocumentParser::prepareToStopParsing() () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#78 0x00007ffff66574f8 in WebCore::HTMLDocumentParser::notifyFinished(WebCore::CachedResource*) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#79 0x00007ffff67cb679 in WebCore::CachedResource::checkNotify() () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#80 0x00007ffff683cec9 in WebCore::SubresourceLoader::didFinishLoading(double) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#81 0x00007ffff70c27a6 in WebCore::readCallback(_GObject*, _GAsyncResult*, void*) () from /opt/gnome-3.0/lib64/libwebkitgtk-3.0.so.0
#82 0x00007ffff16c3187 in async_ready_callback_wrapper (source_object=0x207b980, res=0x20762d0, user_data=0x7fff95934888) at ginputstream.c:530
#83 0x00007ffff16f0cb8 in g_task_return_now (task=0x20762d0) at gtask.c:1105
#84 0x00007ffff16f0ce9 in complete_in_idle_cb (task=0x20762d0) at gtask.c:1114
#85 0x00007ffff0eef9a7 in g_idle_dispatch (source=0x7fff60001290, callback=0x7ffff16f0cd1 <complete_in_idle_cb>, user_data=0x20762d0) at gmain.c:5205
#86 0x00007ffff0eed225 in g_main_dispatch (context=0x721660) at gmain.c:3054
#87 0x00007ffff0eedf8a in g_main_context_dispatch (context=0x721660) at gmain.c:3630
#88 0x00007ffff0eee17a in g_main_context_iterate (context=0x721660, block=1, dispatch=1, self=0x762060) at gmain.c:3701
#89 0x00007ffff0eee23e in g_main_context_iteration (context=0x721660, may_block=1) at gmain.c:3762
#90 0x00007ffff171b3d1 in g_application_run (application=0x8d3000, argc=1, argv=0x7fffffffd658) at gapplication.c:1620
#91 0x00000000004313cd in main (argc=1, argv=0x7fffffffd658) at ephy-main.c:474
Comment 1 Claudio Saavedra 2013-02-18 11:56:16 PST
Sorry, I think I should not be filing bugs when I haven't had a proper meal.
Comment 2 Behdad Esfahbod 2013-02-18 23:34:04 PST
Right... I should have expected this coming.

The way webkit calls harfbuzz is buggy.  I'm compiling webkitgtk now to try to fix this.
Comment 3 Behdad Esfahbod 2013-02-19 02:08:23 PST
I've got webkitgtk build here.  Any easy way to reproduce the crash (ideally in gdb)?
Comment 4 Claudio Saavedra 2013-02-19 02:20:11 PST
Run epiphany against your wk build (from master) and just start the browser. That should do it.
Comment 5 Behdad Esfahbod 2013-02-19 07:49:21 PST
This should do it, though, this area really needs more work.  We should simply make direction available to HarfBuzz unconditionally.  It's recipe for disaster otherwise.

--- Source/WebCore/platform/graphics/harfbuzz/HarfBuzzShaper.cpp.orig	2013-02-19 10:43:39.413756909 -0500
+++ Source/WebCore/platform/graphics/harfbuzz/HarfBuzzShaper.cpp	2013-02-19 10:43:06.730116605 -0500
@@ -327,6 +327,9 @@
         hb_buffer_set_script(harfBuzzBuffer.get(), currentRun->script());
         if (shouldSetDirection)
             hb_buffer_set_direction(harfBuzzBuffer.get(), currentRun->rtl() ? HB_DIRECTION_RTL : HB_DIRECTION_LTR);
+        else
+            // Leaving direction to HarfBuzz to guess is *really* bad, but will do for now.
+            hb_buffer_guess_segment_properties (harfBuzzBuffer.get());
 
         // Add a space as pre-context to the buffer. This prevents showing dotted-circle
         // for combining marks at the beginning of runs.
Comment 6 Behdad Esfahbod 2013-02-19 08:15:00 PST
FWIW, this was caused by the following HarfBuzz commit:

commit c462b32dcb883a7aca066af24c4d28c7a2b7fa28
Author: Behdad Esfahbod <behdad@behdad.org>
Date:   Fri Feb 15 07:51:47 2013 -0500

    Disable automatic segment properties guessing
    
    Before, if one called hb_shape() without setting script, language, and
    direction on the buffer, hb_shape() was calling
    hb_buffer_guess_segment_properties() on the user's behalf to guess
    these.
    
    This is very dangerous, since any serious user of HarfBuzz must set
    these properly (specially important is direction).  So now, we don't
    guess properties by default.  People not setting direction will get
    an abort() now.  If the old behavior is desired (fragile, good for
    simple testing only), users can call
    hb_buffer_guess_segment_properties() on the buffer just before calling
    hb_shape().
Comment 7 Claudio Saavedra 2013-02-19 08:15:10 PST
Created attachment 189091 [details]
Patch
Comment 8 Claudio Saavedra 2013-02-19 08:16:51 PST
Who can I cc for a review on this area?
Comment 9 Claudio Saavedra 2013-02-19 08:22:37 PST
Hi Tony, I was told you might be able to review this :) TIA
Comment 10 EFL EWS Bot 2013-02-19 08:27:06 PST
Comment on attachment 189091 [details]
Patch

Attachment 189091 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16616727
Comment 11 Stephen Chenney 2013-02-19 08:36:35 PST
Could we have a follow up bug for this to fix the underlying problem? According to comment #6 the real error is in WebKit failing to set all the properties on the text run correctly. Just hacking it in is not the right fix - we should be setting the properties.

There is also no test. How are we to reproduce this without building for gtk and running some other piece of software? How will we detect if it fails again?
Comment 12 Behdad Esfahbod 2013-02-19 08:40:16 PST
It will crash with *any* complex script test, with recent-enough harfbuzz.  The reason only GTK build noticed this is that Chrome runs a static version of harfbuzz.
Comment 13 Martin Robinson 2013-02-19 08:42:24 PST
(In reply to comment #12)
> It will crash with *any* complex script test, with recent-enough harfbuzz.  The reason only GTK build noticed this is that Chrome runs a static version of harfbuzz.

And the GTK+ bots are using an old version.
Comment 14 Stephen Chenney 2013-02-19 08:51:49 PST
(In reply to comment #12)
> It will crash with *any* complex script test, with recent-enough harfbuzz.  The reason only GTK build noticed this is that Chrome runs a static version of harfbuzz.

Follow up bug then, so we fix this properly?
Comment 15 Behdad Esfahbod 2013-02-19 09:01:42 PST
Bug 110230 - [harfbuzz] Always pass correct text direction to HarfBuzz
Comment 16 Claudio Saavedra 2013-02-19 09:13:41 PST
Comment on attachment 189091 [details]
Patch

Already landed on: http://trac.webkit.org/changeset/143337
Build fix in: http://trac.webkit.org/changeset/143345
Comment 17 Csaba Osztrogonác 2013-02-19 09:29:30 PST
Comment on attachment 189091 [details]
Patch

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

> Source/WebCore/platform/graphics/harfbuzz/HarfBuzzShaper.cpp:334
> +        else
> +            // Leaving direction to HarfBuzz to guess is *really* bad, but will do for now.
> +            hb_buffer_guess_segment_properties(harfBuzzBuffer.get());

Ouch. Is it correct without { } ?
Comment 18 Chris Dumez 2013-02-19 09:33:23 PST
Comment on attachment 189091 [details]
Patch

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

>> Source/WebCore/platform/graphics/harfbuzz/HarfBuzzShaper.cpp:334
>> +            hb_buffer_guess_segment_properties(harfBuzzBuffer.get());
> 
> Ouch. Is it correct without { } ?

In any case, WebKit coding style recommends to use braces in this case:
"One-line control clauses should not use braces unless comments are included or a single statement spans multiple lines."
Comment 19 Claudio Saavedra 2013-02-19 09:53:09 PST
(In reply to comment #18)

> In any case, WebKit coding style recommends to use braces in this case:
> "One-line control clauses should not use braces unless comments are included or a single statement spans multiple lines."

My bad. check-webkit-style didn't complain on this one. Do you want me to fix this?
Comment 20 Chris Dumez 2013-02-19 09:58:35 PST
(In reply to comment #19)
> (In reply to comment #18)
> 
> > In any case, WebKit coding style recommends to use braces in this case:
> > "One-line control clauses should not use braces unless comments are included or a single statement spans multiple lines."
> 
> My bad. check-webkit-style didn't complain on this one. Do you want me to fix this?

The code works and the patch already landed. I personally think it is fine to leave it as it is. I merely commented for future reference.