Bug 103745

Created attachment 194668 [details] Patch

Comment on attachment 194668 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194668&action=review > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:554 > + evas_object_show(sd->image); think this snippet could be a static function accepting 'sd' > Source/WebKit2/UIProcess/API/efl/EwkView.h:165 > + bool isAccelerated() { return m_isAccelerated; } We actually do not need this getter, m_isAccelerated is still accessed within the class only

Comment on attachment 194668 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194668&action=review >> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:554 >> + evas_object_show(sd->image); > > think this snippet could be a static function accepting 'sd' Do you mean the above 3 lines of code could be a staic function? If so, it could be only used 2 times. >> Source/WebKit2/UIProcess/API/efl/EwkView.h:165 >> + bool isAccelerated() { return m_isAccelerated; } > > We actually do not need this getter, m_isAccelerated is still accessed within the class only Okay, I will fix this.

(In reply to comment #3) > (From update of attachment 194668 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=194668&action=review > > >> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:554 > >> + evas_object_show(sd->image); > > > > think this snippet could be a static function accepting 'sd' > > Do you mean the above 3 lines of code could be a staic function? If so, it could be only used 2 times. yeah, I mean those 3 lines. Think it will improve code-readability.

Created attachment 195338 [details] Patch

Comment on attachment 195338 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195338&action=review > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:561 > + // Make Evas objects visible here in order not to paint empty Evas objects with black color. > + showEvasObjects(sd); Isn't it more a showEvasObjectsIfNeeded(), I suppose those methods does nothing if already visible...

(In reply to comment #6) > (From update of attachment 195338 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=195338&action=review > > > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:561 > > + // Make Evas objects visible here in order not to paint empty Evas objects with black color. > > + showEvasObjects(sd); > > Isn't it more a showEvasObjectsIfNeeded(), I suppose those methods does nothing if already visible... showEvasObjectsIfNeeded makes more sense. I will update the patch soon.

Created attachment 195372 [details] Patch

You need an owner to sign this one off

Comment on attachment 195372 [details] Patch I sign off on this for WebKit2.

Comment on attachment 195372 [details] Patch Rejecting attachment 195372 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 195372, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: ource/WebKit2/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebKit2/UIProcess/API/efl/EwkView.cpp Hunk #1 succeeded at 130 (offset 2 lines). Hunk #2 FAILED at 559. Hunk #3 succeeded at 1133 (offset -14 lines). 1 out of 3 hunks FAILED -- saving rejects to file Source/WebKit2/UIProcess/API/efl/EwkView.cpp.rej Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Kenneth Rohde Christiansen']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: http://webkit-queues.appspot.com/results/32149

Created attachment 198601 [details] Patch

(In reply to comment #12) > Created an attachment (id=198601) [details] > Patch Patch rebased on master

Comment on attachment 198601 [details] Patch Clearing flags on attachment: 198601 Committed r148633: <http://trac.webkit.org/changeset/148633>