RESOLVED FIXED 78047
[Qt][WK2] QQuickWebView::event should lookup faster which events QQuickWebPage can handle
https://bugs.webkit.org/show_bug.cgi?id=78047
Summary [Qt][WK2] QQuickWebView::event should lookup faster which events QQuickWebPag...
Rafael Brandao
Reported 2012-02-07 15:09:09 PST
[Qt][WK2] QQuickWebView::event should lookup faster which events QQuickWebPage can handle
Attachments
Patch (4.00 KB, patch)
2012-02-07 15:16 PST, Rafael Brandao
no flags
Patch (19.03 KB, patch)
2012-02-24 04:51 PST, Simon Hausmann
no flags
Patch (20.84 KB, patch)
2012-02-29 07:03 PST, Simon Hausmann
no flags
Rafael Brandao
Comment 1 2012-02-07 15:16:35 PST
Rafael Brandao
Comment 2 2012-02-07 15:24:06 PST
This is how the assembly code for QQuickWebView::event was before this patch: => 0x00007ffff60a5260 <+0>: mov %rbx,-0x10(%rsp) 0x00007ffff60a5265 <+5>: mov %rbp,-0x8(%rsp) 0x00007ffff60a526a <+10>: sub $0x18,%rsp 0x00007ffff60a526e <+14>: movzwl 0x10(%rsi),%eax 0x00007ffff60a5272 <+18>: mov %rdi,%rbp 0x00007ffff60a5275 <+21>: mov %rsi,%rbx 0x00007ffff60a5278 <+24>: mov 0x20(%rdi),%rdx 0x00007ffff60a527c <+28>: cmp $0x3f,%eax 0x00007ffff60a527f <+31>: jg 0x7ffff60a52c8 <_ZN13QQuickWebView5eventEP6QEvent+104> 0x00007ffff60a5281 <+33>: cmp $0x3c,%eax 0x00007ffff60a5284 <+36>: jl 0x7ffff60a52e8 <_ZN13QQuickWebView5eventEP6QEvent+136> 0x00007ffff60a5286 <+38>: mov 0x90(%rdx),%rdi 0x00007ffff60a528d <+45>: callq 0x7ffff60a3160 <_ZNK13QQuickWebPage12eventHandlerEv> 0x00007ffff60a5292 <+50>: mov %rbx,%rsi 0x00007ffff60a5295 <+53>: mov %rax,%rdi 0x00007ffff60a5298 <+56>: callq 0x7ffff61143f0 <_ZN21QtWebPageEventHandler11handleEventEP6QEvent> 0x00007ffff60a529d <+61>: test %al,%al 0x00007ffff60a529f <+63>: jne 0x7ffff60a5318 <_ZN13QQuickWebView5eventEP6QEvent+184> 0x00007ffff60a52a1 <+65>: movzwl 0x10(%rbx),%eax 0x00007ffff60a52a5 <+69>: cmp $0x53,%eax 0x00007ffff60a52a8 <+72>: je 0x7ffff60a5300 <_ZN13QQuickWebView5eventEP6QEvent+160> 0x00007ffff60a52aa <+74>: mov %rbx,%rsi 0x00007ffff60a52ad <+77>: mov %rbp,%rdi 0x00007ffff60a52b0 <+80>: mov 0x8(%rsp),%rbx 0x00007ffff60a52b5 <+85>: mov 0x10(%rsp),%rbp 0x00007ffff60a52ba <+90>: add $0x18,%rsp 0x00007ffff60a52be <+94>: jmpq 0x7ffff5ff4e60 <_ZN10QQuickItem5eventEP6QEvent@plt> 0x00007ffff60a52c3 <+99>: nopl 0x0(%rax,%rax,1) 0x00007ffff60a52c8 <+104>: cmp $0x7f,%eax 0x00007ffff60a52cb <+107>: jl 0x7ffff60a52a5 <_ZN13QQuickWebView5eventEP6QEvent+69> 0x00007ffff60a52cd <+109>: cmp $0x81,%eax 0x00007ffff60a52d2 <+114>: jle 0x7ffff60a5286 <_ZN13QQuickWebView5eventEP6QEvent+38> 0x00007ffff60a52d4 <+116>: lea -0xc2(%rax),%ecx 0x00007ffff60a52da <+122>: cmp $0x2,%ecx 0x00007ffff60a52dd <+125>: ja 0x7ffff60a52a5 <_ZN13QQuickWebView5eventEP6QEvent+69> 0x00007ffff60a52df <+127>: jmp 0x7ffff60a5286 <_ZN13QQuickWebView5eventEP6QEvent+38> 0x00007ffff60a52e1 <+129>: nopl 0x0(%rax) 0x00007ffff60a52e8 <+136>: cmp $0x2,%eax 0x00007ffff60a52eb <+139>: jl 0x7ffff60a52a5 <_ZN13QQuickWebView5eventEP6QEvent+69> 0x00007ffff60a52ed <+141>: cmp $0x9,%eax 0x00007ffff60a52f0 <+144>: jle 0x7ffff60a5286 <_ZN13QQuickWebView5eventEP6QEvent+38> 0x00007ffff60a52f2 <+146>: cmp $0x1f,%eax 0x00007ffff60a52f5 <+149>: jne 0x7ffff60a52a5 <_ZN13QQuickWebView5eventEP6QEvent+69> 0x00007ffff60a52f7 <+151>: jmp 0x7ffff60a5286 <_ZN13QQuickWebView5eventEP6QEvent+38> 0x00007ffff60a52f9 <+153>: nopl 0x0(%rax) 0x00007ffff60a5300 <+160>: xor %eax,%eax 0x00007ffff60a5302 <+162>: mov 0x8(%rsp),%rbx 0x00007ffff60a5307 <+167>: mov 0x10(%rsp),%rbp 0x00007ffff60a530c <+172>: add $0x18,%rsp 0x00007ffff60a5310 <+176>: retq 0x00007ffff60a5311 <+177>: nopl 0x0(%rax) 0x00007ffff60a5318 <+184>: mov $0x1,%eax 0x00007ffff60a531d <+189>: mov 0x8(%rsp),%rbx 0x00007ffff60a5322 <+194>: mov 0x10(%rsp),%rbp 0x00007ffff60a5327 <+199>: add $0x18,%rsp 0x00007ffff60a532b <+203>: retq And now: => 0x00007ffff60a4b50 <+0>: mov %rbx,-0x10(%rsp) 0x00007ffff60a4b55 <+5>: mov %rbp,-0x8(%rsp) 0x00007ffff60a4b5a <+10>: sub $0x18,%rsp 0x00007ffff60a4b5e <+14>: movzwl 0x10(%rsi),%eax 0x00007ffff60a4b62 <+18>: mov %rdi,%rbp 0x00007ffff60a4b65 <+21>: mov %rsi,%rbx 0x00007ffff60a4b68 <+24>: mov 0x20(%rdi),%rdx 0x00007ffff60a4b6c <+28>: cmp $0xff,%eax 0x00007ffff60a4b71 <+33>: jg 0x7ffff60a4b83 <_ZN13QQuickWebView5eventEP6QEvent+51> 0x00007ffff60a4b73 <+35>: lea 0x1d0b7e6(%rip),%rsi # 0x7ffff7db0360 <_ZL20s_eventHandledByPage> 0x00007ffff60a4b7a <+42>: movslq %eax,%rcx 0x00007ffff60a4b7d <+45>: cmpb $0x0,(%rsi,%rcx,1) 0x00007ffff60a4b81 <+49>: jne 0x7ffff60a4ba8 <_ZN13QQuickWebView5eventEP6QEvent+88> 0x00007ffff60a4b83 <+51>: cmp $0x53,%eax 0x00007ffff60a4b86 <+54>: je 0x7ffff60a4be0 <_ZN13QQuickWebView5eventEP6QEvent+144> 0x00007ffff60a4b88 <+56>: mov %rbx,%rsi 0x00007ffff60a4b8b <+59>: mov %rbp,%rdi 0x00007ffff60a4b8e <+62>: mov 0x8(%rsp),%rbx 0x00007ffff60a4b93 <+67>: mov 0x10(%rsp),%rbp 0x00007ffff60a4b98 <+72>: add $0x18,%rsp 0x00007ffff60a4b9c <+76>: jmpq 0x7ffff5ff4e60 <_ZN10QQuickItem5eventEP6QEvent@plt> 0x00007ffff60a4ba1 <+81>: nopl 0x0(%rax) 0x00007ffff60a4ba8 <+88>: mov 0x90(%rdx),%rdi 0x00007ffff60a4baf <+95>: callq 0x7ffff60a3160 <_ZNK13QQuickWebPage12eventHandlerEv> 0x00007ffff60a4bb4 <+100>: mov %rbx,%rsi 0x00007ffff60a4bb7 <+103>: mov %rax,%rdi 0x00007ffff60a4bba <+106>: callq 0x7ffff61144a0 <_ZN21QtWebPageEventHandler11handleEventEP6QEvent> 0x00007ffff60a4bbf <+111>: test %al,%al 0x00007ffff60a4bc1 <+113>: je 0x7ffff60a4b88 <_ZN13QQuickWebView5eventEP6QEvent+56> 0x00007ffff60a4bc3 <+115>: mov $0x1,%eax 0x00007ffff60a4bc8 <+120>: mov 0x8(%rsp),%rbx 0x00007ffff60a4bcd <+125>: mov 0x10(%rsp),%rbp 0x00007ffff60a4bd2 <+130>: add $0x18,%rsp 0x00007ffff60a4bd6 <+134>: retq 0x00007ffff60a4bd7 <+135>: nopw 0x0(%rax,%rax,1) 0x00007ffff60a4be0 <+144>: xor %eax,%eax 0x00007ffff60a4be2 <+146>: mov 0x8(%rsp),%rbx 0x00007ffff60a4be7 <+151>: mov 0x10(%rsp),%rbp 0x00007ffff60a4bec <+156>: add $0x18,%rsp 0x00007ffff60a4bf0 <+160>: retq The main motivation to make this change is that QQuickWebView::event is used a lot and the lookup would grow logarithmically by the number of events handled by QQuickWebPage when we could do this in O(1) instead. Adding some people to CC... What you think of this?
Noam Rosenthal
Comment 3 2012-02-07 16:03:08 PST
Comment on attachment 125940 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125940&action=review Why use those raw bool arrays instead of the nice abstraction called BitVector? > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:96 > +static inline bool shouldPageHandleEvent(const QEvent::Type& event) why const&? it's an enum, just pass by value. > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:1255 > + } else if (ev->type() == QEvent::InputMethod) Why remove the comment?
Igor Trindade Oliveira
Comment 4 2012-02-07 16:21:19 PST
What valgrind/cachegrind says about this patch? (In reply to comment #2) > This is how the assembly code for QQuickWebView::event was before this patch: > > => 0x00007ffff60a5260 <+0>: mov %rbx,-0x10(%rsp) > 0x00007ffff60a5265 <+5>: mov %rbp,-0x8(%rsp) > 0x00007ffff60a526a <+10>: sub $0x18,%rsp > 0x00007ffff60a526e <+14>: movzwl 0x10(%rsi),%eax > 0x00007ffff60a5272 <+18>: mov %rdi,%rbp > 0x00007ffff60a5275 <+21>: mov %rsi,%rbx > 0x00007ffff60a5278 <+24>: mov 0x20(%rdi),%rdx > 0x00007ffff60a527c <+28>: cmp $0x3f,%eax > 0x00007ffff60a527f <+31>: jg 0x7ffff60a52c8 <_ZN13QQuickWebView5eventEP6QEvent+104> > 0x00007ffff60a5281 <+33>: cmp $0x3c,%eax > 0x00007ffff60a5284 <+36>: jl 0x7ffff60a52e8 <_ZN13QQuickWebView5eventEP6QEvent+136> > 0x00007ffff60a5286 <+38>: mov 0x90(%rdx),%rdi > 0x00007ffff60a528d <+45>: callq 0x7ffff60a3160 <_ZNK13QQuickWebPage12eventHandlerEv> > 0x00007ffff60a5292 <+50>: mov %rbx,%rsi > 0x00007ffff60a5295 <+53>: mov %rax,%rdi > 0x00007ffff60a5298 <+56>: callq 0x7ffff61143f0 <_ZN21QtWebPageEventHandler11handleEventEP6QEvent> > 0x00007ffff60a529d <+61>: test %al,%al > 0x00007ffff60a529f <+63>: jne 0x7ffff60a5318 <_ZN13QQuickWebView5eventEP6QEvent+184> > 0x00007ffff60a52a1 <+65>: movzwl 0x10(%rbx),%eax > 0x00007ffff60a52a5 <+69>: cmp $0x53,%eax > 0x00007ffff60a52a8 <+72>: je 0x7ffff60a5300 <_ZN13QQuickWebView5eventEP6QEvent+160> > 0x00007ffff60a52aa <+74>: mov %rbx,%rsi > 0x00007ffff60a52ad <+77>: mov %rbp,%rdi > 0x00007ffff60a52b0 <+80>: mov 0x8(%rsp),%rbx > 0x00007ffff60a52b5 <+85>: mov 0x10(%rsp),%rbp > 0x00007ffff60a52ba <+90>: add $0x18,%rsp > 0x00007ffff60a52be <+94>: jmpq 0x7ffff5ff4e60 <_ZN10QQuickItem5eventEP6QEvent@plt> > 0x00007ffff60a52c3 <+99>: nopl 0x0(%rax,%rax,1) > 0x00007ffff60a52c8 <+104>: cmp $0x7f,%eax > 0x00007ffff60a52cb <+107>: jl 0x7ffff60a52a5 <_ZN13QQuickWebView5eventEP6QEvent+69> > 0x00007ffff60a52cd <+109>: cmp $0x81,%eax > 0x00007ffff60a52d2 <+114>: jle 0x7ffff60a5286 <_ZN13QQuickWebView5eventEP6QEvent+38> > 0x00007ffff60a52d4 <+116>: lea -0xc2(%rax),%ecx > 0x00007ffff60a52da <+122>: cmp $0x2,%ecx > 0x00007ffff60a52dd <+125>: ja 0x7ffff60a52a5 <_ZN13QQuickWebView5eventEP6QEvent+69> > 0x00007ffff60a52df <+127>: jmp 0x7ffff60a5286 <_ZN13QQuickWebView5eventEP6QEvent+38> > 0x00007ffff60a52e1 <+129>: nopl 0x0(%rax) > 0x00007ffff60a52e8 <+136>: cmp $0x2,%eax > 0x00007ffff60a52eb <+139>: jl 0x7ffff60a52a5 <_ZN13QQuickWebView5eventEP6QEvent+69> > 0x00007ffff60a52ed <+141>: cmp $0x9,%eax > 0x00007ffff60a52f0 <+144>: jle 0x7ffff60a5286 <_ZN13QQuickWebView5eventEP6QEvent+38> > 0x00007ffff60a52f2 <+146>: cmp $0x1f,%eax > 0x00007ffff60a52f5 <+149>: jne 0x7ffff60a52a5 <_ZN13QQuickWebView5eventEP6QEvent+69> > 0x00007ffff60a52f7 <+151>: jmp 0x7ffff60a5286 <_ZN13QQuickWebView5eventEP6QEvent+38> > 0x00007ffff60a52f9 <+153>: nopl 0x0(%rax) > 0x00007ffff60a5300 <+160>: xor %eax,%eax > 0x00007ffff60a5302 <+162>: mov 0x8(%rsp),%rbx > 0x00007ffff60a5307 <+167>: mov 0x10(%rsp),%rbp > 0x00007ffff60a530c <+172>: add $0x18,%rsp > 0x00007ffff60a5310 <+176>: retq > 0x00007ffff60a5311 <+177>: nopl 0x0(%rax) > 0x00007ffff60a5318 <+184>: mov $0x1,%eax > 0x00007ffff60a531d <+189>: mov 0x8(%rsp),%rbx > 0x00007ffff60a5322 <+194>: mov 0x10(%rsp),%rbp > 0x00007ffff60a5327 <+199>: add $0x18,%rsp > 0x00007ffff60a532b <+203>: retq > > And now: > > => 0x00007ffff60a4b50 <+0>: mov %rbx,-0x10(%rsp) > 0x00007ffff60a4b55 <+5>: mov %rbp,-0x8(%rsp) > 0x00007ffff60a4b5a <+10>: sub $0x18,%rsp > 0x00007ffff60a4b5e <+14>: movzwl 0x10(%rsi),%eax > 0x00007ffff60a4b62 <+18>: mov %rdi,%rbp > 0x00007ffff60a4b65 <+21>: mov %rsi,%rbx > 0x00007ffff60a4b68 <+24>: mov 0x20(%rdi),%rdx > 0x00007ffff60a4b6c <+28>: cmp $0xff,%eax > 0x00007ffff60a4b71 <+33>: jg 0x7ffff60a4b83 <_ZN13QQuickWebView5eventEP6QEvent+51> > 0x00007ffff60a4b73 <+35>: lea 0x1d0b7e6(%rip),%rsi # 0x7ffff7db0360 <_ZL20s_eventHandledByPage> > 0x00007ffff60a4b7a <+42>: movslq %eax,%rcx > 0x00007ffff60a4b7d <+45>: cmpb $0x0,(%rsi,%rcx,1) > 0x00007ffff60a4b81 <+49>: jne 0x7ffff60a4ba8 <_ZN13QQuickWebView5eventEP6QEvent+88> > 0x00007ffff60a4b83 <+51>: cmp $0x53,%eax > 0x00007ffff60a4b86 <+54>: je 0x7ffff60a4be0 <_ZN13QQuickWebView5eventEP6QEvent+144> > 0x00007ffff60a4b88 <+56>: mov %rbx,%rsi > 0x00007ffff60a4b8b <+59>: mov %rbp,%rdi > 0x00007ffff60a4b8e <+62>: mov 0x8(%rsp),%rbx > 0x00007ffff60a4b93 <+67>: mov 0x10(%rsp),%rbp > 0x00007ffff60a4b98 <+72>: add $0x18,%rsp > 0x00007ffff60a4b9c <+76>: jmpq 0x7ffff5ff4e60 <_ZN10QQuickItem5eventEP6QEvent@plt> > 0x00007ffff60a4ba1 <+81>: nopl 0x0(%rax) > 0x00007ffff60a4ba8 <+88>: mov 0x90(%rdx),%rdi > 0x00007ffff60a4baf <+95>: callq 0x7ffff60a3160 <_ZNK13QQuickWebPage12eventHandlerEv> > 0x00007ffff60a4bb4 <+100>: mov %rbx,%rsi > 0x00007ffff60a4bb7 <+103>: mov %rax,%rdi > 0x00007ffff60a4bba <+106>: callq 0x7ffff61144a0 <_ZN21QtWebPageEventHandler11handleEventEP6QEvent> > 0x00007ffff60a4bbf <+111>: test %al,%al > 0x00007ffff60a4bc1 <+113>: je 0x7ffff60a4b88 <_ZN13QQuickWebView5eventEP6QEvent+56> > 0x00007ffff60a4bc3 <+115>: mov $0x1,%eax > 0x00007ffff60a4bc8 <+120>: mov 0x8(%rsp),%rbx > 0x00007ffff60a4bcd <+125>: mov 0x10(%rsp),%rbp > 0x00007ffff60a4bd2 <+130>: add $0x18,%rsp > 0x00007ffff60a4bd6 <+134>: retq > 0x00007ffff60a4bd7 <+135>: nopw 0x0(%rax,%rax,1) > 0x00007ffff60a4be0 <+144>: xor %eax,%eax > 0x00007ffff60a4be2 <+146>: mov 0x8(%rsp),%rbx > 0x00007ffff60a4be7 <+151>: mov 0x10(%rsp),%rbp > 0x00007ffff60a4bec <+156>: add $0x18,%rsp > 0x00007ffff60a4bf0 <+160>: retq > > The main motivation to make this change is that QQuickWebView::event is used a lot and the lookup would grow logarithmically by the number of events handled by QQuickWebPage when we could do this in O(1) instead. > > Adding some people to CC... What you think of this?
Alexis Menard (darktears)
Comment 5 2012-02-07 18:35:49 PST
Comment on attachment 125940 [details] Patch Wait wait wait, what is the improvement here? How much faster is that? I want to see numbers, assembly is not enough. This is a huge hack for maybe nothing. I don't remember seeing ::event being a bottleneck while working in Qt (but I maybe wrong). We're not yet in the state of micro-optimizing our code. I believe that callgrind will show much more bottleneck than this one.
Alexis Menard (darktears)
Comment 6 2012-02-07 18:42:16 PST
(In reply to comment #5) > (From update of attachment 125940 [details]) > Wait wait wait, what is the improvement here? How much faster is that? I want to see numbers, assembly is not enough. This is a huge hack for maybe nothing. I don't remember seeing ::event being a bottleneck while working in Qt (but I maybe wrong). We're not yet in the state of micro-optimizing our code. I believe that callgrind will show much more bottleneck than this one. Also what is the balance here memory usage of this map vs the tiny optimization? One way to optimize the assembly code here is to group each case to avoid big hole (which is the case today) starting let say from the smallest value to the highest, that would help the compiler to maybe generate a jump table for this switch/case and maybe lead to worst case O(n) best case O(1).
Rafael Brandao
Comment 7 2012-02-07 18:50:12 PST
(In reply to comment #6) > (In reply to comment #5) > > (From update of attachment 125940 [details] [details]) > > Wait wait wait, what is the improvement here? How much faster is that? I want to see numbers, assembly is not enough. This is a huge hack for maybe nothing. I don't remember seeing ::event being a bottleneck while working in Qt (but I maybe wrong). We're not yet in the state of micro-optimizing our code. I believe that callgrind will show much more bottleneck than this one. > > Also what is the balance here memory usage of this map vs the tiny optimization? > > One way to optimize the assembly code here is to group each case to avoid big hole (which is the case today) starting let say from the smallest value to the highest, that would help the compiler to maybe generate a jump table for this switch/case and maybe lead to worst case O(n) best case O(1). Ok I'll look into numbers for it (valgrind/cachegrind is an unknown tool for me yet). Still it looks wrong to me that we are not already doing this fast enough, but I believe you may be right, this may not be a bottleneck. In the worse case we can skip this task and fix at some point later (when we get into that stage of micro-optimization), but I can only say more about it tomorrow. Thanks for the feedback! :-)
Simon Hausmann
Comment 8 2012-02-08 02:00:33 PST
Comment on attachment 125940 [details] Patch I think the best solution is to get rid of this code altogether. With and without your patch we do a type switch _three_ times. Once in QtQuick, which calls our specific mouseFooEvent, touchEvent, etc. handlers in QQuickWebView. We throw away that result and dispatch once more in QQuickWebView::event, just to throw that away again and do another type dispatch in QtWebPageEventHandler. Let's get rid of our own type dispatch and simply change for example void QQuickWebView::mouseReleaseEvent(QMouseEvent* event) to: void QQuickWebView::mouseReleaseEvent(QMouseEvent* event) { d->pageView->eventHandler()->mouseReleaseEvent(event); } Am I missing something?
Andras Becsi
Comment 9 2012-02-08 03:10:14 PST
(In reply to comment #8) > (From update of attachment 125940 [details]) > I think the best solution is to get rid of this code altogether. I agree with Simon on this, AFAIR the function originates from the time where we did not have the page event handler yet and the item receiving the events was the page, not the view.
Rafael Brandao
Comment 10 2012-02-08 04:52:00 PST
(In reply to comment #8) > (From update of attachment 125940 [details]) > I think the best solution is to get rid of this code altogether. With and without your patch we do a type switch _three_ times. Once in QtQuick, which calls our specific mouseFooEvent, touchEvent, etc. handlers in QQuickWebView. We throw away that result and dispatch once more in QQuickWebView::event, just to throw that away again and do another type dispatch in QtWebPageEventHandler. Let's get rid of our own type dispatch and simply change for example void QQuickWebView::mouseReleaseEvent(QMouseEvent* event) to: > > void QQuickWebView::mouseReleaseEvent(QMouseEvent* event) > { > d->pageView->eventHandler()->mouseReleaseEvent(event); > } > > Am I missing something? Good point Simon, it seems more reasonable.
Simon Hausmann
Comment 11 2012-02-24 04:51:01 PST
Alexis Menard (darktears)
Comment 12 2012-02-24 04:53:06 PST
Comment on attachment 128710 [details] Patch LGTM. Nice cleanup.
Kenneth Rohde Christiansen
Comment 13 2012-02-24 04:53:18 PST
Comment on attachment 128710 [details] Patch Nice!
Rafael Brandao
Comment 14 2012-02-24 09:32:12 PST
Comment on attachment 128710 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=128710&action=review nice :) > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:1292 > + d->pageView->eventHandler()->handleInputMethodEvent(event); Can you confirm if we're still propagating this event? I recall that I've tried to reproduce it when I was looking into this but I couldn't find out. If we are not using it, maybe we should just remove it.
Alexis Menard (darktears)
Comment 15 2012-02-24 09:35:30 PST
Comment on attachment 128710 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=128710&action=review >> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:1292 >> + d->pageView->eventHandler()->handleInputMethodEvent(event); > > Can you confirm if we're still propagating this event? I recall that I've tried to reproduce it when I was looking into this but I couldn't find out. If we are not using it, maybe we should just remove it. It is used whenever an input method exists in the app :D.
Simon Hausmann
Comment 16 2012-02-29 07:03:42 PST
Simon Hausmann
Comment 17 2012-02-29 07:07:19 PST
(In reply to comment #16) > Created an attachment (id=129437) [details] > Patch New patch. I landed the original patch but had to roll it out again, because many WK2 tests failed. It turned out that this is caused by WTR sending events directly to the QQuickItem instead of sending them through the QQuickCanvas. The latter dispatches the events directly to QQuickItem::fooEvent(QFooEvent*), _not_ going through ::event. I'll change WTR to send the events through the canvas, like it would happen in a "real" application.
Simon Hausmann
Comment 18 2012-03-01 01:33:31 PST
Simon Hausmann
Comment 19 2012-03-01 07:52:43 PST
Comment on attachment 129437 [details] Patch Clearning review. Strange that webkit-patch didn't do it this time around :)
Note You need to log in before you can comment on or make changes to this bug.