Bug 33344 - [Qt] unskip plugins/get-url-that-the-resource-load-delegate-will-disallow.html
: [Qt] unskip plugins/get-url-that-the-resource-load-delegate-will-disallow.html
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Tools / Tests
: 528+ (Nightly build)
: Other Mac OS X 10.5
: P2 Normal
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-01-07 13:46 PST by Robert Hogan
Modified: 2010-06-21 15:17 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.72 KB, patch)
2010-06-14 11:44 PDT, Robert Hogan
no flags Details | Formatted Diff | Diff
Patch (3.20 KB, patch)
2010-06-21 14:13 PDT, Robert Hogan
kenneth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Hogan 2010-01-07 13:46:25 PST
The code that this test relates to is gone. It was added by http://trac.webkit.org/changeset/20694, which fixed a crash in WebKit/Plugins/WebNetscapePluginStream.mm. The crash was caused by the lines removed below:

     loader->documentLoader()->addPlugInStreamLoader(_loader); 
-    if (!_loader->load(request)) 
-        _loader->documentLoader()->removePlugInStreamLoader(_loader); 
+    _loader->load(request); 

The implementation is now shared, or at least WebBaseNetscapePluginStream.mm maintains a copy of PluginStream.cpp which has always had:

void PluginStream::start()
{
    ASSERT(!m_loadManually);

    m_loader = NetscapePlugInStreamLoader::create(m_frame, this);

    m_loader->setShouldBufferData(false);
    m_loader->documentLoader()->addPlugInStreamLoader(m_loader.get());
    m_loader->load(m_resourceRequest);
}

Also, the crash situation doesn't arise for ports that do not implement a ResourceLoadDelegate. So should such ports implement the layoutTestController method the test requires and some drt specific code into the frameloaders to pass this test or should it be removed/moved to platform-specific tests?
Comment 1 Robert Hogan 2010-06-14 11:44:01 PDT
Created attachment 58677 [details]
Patch
Comment 2 Csaba Osztrogonác 2010-06-21 00:41:53 PDT
(In reply to comment #1)
> Created an attachment (id=58677) [details]
> Patch

LGTM, but you only missed unskipping plugins/get-url-that-the-resource-load-delegate-will-disallow.html
Comment 3 Robert Hogan 2010-06-21 14:13:11 PDT
Created attachment 59287 [details]
Patch
Comment 4 Robert Hogan 2010-06-21 14:15:12 PDT
(In reply to comment #2)
> (In reply to comment #1)
> > Created an attachment (id=58677) [details] [details]
> > Patch
> 
> LGTM, but you only missed unskipping plugins/get-url-that-the-resource-load-delegate-will-disallow.html

whoops!
Comment 5 Robert Hogan 2010-06-21 14:39:45 PDT
Committed r61580: <http://trac.webkit.org/changeset/61580>
Comment 6 WebKit Review Bot 2010-06-21 15:17:45 PDT
http://trac.webkit.org/changeset/61580 might have broken SnowLeopard Intel Release (Tests)
The following changes are on the blame list:
http://trac.webkit.org/changeset/61579
http://trac.webkit.org/changeset/61580