RESOLVED FIXED 36317
[EFL] Add EFL-specific code to Widget.h
https://bugs.webkit.org/show_bug.cgi?id=36317
Summary [EFL] Add EFL-specific code to Widget.h
Leandro Pereira
Reported Thursday, March 18, 2010 8:36:06 PM UTC
Attached patch adds EFL-specific code to WebCore/platform/Widget.h.
Attachments
Add EFL-specific code to Widget.h (2.33 KB, patch)
2010-03-18 12:36 PDT, Leandro Pereira
no flags
Add EFL-specific code to Widget.h, with comments. (2.51 KB, patch)
2010-04-12 17:32 PDT, Gustavo Sverzut Barbieri
no flags
Add EFL-specific code to Widget.h. (2.80 KB, patch)
2010-04-14 07:11 PDT, Leandro Pereira
eric: review-
Add EFL-specific code to Widget.h. (3.29 KB, patch)
2010-04-14 13:47 PDT, Leandro Pereira
no flags
Add EFL-specific code to Widget.h and Widget.cpp (3.26 KB, patch)
2010-04-14 14:28 PDT, Leandro Pereira
no flags
The full log from the commit-queue (70.08 KB, text/plain)
2010-04-21 04:18 PDT, Eric Seidel (no email)
no flags
Add Widget.h and Widget.cpp changes to enable EFL (3.93 KB, patch)
2010-04-28 14:35 PDT, Gustavo Sverzut Barbieri
no flags
Leandro Pereira
Comment 1 Thursday, March 18, 2010 8:36:46 PM UTC
Created attachment 51069 [details] Add EFL-specific code to Widget.h
WebKit Review Bot
Comment 2 Thursday, March 18, 2010 8:41:25 PM UTC
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.
Kenneth Rohde Christiansen
Comment 3 Thursday, March 25, 2010 5:29:39 AM UTC
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?
Eric Seidel (no email)
Comment 4 Thursday, March 25, 2010 7:51:24 AM UTC
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
Gustavo Sverzut Barbieri
Comment 5 Thursday, March 25, 2010 6:55:32 PM UTC
(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.
Gustavo Noronha (kov)
Comment 6 Thursday, April 8, 2010 8:14:46 PM UTC
(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.
Gustavo Sverzut Barbieri
Comment 7 Thursday, April 8, 2010 8:30:26 PM UTC
(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.
Gustavo Noronha (kov)
Comment 8 Friday, April 9, 2010 7:03:29 PM UTC
(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.
Gustavo Sverzut Barbieri
Comment 9 Sunday, April 11, 2010 2:00:55 AM UTC
(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.
Gustavo Noronha (kov)
Comment 10 Monday, April 12, 2010 7:53:09 PM UTC
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.
Gustavo Sverzut Barbieri
Comment 11 Tuesday, April 13, 2010 1:32:42 AM UTC
Created attachment 53205 [details] Add EFL-specific code to Widget.h, with comments. Add comments as requested by Gustavo Noronha (kov).
WebKit Review Bot
Comment 12 Tuesday, April 13, 2010 1:37:08 AM UTC
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.
Gustavo Sverzut Barbieri
Comment 13 Tuesday, April 13, 2010 2:52:49 AM UTC
(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".
Dave Hyatt
Comment 14 Tuesday, April 13, 2010 8:20:49 PM UTC
Comment on attachment 53205 [details] Add EFL-specific code to Widget.h, with comments. r=me
WebKit Commit Bot
Comment 15 Tuesday, April 13, 2010 9:00:15 PM UTC
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
Eric Seidel (no email)
Comment 16 Tuesday, April 13, 2010 9:24:47 PM UTC
Something is wrong with teh commit-queue. Will ix.
Eric Seidel (no email)
Comment 17 Tuesday, April 13, 2010 10:17:19 PM UTC
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.
Chris Jerdonek
Comment 18 Tuesday, April 13, 2010 11:59:37 PM UTC
(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?
Chris Jerdonek
Comment 19 Wednesday, April 14, 2010 12:06:44 AM UTC
(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
Leandro Pereira
Comment 20 Wednesday, April 14, 2010 3:11:52 PM UTC
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.
Eric Seidel (no email)
Comment 21 Wednesday, April 14, 2010 5:09:04 PM UTC
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?
Eric Seidel (no email)
Comment 22 Wednesday, April 14, 2010 5:11:33 PM UTC
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.
Leandro Pereira
Comment 23 Wednesday, April 14, 2010 5:55:04 PM UTC
(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.
Eric Seidel (no email)
Comment 24 Wednesday, April 14, 2010 5:56:50 PM UTC
It delcares an empty virtual method, which could be overridden by a virtual method in a Widget subclass, no?
Eric Seidel (no email)
Comment 25 Wednesday, April 14, 2010 8:56:03 PM UTC
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;
Leandro Pereira
Comment 26 Wednesday, April 14, 2010 9:47:15 PM UTC
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.
Eric Seidel (no email)
Comment 27 Wednesday, April 14, 2010 9:50:37 PM UTC
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. :)
Leandro Pereira
Comment 28 Wednesday, April 14, 2010 10:28:35 PM UTC
Created attachment 53369 [details] Add EFL-specific code to Widget.h and Widget.cpp
Eric Seidel (no email)
Comment 29 Wednesday, April 14, 2010 10:30:43 PM UTC
Comment on attachment 53369 [details] Add EFL-specific code to Widget.h and Widget.cpp Thanks.
WebKit Commit Bot
Comment 30 Thursday, April 15, 2010 3:18:44 AM UTC
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
Gustavo Sverzut Barbieri
Comment 31 Thursday, April 15, 2010 5:59:36 AM UTC
(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
Eric Seidel (no email)
Comment 32 Thursday, April 15, 2010 6:36:56 AM UTC
Comment on attachment 53369 [details] Add EFL-specific code to Widget.h and Widget.cpp Trying again.
WebKit Commit Bot
Comment 33 Thursday, April 15, 2010 11:28:10 AM UTC
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
Eric Seidel (no email)
Comment 34 Thursday, April 15, 2010 11:32:54 AM UTC
I suspect this is attempting to post a huge log and timing out. I suspect it breaks mac somehow.
Leandro Pereira
Comment 35 Thursday, April 15, 2010 9:14:00 PM UTC
(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.
Eric Seidel (no email)
Comment 36 Monday, April 19, 2010 12:26:06 AM UTC
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.
WebKit Commit Bot
Comment 37 Monday, April 19, 2010 8:13:57 PM UTC
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.
WebKit Commit Bot
Comment 38 Tuesday, April 20, 2010 2:03:33 PM UTC
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
Gustavo Sverzut Barbieri
Comment 39 Tuesday, April 20, 2010 2:51:26 PM UTC
(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?
Eric Seidel (no email)
Comment 40 Tuesday, April 20, 2010 8:31:41 PM UTC
Restarted the cq. Jesus should be able to cq+ things now.
Eric Seidel (no email)
Comment 41 Tuesday, April 20, 2010 8:44:06 PM UTC
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.
Eric Seidel (no email)
Comment 42 Wednesday, April 21, 2010 12:18:53 PM UTC
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.
Gustavo Sverzut Barbieri
Comment 43 Thursday, April 22, 2010 2:25:25 PM UTC
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?
Nikolas Zimmermann
Comment 44 Thursday, April 22, 2010 2:31:47 PM UTC
(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.
Leandro Pereira
Comment 45 Monday, April 26, 2010 6:18:26 PM UTC
Any updates as to why this patch is failing to be applied?
Eric Seidel (no email)
Comment 46 Monday, April 26, 2010 7:02:30 PM UTC
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.
Antonio Gomes
Comment 47 Wednesday, April 28, 2010 3:32:03 PM UTC
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
WebKit Commit Bot
Comment 48 Wednesday, April 28, 2010 7:52:43 PM UTC
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
Gustavo Sverzut Barbieri
Comment 49 Wednesday, April 28, 2010 7:58:01 PM UTC
(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.
Eric Seidel (no email)
Comment 50 Wednesday, April 28, 2010 9:54:52 PM UTC
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
Gustavo Sverzut Barbieri
Comment 51 Wednesday, April 28, 2010 10:35:35 PM UTC
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.
WebKit Commit Bot
Comment 52 Thursday, April 29, 2010 9:46:43 AM UTC
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>
WebKit Commit Bot
Comment 53 Thursday, April 29, 2010 9:46:53 AM UTC
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.