WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Add EFL-specific code to Widget.h, with comments.
(2.51 KB, patch)
2010-04-12 17:32 PDT
,
Gustavo Sverzut Barbieri
no flags
Details
Formatted Diff
Diff
Add EFL-specific code to Widget.h.
(2.80 KB, patch)
2010-04-14 07:11 PDT
,
Leandro Pereira
eric
: review-
Details
Formatted Diff
Diff
Add EFL-specific code to Widget.h.
(3.29 KB, patch)
2010-04-14 13:47 PDT
,
Leandro Pereira
no flags
Details
Formatted Diff
Diff
Add EFL-specific code to Widget.h and Widget.cpp
(3.26 KB, patch)
2010-04-14 14:28 PDT
,
Leandro Pereira
no flags
Details
Formatted Diff
Diff
The full log from the commit-queue
(70.08 KB, text/plain)
2010-04-21 04:18 PDT
,
Eric Seidel (no email)
no flags
Details
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
Details
Formatted Diff
Diff
Show Obsolete
(5)
View All
Add attachment
proposed patch, testcase, etc.
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.
Top of Page
Format For Printing
XML
Clone This Bug