Changeset 261898 in webkit


Ignore:
Timestamp:
May 19, 2020 5:27:20 PM (4 years ago)
Author:
commit-queue@webkit.org
Message:

Add _WKDownloadDelegate callback including totalBytesWritten
​https://bugs.webkit.org/show_bug.cgi?id=212110
<rdar://problem/63358981>

Patch by Alex Christensen <​achristensen@webkit.org> on 2020-05-19
Reviewed by Geoffrey Garen.

Source/WebKit:

Without this new callback, after resuming a download, a client has no way to tell whether the download was successfully
resumed by a server that has proper etag and range request support or whether the download began at the beginning again.
A client was guessing that the download did not restart, causing incorrect reported download sizes when the download did restart.
Luckily, the data on disk was not corrupted, just the UI. This allows us to fix the UI.

Testing covered by expanding the API test for resuming downloads.

  • NetworkProcess/Downloads/Download.cpp:

(WebKit::Download::didReceiveData):

  • NetworkProcess/Downloads/Download.h:
  • NetworkProcess/NetworkDataTaskBlob.cpp:

(WebKit::NetworkDataTaskBlob::writeDownload):

  • NetworkProcess/NetworkDataTaskBlob.h:
  • NetworkProcess/cocoa/NetworkSessionCocoa.mm:

(-[WKNetworkSessionDelegate URLSession:downloadTask:didWriteData:totalBytesWritten:totalBytesExpectedToWrite:]):

  • UIProcess/API/APIDownloadClient.h:

(API::DownloadClient::didReceiveData):

  • UIProcess/API/C/WKContext.cpp:

(WKContextSetDownloadClient):

  • UIProcess/API/Cocoa/_WKDownloadDelegate.h:
  • UIProcess/Cocoa/DownloadClient.h:
  • UIProcess/Cocoa/DownloadClient.mm:

(WebKit::DownloadClient::DownloadClient):
(WebKit::DownloadClient::didReceiveResponse):
(WebKit::DownloadClient::didReceiveData):

  • UIProcess/Downloads/DownloadProxy.cpp:

(WebKit::DownloadProxy::didReceiveData):

  • UIProcess/Downloads/DownloadProxy.h:

(WebKit::DownloadProxy::expectedContentLength const): Deleted.
(WebKit::DownloadProxy::setExpectedContentLength): Deleted.
(WebKit::DownloadProxy::bytesLoaded const): Deleted.
(WebKit::DownloadProxy::setBytesLoaded): Deleted.

  • UIProcess/Downloads/DownloadProxy.messages.in:

Tools:

  • TestWebKitAPI/Tests/WebKitCocoa/Download.mm:

(TEST):

  • TestWebKitAPI/cocoa/TestDownloadDelegate.h:
  • TestWebKitAPI/cocoa/TestDownloadDelegate.mm:

(-[TestDownloadDelegate _download:didWriteData:totalBytesWritten:totalBytesExpectedToWrite:]):
(-[TestDownloadDelegate _download:didReceiveData:]): Deleted.

Location:
trunk
Files:
20 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r261892 r261898  
     12020-05-19  Alex Christensen  <achristensen@webkit.org>
     2
     3        Add _WKDownloadDelegate callback including totalBytesWritten
     4        https://bugs.webkit.org/show_bug.cgi?id=212110
     5        <rdar://problem/63358981>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        Without this new callback, after resuming a download, a client has no way to tell whether the download was successfully
     10        resumed by a server that has proper etag and range request support or whether the download began at the beginning again.
     11        A client was guessing that the download did not restart, causing incorrect reported download sizes when the download did restart.
     12        Luckily, the data on disk was not corrupted, just the UI.  This allows us to fix the UI.
     13
     14        Testing covered by expanding the API test for resuming downloads.
     15
     16        * NetworkProcess/Downloads/Download.cpp:
     17        (WebKit::Download::didReceiveData):
     18        * NetworkProcess/Downloads/Download.h:
     19        * NetworkProcess/NetworkDataTaskBlob.cpp:
     20        (WebKit::NetworkDataTaskBlob::writeDownload):
     21        * NetworkProcess/NetworkDataTaskBlob.h:
     22        * NetworkProcess/cocoa/NetworkSessionCocoa.mm:
     23        (-[WKNetworkSessionDelegate URLSession:downloadTask:didWriteData:totalBytesWritten:totalBytesExpectedToWrite:]):
     24        * UIProcess/API/APIDownloadClient.h:
     25        (API::DownloadClient::didReceiveData):
     26        * UIProcess/API/C/WKContext.cpp:
     27        (WKContextSetDownloadClient):
     28        * UIProcess/API/Cocoa/_WKDownloadDelegate.h:
     29        * UIProcess/Cocoa/DownloadClient.h:
     30        * UIProcess/Cocoa/DownloadClient.mm:
     31        (WebKit::DownloadClient::DownloadClient):
     32        (WebKit::DownloadClient::didReceiveResponse):
     33        (WebKit::DownloadClient::didReceiveData):
     34        * UIProcess/Downloads/DownloadProxy.cpp:
     35        (WebKit::DownloadProxy::didReceiveData):
     36        * UIProcess/Downloads/DownloadProxy.h:
     37        (WebKit::DownloadProxy::expectedContentLength const): Deleted.
     38        (WebKit::DownloadProxy::setExpectedContentLength): Deleted.
     39        (WebKit::DownloadProxy::bytesLoaded const): Deleted.
     40        (WebKit::DownloadProxy::setBytesLoaded): Deleted.
     41        * UIProcess/Downloads/DownloadProxy.messages.in:
     42
    1432020-05-19  Kate Cheney  <katherine_cheney@apple.com>
    244

  • trunk/Source/WebKit/NetworkProcess/Downloads/Download.cpp

    r252274 r261898  
    118118}
    119119
    120 void Download::didReceiveData(uint64_t length)
     120void Download::didReceiveData(uint64_t bytesWritten, uint64_t totalBytesWritten, uint64_t totalBytesExpectedToWrite)
    121121{
    122122    if (!m_hasReceivedData) {
    … …  
    125125    }
    126126   
    127     m_monitor.downloadReceivedBytes(length);
    128 
    129     send(Messages::DownloadProxy::DidReceiveData(length));
     127    m_monitor.downloadReceivedBytes(bytesWritten);
     128
     129    send(Messages::DownloadProxy::DidReceiveData(bytesWritten, totalBytesWritten, totalBytesExpectedToWrite));
    130130}
    131131

  • trunk/Source/WebKit/NetworkProcess/Downloads/Download.h

    r255533 r261898  
    8989    void didReceiveChallenge(const WebCore::AuthenticationChallenge&, ChallengeCompletionHandler&&);
    9090    void didCreateDestination(const String& path);
    91     void didReceiveData(uint64_t length);
     91    void didReceiveData(uint64_t bytesWritten, uint64_t totalBytesWritten, uint64_t totalBytesExpectedToWrite);
    9292    void didFinish();
    9393    void didFail(const WebCore::ResourceError&, const IPC::DataReference& resumeData);

  • trunk/Source/WebKit/NetworkProcess/NetworkDataTaskBlob.cpp

    r255899 r261898  
    484484    ASSERT(isDownload());
    485485    int bytesWritten = FileSystem::writeToFile(m_downloadFile, data, bytesRead);
    486     if (bytesWritten == -1) {
     486    if (bytesWritten != bytesRead) {
    487487        didFailDownload(cancelledError(m_firstRequest));
    488488        return false;
    489489    }
    490490
    491     ASSERT(bytesWritten == bytesRead);
     491    m_downloadBytesWritten += bytesWritten;
    492492    auto* download = m_networkProcess->downloadManager().download(m_pendingDownloadID);
    493493    ASSERT(download);
    494     download->didReceiveData(bytesWritten);
     494    download->didReceiveData(bytesWritten, m_downloadBytesWritten, m_totalSize);
    495495    return true;
    496496}

  • trunk/Source/WebKit/NetworkProcess/NetworkDataTaskBlob.h

    r241317 r261898  
    109109    long long m_rangeSuffixLength { kPositionNotSpecified };
    110110    long long m_totalSize { 0 };
     111    long long m_downloadBytesWritten { 0 };
    111112    long long m_totalRemainingSize { 0 };
    112113    long long m_currentItemReadSize { 0 };

  • trunk/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm

    r261762 r261898  
    951951    if (!download)
    952952        return;
    953     download->didReceiveData(bytesWritten);
     953    download->didReceiveData(bytesWritten, totalBytesWritten, totalBytesExpectedToWrite);
    954954}
    955955

  • trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp

    r260707 r261898  
    993993    auto* download = m_session->networkProcess().downloadManager().download(m_pendingDownloadID);
    994994    ASSERT(download);
    995     download->didReceiveData(bytesWritten);
     995    download->didReceiveData(bytesWritten, 0, 0);
    996996    read();
    997997}

  • trunk/Source/WebKit/UIProcess/API/APIDownloadClient.h

    r250292 r261898  
    5757    virtual void didReceiveAuthenticationChallenge(WebKit::DownloadProxy&, WebKit::AuthenticationChallengeProxy& challenge) { challenge.listener().completeChallenge(WebKit::AuthenticationChallengeDisposition::Cancel); }
    5858    virtual void didReceiveResponse(WebKit::DownloadProxy&, const WebCore::ResourceResponse&) { }
    59     virtual void didReceiveData(WebKit::DownloadProxy&, uint64_t) { }
     59    virtual void didReceiveData(WebKit::DownloadProxy&, uint64_t, uint64_t, uint64_t) { }
    6060    virtual void decideDestinationWithSuggestedFilename(WebKit::DownloadProxy&, const WTF::String&, Function<void(WebKit::AllowOverwrite, WTF::String)>&& completionHandler) { completionHandler(WebKit::AllowOverwrite::No, { }); }
    6161    virtual void didCreateDestination(WebKit::DownloadProxy&, const WTF::String&) { }

  • trunk/Source/WebKit/UIProcess/API/C/WKContext.cpp

    r259705 r261898  
    197197            m_client.didReceiveResponse(m_context, WebKit::toAPI(&downloadProxy), WebKit::toAPI(API::URLResponse::create(response).ptr()), m_client.base.clientInfo);
    198198        }
    199         void didReceiveData(WebKit::DownloadProxy& downloadProxy, uint64_t length) final
     199        void didReceiveData(WebKit::DownloadProxy& downloadProxy, uint64_t length, uint64_t, uint64_t) final
    200200        {
    201201            if (!m_client.didReceiveData)

  • trunk/Source/WebKit/UIProcess/API/Cocoa/_WKDownloadDelegate.h

    r243376 r261898  
    3636- (void)_download:(_WKDownload *)download didReceiveResponse:(NSURLResponse *)response;
    3737- (void)_download:(_WKDownload *)download didReceiveData:(uint64_t)length;
     38- (void)_download:(_WKDownload *)download didWriteData:(uint64_t)bytesWritten totalBytesWritten:(uint64_t)totalBytesWritten totalBytesExpectedToWrite:(uint64_t)totalBytesExpectedToWrite WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
    3839- (void)_download:(_WKDownload *)download decideDestinationWithSuggestedFilename:(NSString *)filename completionHandler:(void (^)(BOOL allowOverwrite, NSString *destination))completionHandler;
    3940- (void)_downloadDidFinish:(_WKDownload *)download;

  • trunk/Source/WebKit/UIProcess/API/glib/WebKitDownloadClient.cpp

    r250292 r261898  
    6969    }
    7070
    71     void didReceiveData(DownloadProxy& downloadProxy, uint64_t length) override
     71    void didReceiveData(DownloadProxy& downloadProxy, uint64_t length, uint64_t, uint64_t) override
    7272    {
    7373        GRefPtr<WebKitDownload> download = webkitWebContextGetOrCreateDownload(&downloadProxy);

  • trunk/Source/WebKit/UIProcess/Cocoa/DownloadClient.h

    r251778 r261898  
    5050    void didStart(DownloadProxy&) final;
    5151    void didReceiveResponse(DownloadProxy&, const WebCore::ResourceResponse&) final;
    52     void didReceiveData(DownloadProxy&, uint64_t length) final;
     52    void didReceiveData(DownloadProxy&, uint64_t, uint64_t, uint64_t) final;
    5353    void decideDestinationWithSuggestedFilename(DownloadProxy&, const String& suggestedFilename, Function<void(AllowOverwrite, String)>&&) final;
    5454    void didFinish(DownloadProxy&) final;
    … …  
    7575        bool downloadDidReceiveResponse : 1;
    7676        bool downloadDidReceiveData : 1;
     77        bool downloadDidWriteDataTotalBytesWrittenTotalBytesExpectedToWrite : 1;
    7778        bool downloadDecideDestinationWithSuggestedFilenameAllowOverwrite : 1;
    7879        bool downloadDecideDestinationWithSuggestedFilenameCompletionHandler : 1;

  • trunk/Source/WebKit/UIProcess/Cocoa/DownloadClient.mm

    r260707 r261898  
    5454    m_delegateMethods.downloadDidReceiveResponse = [delegate respondsToSelector:@selector(_download:didReceiveResponse:)];
    5555    m_delegateMethods.downloadDidReceiveData = [delegate respondsToSelector:@selector(_download:didReceiveData:)];
     56    m_delegateMethods.downloadDidWriteDataTotalBytesWrittenTotalBytesExpectedToWrite = [delegate respondsToSelector:@selector(_download:didWriteData:totalBytesWritten:totalBytesExpectedToWrite:)];
    5657    m_delegateMethods.downloadDecideDestinationWithSuggestedFilenameAllowOverwrite = [delegate respondsToSelector:@selector(_download:decideDestinationWithSuggestedFilename:allowOverwrite:)];
    5758    m_delegateMethods.downloadDecideDestinationWithSuggestedFilenameCompletionHandler = [delegate respondsToSelector:@selector(_download:decideDestinationWithSuggestedFilename:completionHandler:)];
    … …  
    9798#if USE(SYSTEM_PREVIEW)
    9899    if (downloadProxy.isSystemPreviewDownload() && response.isSuccessful()) {
    99         downloadProxy.setExpectedContentLength(response.expectedContentLength());
    100         downloadProxy.setBytesLoaded(0);
    101100        if (auto* controller = systemPreviewController(downloadProxy))
    102101            controller->updateProgress(0);
    … …  
    109108}
    110109
    111 void DownloadClient::didReceiveData(DownloadProxy& downloadProxy, uint64_t length)
    112 {
    113 #if USE(SYSTEM_PREVIEW)
    114     if (downloadProxy.isSystemPreviewDownload()) {
    115         downloadProxy.setBytesLoaded(downloadProxy.bytesLoaded() + length);
    116         if (auto* controller = systemPreviewController(downloadProxy))
    117             controller->updateProgress(static_cast<float>(downloadProxy.bytesLoaded()) / downloadProxy.expectedContentLength());
    118         return;
    119     }
    120 #endif
    121 
    122     if (m_delegateMethods.downloadDidReceiveData)
    123         [m_delegate _download:wrapper(downloadProxy) didReceiveData:length];
     110void DownloadClient::didReceiveData(DownloadProxy& downloadProxy, uint64_t bytesWritten, uint64_t totalBytesWritten, uint64_t totalBytesExpectedToWrite)
     111{
     112#if USE(SYSTEM_PREVIEW)
     113    if (downloadProxy.isSystemPreviewDownload()) {
     114        if (auto* controller = systemPreviewController(downloadProxy))
     115            controller->updateProgress(static_cast<float>(totalBytesWritten) / totalBytesExpectedToWrite);
     116        return;
     117    }
     118#endif
     119
     120    if (m_delegateMethods.downloadDidWriteDataTotalBytesWrittenTotalBytesExpectedToWrite)
     121        [m_delegate _download:wrapper(downloadProxy) didWriteData:bytesWritten totalBytesWritten:totalBytesWritten totalBytesExpectedToWrite:totalBytesExpectedToWrite];
     122    else if (m_delegateMethods.downloadDidReceiveData)
     123        [m_delegate _download:wrapper(downloadProxy) didReceiveData:bytesWritten];
    124124}
    125125

  • trunk/Source/WebKit/UIProcess/Downloads/DownloadProxy.cpp

    r258741 r261898  
    167167}
    168168
    169 void DownloadProxy::didReceiveData(uint64_t length)
    170 {
    171     if (!m_processPool)
    172         return;
    173 
    174     m_processPool->downloadClient().didReceiveData(*this, length);
     169void DownloadProxy::didReceiveData(uint64_t bytesWritten, uint64_t totalBytesWritten, uint64_t totalBytesExpectedToWrite)
     170{
     171    if (!m_processPool)
     172        return;
     173
     174    m_processPool->downloadClient().didReceiveData(*this, bytesWritten, totalBytesWritten, totalBytesExpectedToWrite);
    175175}
    176176

  • trunk/Source/WebKit/UIProcess/Downloads/DownloadProxy.h

    r255845 r261898  
    9090    void setDestinationFilename(const String& d) { m_destinationFilename = d; }
    9191
    92     uint64_t expectedContentLength() const { return m_expectedContentLength; }
    93     void setExpectedContentLength(uint64_t expectedContentLength) { m_expectedContentLength = expectedContentLength; }
    94 
    95     uint64_t bytesLoaded() const { return m_bytesLoaded; }
    96     void setBytesLoaded(uint64_t bytesLoaded) { m_bytesLoaded = bytesLoaded; }
    97 
    9892#if USE(SYSTEM_PREVIEW)
    9993    bool isSystemPreviewDownload() const { return request().isSystemPreview(); }
    … …  
    117111    void didReceiveAuthenticationChallenge(WebCore::AuthenticationChallenge&&, uint64_t challengeID);
    118112    void didReceiveResponse(const WebCore::ResourceResponse&);
    119     void didReceiveData(uint64_t length);
     113    void didReceiveData(uint64_t bytesWritten, uint64_t totalBytesWritten, uint64_t totalBytesExpectedToWrite);
    120114    void shouldDecodeSourceDataOfMIMEType(const String& mimeType, bool& result);
    121115    void didCreateDestination(const String& path);
    … …  
    135129    String m_suggestedFilename;
    136130    String m_destinationFilename;
    137     uint64_t m_expectedContentLength { 0 };
    138     uint64_t m_bytesLoaded { 0 };
    139131
    140132    WeakPtr<WebPageProxy> m_originatingPage;

  • trunk/Source/WebKit/UIProcess/Downloads/DownloadProxy.messages.in

    r246490 r261898  
    2828
    2929    DidReceiveResponse(WebCore::ResourceResponse response)
    30     DidReceiveData(uint64_t length)
     30    DidReceiveData(uint64_t bytesWritten, uint64_t totalBytesWritten, uint64_t totalBytesExpectedToWrite)
    3131    DidCreateDestination(String path)
    3232    DidFinish()

  • trunk/Tools/ChangeLog

    r261897 r261898  
     12020-05-19  Alex Christensen  <achristensen@webkit.org>
     2
     3        Add _WKDownloadDelegate callback including totalBytesWritten
     4        https://bugs.webkit.org/show_bug.cgi?id=212110
     5        <rdar://problem/63358981>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        * TestWebKitAPI/Tests/WebKitCocoa/Download.mm:
     10        (TEST):
     11        * TestWebKitAPI/cocoa/TestDownloadDelegate.h:
     12        * TestWebKitAPI/cocoa/TestDownloadDelegate.mm:
     13        (-[TestDownloadDelegate _download:didWriteData:totalBytesWritten:totalBytesExpectedToWrite:]):
     14        (-[TestDownloadDelegate _download:didReceiveData:]): Deleted.
     15
    1162020-05-19  Daniel Bates  <dabates@apple.com>
    217

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Download.mm

    r261762 r261898  
    12241224    };
    12251225
    1226     enum class Callback : uint8_t { Start, ReceiveData, DecideDestination, CreateDestination, Cancel, Finish };
     1226    enum class Callback : uint8_t { Start, WriteData, DecideDestination, CreateDestination, Cancel, Finish };
    12271227    __block Vector<Callback> callbacks;
    12281228    __block bool didCancel = false;
    … …  
    12371237        completionHandler(YES, [tempDir URLByAppendingPathComponent:suggestedFilename].path);
    12381238    };
    1239     downloadDelegate.get().didReceiveData = ^(_WKDownload *download, uint64_t length) {
    1240         callbacks.append(Callback::ReceiveData);
    1241         EXPECT_EQ(length, 5000u);
     1239    downloadDelegate.get().didWriteData = ^(_WKDownload *download, uint64_t bytesWritten, uint64_t totalBytesWritten, uint64_t totalBytesExpectedToWrite) {
     1240        callbacks.append(Callback::WriteData);
     1241        EXPECT_EQ(bytesWritten, 5000u);
     1242        EXPECT_EQ(totalBytesWritten, didCancel ? 10000u : 5000u);
     1243        EXPECT_EQ(totalBytesExpectedToWrite, 10000u);
    12421244        receivedData = true;
    12431245    };
    … …  
    12751277    EXPECT_EQ(callbacks[1], Callback::DecideDestination);
    12761278    EXPECT_EQ(callbacks[2], Callback::CreateDestination);
    1277     EXPECT_EQ(callbacks[3], Callback::ReceiveData);
     1279    EXPECT_EQ(callbacks[3], Callback::WriteData);
    12781280    EXPECT_EQ(callbacks[4], Callback::Cancel);
    1279     EXPECT_EQ(callbacks[5], Callback::ReceiveData);
     1281    EXPECT_EQ(callbacks[5], Callback::WriteData);
    12801282    EXPECT_EQ(callbacks[6], Callback::Finish);
    12811283

  • trunk/Tools/TestWebKitAPI/cocoa/TestDownloadDelegate.h

    r260518 r261898  
    3131@property (nonatomic, copy) void (^didReceiveServerRedirectToURL)(_WKDownload *, NSURL *);
    3232@property (nonatomic, copy) void (^didReceiveResponse)(_WKDownload *, NSURLResponse *);
    33 @property (nonatomic, copy) void (^didReceiveData)(_WKDownload *, uint64_t);
     33@property (nonatomic, copy) void (^didWriteData)(_WKDownload *, uint64_t, uint64_t, uint64_t);
    3434@property (nonatomic, copy) void (^decideDestinationWithSuggestedFilename)(_WKDownload *, NSString *, void (^)(BOOL, NSString *));
    3535@property (nonatomic, copy) void (^downloadDidFinish)(_WKDownload *);

  • trunk/Tools/TestWebKitAPI/cocoa/TestDownloadDelegate.mm

    r260518 r261898  
    4747}
    4848
    49 - (void)_download:(_WKDownload *)download didReceiveData:(uint64_t)length
     49- (void)_download:(_WKDownload *)download didWriteData:(uint64_t)bytesWritten totalBytesWritten:(uint64_t)totalBytesWritten totalBytesExpectedToWrite:(uint64_t)totalBytesExpectedToWrite
    5050{
    51     if (_didReceiveData)
    52         _didReceiveData(download, length);
     51    if (_didWriteData)
     52        _didWriteData(download, bytesWritten, totalBytesWritten, totalBytesExpectedToWrite);
    5353}
    5454
Note: See TracChangeset for help on using the changeset viewer.