Bug 61515 – [Chromium] fix a layout test crash except for mac (LayoutTest/fast/forms/select-script-onchange.html)

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: Tools / Tests

[Takashi Toyoshima]

LayoutTest/fast/forms/select-script-onchange.html crashes on chromium-linux and chromium-win.

Pop-up keystrokes depends on platform and classed into two groups.
As WebCore/dom/SelectElement.cpp said like:

// Configure platform-specific behavior when focused pop-up receives arrow/space/return keystroke.
// (PLATFORM(MAC) and PLATFORM(GTK) are always false in Chromium, hence the extra tests.)
#if PLATFORM(MAC) || (PLATFORM(CHROMIUM) && OS(DARWIN))
#define ARROW_KEYS_POP_MENU 1
#define SPACE_OR_RETURN_POP_MENU 0
#elif PLATFORM(GTK) || (PLATFORM(CHROMIUM) && OS(UNIX))
#define ARROW_KEYS_POP_MENU 0
#define SPACE_OR_RETURN_POP_MENU 1
#else
#define ARROW_KEYS_POP_MENU 0
#define SPACE_OR_RETURN_POP_MENU 0
#endif

This test sends "t\r" keyDown event to change the selected item.
It cause calling WebViewHost::createPopupMenu() on SPACE_OR_RETURN_POP_MENU platform,
and it's not implemented by DumpRenderTree for chromium
(Tools/DumpRenderTree/chromium/WebViewHost.cpp:241).

Originally "\r" must be intended to call htmlForm->submitImplicitly() on ARROW_KEYS_POP_MENU platform.
But this function isn't called on SPACE_OR_RETURN_POP_MENU platform.
So, I just skip it on SPACE_OR_RETURN_POP_MENU platform.

Crash dump is following
[11297:11297:4333840847280:ERROR:process_util_posix.cc(108)] Received signal 11
        base::debug::StackTrace::StackTrace() [0x91b81a]
        base::(anonymous namespace)::StackDumpSignalHandler() [0x8e42cb]
        0x7f8ebbed3af0
        WebKit::WebPopupMenuImpl::Init() [0x4fbb82]
        WebKit::ChromeClientImpl::popupOpened() [0x4be183]
        WebCore::PopupContainer::showPopup() [0xd6cf6d]
        WebCore::PopupContainer::showInRect() [0xd6d98c]
        WebCore::PopupMenuChromium::show() [0xd71993]
        WebCore::RenderMenuList::showPopup() [0x159fbb2]
        WebCore::SelectElement::menuListDefaultEventHandler() [0x10a0a3b]
        WebCore::SelectElement::defaultEventHandler() [0x10a1a9c]
        WebCore::HTMLSelectElement::defaultEventHandler() [0x675866]
        WebCore::EventDispatcher::dispatchEvent() [0x1046f51]
        WebCore::EventDispatchMediator::dispatchEvent() [0x104572c]
        WebCore::KeyboardEventDispatchMediator::dispatchEvent() [0x1054729]
        WebCore::EventDispatcher::dispatchEvent() [0x10458dd]
        WebCore::Node::dispatchKeyEvent() [0x106819f]
        WebCore::EventHandler::keyEvent() [0x126ba76]
        WebKit::WebViewImpl::charEvent() [0x4a8a5f]
        WebKit::WebViewImpl::handleInputEvent() [0x4aa330]
        EventSender::keyDown() [0x43083d]
(snip)

[Takashi Toyoshima]

Created attachment 94954 [details]
Patch

[WebKit Review Bot]

Attachment 94954 [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

LayoutTests/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
LayoutTests/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Takashi Toyoshima]

This change use navigator.userAgent to switch codes which depends on platform.
Another idea is preparing two different tests for each platform and just skip a test for another.
Actually, I'm not familier with platform dependent tests.
So I could not decide which is better approach.

[Takashi Toyoshima]

Created attachment 94956 [details]
Fix style bugs

[Kent Tamura]

Comment on attachment 94956 [details]
Fix style bugs

(In reply to comment #0)
> It cause calling WebViewHost::createPopupMenu() on SPACE_OR_RETURN_POP_MENU platform,
> and it's not implemented by DumpRenderTree for chromium
> (Tools/DumpRenderTree/chromium/WebViewHost.cpp:241).

Right.  So we should implement popup menu in DRT/Chromium.

> Originally "\r" must be intended to call htmlForm->submitImplicitly() on ARROW_KEYS_POP_MENU platform.

No.  Select elements in the test don't have their parent forms.  So submitImplicitly() is not called.

[Takashi Toyoshima]

Hi, Kent-san.
Thank you for review.

(In reply to comment #5)
> Right.  So we should implement popup menu in DRT/Chromium.

OK. I understand DRT/Chromium needs pseudo popup implementation.

But, originally this test doesn't require popup menu.
So I think it's independent issue.

This test works on Mac as...
1. popup.focus()
  Set active focus to second select form.
2. eventSender.keyDown("t", null);
  Select the second item 'two' by initial word 't'.
3. eventSender.keyDown("\r", null);
  Fix the selection.

And on Linux and Windows as...
1. popup.focus()
  Set active focus to second select form.
2. eventSender.keyDown("t", null);
  Select the second item 'two' by initial word 't'.
3. eventSender.keyDown("\r", null);
  Show pulldown menu for select items.

The process 3 cause quite different action for Linux and Windows.
Because pop-up keystrokes configuration is different as I described.
Popup menu is not intended. That's why I just skip the "\r" event in this change.

[Kent Tamura]

(In reply to comment #6)
> But, originally this test doesn't require popup menu.

It's right. But showing a pop-menu should be harmless. Actually this test doesn't fail on Qt and Apple Windows.  So we don't need to remove the \r.

[Takashi Toyoshima]

> It's right. But showing a pop-menu should be harmless. Actually this test doesn't fail on Qt and Apple Windows.  So we don't need to remove the \r.

OK.
I see the background.
I withdraw this patch.

[Eric Seidel (no email)]

What's the status of this old patch?

[Kent Tamura]

I think the problem was resolved by another patch.
https://bugs.webkit.org/show_bug.cgi?id=63857

[Eric Seidel (no email)]

*** This bug has been marked as a duplicate of bug 63857 ***