Bug 99053 - [Qt] Handle synchronous GET of blob URLs
Summary: [Qt] Handle synchronous GET of blob URLs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 420+
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Allan Sandfeld Jensen
URL:
Keywords:
Depends on: 99178
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-11 06:32 PDT by Allan Sandfeld Jensen
Modified: 2012-10-22 02:54 PDT (History)
2 users (show)

See Also:


Attachments
Patch (8.86 KB, patch)
2012-10-11 06:38 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (8.48 KB, patch)
2012-10-15 05:51 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Allan Sandfeld Jensen 2012-10-11 06:32:27 PDT
In the patch for bug #72329 I implemented uploading of BlobData content in POST requests. We are however still missing the capability of making XHR GET request for blob URLs. Fortunately that can be handled completely inside WebCore, and we already have tests for it.
Comment 1 Allan Sandfeld Jensen 2012-10-11 06:38:31 PDT
Created attachment 168219 [details]
Patch
Comment 2 Allan Sandfeld Jensen 2012-10-11 07:33:43 PDT
Comment on attachment 168219 [details]
Patch

Misnamed expectation file, and two of the tests are flaky and should be investigated.
Comment 3 Allan Sandfeld Jensen 2012-10-15 05:51:53 PDT
Created attachment 168691 [details]
Patch

Reupload. Note two of the tests are flaky and depends on the patch in bug #99178 to become reliable.
Comment 4 Simon Hausmann 2012-10-21 23:13:31 PDT
Comment on attachment 168691 [details]
Patch

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

r=me with one minor question.

> Source/WebCore/platform/network/qt/ResourceHandleQt.cpp:156
> +        blobRegistry().loadResourceSynchronously(request, error, response, data);

Hmm, shouldn't this "handle" the case if BlogRegistryImpl::shouldLoadResource returning false (i.e. the cause of loadResourceSynchronously returning false) ?
Comment 5 Allan Sandfeld Jensen 2012-10-22 02:27:02 PDT
(In reply to comment #4)
> (From update of attachment 168691 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=168691&action=review
> 
> r=me with one minor question.
> 
> > Source/WebCore/platform/network/qt/ResourceHandleQt.cpp:156
> > +        blobRegistry().loadResourceSynchronously(request, error, response, data);
> 
> Hmm, shouldn't this "handle" the case if BlogRegistryImpl::shouldLoadResource returning false (i.e. the cause of loadResourceSynchronously returning false) ?

We will not be able to "handle" that any better using our platform code. In we will get worse error-codes (unknown protocol instead of GET not allowed).
Comment 6 Allan Sandfeld Jensen 2012-10-22 02:32:16 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 168691 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=168691&action=review
> > 
> > r=me with one minor question.
> > 
> > > Source/WebCore/platform/network/qt/ResourceHandleQt.cpp:156
> > > +        blobRegistry().loadResourceSynchronously(request, error, response, data);
> > 
> > Hmm, shouldn't this "handle" the case if BlogRegistryImpl::shouldLoadResource returning false (i.e. the cause of loadResourceSynchronously returning false) ?
> 
> We will not be able to "handle" that any better using our platform code. In we will get worse error-codes (unknown protocol instead of GET not allowed).

Okay, we won't actually set an error code in either case, but the end result is still the same.
Comment 7 WebKit Review Bot 2012-10-22 02:54:51 PDT
Comment on attachment 168691 [details]
Patch

Clearing flags on attachment: 168691

Committed r132053: <http://trac.webkit.org/changeset/132053>
Comment 8 WebKit Review Bot 2012-10-22 02:54:54 PDT
All reviewed patches have been landed.  Closing bug.