Re: [webkit-dev] [webkit-changes] [68146] trunk/WebCore
Sorry. I should really have made better comments. This code is not used yet, I just aded it yesterday and plan to use it soon. The assertion, that I have introduced, was wrong because the case callType == CallTypeNone must be handled. It is not about correcting existing problems because I just introduced this code. It will not cause any regressions because the code that will use it did not land yet, it is in review process. I am sorry if I made you spend your time. Next time please contact me, it is easy to find me. I have been working actively in webkit for a while, I am always in iRC (lca), my name, email and IRC user can be found in http://trac.webkit.org/wiki/WebKit%20Team and I am willing to help, answer, change or whatever is needed. And of course I would reply to any comment in the bug. Luiz 2010/9/23 Alexey Proskuryakov > > It is unfortunate that this fix changes unused code. Will it be covered by > existing layout tests when ScriptFunctionCall and ScriptCallback start being > used? > > The patch and bug were highly confusing. Without any explanation of why > this assertion was wrong, a test case, or an explanation of why one can't be > made, I had to spend considerable time figuring out why it shouldn't be > rolled out immediately. > > - WBR, Alexey Proskuryakov > > Начало переадресованного сообщения: > > *От: *commit-qu...@webkit.org > *Дата: *23 сентября 2010 г. 8:57:01 Тихоокеанское летнее время > *Кому: *webkit-chan...@lists.webkit.org > *Тема: **[webkit-changes] [68146] trunk/WebCore* > > Revision 68146 <http://trac.webkit.org/projects/webkit/changeset/68146> > Author commit-qu...@webkit.org Date 2010-09-23 08:56:59 -0700 (Thu, 23 Sep > 2010) Log Message > > 2010-09-23 Luiz Agostini > > Reviewed by Andreas Kling. > > Invalid assertion in ScriptCallback > https://bugs.webkit.org/show_bug.cgi?id=46348 > > Removing invalid ASSERT from method ScriptCallback::call(). > > * bindings/js/ScriptFunctionCall.cpp: > (WebCore::ScriptCallback::call): > > Modified Paths > >- trunk/WebCore/ChangeLog >- trunk/WebCore/bindings/js/ScriptFunctionCall.cpp > > Diff > Modified: trunk/WebCore/ChangeLog (68145 => 68146) > > --- trunk/WebCore/ChangeLog 2010-09-23 15:51:41 UTC (rev 68145) > +++ trunk/WebCore/ChangeLog 2010-09-23 15:56:59 UTC (rev 68146)@@ -1,3 > +1,15 @@+2010-09-23 Luiz Agostini > + > +Reviewed by Andreas Kling. > + > +Invalid assertion in ScriptCallback > +https://bugs.webkit.org/show_bug.cgi?id=46348 > + > +Removing invalid ASSERT from method ScriptCallback::call(). > + > +* bindings/js/ScriptFunctionCall.cpp: > +(WebCore::ScriptCallback::call): > + 2010-09-23 Martin RobinsonReviewed by > Ariya Hidayat. > > Modified: trunk/WebCore/bindings/js/ScriptFunctionCall.cpp (68145 => > 68146) > > --- trunk/WebCore/bindings/js/ScriptFunctionCall.cpp 2010-09-23 15:51:41 UTC > (rev 68145) > +++ trunk/WebCore/bindings/js/ScriptFunctionCall.cpp 2010-09-23 15:56:59 UTC > (rev 68146)@@ -215,9 +215,9 @@ CallData callData; CallType callType > = getCallData(m_function.jsValue(), callData);+if (callType == > CallTypeNone) > +return ScriptValue(); -ASSERT(callType != CallTypeNone); > - JSValue result = JSC::call(m_exec, m_function.jsValue(), callType, > callData, m_function.jsValue(), m_arguments); hadException = > m_exec->hadException(); > > ___ > webkit-changes mailing list > webkit-chan...@lists.webkit.org > http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes > > > > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Clean way to allow delegates for PopupMenu?
2010/4/16 Gustavo Sverzut Barbieri
> On Fri, Apr 16, 2010 at 10:30 AM, Luiz Agostini
> wrote:
> >
> > 2010/4/15 Gustavo Sverzut Barbieri
> >>
> >> Hello all,
> >>
> >> I'm part of the EFL port team and we're implementing the PopupMenu,
> >> however EFL is a different platform as for our port it is just a
> >> state-full canvas, not relying on any widget set/toolkit, we do not
> >> have the concept of a menu at all!
> >>
> >> Looking at all existing implementations, they all go straight to
> >> native platform menus. But we can't as we want to avoid such
> >> dependencies. What we'd like to have is a delegate, that WebCore calls
> >> the ChromeClient that is overloaded in WebKit, giving our View user a
> >> callback with all data.
> >>
> >> The good news is: we did it and it works quite well.
> >>
> >> The bad news is: we cheated and include WebKit/efl/WebCoreSupport from
> >> PopupMenuEfl.cpp, what a shame! (although some ports *cough qt*
> >> include in WebCore defines from WebKit)
> >
> > In Qt we have an abstract class for popup delegates in
> WebCore/platform/qt
> > and we do the actual popup delegate implementation in
> > WebKit/qt/WebCoreSupport. The delegates are implemented in WebKit layer
> and
> > PopupMenuQt has no knowledge about any specific delegates. The only
> reason
> > to include WebKit/qt/WebCoreSupport in PopupMenuQt is to make the call
> that
> > creates the delegate object. It could be avoided adding a method to
> Chrome
> > but at the time I was working on it it was decided that it was not
> needed.
>
> Yes, it is kinda similar to our, since we looked at your
> implementation before doing our. But the problem here is you still
> include WebKit files from WebCore. For *me* it looks like a hack, but
> if people do not complain then I can kindly keep it as is, after all
> it is working already :-)
>
>
> >> In order to have a clean design, we'd like to know the general opinion
> >> on how to do it. From what I see Mac already defines a similar call in
> >> ChromeClient.h:
> >>
> >> #if PLATFORM(MAC)
> >> ...
> >>virtual void willPopUpMenu(NSMenu *) { }
> >> #endif
> >>
> >> in our case, we'd need:
> >>
> >> #if PLATFORM(EFL)
> >>virtual void showPopupMenu(const IntRect& rect, FrameView*
> >> view, int index) { }
> >>virtual void hidePopupMenu() { }
> >> #endif
> >>
> >> so our PopupMenuEfl.cpp will just proxy to these calls.
> >>
> >> However, although Mac does that it may not be the best solution, so to
> >> avoid endless patches to be discussed at bugzilla I'd like to know
> >> your opinion on the best way so our patch is right from beginning.
> >
> >
> > I think that we could add methods to Chrome and ChromeClient to create
> the
> > delegates. For example:
> > PopupMenuDelegate* Chrome::createPopupMenuDelegate(PopupMenuClient* c) {
> > return m_client->createPopupMenuDelegate(c); }
> > virtual PopupMenuDelegate*
> > ChromeClient::createPopupMenuDelegate(PopupMenuClient*) { return 0; }
> > Each port could then provide its typedef for PopupMenuDelegate and
> > override ChromeClient::createPopupMenuDelegate.
>
> Well, as I said we just need methods to show/hide to keep it done, but
> if you want to implement another class "PopupMenuDelegate" to do it,
> then we could do such work as well. We'd like to avoid this delegate
> class as our port is C, so we'd need to do it all in C++ plus what we
> have done in C already, just to proxy it.
>
I did not suggest a class for PopupMenuDelegate: "Each port could then
provide its typedef for PopupMenuDelegate".
Remember PlatformWidget? :-)
>
>
> BR,
>
> --
> Gustavo Sverzut Barbieri
> http://profusion.mobi embedded systems
> --
> MSN: barbi...@gmail.com
> Skype: gsbarbieri
> Mobile: +55 (19) 9225-2202
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Clean way to allow delegates for PopupMenu?
2010/4/15 Gustavo Sverzut Barbieri
> Hello all,
>
> I'm part of the EFL port team and we're implementing the PopupMenu,
> however EFL is a different platform as for our port it is just a
> state-full canvas, not relying on any widget set/toolkit, we do not
> have the concept of a menu at all!
>
> Looking at all existing implementations, they all go straight to
> native platform menus. But we can't as we want to avoid such
> dependencies. What we'd like to have is a delegate, that WebCore calls
> the ChromeClient that is overloaded in WebKit, giving our View user a
> callback with all data.
>
> The good news is: we did it and it works quite well.
>
> The bad news is: we cheated and include WebKit/efl/WebCoreSupport from
> PopupMenuEfl.cpp, what a shame! (although some ports *cough qt*
> include in WebCore defines from WebKit)
>
In Qt we have an abstract class for popup delegates in WebCore/platform/qt
and we do the actual popup delegate implementation in
WebKit/qt/WebCoreSupport. The delegates are implemented in WebKit layer and
PopupMenuQt has no knowledge about any specific delegates. The only reason
to include WebKit/qt/WebCoreSupport in PopupMenuQt is to make the call that
creates the delegate object. It could be avoided adding a method to Chrome
but at the time I was working on it it was decided that it was not needed.
>
> In order to have a clean design, we'd like to know the general opinion
> on how to do it. From what I see Mac already defines a similar call in
> ChromeClient.h:
>
> #if PLATFORM(MAC)
> ...
>virtual void willPopUpMenu(NSMenu *) { }
> #endif
>
> in our case, we'd need:
>
> #if PLATFORM(EFL)
>virtual void showPopupMenu(const IntRect& rect, FrameView*
> view, int index) { }
>virtual void hidePopupMenu() { }
> #endif
>
> so our PopupMenuEfl.cpp will just proxy to these calls.
>
> However, although Mac does that it may not be the best solution, so to
> avoid endless patches to be discussed at bugzilla I'd like to know
> your opinion on the best way so our patch is right from beginning.
>
I think that we could add methods to Chrome and ChromeClient to create the
delegates. For example:
PopupMenuDelegate* Chrome::createPopupMenuDelegate(PopupMenuClient* c) {
return m_client->createPopupMenuDelegate(c); }
virtual PopupMenuDelegate*
ChromeClient::createPopupMenuDelegate(PopupMenuClient*) { return 0; }
Each port could then provide its typedef for PopupMenuDelegate and
override ChromeClient::createPopupMenuDelegate.
br
Luiz
>
> Regards,
>
> --
> Gustavo Sverzut Barbieri
> http://profusion.mobi embedded systems
> --
> MSN: barbi...@gmail.com
> Skype: gsbarbieri
> Mobile: +55 (19) 9225-2202
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
