[Webkit-unassigned] [Bug 39083] Refactor FormData and Blob for better support of Blobs synthesized by BlobBuilder

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=39083





--- Comment #20 from Kinuko Yasuda <kinuko at chromium.org>  2010-05-19 15:58:04 PST ---
Thanks, I've moved the BlobItem under platform and fixed dependencies.

(In reply to comment #18)
> You might need to add the new file BlobItem.cpp to the new make file: CMakeLists.txt.

Done.

> WebCore/bindings/v8/SerializedScriptValue.cpp:562
> WebCore/html/Blob.h:50
>  +      static PassRefPtr<Blob> createFromFile(const String& path, const String type = String())

In this patch I left it untouched and removed the type parameter from the Blob constructor (as no one is calling Blob::create(type) yet - we'll need it when we add slice(type) or BlobBuilder).

> WebCore/html/BlobItem.cpp:68
>  +  static const double invalidModificationTime = -1;
> Why do we use -1, instead of 0?

Changed to 0.  Back then I wanted to differentiate uninitialized motd (when the item is not a lice) and invalid one (initialization failure) but now we won't need it because the base class doesn't have the field.

> WebCore/html/BlobItem.h:74
>  +      static PassRefPtr<BlobItem> createFileRangeBlobItem(const String& path, long long start, long long length, double snapshotModificationTime);
> It would be better to return "PassRefPtr<FileRangeBlobItem>".

Hmm, I think it would be easier to return PassRefPtr<BlobItem> for all create methods because in that way we can uniformly assign the return value to RefPtr<BlobItem>.

> WebCore/html/BlobItem.h:61
>  +      virtual const char* data() const { return 0; }
> data() should only make sense for StringBlobItem, BytesArrayBlobItem and DataRangeBlobItem. It is a little bit wield to put it in the base class. Can we move it to the subclass? Since we do not want to add it for each of these subclasses, how about restructure BlobItem classes like the following:

Sounds great.  I made the change.

> WebCore/html/File.cpp: 
>  +      // We don't use MIMETypeRegistry::getMIMETypeForPath() because it returns "application/octet-stream" upon failure.
> Can we keep the comment to help people better understand why we call MIMETypeRegistry::getMIMETypeForExtension?

Done.

> WebCore/platform/network/FormData.h:23
>  +  #include "Blob.h"
> As Darin pointed out, FormData is under platform/network so it should not depend on other non-platform stuff. We could move BlobItem to platform.

Done.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.