Bug 185344 - Cleanup XMLHttpRequestUpload a little
Summary: Cleanup XMLHttpRequestUpload a little
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-05-04 22:45 PDT by Sam Weinig
Modified: 2018-05-06 17:02 PDT (History)
4 users (show)

See Also:


Attachments
Patch (6.90 KB, patch)
2018-05-04 22:48 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2018-05-04 22:45:27 PDT
Cleanup XMLHttpRequestUpload a little
Comment 1 Sam Weinig 2018-05-04 22:48:18 PDT
Created attachment 339636 [details]
Patch
Comment 2 Yusuke Suzuki 2018-05-04 23:53:00 PDT
Comment on attachment 339636 [details]
Patch

r=me
Comment 3 WebKit Commit Bot 2018-05-05 16:45:20 PDT
Comment on attachment 339636 [details]
Patch

Clearing flags on attachment: 339636

Committed r231398: <https://trac.webkit.org/changeset/231398>
Comment 4 WebKit Commit Bot 2018-05-05 16:45:21 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Darin Adler 2018-05-06 17:01:29 PDT
Comment on attachment 339636 [details]
Patch

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

> Source/WebCore/xml/XMLHttpRequestUpload.h:45
> +    EventTargetInterface eventTargetInterface() const override { return XMLHttpRequestUploadEventTargetInterfaceType; }
> +    ScriptExecutionContext* scriptExecutionContext() const override { return m_xmlHttpRequest.scriptExecutionContext(); }

I’d have made these private, and used final instead of override.

> Source/WebCore/xml/XMLHttpRequestUpload.h:54
> +    XMLHttpRequest& m_xmlHttpRequest;

We should rename this to m_request or m_underlyingRequest. I think maybe the member function too.

> Source/WebCore/xml/XMLHttpRequestUpload.h:57
> +    bool m_lengthComputable { false };
> +    unsigned long long m_loaded { 0 };
> +    unsigned long long m_total { 0 };

I really don’t like these names, but they are straight out of the progress event specification. I kind of wish this was a ProgressEvent::Init structure. But that also has Event::Init as its base class.
Comment 6 Radar WebKit Bug Importer 2018-05-06 17:02:26 PDT
<rdar://problem/40011842>