Bug 64220 - Scrollbar color heuristic needs to be hooked up in WebKit1
: Scrollbar color heuristic needs to be hooked up in WebKit1
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: WebCore Misc.
: 528+ (Nightly build)
: Macintosh Unspecified
: P2 Normal
Assigned To: Tim Horton
: InRadar
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-08 17:24 PDT by Tim Horton
Modified: 2011-07-21 12:40 PDT (History)
10 users (show)

See Also:


Attachments
Patch (1.82 KB, patch)
2011-07-08 17:46 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
totally new patch, utilizing Darin's suggestions (22.88 KB, patch)
2011-07-18 15:03 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
same as last + radar URL (22.98 KB, patch)
2011-07-18 15:05 PDT, Tim Horton
gyuyoung.kim: commit‑queue-
Details | Formatted Diff | Diff
this time, without breaking all of the other platforms! (23.21 KB, patch)
2011-07-18 15:40 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
work around <rdar://problem/9797253> (24.77 KB, patch)
2011-07-18 17:34 PDT, Tim Horton
darin: review+
webkit.review.bot: commit‑queue-
Details | Formatted Diff | Diff
remove updateStyleIfNeeded() (24.66 KB, patch)
2011-07-19 10:40 PDT, Tim Horton
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2011-07-08 17:24:43 PDT
We don't currently set the color of the scrollbar based on the content for WebKit1 views.

<rdar://problem/9589140>

Patch forthcoming.
Comment 1 Tim Horton 2011-07-08 17:46:48 PDT
Created attachment 100192 [details]
Patch

Not commit-worthy yet, I don't think that putting this in repaint is the right place, though it's not clear there's a better place.
Comment 2 Darin Adler 2011-07-11 17:02:55 PDT
Comment on attachment 100192 [details]
Patch

Looking back at the implementation of this that is already checked in:

I was surprised to see that in ScrollbarThemeMac::paint, the knob style is recomputed each time we draw a scrollbar. That is not a good design. Even if setting the style on the painter each time was efficient enough, it’s not conceptually OK to simply draw the scrollbar in a different style without making sure we repaint existing visible scrollbars when the style changes. Generally speaking such things are set up as part of style computation and layout.

It’s also not good that we added a getDocumentBackgroundColor function to Frame. This is not the right class for a function like that. For one thing, we should not be adding new functions to the Frame class at this time since it is just the center of a set of related classes. It is probably reasonable to put this function in the FrameView class.

The name of the getDocumentBackgroundColor function, starting with get, is correct style inside WebKit2, but not correct WebCore. The get prefix should have been removed.

The getDocumentBackgroundColor function gets at renderers without triggering style recalculation first. This is not correct unless the caller somehow guarantees rendering is up to date. Normally a function like this needs to make sure style is calculated before looking at renderers or must only be called when style is guaranteed to be up to date.

After talking to Maciej we agree that the right design for this is to store the scrollbar overlay style in the ScrollableArea. If the scrollbar overlay style changes, the scrollable area then schedules a scrollbar repaint. In the case of a scrollable area that is implemented with ScrollView, it would call a function from the ScrollView class.

We’d also need code to schedule a scrollbar overlay style recalculation that works either immediately or based on a timer. That would be called by any code that could change the background color. This would need to be done any time the color of the document element or the body element changes, presumably in RenderBox::styleDidChange, or any time the view's base background color changes, which would be in FrameView::setBaseBackgroundColor.
Comment 3 Tim Horton 2011-07-15 11:31:22 PDT
(In reply to comment #2)
Ok, I'm a lot more familiar with the scrolling code after the last two days, so I'll take a crack at this and see what happens. Thanks, Darin!
Comment 4 Tim Horton 2011-07-18 15:03:48 PDT
Created attachment 101212 [details]
totally new patch, utilizing Darin's suggestions
Comment 5 Tim Horton 2011-07-18 15:05:59 PDT
Created attachment 101213 [details]
same as last + radar URL
Comment 6 Gyuyoung Kim 2011-07-18 15:22:06 PDT
Comment on attachment 101213 [details]
same as last + radar URL

Attachment 101213 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9108002
Comment 7 Early Warning System Bot 2011-07-18 15:32:52 PDT
Comment on attachment 101213 [details]
same as last + radar URL

Attachment 101213 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9117373
Comment 8 Gustavo Noronha (kov) 2011-07-18 15:37:00 PDT
Comment on attachment 101213 [details]
same as last + radar URL

Attachment 101213 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9100022
Comment 9 Tim Horton 2011-07-18 15:40:03 PDT
Created attachment 101224 [details]
this time, without breaking all of the other platforms!
Comment 10 Tim Horton 2011-07-18 16:45:10 PDT
Interesting. Works great for WK1, but for WK2, somewhere between setting the correct overlay style (I've added an updateScrollbarOverlayStyle call to registerScrollbar and setNewPainterForScrollbar) and going to paint, the NSScrollerImp is losing its style (getting reset to default). Will continue investigating (if all else fails, we could just set the cached style during every paint(); we still get 99% of the benefit).
Comment 11 Darin Adler 2011-07-18 16:49:28 PDT
Comment on attachment 101224 [details]
this time, without breaking all of the other platforms!

View in context: https://bugs.webkit.org/attachment.cgi?id=101224&action=review

> Source/WebCore/page/FrameView.cpp:333
> +    Color bgColor = documentBackgroundColor();

I’d suggest not abbreviated background to “bg”.

> Source/WebCore/page/FrameView.cpp:2390
> +    // start as invalid colors

I’d suggest changing this comment to sentence style:

    // Start with invalid colors.

> Source/WebCore/platform/ScrollView.cpp:790
> +    if (horizontalScrollbar())
> +        ScrollbarTheme::nativeTheme()->updateScrollbarOverlayStyle(horizontalScrollbar());
> +    if (verticalScrollbar())
> +        ScrollbarTheme::nativeTheme()->updateScrollbarOverlayStyle(verticalScrollbar());

Why is this code here instead of in the ScrollableArea base class?

> Source/WebCore/platform/mac/ScrollbarThemeMac.mm:239
> +    switch (style) {
> +        case ScrollbarOverlayStyleDark:
> +            return wkScrollerKnobStyleDark;
> +        case ScrollbarOverlayStyleLight:
> +            return wkScrollerKnobStyleLight;
> +        default:
> +            return wkScrollerKnobStyleDefault;
> +    }

The old indenting style is the correct style for WebKit, believe it or not.
Comment 12 Tim Horton 2011-07-18 16:54:49 PDT
Comment on attachment 101224 [details]
this time, without breaking all of the other platforms!

View in context: https://bugs.webkit.org/attachment.cgi?id=101224&action=review

>> Source/WebCore/page/FrameView.cpp:333
>> +    Color bgColor = documentBackgroundColor();
> 
> I’d suggest not abbreviated background to “bg”.

Ok.

>> Source/WebCore/page/FrameView.cpp:2390
>> +    // start as invalid colors
> 
> I’d suggest changing this comment to sentence style:
> 
>     // Start with invalid colors.

Ok, sure!

>> Source/WebCore/platform/ScrollView.cpp:790
>> +        ScrollbarTheme::nativeTheme()->updateScrollbarOverlayStyle(verticalScrollbar());
> 
> Why is this code here instead of in the ScrollableArea base class?

Fixed.

>> Source/WebCore/platform/mac/ScrollbarThemeMac.mm:239
>> +    }
> 
> The old indenting style is the correct style for WebKit, believe it or not.

That's odd; does Xcode reindent code when you copy/paste it around?
Comment 13 Tim Horton 2011-07-18 17:34:33 PDT
Created attachment 101242 [details]
work around <rdar://problem/9797253>
Comment 14 WebKit Review Bot 2011-07-18 18:12:45 PDT
Comment on attachment 101242 [details]
work around <rdar://problem/9797253>

Attachment 101242 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9112503

New failing tests:
fast/css/stylesheet-candidate-nodes-crash.xhtml
Comment 15 Darin Adler 2011-07-18 18:25:35 PDT
Comment on attachment 101242 [details]
work around <rdar://problem/9797253>

View in context: https://bugs.webkit.org/attachment.cgi?id=101242&action=review

I think the EWS failure was a false positive.

> Source/WebCore/platform/ScrollableArea.cpp:227
> +    if (horizontalScrollbar())
> +        ScrollbarTheme::nativeTheme()->updateScrollbarOverlayStyle(horizontalScrollbar());
> +    if (verticalScrollbar())
> +        ScrollbarTheme::nativeTheme()->updateScrollbarOverlayStyle(verticalScrollbar());
> +
> +    if (horizontalScrollbar())
> +        horizontalScrollbar()->invalidate();
> +
> +    if (verticalScrollbar())
> +        verticalScrollbar()->invalidate();

Could we merge these if statements for clarity?

> Source/WebCore/platform/mac/ScrollViewMac.mm:215
> +    switch (style) {
> +        case ScrollbarOverlayStyleDark:
> +            return NSScrollerKnobStyleDark;
> +        case ScrollbarOverlayStyleLight:
> +            return NSScrollerKnobStyleLight;
> +        default:
> +            return NSScrollerKnobStyleDefault;
> +    }

Same switch statement indentation problem.
Comment 16 WebKit Review Bot 2011-07-18 19:27:12 PDT
Comment on attachment 101242 [details]
work around <rdar://problem/9797253>

Rejecting attachment 101242 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 2

Last 500 characters of output:

  perf/object-keys.html = TEXT PASS


Regressions: Unexpected image mismatch : (5)
  fast/text/atsui-multiple-renderers.html = IMAGE
  fast/text/international/danda-space.html = IMAGE
  fast/text/international/thai-baht-space.html = IMAGE
  fast/text/international/thai-line-breaks.html = IMAGE
  platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html = IMAGE

Regressions: Unexpected DumpRenderTree crashes : (1)
  fast/css/stylesheet-candidate-nodes-crash.xhtml = CRASH



Full output: http://queues.webkit.org/results/9136008
Comment 17 Tim Horton 2011-07-19 10:23:42 PDT
(In reply to comment #16)

While the rest of the failures seem to be happening to everyone, this one:

fast/css/stylesheet-candidate-nodes-crash.xhtml = CRASH

seems to be legitimate (can reproduce with patch, cannot without). But the backtrace certainly doesn't make it obvious how this patch is causing this failure. I'll take a look.
Comment 18 Tim Horton 2011-07-19 10:32:28 PDT
(In reply to comment #17)
> (In reply to comment #16)
> 
> While the rest of the failures seem to be happening to everyone, this one:
> 
> fast/css/stylesheet-candidate-nodes-crash.xhtml = CRASH
> 
> seems to be legitimate (can reproduce with patch, cannot without). But the backtrace certainly doesn't make it obvious how this patch is causing this failure. I'll take a look.

It was the updateStyleIfNeeded() added to FrameView::documentBackgroundColor.
Comment 19 Darin Adler 2011-07-19 10:33:48 PDT
(In reply to comment #18)
> It was the updateStyleIfNeeded() added to FrameView::documentBackgroundColor.

Very interesting. Maybe we need to make that change all alone in a separate patch and figure out what callers have difficulty.

I guess that could be done *after* this fix if the fix works without it. Or *before* if it really doesn't.
Comment 20 Tim Horton 2011-07-19 10:40:34 PDT
Created attachment 101341 [details]
remove updateStyleIfNeeded()

> I guess that could be done *after* this fix if the fix works without it. Or *before* if it really doesn't.

Everything works fine without it as far as my manual tests go (I added it solely because of your comment, though it certainly makes sense), so my vote is for solve that mystery after this patch is landed.
Comment 21 WebKit Review Bot 2011-07-20 18:32:30 PDT
Comment on attachment 101341 [details]
remove updateStyleIfNeeded()

Clearing flags on attachment: 101341

Committed r91435: <http://trac.webkit.org/changeset/91435>
Comment 22 WebKit Review Bot 2011-07-20 18:32:36 PDT
All reviewed patches have been landed.  Closing bug.
Comment 23 Ryosuke Niwa 2011-07-20 18:49:54 PDT
It seems like this broke Snow Leopard build:
http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20%28Build%29/builds/32163
Comment 24 Ryosuke Niwa 2011-07-20 23:12:39 PDT
This patch appears to have caused valgrind error:

19:02:10 memcheck_analyze.py [ERROR] FAIL! There were 1 errors: 
19:02:10 memcheck_analyze.py [ERROR] Command: src/sconsbuild/Release/test_shell_tests --gtest_filter=-BMPImageDecoderTest.FLAKY_ChunkedDecodingSlow:MediaLeakTest.VideoBear:MediaLeakTest.FAILS_VideoBear:WebFrameTest.GetFullHtmlOfPage:BMPImageDecoderTest.FLAKY_DecodingSlow:BMPImageDecoderTest.FAILS_ChunkedDecodingSlow:MediaLeakTest.FLAKY_VideoBear:MediaLeakTest.ManyVideoBear:BMPImageDecoderTest.DecodingSlow:MediaLeakTest.FAILS_ManyVideoBear:WebFrameTest.FAILS_GetFullHtmlOfPage:MediaLeakTest.FLAKY_ManyVideoBear:BMPImageDecoderTest.ChunkedDecodingSlow:BMPImageDecoderTest.FAILS_DecodingSlow:WebFrameTest.FLAKY_GetFullHtmlOfPage --gtest_print_time
UninitCondition
Conditional jump or move depends on uninitialised value(s)
  WebCore::FrameView::recalculateScrollbarOverlayStyle() (third_party/WebKit/Source/WebCore/page/FrameView.cpp:344)
  WebCore::RenderBox::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*) (third_party/WebKit/Source/WebCore/rendering/RenderBox.cpp:341)
  WebCore::RenderBlock::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*) (third_party/WebKit/Source/WebCore/rendering/RenderBlock.cpp:233)
  WebCore::RenderObject::setStyle(WTF::PassRefPtr<WebCore::RenderStyle>) (third_party/WebKit/Source/WebCore/rendering/RenderObject.cpp:1631)
  WebCore::RenderObject::setAnimatableStyle(WTF::PassRefPtr<WebCore::RenderStyle>) (third_party/WebKit/Source/WebCore/rendering/RenderObject.cpp:1544)
  WebCore::NodeRendererFactory::createRendererAndStyle() (third_party/WebKit/Source/WebCore/dom/NodeRenderingContext.cpp:287)
  WebCore::NodeRendererFactory::createRendererIfNeeded() (third_party/WebKit/Source/WebCore/dom/NodeRenderingContext.cpp:316)
  WebCore::Node::createRendererIfNeeded() (third_party/WebKit/Source/WebCore/dom/Node.cpp:1454)
  WebCore::Element::attach() (third_party/WebKit/Source/WebCore/dom/Element.cpp:1015)
  WTF::PassRefPtr<WebCore::Element> WebCore::HTMLConstructionSite::attach<WebCore::Element>(WebCore::ContainerNode*, WTF::PassRefPtr<WebCore::Element>) (third_party/WebKit/Source/WebCore/html/parser/HTMLConstructionSite.cpp:111)
  WebCore::HTMLConstructionSite::insertHTMLHtmlStartTagBeforeHTML(WebCore::AtomicHTMLToken&) (third_party/WebKit/Source/WebCore/html/parser/HTMLConstructionSite.cpp:186)
  WebCore::HTMLTreeBuilder::defaultForBeforeHTML() (third_party/WebKit/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2685)
  WebCore::HTMLTreeBuilder::processEndOfFile(WebCore::AtomicHTMLToken&) (third_party/WebKit/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2594)
  WebCore::HTMLTreeBuilder::processToken(WebCore::AtomicHTMLToken&) (third_party/WebKit/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:493)
  WebCore::HTMLTreeBuilder::constructTreeFromAtomicToken(WebCore::AtomicHTMLToken&) (third_party/WebKit/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:462)
  WebCore::HTMLTreeBuilder::constructTreeFromToken(WebCore::HTMLToken&) (third_party/WebKit/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:452)
  WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode) (third_party/WebKit/Source/WebCore/html/parser/HTMLDocumentParser.cpp:276)
  WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode) (third_party/WebKit/Source/WebCore/html/parser/HTMLDocumentParser.cpp:175)
  WebCore::HTMLDocumentParser::prepareToStopParsing() (third_party/WebKit/Source/WebCore/html/parser/HTMLDocumentParser.cpp:140)
  WebCore::HTMLDocumentParser::attemptToEnd() (third_party/WebKit/Source/WebCore/html/parser/HTMLDocumentParser.cpp:399)
  WebCore::HTMLDocumentParser::finish() (third_party/WebKit/Source/WebCore/html/parser/HTMLDocumentParser.cpp:426)
  WebCore::DocumentWriter::endIfNotLoadingMainResource() (third_party/WebKit/Source/WebCore/loader/DocumentWriter.cpp:226)
  WebCore::DocumentWriter::end() (third_party/WebKit/Source/WebCore/loader/DocumentWriter.cpp:209)
  WebCore::FrameLoader::init() (third_party/WebKit/Source/WebCore/loader/FrameLoader.cpp:233)
  WebCore::Frame::init() (third_party/WebKit/Source/WebCore/page/Frame.h:286)
  WebKit::WebFrameImpl::initializeAsMainFrame(WebKit::WebViewImpl*) (third_party/WebKit/Source/WebKit/chromium/src/WebFrameImpl.cpp:1883)
  WebKit::WebViewImpl::initializeMainFrame(WebKit::WebFrameClient*) (third_party/WebKit/Source/WebKit/chromium/src/WebViewImpl.cpp:271)
  WebViewHost::Create(_GtkWidget*, TestWebViewDelegate*, WebKit::WebDevToolsAgentClient*, WebPreferences const&) (webkit/tools/test_shell/webview_host_gtk.cc:34)
  TestShell::Initialize(GURL const&) (webkit/tools/test_shell/test_shell_gtk.cc:349)
  TestShell::CreateNewWindow(GURL const&, TestShell**) (webkit/tools/test_shell/test_shell.cc:203)
Suppression (error hash=#1E5287B6D215189B#):
  For more info on using suppressions see http://dev.chromium.org/developers/how-tos/using-valgrind#TOC-Suppressing-Errors
{
   <insert_a_suppression_name_here>
   Memcheck:Cond
   fun:_ZN7WebCore9FrameView32recalculateScrollbarOverlayStyleEv
   fun:_ZN7WebCore9RenderBox14styleDidChangeENS_15StyleDifferenceEPKNS_11RenderStyleE
   fun:_ZN7WebCore11RenderBlock14styleDidChangeENS_15StyleDifferenceEPKNS_11RenderStyleE
   fun:_ZN7WebCore12RenderObject8setStyleEN3WTF10PassRefPtrINS_11RenderStyleEEE
   fun:_ZN7WebCore12RenderObject18setAnimatableStyleEN3WTF10PassRefPtrINS_11RenderStyleEEE
   fun:_ZN7WebCore19NodeRendererFactory22createRendererAndStyleEv
   fun:_ZN7WebCore19NodeRendererFactory22createRendererIfNeededEv
   fun:_ZN7WebCore4Node22createRendererIfNeededEv
   fun:_ZN7WebCore7Element6attachEv
   fun:_ZN7WebCore20HTMLConstructionSite6attachINS_7ElementEEEN3WTF10PassRefPtrIT_EEPNS_13ContainerNodeES6_
   fun:_ZN7WebCore20HTMLConstructionSite32insertHTMLHtmlStartTagBeforeHTMLERNS_15AtomicHTMLTokenE
   fun:_ZN7WebCore15HTMLTreeBuilder20defaultForBeforeHTMLEv
   fun:_ZN7WebCore15HTMLTreeBuilder16processEndOfFileERNS_15AtomicHTMLTokenE
   fun:_ZN7WebCore15HTMLTreeBuilder12processTokenERNS_15AtomicHTMLTokenE
   fun:_ZN7WebCore15HTMLTreeBuilder28constructTreeFromAtomicTokenERNS_15AtomicHTMLTokenE
   fun:_ZN7WebCore15HTMLTreeBuilder22constructTreeFromTokenERNS_9HTMLTokenE
   fun:_ZN7WebCore18HTMLDocumentParser13pumpTokenizerENS0_15SynchronousModeE
   fun:_ZN7WebCore18HTMLDocumentParser23pumpTokenizerIfPossibleENS0_15SynchronousModeE
   fun:_ZN7WebCore18HTMLDocumentParser20prepareToStopParsingEv
   fun:_ZN7WebCore18HTMLDocumentParser12attemptToEndEv
   fun:_ZN7WebCore18HTMLDocumentParser6finishEv
   fun:_ZN7WebCore14DocumentWriter27endIfNotLoadingMainResourceEv
}
Comment 25 Ryosuke Niwa 2011-07-20 23:13:15 PDT
It appears that m_scrollbarOverlayStyle added to ScrollableArea is never initialized :(
Comment 26 Tim Horton 2011-07-20 23:16:53 PDT
(In reply to comment #25)
> It appears that m_scrollbarOverlayStyle added to ScrollableArea is never initialized :(

Eek! It needs to be initialized to ScrollbarOverlayStyleDefault. I'll do that tomorrow when I get back to my machine.

Is there a way to run valgrind tests on my machine? (though this one should have been caught simply by not making the mistake in the first place)
Comment 27 Ryosuke Niwa 2011-07-20 23:27:20 PDT
(In reply to comment #26)
> (In reply to comment #25)
> > It appears that m_scrollbarOverlayStyle added to ScrollableArea is never initialized :(
> 
> Eek! It needs to be initialized to ScrollbarOverlayStyleDefault. I'll do that tomorrow when I get back to my machine.

Indeed!  Fixed in http://trac.webkit.org/changeset/91447.

> Is there a way to run valgrind tests on my machine? (though this one should have been caught simply by not making the mistake in the first place)

There's a documentation on chromium.org: http://www.chromium.org/developers/how-tos/using-valgrind

But I've never used it myself so I'm not sure how much of that is applicable to Mac port.

If anything, Chromium Linux has Valgrind bot:
http://build.chromium.org/p/chromium.webkit/waterfall?builder=Linux%20Valgrind

We also have a shiny Snow Leopard Clang bot:
http://build.chromium.org/p/chromium.webkit/waterfall?builder=Mac%20Clang%20Builder%20%28dbg%29
Comment 28 Tim Horton 2011-07-20 23:29:03 PDT
(In reply to comment #27)
> (In reply to comment #26)
> > (In reply to comment #25)
> > > It appears that m_scrollbarOverlayStyle added to ScrollableArea is never initialized :(
> > 
> > Eek! It needs to be initialized to ScrollbarOverlayStyleDefault. I'll do that tomorrow when I get back to my machine.
> 
> Indeed!  Fixed in http://trac.webkit.org/changeset/91447.

Ah, you've been great today, cleaning up after all my mistakes. Thanks!

> > Is there a way to run valgrind tests on my machine? (though this one should have been caught simply by not making the mistake in the first place)
> 
> There's a documentation on chromium.org: http://www.chromium.org/developers/how-tos/using-valgrind
> 
> But I've never used it myself so I'm not sure how much of that is applicable to Mac port.
> 
> If anything, Chromium Linux has Valgrind bot:
> http://build.chromium.org/p/chromium.webkit/waterfall?builder=Linux%20Valgrind
> 
> We also have a shiny Snow Leopard Clang bot:
> http://build.chromium.org/p/chromium.webkit/waterfall?builder=Mac%20Clang%20Builder%20%28dbg%29

Excellent, I'll take a look!
Comment 29 Simon Fraser (smfr) 2011-07-21 12:40:33 PDT
Comment on attachment 101341 [details]
remove updateStyleIfNeeded()

View in context: https://bugs.webkit.org/attachment.cgi?id=101341&action=review

> Source/WebCore/ChangeLog:14
> +        No new tests, since this code is only enabled on future versions of Mac OS X.

No longer future!