Bug 36317

Summary: [EFL] Add EFL-specific code to Widget.h
Product: WebKit Reporter: Leandro Pereira <leandro>
Component: WebCore Misc.Assignee: Leandro Pereira <leandro>
Status: RESOLVED FIXED    
Severity: Normal CC: barbieri, cjerdonek, commit-queue, dbates, eric, gns, rakuco, tonikitoo, webkit.review.bot, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Add EFL-specific code to Widget.h
none
Add EFL-specific code to Widget.h, with comments.
none
Add EFL-specific code to Widget.h.
eric: review-
Add EFL-specific code to Widget.h.
none
Add EFL-specific code to Widget.h and Widget.cpp
none
The full log from the commit-queue
none
Add Widget.h and Widget.cpp changes to enable EFL none

Description Leandro Pereira 2010-03-18 12:36:06 PDT
Attached patch adds EFL-specific code to WebCore/platform/Widget.h.
Comment 1 Leandro Pereira 2010-03-18 12:36:46 PDT
Created attachment 51069 [details]
Add EFL-specific code to Widget.h
Comment 2 WebKit Review Bot 2010-03-18 12:41:25 PDT
Attachment 51069 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/Widget.h:84:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Kenneth Rohde Christiansen 2010-03-24 21:29:39 PDT
Comment on attachment 51069 [details]
Add EFL-specific code to Widget.h


> +#if PLATFORM(EFL)
> +    virtual void frameRectsChanged();
> +#else
>      virtual void frameRectsChanged() {}
> +#endif

What was this one introduced for? Maybe that should be part of the patch that introduced it?
Comment 4 Eric Seidel (no email) 2010-03-24 23:51:24 PDT
Comment on attachment 51069 [details]
Add EFL-specific code to Widget.h

Insufficient explaination in the ChagneLog for me to make sense of this patch.  See http://webkit.org/coding/contributing.html
Comment 5 Gustavo Sverzut Barbieri 2010-03-25 10:55:32 PDT
(In reply to comment #3)
> (From update of attachment 51069 [details])
> 
> > +#if PLATFORM(EFL)
> > +    virtual void frameRectsChanged();
> > +#else
> >      virtual void frameRectsChanged() {}
> > +#endif
> 
> What was this one introduced for? Maybe that should be part of the patch that
> introduced it?

It is used to move our scrollbars and other widgets like plugins. Our platform is bit different and requires this.
Comment 6 Gustavo Noronha (kov) 2010-04-08 12:14:46 PDT
(In reply to comment #5)
> > What was this one introduced for? Maybe that should be part of the patch that
> > introduced it?
> 
> It is used to move our scrollbars and other widgets like plugins. Our platform
> is bit different and requires this.

I think this should go in a port-specific file rather than on the cross-platform one. I would mimic what is done in, say, ScrollbarGtk.h. These functions are virtual exactly so they can be overriden.
Comment 7 Gustavo Sverzut Barbieri 2010-04-08 12:30:26 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > > What was this one introduced for? Maybe that should be part of the patch that
> > > introduced it?
> > 
> > It is used to move our scrollbars and other widgets like plugins. Our platform
> > is bit different and requires this.
> 
> I think this should go in a port-specific file rather than on the
> cross-platform one. I would mimic what is done in, say, ScrollbarGtk.h. These
> functions are virtual exactly so they can be overriden.

We need this at the widget level, we must be informed about the frameRect changed so we can act. Unlike other ports, our plugins and other visual elements (scrollbars) are not moved automatically. We also need to update our clippers if we use backing store that is bigger than the current viewport.

Also, note that this patch just add an exception for EFL, it does not change the behavior of old platforms. We just need to be able to implement our own frameRectChanged() in WidgetEfl.cpp, see http://trac.webkit.org/browser/trunk/WebCore/platform/efl/WidgetEfl.cpp#L183

If we don't have this, then moving our object into the window will leave stuff behind :-/  Note that EFL is just managing objects into a scene, it is windowless, there is no X11 windows being moved that move their children automatically.
Comment 8 Gustavo Noronha (kov) 2010-04-09 11:03:29 PDT
(In reply to comment #7)
> We need this at the widget level, we must be informed about the frameRect
> changed so we can act. Unlike other ports, our plugins and other visual
> elements (scrollbars) are not moved automatically. We also need to update our
> clippers if we use backing store that is bigger than the current viewport.

Sorry, I may not have been clear enough. I am not suggesting you move it outside of the Widget level. I'm just suggesting you use a port specific WidgetEfl.h, like other classes do. I haven't looked at how difficult that would be, but I don't think it should be too difficult.
Comment 9 Gustavo Sverzut Barbieri 2010-04-10 18:00:55 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > We need this at the widget level, we must be informed about the frameRect
> > changed so we can act. Unlike other ports, our plugins and other visual
> > elements (scrollbars) are not moved automatically. We also need to update our
> > clippers if we use backing store that is bigger than the current viewport.
> 
> Sorry, I may not have been clear enough. I am not suggesting you move it
> outside of the Widget level. I'm just suggesting you use a port specific
> WidgetEfl.h, like other classes do. I haven't looked at how difficult that
> would be, but I don't think it should be too difficult.

But the patch would be as intrusive, no? What is good in leaving the #ifndef in Widget.h and the #ifdef in the WidgetEfl.h? I guess it is overcomplicating for no greater benefit. MAC already have similar pattern down in the same file.
Comment 10 Gustavo Noronha (kov) 2010-04-12 11:53:09 PDT
There would be no #if's in that case. But that was just a suggestion. If you
fix the changelog to detail the change as requested by Eric, and perhaps a
comment to make it obvious, I have no problem with this patch going in.
Comment 11 Gustavo Sverzut Barbieri 2010-04-12 17:32:42 PDT
Created attachment 53205 [details]
Add EFL-specific code to Widget.h, with comments.

Add comments as requested by Gustavo Noronha (kov).
Comment 12 WebKit Review Bot 2010-04-12 17:37:08 PDT
Attachment 53205 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 Gustavo Sverzut Barbieri 2010-04-12 18:52:49 PDT
(In reply to comment #12)
> Attachment 53205 [details] did not pass style-queue:
> 
> Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
> Total errors found: 0 in 0 files
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.

These errors were caused by EFL's type using the format like "Evas_Object" and "Ecore_Evas".
Comment 14 Dave Hyatt 2010-04-13 12:20:49 PDT
Comment on attachment 53205 [details]
Add EFL-specific code to Widget.h, with comments.

r=me
Comment 15 WebKit Commit Bot 2010-04-13 13:00:15 PDT
Comment on attachment 53205 [details]
Add EFL-specific code to Widget.h, with comments.

Rejecting patch 53205 from commit-queue.

Found no modified ChangeLogs, cannot create a commit message.
All changes require a ChangeLog.  See:
http://webkit.org/coding/contributing.html
Comment 16 Eric Seidel (no email) 2010-04-13 13:24:47 PDT
Something is wrong with teh commit-queue.  Will ix.
Comment 17 Eric Seidel (no email) 2010-04-13 14:17:19 PDT
svn-apply seems to think that this patch is invalid:


% curl "https://bugs.webkit.org/attachment.cgi?id=53205" --location | svn-apply
Failed to find 'Index:' in:
-------------------------------------------------------------------
--- WebCore/ChangeLog
+++ WebCore/ChangeLog
@@ -1,3 +1,16 @@
+2010-03-18  Leandro Pereira  <leandro@profusion.mobi>

Died at WebKitTools/Scripts/svn-apply line 337, <> line 94.
% echo $?
255

I'm not sure why the commit-queue got all the way to the landing stage if svn-apply failed as such.
Comment 18 Chris Jerdonek 2010-04-13 15:59:37 PDT
(In reply to comment #17)
> svn-apply seems to think that this patch is invalid:
> 
> 
> % curl "https://bugs.webkit.org/attachment.cgi?id=53205" --location | svn-apply
> Failed to find 'Index:' in:
> -------------------------------------------------------------------
> --- WebCore/ChangeLog
> +++ WebCore/ChangeLog
> @@ -1,3 +1,16 @@
> +2010-03-18  Leandro Pereira  <leandro@profusion.mobi>
> 
> Died at WebKitTools/Scripts/svn-apply line 337, <> line 94.
> % echo $?
> 255
> 
> I'm not sure why the commit-queue got all the way to the landing stage if
> svn-apply failed as such.

The submitted patch has no "Index:" line (as the svn-apply error states):

--- WebCore/ChangeLog
+++ WebCore/ChangeLog
@@ -1,3 +1,16 @@
+2010-03-18  Leandro Pereira  <leandro@profusion.mobi>
+
+        Reviewed by NOBODY (OOPS!).
+
+        Add EFL-specific code to platform/Widget.h. We need

(from https://bugs.webkit.org/attachment.cgi?id=53205 )

FYI, Leandro seems to have had similar problems in the past:

https://bugs.webkit.org/show_bug.cgi?id=35234#c2
https://bugs.webkit.org/show_bug.cgi?id=37478#c10

Leandro, are you using svn-create-patch, git diff, or an equivalent?  Are you modifying your patches by hand in some way prior to submitting?
Comment 19 Chris Jerdonek 2010-04-13 16:06:44 PDT
(In reply to comment #18)
> FYI, Leandro seems to have had similar problems in the past:
> 
> https://bugs.webkit.org/show_bug.cgi?id=35234#c2
> https://bugs.webkit.org/show_bug.cgi?id=37478#c10
> 
> Leandro, are you using svn-create-patch, git diff, or an equivalent?  Are you
> modifying your patches by hand in some way prior to submitting?

FYI, here is a previous note to Leandro requesting that our tools be used to create patches:

https://bugs.webkit.org/show_bug.cgi?id=35749#c2
Comment 20 Leandro Pereira 2010-04-14 07:11:52 PDT
Created attachment 53328 [details]
Add EFL-specific code to Widget.h.

The previous patch was made by Gustavo because we're in a hurry -- he didn't know about the WebKit tools.

This patch has been already r+'d by Hyatt, it's just updated both to fix the patch format, and to update to a newer Widget.h.
Comment 21 Eric Seidel (no email) 2010-04-14 09:09:04 PDT
Comment on attachment 53328 [details]
Add EFL-specific code to Widget.h.

I'm confused.  Why wouldn't EFL subclass Widget like how WidgetMac does?
Comment 22 Eric Seidel (no email) 2010-04-14 09:11:33 PDT
Comment on attachment 53328 [details]
Add EFL-specific code to Widget.h.

I have no idea why Hyatt r+'d this before.

Two reviewers have now suggested using WidgetEFL, and there has been no explanation from teh author why we would not, neither in the bug nor the ChangeLog.

Please update the ChangeLog to explain why this change is done this way, or fix ti to use WidgetEFL (assuming that's the modern way to do this type of change?).  #ifdefs here seem super-ugly.
Comment 23 Leandro Pereira 2010-04-14 09:55:04 PDT
(In reply to comment #21)
> (From update of attachment 53328 [details])
> I'm confused.  Why wouldn't EFL subclass Widget like how WidgetMac does?

No other port override this method, not even WidgetMac:

leandro:~/git/webkit/WebCore/platform$ grep 'void\s*Widget::frameRectsChanged' * -r
efl/WidgetEfl.cpp:void Widget::frameRectsChanged()
leandro:~/git/webkit/WebCore/platform$ 

Widget.h declares an empty method for all ports, but we do need to override this at widget level.

I thought that the ChangeLog message was clear enough.
Comment 24 Eric Seidel (no email) 2010-04-14 09:56:50 PDT
It delcares an empty virtual method, which could be overridden by a virtual method in a Widget subclass, no?
Comment 25 Eric Seidel (no email) 2010-04-14 12:56:03 PDT
Comment on attachment 53328 [details]
Add EFL-specific code to Widget.h.

OK.  I think this can be OK.  We talked about this at length over IRC.

I'd like to see a patch posted with either // FIXME: comments or just normal comments expaining why EFL is adding all this junk to Widget.

+#if PLATFORM(EFL)
+    // EFL needs to know whenever any object was moved or resized, so a
+    // null frameRectsChanged() implementation can't be used.
+    virtual void frameRectsChanged();
+#else
     virtual void frameRectsChanged() {}
+#endif
 
Should be done with a WidgetEFL.cpp file, like all the other overrides in Widget.cpp are done.

+#if PLATFORM(EFL)
+    Evas* evas() const;
+
+    void setEvasObject(Evas_Object*);
+    Evas_Object* evasObject() const;
+
+    const String edjeTheme() const;
+    void setEdjeTheme(const String &);
+    const String edjeThemeRecursive() const;
+#endif

Shouldn't these just be free methods in some sort of WidgetEFL.h file?  They coudl take the Widget or PlatformWidget as an argument since I"m sure that's what you're using to compute this data.  Free methods are very c-like after all. :)


I'm not sure what this is:
+    Ecore_Evas* ecoreEvas() const;
Comment 26 Leandro Pereira 2010-04-14 13:47:15 PDT
Created attachment 53361 [details]
 Add EFL-specific code to Widget.h.

After some discussion with eseidel on IRC, we agreed to include the patch as-is, but with some comments indicating what should be done to have as few platform-specific stuff as possible inside Widget.h. This revised patch includes these FIXME comments.
Comment 27 Eric Seidel (no email) 2010-04-14 13:50:37 PDT
Comment on attachment 53361 [details]
 Add EFL-specific code to Widget.h.

EFL port should subclass the Widget class and 
 205     // override this method.
That's wrong.  I was originally confused.

Widget doesn't get subclassed it appears.  But Widget.cpp could add an empty implementation which was wrapped in the #ifdef and WidgetEFL.cpp would have the implementation which was non-empty.

I'm actually not sure why we don't do that instead of writing the FIXME as that would actually be fewer lines of code. :)
Comment 28 Leandro Pereira 2010-04-14 14:28:35 PDT
Created attachment 53369 [details]
Add EFL-specific code to Widget.h and Widget.cpp
Comment 29 Eric Seidel (no email) 2010-04-14 14:30:43 PDT
Comment on attachment 53369 [details]
Add EFL-specific code to Widget.h and Widget.cpp

Thanks.
Comment 30 WebKit Commit Bot 2010-04-14 19:18:44 PDT
Comment on attachment 53369 [details]
Add EFL-specific code to Widget.h and Widget.cpp

Rejecting patch 53369 from commit-queue.

Unexpected failure when landing patch!  Please file a bug against webkit-patch.
Failed to run "['WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--build', '--test', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', '53369', '--parent-command=commit-queue', '--no-update']" exit_code: 1
Last 500 characters of output:
date_status
    return NetworkTransaction().run(lambda: self._post_status_to_server(queue_name, status, patch, results_file))
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/common/net/networktransaction.py", line 56, in run
    self._check_for_timeout()
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/common/net/networktransaction.py", line 63, in _check_for_timeout
    raise NetworkTimeout()
webkitpy.common.net.networktransaction.NetworkTimeout
Comment 31 Gustavo Sverzut Barbieri 2010-04-14 21:59:36 PDT
(In reply to comment #30)
> (From update of attachment 53369 [details])
> Rejecting patch 53369 from commit-queue.
> 
> Unexpected failure when landing patch!  Please file a bug against webkit-patch.
> Failed to run "['WebKitTools/Scripts/webkit-patch',
> '--status-host=webkit-commit-queue.appspot.com', 'land-attachment',
> '--force-clean', '--build', '--test', '--non-interactive', '--ignore-builders',
> '--build-style=both', '--quiet', '53369', '--parent-command=commit-queue',
> '--no-update']" exit_code: 1
> Last 500 characters of output:
> date_status
>     return NetworkTransaction().run(lambda:
> self._post_status_to_server(queue_name, status, patch, results_file))
>   File
> "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/common/net/networktransaction.py",
> line 56, in run
>     self._check_for_timeout()
>   File
> "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/common/net/networktransaction.py",
> line 63, in _check_for_timeout
>     raise NetworkTimeout()
> webkitpy.common.net.networktransaction.NetworkTimeout

This bug is cursed, it is the only explanation I can see :-D
Comment 32 Eric Seidel (no email) 2010-04-14 22:36:56 PDT
Comment on attachment 53369 [details]
Add EFL-specific code to Widget.h and Widget.cpp

Trying again.
Comment 33 WebKit Commit Bot 2010-04-15 03:28:10 PDT
Comment on attachment 53369 [details]
Add EFL-specific code to Widget.h and Widget.cpp

Rejecting patch 53369 from commit-queue.

Unexpected failure when landing patch!  Please file a bug against webkit-patch.
Failed to run "['WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--build', '--test', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', '53369', '--parent-command=commit-queue', '--no-update']" exit_code: 1
Last 500 characters of output:
date_status
    return NetworkTransaction().run(lambda: self._post_status_to_server(queue_name, status, patch, results_file))
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/common/net/networktransaction.py", line 56, in run
    self._check_for_timeout()
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/common/net/networktransaction.py", line 63, in _check_for_timeout
    raise NetworkTimeout()
webkitpy.common.net.networktransaction.NetworkTimeout
Comment 34 Eric Seidel (no email) 2010-04-15 03:32:54 PDT
I suspect this is attempting to post a huge log and timing out.  I suspect it breaks mac somehow.
Comment 35 Leandro Pereira 2010-04-15 13:14:00 PDT
(In reply to comment #34)
> I suspect this is attempting to post a huge log and timing out.  I suspect it
> breaks mac somehow.

I've built WebCore (from r57639) on my personal Mac today with this patch and it built successfully. Any other clues on what might be happening?

FWIW: MacBook 2,1, OSX 10.6.3, Xcode 3.2.1, default x86_64|Release build by running "make" on the top level source directory.
Comment 36 Eric Seidel (no email) 2010-04-18 16:26:06 PDT
Comment on attachment 53205 [details]
Add EFL-specific code to Widget.h, with comments.

Cleared David Hyatt's review+ from obsolete attachment 53205 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 37 WebKit Commit Bot 2010-04-19 12:13:57 PDT
Comment on attachment 53369 [details]
Add EFL-specific code to Widget.h and Widget.cpp

Rejecting patch 53369 from commit-queue.

jesus@webkit.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in WebKitTools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  Due to bug 30084 the commit-queue will require a restart after your change.  Please contact eseidel@chromium.org to request a commit-queue restart.  After restart the commit-queue will correctly respect your committer rights.
Comment 38 WebKit Commit Bot 2010-04-20 06:03:33 PDT
Comment on attachment 53369 [details]
Add EFL-specific code to Widget.h and Widget.cpp

Rejecting patch 53369 from commit-queue.

Unexpected failure when landing patch!  Please file a bug against webkit-patch.
Failed to run "['WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--build', '--test', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', '53369', '--parent-command=commit-queue', '--no-update']" exit_code: 1
Last 500 characters of output:
date_status
    return NetworkTransaction().run(lambda: self._post_status_to_server(queue_name, status, patch, results_file))
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/common/net/networktransaction.py", line 56, in run
    self._check_for_timeout()
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/common/net/networktransaction.py", line 63, in _check_for_timeout
    raise NetworkTimeout()
webkitpy.common.net.networktransaction.NetworkTimeout
Comment 39 Gustavo Sverzut Barbieri 2010-04-20 06:51:26 PDT
(In reply to comment #38)
> (From update of attachment 53369 [details])
> Rejecting patch 53369 from commit-queue.
> 
> Unexpected failure when landing patch!  Please file a bug against webkit-patch.
> Failed to run "['WebKitTools/Scripts/webkit-patch',
> '--status-host=webkit-commit-queue.appspot.com', 'land-attachment',
> '--force-clean', '--build', '--test', '--non-interactive', '--ignore-builders',
> '--build-style=both', '--quiet', '53369', '--parent-command=commit-queue',
> '--no-update']" exit_code: 1
> Last 500 characters of output:
> date_status
>     return NetworkTransaction().run(lambda:
> self._post_status_to_server(queue_name, status, patch, results_file))
>   File
> "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/common/net/networktransaction.py",
> line 56, in run
>     self._check_for_timeout()
>   File
> "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/common/net/networktransaction.py",
> line 63, in _check_for_timeout
>     raise NetworkTimeout()
> webkitpy.common.net.networktransaction.NetworkTimeout

Eric, what to do for this? This patch is hanging around for some time and the error is with python scripts to check it. You previously mentioned it may be a problem with some port, producing huge logs, but could you confirm if that is the problem and if so which problem it is so we can fix?
Comment 40 Eric Seidel (no email) 2010-04-20 12:31:41 PDT
Restarted the cq.  Jesus should be able to cq+ things now.
Comment 41 Eric Seidel (no email) 2010-04-20 12:44:06 PDT
Comment on attachment 53369 [details]
Add EFL-specific code to Widget.h and Widget.cpp

I'll look at the cq logs to find out bout the network transaction issue.
Comment 42 Eric Seidel (no email) 2010-04-21 04:18:53 PDT
Created attachment 53940 [details]
The full log from the commit-queue

Unfortunately the commit-queue is not currently configured to keep full build logs.  I should fix that.  But from the log we can see that it is the build which is failing here.
Comment 43 Gustavo Sverzut Barbieri 2010-04-22 06:25:25 PDT
Eric, from your log I see that what failed is:


   platform/mac/accessibility/aria-liveregion-on-image.html -> failed

Looking at the attached patch, every change except by the following are done inside #if PLATFORM(EFL):

    WebCore/platform/Widget.h:
        -    virtual void frameRectsChanged() {}
        +    virtual void frameRectsChanged();
    WebCore/platform/Widget.cpp:
        +#if !PLATFORM(EFL)
        +void Widget::frameRectsChanged()
        +{
        +}
        +#endif

Would you or anyone from Mac review how these would fail such accessibility test?

I guess this happened due some other patch that was landed at the time. No?
Comment 44 Nikolas Zimmermann 2010-04-22 06:31:47 PDT
(In reply to comment #43)
> Would you or anyone from Mac review how these would fail such accessibility
> test?
> 
> I guess this happened due some other patch that was landed at the time. No?

It's either flakyness or your reason. I don't see how this could possibly break mac, as WebCore.xcodeproj includes both Widget.cpp and Widget.h, so your patch looks fine to me.
Comment 45 Leandro Pereira 2010-04-26 10:18:26 PDT
Any updates as to why this patch is failing to be applied?
Comment 46 Eric Seidel (no email) 2010-04-26 11:02:30 PDT
The very last failure in that log is:
Failed to run "['WebKitTools/Scripts/build-webkit', '--debug']" exit_code: 1

Which means it failed to build on Mac.  Our upload size is too large for App Engine, so we failed to upload the log.
Comment 47 Antonio Gomes 2010-04-28 07:32:03 PDT
Comment on attachment 53369 [details]
Add EFL-specific code to Widget.h and Widget.cpp

and yet another change, as requested by Gustavo Barbieri on IRC
Comment 48 WebKit Commit Bot 2010-04-28 11:52:43 PDT
Comment on attachment 53369 [details]
Add EFL-specific code to Widget.h and Widget.cpp

Rejecting patch 53369 from commit-queue.

Unexpected failure when landing patch!  Please file a bug against webkit-patch.
Failed to run "['WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--build', '--test', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 53369, '--parent-command=commit-queue', '--no-update']" exit_code: 1
Last 500 characters of output:
date_status
    return NetworkTransaction().run(lambda: self._post_status_to_server(queue_name, status, patch, results_file))
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/common/net/networktransaction.py", line 56, in run
    self._check_for_timeout()
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/common/net/networktransaction.py", line 63, in _check_for_timeout
    raise NetworkTimeout()
webkitpy.common.net.networktransaction.NetworkTimeout
Comment 49 Gustavo Sverzut Barbieri 2010-04-28 11:58:01 PDT
(In reply to comment #48)
> (From update of attachment 53369 [details])
> Rejecting patch 53369 from commit-queue.

Again :~(

Could anyone from Apple offer help to check what fails in Mac build? We checked it here and seems nothing fails, the patch is rather simple.
Comment 50 Eric Seidel (no email) 2010-04-28 13:54:52 PDT
Ld /Users/eseidel/Projects/WebKit/WebKitBuild/Release/WebKit.framework/Versions/A/WebKit normal i386
    cd /Users/eseidel/Projects/WebKit/WebKit
    /Developer/usr/bin/g++-4.2 -arch i386 -dynamiclib -L/Users/eseidel/Projects/WebKit/WebKitBuild/Release -F/Users/eseidel/Projects/WebKit/WebKitBuild/Release -F/System/Library/Frameworks/WebKit.framework/Versions/A/Frameworks -F/System/Library/Frameworks/ApplicationServices.framework/Frameworks -F/System/Library/Frameworks/Carbon.framework/Frameworks -F/System/Library/Frameworks/Quartz.framework/Frameworks -F/System/Library/PrivateFrameworks -filelist /Users/eseidel/Projects/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/Objects-normal/i386/WebKit.LinkFileList -Wl,--no-demangle -exported_symbols_list mac/WebKit.exp -install_name /Users/eseidel/Projects/WebKit/WebKitBuild/Release/WebKit.framework/Versions/A/WebKit -mmacosx-version-min=10.5 -Wl,-dead_strip -sub_umbrella WebCore -lWebKitSystemInterfaceLeopard -framework Carbon -framework Cocoa -framework JavaScriptCore -licucore -framework OpenGL -framework QuartzCore -framework Security -framework WebCore -Wl,-single_module -compatibility_version 1 -current_version 533.7 -o /Users/eseidel/Projects/WebKit/WebKitBuild/Release/WebKit.framework/Versions/A/WebKit
Undefined symbols:
  "__ZN7WebCore6Widget17frameRectsChangedEv", referenced from:
      __ZTV20NetscapePluginWidget in WebFrameLoaderClient.o
ld: symbol(s) not found
collect2: ld returned 1 exit status
Comment 51 Gustavo Sverzut Barbieri 2010-04-28 14:35:35 PDT
Created attachment 54622 [details]
Add Widget.h and Widget.cpp changes to enable EFL

This is a new version that modifies WebCore.base.exp to export the new frameRectsChanged() so Mac builds will not fail anymore.
Comment 52 WebKit Commit Bot 2010-04-29 01:46:43 PDT
Comment on attachment 54622 [details]
Add Widget.h and Widget.cpp changes to enable EFL

Clearing flags on attachment: 54622

Committed r58487: <http://trac.webkit.org/changeset/58487>
Comment 53 WebKit Commit Bot 2010-04-29 01:46:53 PDT
All reviewed patches have been landed.  Closing bug.