Bug 70231

Created attachment 111253 [details] Patch

Attachment 111253 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2 Updating OpenSource Current branch master is up to date. Updating chromium port dependencies using gclient... Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again. Re-trying 'depot_tools/gclient sync' Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again. Re-trying 'depot_tools/gclient sync' Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again. Error: 'depot_tools/gclient sync' failed 3 tries and returned 256 at Tools/Scripts/update-webkit-chromium line 107. Re-trying 'depot_tools/gclient sync' No such file or directory at Tools/Scripts/update-webkit line 104. If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 111255 [details] try again to resolve style error

Attachment 111255 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2 Updating OpenSource From git://git.webkit.org/WebKit f208d85..3d928fb master -> origin/master A LayoutTests/platform/qt/fast/events/reveal-link-when-focused-expected.png A LayoutTests/platform/qt/fast/events/reveal-link-when-focused-expected.txt M LayoutTests/platform/qt/Skipped M LayoutTests/ChangeLog r97616 = 5014b6ef7e1669f4e7dfbfdb361c2ddba22542ae (refs/remotes/trunk) M LayoutTests/platform/qt/fast/selectors/061-expected.txt M LayoutTests/platform/qt/fast/selectors/017-expected.txt M LayoutTests/platform/qt/fast/selectors/061-expected.png M LayoutTests/platform/qt/fast/selectors/017-expected.png M LayoutTests/platform/qt/css3/selectors3/xml/css3-modsel-61-expected.png M LayoutTests/platform/qt/css3/selectors3/xml/css3-modsel-17-expected.png M LayoutTests/platform/qt/css3/selectors3/xml/css3-modsel-61-expected.txt M LayoutTests/platform/qt/css3/selectors3/xml/css3-modsel-17-expected.txt M LayoutTests/platform/qt/css3/selectors3/html/css3-modsel-61-expected.png M LayoutTests/platform/qt/css3/selectors3/html/css3-modsel-61-expected.txt M LayoutTests/platform/qt/css3/selectors3/html/css3-modsel-17-expected.png M LayoutTests/platform/qt/css3/selectors3/html/css3-modsel-17-expected.txt M LayoutTests/platform/qt/css3/selectors3/xhtml/css3-modsel-61-expected.png M LayoutTests/platform/qt/css3/selectors3/xhtml/css3-modsel-17-expected.png M LayoutTests/platform/qt/css3/selectors3/xhtml/css3-modsel-61-expected.txt M LayoutTests/platform/qt/css3/selectors3/xhtml/css3-modsel-17-expected.txt M LayoutTests/ChangeLog r97617 = 53da0d4c5265e8aa510edeab6fe3cb057e5d047b (refs/remotes/trunk) M Source/JavaScriptCore/runtime/Executable.cpp M Source/JavaScriptCore/ChangeLog r97618 = 3d928fb1a74828744568746427e2acc309df63ea (refs/remotes/trunk) First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/trunk. Updating chromium port dependencies using gclient... Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again. Re-trying 'depot_tools/gclient sync' Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again. Re-trying 'depot_tools/gclient sync' Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again. Error: 'depot_tools/gclient sync' failed 3 tries and returned 256 at Tools/Scripts/update-webkit-chromium line 107. Re-trying 'depot_tools/gclient sync' No such file or directory at Tools/Scripts/update-webkit line 104. If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 111255 [details] try again to resolve style error View in context: https://bugs.webkit.org/attachment.cgi?id=111255&action=review From the ChangeLog, it is not clear to me what these files are supposed to do, and it looks like they have not been integrated into the buildsystem, nor are they called by other code. > Source/WebKit2/Shared/efl/WebCoreArgumentCodersEfl.cpp:64 > + Extra empty line. > Source/WebKit2/Shared/efl/WebCoreArgumentCodersEfl.cpp:83 > + Ditto.

Created attachment 111384 [details] modify ChangeLog, and fix style error

Created attachment 111575 [details] not to return NULL on WKURLRequestEflCopyCookies() to following after Core Foundation

(In reply to comment #5) > (From update of attachment 111255 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=111255&action=review > > From the ChangeLog, it is not clear to me what these files are supposed to do, and it looks like they have not been integrated into the buildsystem, nor are they called by other code. This has not been answered yet.

Hi, thank you for the review. I mentioned about it on ChangeLog. Sorry for the confusion. If you think it is not sufficient, please let me know. Thank you.

Comment on attachment 111575 [details] not to return NULL on WKURLRequestEflCopyCookies() to following after Core Foundation View in context: https://bugs.webkit.org/attachment.cgi?id=111575&action=review > Source/WebKit2/ChangeLog:6 > + These codes are only for EFL port, and will be imported to buildsystem when metabug for EFL port(61838) is complete. I don't understand what above meaning. what is buildsyatem ? Do you mean EFL-WebKit2 build system? If it is so, I think this description is unneeded.

(In reply to comment #10) > (From update of attachment 111575 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=111575&action=review > > > Source/WebKit2/ChangeLog:6 > > + These codes are only for EFL port, and will be imported to buildsystem when metabug for EFL port(61838) is complete. > > I don't understand what above meaning. what is buildsyatem ? Do you mean EFL-WebKit2 build system? If it is so, I think this description is unneeded. O.K. I'll delete the statement from ChangeLog. Thank you.

Created attachment 111736 [details] delete unused line from ChangeLog

Please check licence in files. Why there is APPLE INC?

(In reply to comment #13) > Please check licence in files. Why there is APPLE INC? Do you mean "Copyright (C) 2010 Apple Inc. All rights reserved." on WebCoreArgumentCodersEfl.cpp? That is because the file format is from WebCoreArgumentCoders.cpp. The file has Apple license. So, I thought the original license should be added. By the way, another files has only Samsung license because they has no original file to refer. If you think it is not necessary, please let me know. Thank you.

The licence that you have used in files has two small issues: - THIS SOFTWARE IS PROVIDED BY APPLE INC - IN NO EVENT SHALL APPLE INC. Correct should be: * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS'' * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF * THE POSSIBILITY OF SUCH DAMAGE.

(In reply to comment #15) > The licence that you have used in files has two small issues: > - THIS SOFTWARE IS PROVIDED BY APPLE INC > - IN NO EVENT SHALL APPLE INC. > > Correct should be: > * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS'' > * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, > * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR > * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS > * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS > * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN > * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) > * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF > * THE POSSIBILITY OF SUCH DAMAGE. Hi, It is BSD license, which is posted on "http://trac.webkit.org/browser/trunk/Source/WebKit/LICENSE". I don't think I can modify that. Thank you.

CC'ing Martin and Xan.

*** Bug 75242 has been marked as a duplicate of this bug. ***

Created attachment 138557 [details] Patch

Sorry for this late patch uploading. I'm starting this bug again. I changed license and did 'git rebase'. Thank you.

(In reply to comment #8) > (In reply to comment #5) > > (From update of attachment 111255 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=111255&action=review > > > > From the ChangeLog, it is not clear to me what these files are supposed to do, and it looks like they have not been integrated into the buildsystem, nor are they called by other code. > > This has not been answered yet. After updated, is it mentioned enough? Patch itself looks fine to me.

AFAIK, EFL WK2 doesn't support layout test yet. So, it is a little difficult to check whether this patch doesn't make any problems during the layout test. So, informal rs+ on my side for now.

Comment on attachment 138557 [details] Patch The patch looks good but I hope EFL port would have WK2 layout test support asap.

(In reply to comment #23) > (From update of attachment 138557 [details]) > The patch looks good but I hope EFL port would have WK2 layout test support asap. Thank you for your review. I think EFL WK2 will run WK2 layout test within 2 months.

Comment on attachment 138557 [details] Patch Clearing flags on attachment: 138557 Committed r119092: <http://trac.webkit.org/changeset/119092>

All reviewed patches have been landed. Closing bug.