Remove the separate worker bindings for URL

RESOLVED FIXED in Firefox 50

Status

()

Core
DOM: Workers
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: khuey, Assigned: baku)

Tracking

(Blocks: 1 bug)

unspecified
mozilla50
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

(Whiteboard: btpp-backlog)

Attachments

(1 attachment)

We'll probably need to introduce some sort of abstract base class here.
Whiteboard: btpp-backlog
(Assignee)

Updated

2 years ago
Assignee: nobody → amarchesini
(Assignee)

Comment 1

2 years ago
Created attachment 8748637 [details] [diff] [review]
url.patch
Attachment #8748637 - Flags: review?(ehsan)

Comment 2

2 years ago
I started reviewing this today but ran out of time.  The patch is very large and it contains a lot of things that look like just moving the code around.  Can you please indicate what things have moved without changing and what things have actually changed?  That should help simplify reviewing this.

(Also sorry for the delay, I'm trying hard to finish reviewing it this week!)
(Assignee)

Comment 3

2 years ago
What this patch does is:

1. It creates a base class for URL (dom/base/URL.h)

2. in dom/base/URL.cpp we have 2 implementations of the base class: URLMainThread and URLWorker.
URLMainThread is the old URL class. Code is moved around. Plus assertions about main-thread only execution.

3. URLWorker is a new class (inheriting URL base class) that is the old dom/worker/URL.cpp code moved here.

Before, in workers, the setup was based on a proxy. There was a URLProxy object that was creating a URL object on the main-thread and then, any getter and setter call was sent to that main-thread object.

Now we have the same proxy that creates a URLMainThread object.
(Assignee)

Comment 4

2 years ago
:ehsan, ping?
Comment on attachment 8748637 [details] [diff] [review]
url.patch

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

::: dom/webidl/URL.webidl
@@ +28,1 @@
>    attribute USVString href;

If the SetterThrows, and the GetterThrows, then just Throws?
Attachment #8748637 - Flags: review?(ehsan) → review+
(Assignee)

Updated

2 years ago
Blocks: 1282852

Comment 6

2 years ago
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/81c91e482d44
Remove the separate worker bindings for URL, r=khuey

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/81c91e482d44
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
as  a note this reduced the num_constructors reported from the build:
https://treeherder.mozilla.org/perf.html#/alerts?id=1626
You need to log in before you can comment on or make changes to this bug.