Bug 66658 – Chrome::setSelectedColorInColorChooser shouldn't be called when color chooser sets a new color

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Keishi Hattori]

Chrome::setSelectedColorInColorChooser is being called when the color chooser selects a color. This causes an infinite loop when the user changes the color very rapidly.

[Keishi Hattori]

Created attachment 104658 [details]
patch

[Kent Tamura]

Comment on attachment 104658 [details]
patch

ok

[WebKit Review Bot]

Comment on attachment 104658 [details]
patch

Clearing flags on attachment: 104658

Committed r93503: <http://trac.webkit.org/changeset/93503>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Normally this kind of change needs a regression test. Why no test in this case?

[Keishi Hattori]

(In reply to comment #5)
> Normally this kind of change needs a regression test. Why no test in this case?

I can't write a layout test because I need to manipulate NSColorPanel for the problem to occur.

[Darin Adler]

(In reply to comment #6)
> I can't write a layout test because I need to manipulate NSColorPanel for the problem to occur.

We can easily add a feature to DumpRenderTree to call the same function that the NSColorPanel would.

[Keishi Hattori]

This is my first time looking at DumpRenderTree and I have a couple of questions.

1. The WebKit part of the patch hasn't landed yet (Bug 65889, Bug 65897). So if I were to do this right now, I need to call a WebCore method from DumpRenderTree. Would it be better to do this after the patches land.

2. This infinite loop only happens in Chromium. The "set to color chooser" and "set from color chooser" IPCs get intertwined causing the loop. 
The loop won't happen in WebKit because the calls are from a single thread and the NSColorPanel checks if the same color is being set. 
Could we use DumpRenderTree to test for this?

[Kent Tamura]

(In reply to comment #8)
> This is my first time looking at DumpRenderTree and I have a couple of questions.
> 
> 1. The WebKit part of the patch hasn't landed yet (Bug 65889, Bug 65897). So if I were to do this right now, I need to call a WebCore method from DumpRenderTree. Would it be better to do this after the patches land.

I think we had better introduce a mock color panel delegate object and inject it to Chrome or ChromeClient by window.internals; e.g. window.internals.useMockColorPanel().

[Keishi Hattori]

Thanks darin, tkent. I think I have an idea how to test this now. I am working on it.