Chromium Mac: Use a custom pattern image for rubber banding overhang area
Created attachment 104728 [details] Patch
Created attachment 104731 [details] Patch
Attachment 104731 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2 Last 3072 characters of output: rements: ['./'] name: third_party/ffmpeg url: From('chromium_deps', 'src/third_party/ffmpeg') parsed_url: http://src.chromium.org/svn/trunk/deps/third_party/ffmpeg/source@97428 should_process: True processed: True requirements: ['./', 'chromium_deps', 'third_party'] name: tools/generate_stubs url: http://src.chromium.org/svn/trunk/src/tools/generate_stubs@97698 parsed_url: http://src.chromium.org/svn/trunk/src/tools/generate_stubs@97698 should_process: True processed: True requirements: ['./'] name: third_party/libjpeg_turbo url: From('chromium_deps', 'src/third_party/libjpeg_turbo') parsed_url: http://src.chromium.org/svn/trunk/deps/third_party/libjpeg_turbo@95800 should_process: True processed: True requirements: ['./', 'chromium_deps', 'third_party'] name: third_party/v8-i18n url: From('chromium_deps', 'src/third_party/v8-i18n') parsed_url: http://v8-i18n.googlecode.com/svn/trunk@4 should_process: True processed: True requirements: ['./', 'chromium_deps', 'third_party'] name: tools/grit url: http://src.chromium.org/svn/trunk/src/tools/grit@97698 parsed_url: http://src.chromium.org/svn/trunk/src/tools/grit@97698 should_process: True processed: True requirements: ['./'] name: base url: http://src.chromium.org/svn/trunk/src/base@97698 parsed_url: http://src.chromium.org/svn/trunk/src/base@97698 should_process: True processed: True requirements: ['./'] name: third_party/leveldb url: From('chromium_deps', 'src/third_party/leveldb') should_process: True requirements: ['./', 'chromium_deps', 'third_party'] name: sql url: http://src.chromium.org/svn/trunk/src/sql@97698 should_process: True requirements: ['./'] name: v8 url: From('chromium_deps', 'src/v8') should_process: True requirements: ['./', 'chromium_deps'] name: testing/gtest url: From('chromium_deps', 'src/testing/gtest') should_process: True requirements: ['./', 'chromium_deps', 'testing'] name: third_party/libwebp url: http://src.chromium.org/svn/trunk/src/third_party/libwebp@97698 should_process: True requirements: ['./', 'third_party'] name: googleurl url: From('chromium_deps', 'src/googleurl') should_process: True requirements: ['./', 'chromium_deps'] name: third_party/skia/include url: From('chromium_deps', 'src/third_party/skia/include') should_process: True requirements: ['./', 'chromium_deps', 'third_party'] name: third_party/ots url: From('chromium_deps', 'src/third_party/ots') should_process: True requirements: ['./', 'chromium_deps', 'third_party'] name: third_party/snappy/src url: From('chromium_deps', 'src/third_party/snappy/src') should_process: True requirements: ['./', 'chromium_deps', 'third_party'] Died at Tools/Scripts/update-webkit-chromium line 69. No such file or directory at Tools/Scripts/update-webkit line 100. If any of these errors are false positives, please file a bug against check-webkit-style.
You'll probably want to add a webkit reviewer. The changes look good to me.
Note: This depends on http://codereview.chromium.org/7711007/
Comment on attachment 104731 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104731&action=review LGTM > Source/WebCore/platform/chromium/ScrollbarThemeChromiumMac.mm:198 > + Image* patternImage = Image::loadPlatformResource("overhangPattern").releaseRef(); releaseRef() documentation says " // FIXME: Remove releaseRef once we change all callers to call leakRef instead.", so do you want to call leakRef() here? > Source/WebCore/platform/chromium/ScrollbarThemeChromiumMac.mm:199 > + m_overhangPattern = Pattern::create(patternImage, true, true); Looks like create() takes a PassRefPtr, so maybe you don't even need the leakRef() call. I'm not sure if http://www.webkit.org/coding/RefPtr.html recommends against that or not.
Created attachment 104843 [details] Patch
(In reply to comment #7) > Created an attachment (id=104843) [details] > Patch I've addressed Nico's concerns from comment 6.
The Chromium revision on which this depends is now WebKit's DEPS, so this is ready to go in once approved. Dmitri, can you review/approve the patch, please?
Comment on attachment 104843 [details] Patch Clearing flags on attachment: 104843 Committed r93645: <http://trac.webkit.org/changeset/93645>
All reviewed patches have been landed. Closing bug.
This broke webkit_unit_tests on mac: http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Mac10.6%20%28CG%29%28dbg%29/builds/174/steps/webkit_unit_tests/logs/stdio
(In reply to comment #12) > This broke webkit_unit_tests on mac: > > http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Mac10.6%20%28CG%29%28dbg%29/builds/174/steps/webkit_unit_tests/logs/stdio Ok, I ain't rubberstampin' these no more.
Reopening, reverted in http://trac.webkit.org/changeset/93676
Seems resource loading takes a different path when running unit tests. This patch now depends on the following Chromium CL: http://codereview.chromium.org/7714036/
Created attachment 105212 [details] Patch
I've updated the patch to include DEPS change to roll the Chromium revision forward to 98101 so that it included the needed Chromium change.
Attachment 105212 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/ChangeLog:6: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 105219 [details] Patch
I've confirmed that webkit_unit_tests pass with this patch and that there is no difference in layout tests before and after this patch.
Comment on attachment 105219 [details] Patch oooh, look, donut!
Comment on attachment 105219 [details] Patch Clearing flags on attachment: 105219 Committed r93879: <http://trac.webkit.org/changeset/93879>