Bug 91459 – [Chromium] TestInterfaces should be responsible for owning and binding AccessibilityController and TextInputController

• Component: New Bugs
• Status: RESOLVED FIXED

[Adam Barth]

[Chromium] TestInterfaces should be responsible for owning and binding AccessibilityController and TextInputController

[Adam Barth]

Created attachment 152673 [details]
Patch

[WebKit Review Bot]

Comment on attachment 152673 [details]
Patch

Clearing flags on attachment: 152673

Committed r122828: <http://trac.webkit.org/changeset/122828>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Abhishek Arya]

Adam, looks like ASAN DRT is broken...

./DumpRenderTree 2>&1 | /tmp/asan_symbolize.py | c++filt
ASAN:SIGSEGV
==23107== ERROR: AddressSanitizer crashed on unknown address 0x000000000000 (pc 0x00000050e512 sp 0x7fff7eb3b520 bp 0x7fff7eb3b650 T0)
AddressSanitizer can not provide additional info. ABORTING
    #0 0x50e512 in TestInterfaces::bindTo(WebKit::WebFrame*) ???:0
    #1 0x4cdb93 in TestShell::bindJSObjectsToWindow(WebKit::WebFrame*) ???:0
    #2 0x2478451 in WebCore::FrameLoader::dispatchDidClearWindowObjectInWorld(WebCore::DOMWrapperWorld*) ???:0
    #3 0x2453e5c in WebCore::FrameLoader::dispatchDidClearWindowObjectsInAllWorlds() ???:0
    #4 0x245333b in WebCore::FrameLoader::receivedFirstData() ???:0
    #5 0x2422ba8 in WebCore::DocumentLoader::commitData(char const*, unsigned long) ???:0
    #6 0x2422016 in WebCore::DocumentLoader::finishedLoading() ???:0
    #7 0x24980d8 in WebCore::MainResourceLoader::didFinishLoading(double) ???:0
    #8 0x2496cb8 in WebCore::MainResourceLoader::continueAfterContentPolicy(WebCore::PolicyAction, WebCore::ResourceResponse const&) ???:0
    #9 0x2496f11 in WebCore::MainResourceLoader::continueAfterContentPolicy(WebCore::PolicyAction) ???:0
    #10 0x24a9ec7 in WebCore::PolicyChecker::continueAfterContentPolicy(WebCore::PolicyAction) ???:0
    #11 0x5e4c09 in WebKit::FrameLoaderClientImpl::dispatchDecidePolicyForResponse(void (WebCore::PolicyChecker::*)(WebCore::PolicyAction), WebCore::ResourceResponse const&, WebCore::ResourceRequest const&) ???:0
    #12 0x2497902 in WebCore::MainResourceLoader::didReceiveResponse(WebCore::ResourceResponse const&) ???:0
    #13 0x2498c22 in WebCore::MainResourceLoader::handleEmptyLoad(WebCore::KURL const&, bool) ???:0
    #14 0x24994fe in WebCore::MainResourceLoader::loadNow(WebCore::ResourceRequest&) ???:0
    #15 0x2499e23 in WebCore::MainResourceLoader::load(WebCore::ResourceRequest const&, WebCore::SubstituteData const&) ???:0
    #16 0x242728c in WebCore::DocumentLoader::startLoadingMainResource() ???:0
    #17 0x2465e36 in WebCore::FrameLoader::continueLoadAfterNavigationPolicy(WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool) ???:0
    #18 0x24660ec in WebCore::FrameLoader::callContinueLoadAfterNavigationPolicy(void*, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool) ???:0
    #19 0x24a6749 in WebCore::PolicyCallback::call(bool) ???:0
    #20 0x24a9471 in WebCore::PolicyChecker::continueAfterNavigationPolicy(WebCore::PolicyAction) ???:0
    #21 0x5e5ba7 in WebKit::FrameLoaderClientImpl::dispatchDecidePolicyForNavigationAction(void (WebCore::PolicyChecker::*)(WebCore::PolicyAction), WebCore::NavigationAction const&, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>) ???:0
    #22 0x24a838b in WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest const&, WebCore::DocumentLoader*, WTF::PassRefPtr<WebCore::FormState>, void (*)(void*, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool), void*) ???:0
    #23 0x2464663 in WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader*, WebCore::FrameLoadType, WTF::PassRefPtr<WebCore::FormState>) ???:0
    #24 0x245b3fb in WebCore::FrameLoader::load(WebCore::DocumentLoader*) ???:0
    #25 0x2463c8a in WebCore::FrameLoader::load(WebCore::ResourceRequest const&, WebCore::SubstituteData const&, bool) ???:0
    #26 0x246377e in WebCore::FrameLoader::load(WebCore::ResourceRequest const&, bool) ???:0
    #27 0x4efb53 in WebViewHost::navigate(TestNavigationEntry const&, bool) ???:0
    #28 0x4efd70 in non-virtual thunk to WebViewHost::navigate(TestNavigationEntry const&, bool) ???:0
    #29 0x4c397c in TestNavigationController::navigateToPendingEntry(bool) ???:0
    #30 0x4ef21a in WebViewHost::loadURLForFrame(WebKit::WebURL const&, WebKit::WebString const&) ???:0
    #31 0x4ee2eb in WebViewHost::~WebViewHost() ???:0
    #32 0x4edf35 in WebViewHost::~WebViewHost() ???:0
    #33 0x4c7334 in TestShell::~TestShell() ???:0
    #34 0x466806 in main ???:0
    #35 0x7f2e88702c4d in ?? ??:0
Stats: 12M malloced (22M for red zones) by 79051 calls
Stats: 2M realloced by 4031 calls
Stats: 10M freed by 66113 calls
Stats: 0M really freed by 0 calls
Stats: 68M (17417 full pages) mmaped in 17 calls
  mmaps   by size class: 8:81915; 9:8191; 10:4095; 11:2047; 12:1024; 13:512; 14:256; 15:128; 16:64; 17:32; 18:32; 19:8;
  mallocs by size class: 8:74444; 9:1762; 10:1231; 11:768; 12:386; 13:148; 14:218; 15:64; 16:10; 17:2; 18:17; 19:1;
  frees   by size class: 8:62348; 9:1476; 10:953; 11:644; 12:290; 13:127; 14:195; 15:55; 16:5; 17:2; 18:17; 19:1;
  rfrees  by size class:
Stats: malloc large: 20 small slow: 263

[WebKit Review Bot]

Re-opened since this is blocked by 91516

Comment on attachment 152673 [details]
Patch

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

> Tools/DumpRenderTree/chromium/TestShell.cpp:182
> +    m_testInterfaces.clear();

this appears to be the bug. We shouldn't delete TestInterfaces before the TestShell destructor has finished

[Adam Barth]

Comment on attachment 152673 [details]
Patch

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

>> Tools/DumpRenderTree/chromium/TestShell.cpp:182
>> +    m_testInterfaces.clear();
> 
> this appears to be the bug. We shouldn't delete TestInterfaces before the TestShell destructor has finished

Ok.  I guess we should just setWebView(0) rather than destructing the whole thing.

[Adam Barth]

Created attachment 152795 [details]
Patch

[Adam Barth]

@rniwa: Would you be willing to review this updated patch?

[Ryosuke Niwa]

Comment on attachment 152795 [details]
Patch

Alright. Let's hope this one sticks :)

[WebKit Review Bot]

Comment on attachment 152795 [details]
Patch

Rejecting attachment 152795 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
/TestShell.cpp
Hunk #1 succeeded at 145 (offset -1 lines).
Hunk #2 succeeded at 171 (offset -2 lines).
Hunk #3 succeeded at 288 (offset -2 lines).
Hunk #4 succeeded at 716 (offset -7 lines).
patching file Tools/DumpRenderTree/chromium/TestShell.h
Hunk #2 succeeded at 92 (offset -2 lines).
Hunk #3 succeeded at 213 (offset -1 lines).

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Ryosuke Ni..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/13317237

[Adam Barth]

Created attachment 153845 [details]
Patch for landing

[Adam Barth]

This patch got slightly uglier when merged.  We probably should make TestInterfaces into a proper API sooner rather than later.

[WebKit Review Bot]

Comment on attachment 153845 [details]
Patch for landing

Clearing flags on attachment: 153845

Committed r123384: <http://trac.webkit.org/changeset/123384>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.