Bug 70021

Created attachment 110839 [details] Patch

Attachment 110839 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/css/CSSRegionStyleRule.cpp:81: Should have a space between // and comment [whitespace/comments] [4] Total errors found: 1 in 19 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 110839 [details] Patch Attachment 110839 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10052212

Comment on attachment 110839 [details] Patch Attachment 110839 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10054222

Comment on attachment 110839 [details] Patch Attachment 110839 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/10062202

Created attachment 110849 [details] Patch 2

Comment on attachment 110849 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=110849&action=review Looks good. Some minor nits. > Source/WebCore/css/CSSGrammar.y:784 > + WEBKIT_REGION_STYLE_RULE_SYM WHITESPACE simple_selector_list maybe_space '{' maybe_space block_rule_list save_block { Why limit to simple selectors? Don't you want selector_list instead? I don't think there's any reason to limit to simple selectors here. > Source/WebCore/css/CSSRegionStyleRule.cpp:40 > + , m_lstCSSRules(rules) I'd prefer this be m_ruleList. > Source/WebCore/css/CSSRegionStyleRule.cpp:42 > + for (unsigned idx = 0; idx < m_lstCSSRules->length(); ++idx) You probably copied this from other crufty old CSS code, but fix the variable name to just be "index" > Source/WebCore/css/CSSRegionStyleRule.cpp:51 > + for (unsigned idx = 0; idx < m_lstCSSRules->length(); ++idx) Same here. > Source/WebCore/css/CSSRegionStyleRule.cpp:64 > + // First add the selectors. > + for (CSSSelector* s = m_selectorList.first(); s; s = CSSSelectorList::next(s)) { > + if (s != m_selectorList.first()) > + result += ", "; > + result += s->selectorText(); > + } I'd add a selectorText() helper to CSSSelectorList and put this code there. > Source/WebCore/css/CSSRegionStyleRule.cpp:70 > + for (unsigned idx = 0; idx < m_lstCSSRules->length(); ++idx) { "index"

Created attachment 111018 [details] Patch 3

Comment on attachment 111018 [details] Patch 3 According to the last comment from Hyatt, this should be good. So I'll r+ this within a few days if nobody comes.

Created attachment 111256 [details] Patch 4 Reworked patch to take into account the latest changes into stylesheet area.

Attachment 111256 [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 4f1dd53..293b636 master -> origin/master M LayoutTests/inspector/profiler/heap-snapshot-test.js M LayoutTests/inspector/profiler/heap-snapshot-expected.txt M LayoutTests/inspector/profiler/heap-snapshot.html M LayoutTests/ChangeLog M Source/WebCore/ChangeLog M Source/WebCore/bindings/js/ScriptProfiler.h M Source/WebCore/bindings/js/ScriptProfiler.cpp M Source/WebCore/bindings/v8/ScriptProfiler.h M Source/WebCore/bindings/v8/ScriptProfiler.cpp M Source/WebCore/inspector/InspectorController.cpp M Source/WebCore/inspector/InspectorProfilerAgent.cpp M Source/WebCore/inspector/front-end/HeapSnapshotProxy.js M Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js M Source/WebCore/inspector/front-end/RemoteObject.js M Source/WebCore/inspector/front-end/heapProfiler.css M Source/WebCore/inspector/front-end/DetailedHeapshotView.js M Source/WebCore/inspector/front-end/HeapSnapshot.js M Source/WebCore/inspector/InspectorProfilerAgent.h M Source/WebCore/inspector/Inspector.json M Source/WebCore/English.lproj/localizedStrings.js r97611 = 90b24e894ca5e0e4163364f1d34546bb19261dc1 (refs/remotes/trunk) M WebKitLibraries/ChangeLog M WebKitLibraries/win/tools/vsprops/common.vsprops r97612 = 546adcace757980dbbf6edef33b728cb592d10c0 (refs/remotes/trunk) M Source/WebKit/chromium/ChangeLog M Source/WebKit/chromium/src/js/Tests.js r97613 = 293b63602c28878237fa90e06017499037cd2727 (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.

(In reply to comment #11) > Attachment 111256 [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 > 4f1dd53..293b636 master -> origin/master > M LayoutTests/inspector/profiler/heap-snapshot-test.js > M LayoutTests/inspector/profiler/heap-snapshot-expected.txt > M LayoutTests/inspector/profiler/heap-snapshot.html > M LayoutTests/ChangeLog > M Source/WebCore/ChangeLog > M Source/WebCore/bindings/js/ScriptProfiler.h > M Source/WebCore/bindings/js/ScriptProfiler.cpp > M Source/WebCore/bindings/v8/ScriptProfiler.h > M Source/WebCore/bindings/v8/ScriptProfiler.cpp > M Source/WebCore/inspector/InspectorController.cpp > M Source/WebCore/inspector/InspectorProfilerAgent.cpp > M Source/WebCore/inspector/front-end/HeapSnapshotProxy.js > M Source/WebCore/inspector/front-end/DetailedHeapshotGridNodes.js > M Source/WebCore/inspector/front-end/RemoteObject.js > M Source/WebCore/inspector/front-end/heapProfiler.css > M Source/WebCore/inspector/front-end/DetailedHeapshotView.js > M Source/WebCore/inspector/front-end/HeapSnapshot.js > M Source/WebCore/inspector/InspectorProfilerAgent.h > M Source/WebCore/inspector/Inspector.json > M Source/WebCore/English.lproj/localizedStrings.js > r97611 = 90b24e894ca5e0e4163364f1d34546bb19261dc1 (refs/remotes/trunk) > M WebKitLibraries/ChangeLog > M WebKitLibraries/win/tools/vsprops/common.vsprops > r97612 = 546adcace757980dbbf6edef33b728cb592d10c0 (refs/remotes/trunk) > M Source/WebKit/chromium/ChangeLog > M Source/WebKit/chromium/src/js/Tests.js > r97613 = 293b63602c28878237fa90e06017499037cd2727 (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. I am confused by this message :)

Comment on attachment 111256 [details] Patch 4 r=me

Comment on attachment 111256 [details] Patch 4 Rejecting attachment 111256 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: bCore/css/CSSSelectorList.cpp patching file Source/WebCore/css/CSSSelectorList.h patching file Source/WebCore/css/CSSStyleSelector.cpp Hunk #2 succeeded at 441 (offset 2 lines). Hunk #3 succeeded at 1989 (offset -46 lines). patching file Source/WebCore/css/CSSStyleSelector.h patching file Source/WebCore/css/StyleBase.h patching file Source/WebCore/css/tokenizer.flex Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'David Hyatt', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/10125041

Created attachment 111415 [details] Patch for landing

Comment on attachment 111415 [details] Patch for landing Clearing flags on attachment: 111415 Committed r97738: <http://trac.webkit.org/changeset/97738>

All reviewed patches have been landed. Closing bug.