Closed Bug 1359718 Opened 3 years ago Closed 3 years ago

Get rid of PBlob

Categories

(Core :: DOM: File, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: baku, Assigned: baku)

References

Details

Attachments

(7 files)

No description provided.
This is the last bit of patches for getting rid of PBlob completely in our code base.
It can land only after the update of IndexedDB and FileHandle to IPCBlob.
Blocks: 1353629
Assignee: nobody → amarchesini
Attachment #8861784 - Flags: review?(bugs)
Attachment #8861785 - Flags: review?(bugs)
Attached patch part 2 - PBlobSplinter Review
Attachment #8861786 - Flags: review?(bugs)
Attachment #8861787 - Flags: review?(bugs)
Attachment #8861788 - Flags: review?(bugs)
Attachment #8861789 - Flags: review?(bugs)
Better to wait for some time before landing in case IPCBlob stuff causes some major issues and need to be backed out?
Definitely. This needs FileHandle and IndexedDB landed first.
Attachment #8861788 - Flags: review?(bugs) → review+
Attachment #8861789 - Flags: review?(bugs) → review+
Attachment #8861787 - Flags: review?(bugs) → review+
Attachment #8861784 - Flags: review?(bugs) → review+
Comment on attachment 8861785 [details] [diff] [review]
part 1 - PBlobStream

rs+
Attachment #8861785 - Flags: review?(bugs) → review+
Comment on attachment 8861786 [details] [diff] [review]
part 2 - PBlob

"// Thi is broken!"

I hope that is removed in some patch.

rs+
Attachment #8861786 - Flags: review?(bugs) → review+
Comment on attachment 8861786 [details] [diff] [review]
part 2 - PBlob

Apparently it isn't.

Please explain, and make that assert or at least fix the comment.
Attachment #8861786 - Flags: review+ → review-
Component: DOM → DOM: File
Duplicate of this bug: 1349685
Attachment #8870297 - Flags: review?(bugs) → review+
Attachment #8861786 - Flags: review?(kchen)
Attachment #8861785 - Flags: review?(kchen)
Comment on attachment 8861786 [details] [diff] [review]
part 2 - PBlob

Review of attachment 8861786 [details] [diff] [review]:
-----------------------------------------------------------------

r+ for removing PBlob from sync-messages.ini
Attachment #8861786 - Flags: review?(kchen) → review+
Comment on attachment 8861785 [details] [diff] [review]
part 1 - PBlobStream

Review of attachment 8861785 [details] [diff] [review]:
-----------------------------------------------------------------

r+ for removing PBlobStream from sync-messages.ini
Attachment #8861785 - Flags: review?(kchen) → review+
Duplicate of this bug: 1350644
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c92612ac17d3
Get rid of PBlob - part 0 - remove PMemoryStream, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b7d055b4207
Get rid of PBlob - part 1 - Remove PBlobStream, r=smaug, r=kanru
https://hg.mozilla.org/integration/mozilla-inbound/rev/537abc431472
Get rid of PBlob - part 2 - PBlob, r=smaug, r=kanru
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ded99aa9c74
Get rid of PBlob - part 3 - nsIRemoteBlob, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/5684176c7662
Get rid of PBlob - part 4 - PBlob DOMTypes, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/58cc2991c47a
Get rid of PBlob - part 5 - Fixing #includes, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/089b1233b9a3
Get rid of PBlob - part 6 - removed unused inputStream params, r=smaug
Backed out for bustage in IPCBlobInputStreamChild.cpp:

https://hg.mozilla.org/integration/mozilla-inbound/rev/7e95c3cac6119326cbbf1cfe4d6bef70423348f1
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f995013f5e180b48a684a053765ac75c356407c
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ca20971f1e02a77ed4a60373c65d8fbd047a24e
https://hg.mozilla.org/integration/mozilla-inbound/rev/a82e7819af64042f6dc9838cf598bffe8f83933d
https://hg.mozilla.org/integration/mozilla-inbound/rev/563b66cb587c0ab304f1bdcd7f66d415de16e22f
https://hg.mozilla.org/integration/mozilla-inbound/rev/da70b41fc27a28556e65e6aecf8b59c64332b4bf
https://hg.mozilla.org/integration/mozilla-inbound/rev/7433cd779fc3b698c5e328b744bd4e14be65bea5

Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=089b1233b9a3b853f3d96668e2ac622a844365f3&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=101258890&repo=mozilla-inbound

[task 2017-05-23T16:20:58.309567Z] 16:20:58     INFO -  In file included from /home/worker/workspace/build/src/obj-firefox/dom/file/ipc/Unified_cpp_dom_file_ipc0.cpp:11:0:
[task 2017-05-23T16:20:58.310369Z] 16:20:58     INFO -  /home/worker/workspace/build/src/dom/file/ipc/IPCBlobInputStreamChild.cpp:86:1: error: expected class-name before '{' token
[task 2017-05-23T16:20:58.310417Z] 16:20:58     INFO -   {
[task 2017-05-23T16:20:58.310453Z] 16:20:58     INFO -   ^
[task 2017-05-23T16:20:58.310548Z] 16:20:58     INFO -  /home/worker/workspace/build/src/dom/file/ipc/IPCBlobInputStreamChild.cpp:92:15: error: 'Status' has not been declared
[task 2017-05-23T16:20:58.310968Z] 16:20:58     INFO -     bool Notify(Status aStatus) override
[task 2017-05-23T16:20:58.311015Z] 16:20:58     INFO -                 ^
[task 2017-05-23T16:20:58.311306Z] 16:20:58     INFO -  /home/worker/workspace/build/src/dom/file/ipc/IPCBlobInputStreamChild.cpp:92:8: error: 'bool mozilla::dom::{anonymous}::IPCBlobInputStreamWorkerHolder::Notify(int)' marked override, but does not override
[task 2017-05-23T16:20:58.311597Z] 16:20:58     INFO -     bool Notify(Status aStatus) override
[task 2017-05-23T16:20:58.311716Z] 16:20:58     INFO -          ^
[task 2017-05-23T16:20:58.312156Z] 16:20:58     INFO -  /home/worker/workspace/build/src/dom/file/ipc/IPCBlobInputStreamChild.cpp: In member function 'bool mozilla::dom::{anonymous}::IPCBlobInputStreamWorkerHolder::Notify(int)':
[task 2017-05-23T16:20:58.312288Z] 16:20:58     INFO -  /home/worker/workspace/build/src/dom/file/ipc/IPCBlobInputStreamChild.cpp:94:19: error: 'Running' was not declared in this scope
...
Flags: needinfo?(amarchesini)
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ac94ff745fc
Get rid of PBlob - part 0 - remove PMemoryStream, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/48ed92a3d18a
Get rid of PBlob - part 1 - Remove PBlobStream, r=smaug, r=kanru
https://hg.mozilla.org/integration/mozilla-inbound/rev/c3251f5022dd
Get rid of PBlob - part 2 - PBlob, r=smaug, r=kanru
https://hg.mozilla.org/integration/mozilla-inbound/rev/b21dc429b5bd
Get rid of PBlob - part 3 - nsIRemoteBlob, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b6c07b71ba4
Get rid of PBlob - part 4 - PBlob DOMTypes, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a2c16255a2c
Get rid of PBlob - part 5 - Fixing #includes, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/e56d1ca34efd
Get rid of PBlob - part 6 - removed unused inputStream params, r=smaug
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/247354884a9a
Get rid of PBlob - part 7 - fixing namespaces, r=me
Backed out for bustage in IPCBlobInputStreamChild.cpp:

https://hg.mozilla.org/integration/mozilla-inbound/rev/d1d2ba65371d0d281dd8070d96c1c856eb9b109f
https://hg.mozilla.org/integration/mozilla-inbound/rev/9aee049bca9526c2ab10ebebc2dab40055786441
https://hg.mozilla.org/integration/mozilla-inbound/rev/6567ca412e0eaa54bb64b781e01fcb22361d49cb
https://hg.mozilla.org/integration/mozilla-inbound/rev/2532664f2c94fa4099f621c79632dc33b107b222
https://hg.mozilla.org/integration/mozilla-inbound/rev/412728e28e7fca3e9bd6748bf97008a911364b4e
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a5f624b597cc8c19f490267f6a0240dd7d9fbe2
https://hg.mozilla.org/integration/mozilla-inbound/rev/207ead78774dd800dd97d40ed3bba7fb186b5b63
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1f4657102382a13c3ff18d4313397771fbf8b1a

Please run the updated patches on Try.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=247354884a9a77e6e07f1b2dce575b530dbc73ed&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=101265026&repo=mozilla-inbound

[task 2017-05-23T17:01:55.309798Z] 17:01:55     INFO -  In file included from /home/worker/workspace/build/src/obj-firefox/dom/file/ipc/Unified_cpp_dom_file_ipc0.cpp:11:0:
[task 2017-05-23T17:01:55.309906Z] 17:01:55     INFO -  /home/worker/workspace/build/src/dom/file/ipc/IPCBlobInputStreamChild.cpp:14:17: error: 'workers' is not a namespace-name
[task 2017-05-23T17:01:55.309941Z] 17:01:55     INFO -   using namespace workers;
[task 2017-05-23T17:01:55.309967Z] 17:01:55     INFO -                   ^
[task 2017-05-23T17:01:55.310022Z] 17:01:55     INFO -  /home/worker/workspace/build/src/dom/file/ipc/IPCBlobInputStreamChild.cpp:14:24: error: expected namespace-name before ';' token
[task 2017-05-23T17:01:55.310051Z] 17:01:55     INFO -   using namespace workers;
[task 2017-05-23T17:01:55.317539Z] 17:01:55     INFO -                          ^
...
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/50416ab469e7
Get rid of PBlob - part 0 - remove PMemoryStream, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/422e67a8d2b7
Get rid of PBlob - part 1 - Remove PBlobStream, r=smaug, r=kanru
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f03e06b92f3
Get rid of PBlob - part 2 - PBlob, r=smaug, r=kanru
https://hg.mozilla.org/integration/mozilla-inbound/rev/34920e954770
Get rid of PBlob - part 3 - nsIRemoteBlob, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/45a310164848
Get rid of PBlob - part 4 - PBlob DOMTypes, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/7114f6c81f54
Get rid of PBlob - part 5 - Fixing #includes, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c945729fadb
Get rid of PBlob - part 6 - removed unused inputStream params, r=smaug
Flags: needinfo?(amarchesini)
As an FYI, this change increased the compiler warning count:

== Change summary for alert #6827 (as of May 24 2017 05:24 UTC) ==

Regressions:

  1%  compiler warnings summary linux32 debug      454.00 -> 459.00
  1%  compiler warnings summary linux64-stylo debug 458.00 -> 463.00
  1%  compiler warnings summary linux64 debug      458.00 -> 463.00

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=6827
You need to log in before you can comment on or make changes to this bug.