Bug 61515 - [Chromium] fix a layout test crash except for mac (LayoutTest/fast/forms/select-script-onchange.html)
Summary: [Chromium] fix a layout test crash except for mac (LayoutTest/fast/forms/sele...
Status: RESOLVED DUPLICATE of bug 63857
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-26 02:33 PDT by Takashi Toyoshima
Modified: 2012-03-16 15:26 PDT (History)
3 users (show)

See Also:


Attachments
Patch (2.12 KB, patch)
2011-05-26 03:31 PDT, Takashi Toyoshima
no flags Details | Formatted Diff | Diff
Fix style bugs (2.13 KB, patch)
2011-05-26 03:41 PDT, Takashi Toyoshima
tkent: review-
toyoshim: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Takashi Toyoshima 2011-05-26 02:33:06 PDT
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)
Comment 1 Takashi Toyoshima 2011-05-26 03:31:50 PDT
Created attachment 94954 [details]
Patch
Comment 2 WebKit Review Bot 2011-05-26 03:34:25 PDT
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.
Comment 3 Takashi Toyoshima 2011-05-26 03:38:47 PDT
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.
Comment 4 Takashi Toyoshima 2011-05-26 03:41:22 PDT
Created attachment 94956 [details]
Fix style bugs
Comment 5 Kent Tamura 2011-05-26 07:25:20 PDT
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.
Comment 6 Takashi Toyoshima 2011-05-26 20:16:24 PDT
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.
Comment 7 Kent Tamura 2011-05-26 20:25:29 PDT
(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.
Comment 8 Takashi Toyoshima 2011-05-26 20:30:11 PDT
> 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.
Comment 9 Eric Seidel (no email) 2011-07-05 20:44:50 PDT
What's the status of this old patch?
Comment 10 Kent Tamura 2011-07-07 04:08:05 PDT
I think the problem was resolved by another patch.
https://bugs.webkit.org/show_bug.cgi?id=63857
Comment 11 Eric Seidel (no email) 2011-07-07 10:17:17 PDT

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