Bug 203962

Summary: Make DownloadID an ObjectIdentifier
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, ggaren, jbedard, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch ggaren: review+

Description Alex Christensen 2019-11-07 09:30:35 PST
Make DownloadID an ObjectIdentifier
Comment 1 Alex Christensen 2019-11-07 09:33:09 PST
Created attachment 383057 [details]
Patch
Comment 2 Chris Dumez 2019-11-07 10:20:39 PST
Looks like there are some build issues.
Comment 3 Alex Christensen 2019-11-07 14:27:50 PST
Created attachment 383072 [details]
Patch
Comment 4 youenn fablet 2019-11-08 06:47:23 PST
Comment on attachment 383072 [details]
Patch

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

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:1970
> +    downloadProxyConnection()->send(Messages::DownloadProxy::DidCancel({ }), downloadID.toUInt64());

We can probably remove toUInt64() here and in every send() call site below.

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:1975
> +    uint64_t destinationID = networkDataTask.pendingDownloadID().toUInt64();

auto destinationID = networkDataTask.pendingDownloadID(); or directly inline the call where used.
Comment 5 Alex Christensen 2019-11-08 13:35:05 PST
http://trac.webkit.org/r252260
Comment 6 Radar WebKit Bug Importer 2019-11-08 13:36:16 PST
<rdar://problem/57032761>
Comment 7 Jonathan Bedard 2019-11-08 16:20:06 PST
Reverted r252260 for reason:

Breaks half of API tests

Committed r252274: <https://trac.webkit.org/changeset/252274>
Comment 8 Jonathan Bedard 2019-11-08 16:21:02 PST
My rollout was a bit blind, but given how conclusive https://results.webkit.org/suites?suite=api-tests was, I thought justified. Seems like this breaks Debug API tests
Comment 9 Alex Christensen 2020-10-05 14:34:13 PDT
That was a case of sending a default-constructed DownloadID instead of using an Optional<DownloadID> and sending nullopt.  Will fix and rebase.
Comment 10 Alex Christensen 2020-10-05 14:35:07 PDT
Created attachment 410560 [details]
Patch
Comment 11 Alex Christensen 2020-10-05 14:36:22 PDT
Created attachment 410562 [details]
Patch
Comment 12 Alex Christensen 2020-10-05 15:26:02 PDT
Created attachment 410569 [details]
Patch
Comment 13 Alex Christensen 2020-10-05 15:42:11 PDT
Created attachment 410573 [details]
Patch
Comment 14 Alex Christensen 2020-10-05 16:31:12 PDT
Created attachment 410585 [details]
Patch
Comment 15 Geoffrey Garen 2020-10-05 16:36:03 PDT
Comment on attachment 410585 [details]
Patch

r=me
Comment 16 Alex Christensen 2020-10-05 16:53:46 PDT
http://trac.webkit.org/r268017