Bug 122771

Created attachment 214732 [details] WIP Patch for EWS testing Patch to check WIN build

Attachment 214732 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/dom/HTMLScriptElement/nested-execution.html', u'LayoutTests/storage/domstorage/localstorage/close-idle-localstorage-databases-immediately.html', u'LayoutTests/storage/domstorage/storage-close-database-on-idle.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/testing/Internals.cpp', u'Source/WebCore/testing/Internals.h', u'Source/WebCore/testing/Internals.idl', u'Source/WebKit/gtk/ChangeLog', u'Source/WebKit/gtk/webkit/webkitglobals.cpp', u'Source/WebKit/gtk/webkit/webkitglobals.h', u'Tools/ChangeLog', u'Tools/DumpRenderTree/TestRunner.cpp', u'Tools/DumpRenderTree/TestRunner.h', u'Tools/DumpRenderTree/blackberry/TestRunnerBlackBerry.cpp', u'Tools/DumpRenderTree/efl/TestRunnerEfl.cpp', u'Tools/DumpRenderTree/gtk/TestRunnerGtk.cpp', u'Tools/DumpRenderTree/mac/TestRunnerMac.mm', u'Tools/DumpRenderTree/win/TestRunnerWin.cpp']" exit_code: 1 LayoutTests/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Tools/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Source/WebCore/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Source/WebKit/gtk/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Source/WebCore/testing/Internals.cpp:2095: Omit int when using unsigned [runtime/unsigned] [1] Source/WebCore/testing/Internals.cpp:2096: Omit int when using unsigned [runtime/unsigned] [1] Source/WebCore/testing/Internals.cpp:2097: Omit int when using unsigned [runtime/unsigned] [1] Source/WebCore/testing/Internals.cpp:2099: Omit int when using unsigned [runtime/unsigned] [1] Total errors found: 8 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 214732 [details] WIP Patch for EWS testing Attachment 214732 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/8858037

Cache model is a WebKit concept, not a WebCore one. I don't think that it makes sense to "test" it in WebCore.

(In reply to comment #4) > Cache model is a WebKit concept, not a WebCore one. I don't think that it makes sense to "test" it in WebCore. Hi, I see, but don't you think that it could be be considered to move to internals since only gtk port implements this feature? if not, could you explain better this issue?

Why are you saying that only Gtk implements this? -[WebView setCacheModel:] is part of Mac WebKit1 API, and this is what's exposed through TestRunner. Looking at the patch, it changes Efl too.

Comment on attachment 214732 [details] WIP Patch for EWS testing Attachment 214732 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/8878166

(In reply to comment #6) > Why are you saying that only Gtk implements this? -[WebView setCacheModel:] is part of Mac WebKit1 API, and this is what's exposed through TestRunner. Looking at the patch, it changes Efl too. Hi Alexey, Yes you are right, but what I'd like to say is, besides Mac port others ports has implementation to this method, for example, Gtk and EFL as well, so according with http://trac.webkit.org/wiki/Internals_Hackathon I thought that this method could be considered to move to Internals (just to reduce duplicated code), as I did in this other issue here https://bugs.webkit.org/show_bug.cgi?id=84398. Although, as you said before, cache model is a webkit concept and not a webcore concept, then my question is, which methods could be considered to move to Internals according the Internals Hackathon? Thanks in advance.

One way to look at this is that we considered doing so in this bug, and decided against doing so. If you agree, you can just remove the suggestion from the wiki page, referencing this bug in a history comment. There is no hard rule - as the obvious things were already moved to internals, moving anything else probably reduces test coverage on platforms that expose the functionality as WebKit API, but at the same time it also reduced code duplication between ports. In a case like this, where it's conceptually a WebKit feature anyway, I think that it's better to keep going through API where one exists.

(In reply to comment #9) > One way to look at this is that we considered doing so in this bug, and decided against doing so. If you agree, you can just remove the suggestion from the wiki page, referencing this bug in a history comment. Agreed, I can update the wiki and refer this bug there, by the way, could you help me in update the wiki with bugs related to Internals, for example this bug https://bugs.webkit.org/show_bug.cgi?id=84395 do you think it is valid to move to internals? > There is no hard rule - as the obvious things were already moved to internals, moving anything else probably reduces test coverage on platforms that expose the functionality as WebKit API, but at the same time it also reduced code duplication between ports. In a case like this, where it's conceptually a WebKit feature anyway, I think that it's better to keep going through API where one exists. Ok, I'll mark this bug as won't fix. Thanks in advance