Moving XMLHttpRequest to modules

[Yutaka Hirano]

Hi,

I'm about to start implementing the Streams API[1] and wondering where to place the source code.

It seems reasonable to me to put it on modules/streams, but there is one problem:

XMLHttpRequest, located on core/xml now, is expected to depend on Streams in the future.

There are two options:

 A: Place streams on core/streams

 B: Move XMLHttpRequest to modules/xmlhttprequest

I believe B is the right way. Is that correct from the modularity point of view?

Currently core/inspector depends on core/xml/XMLHttpRequest*.

Is there any concern from inspector developers to remove the dependency?

Thanks, 

1: https://groups.google.com/a/chromium.org/forum/#!searchin/blink-dev/Intent$20to$20implement$20Streams/blink-dev/cCPJTZbnjCw/9vFx8DHjjAEJ

[Takeshi Yoshino]

Some of you may find Stream.idl under core/fileapi. Never mind about that.

I chose the directory when adding the initial implementation of the Streams API because at that point the Streams API was kinda new Blob with different limitation and features. Now, the Streams API has been more generalized. It no longer makes much sense for it to live in the fileapi directory.

[Anne van Kesteren]

On Mon, Jul 14, 2014 at 8:53 AM, Yutaka Hirano <yhi...@chromium.org> wrote:
> XMLHttpRequest, located on core/xml now, is expected to depend on Streams in
> the future.

Given http://fetch.spec.whatwg.org/#fetch-api I think we should start
looking into adding new fetch-layer features there rather than
XMLHttpRequest. In particular, fetch() is assuming we'll have streams
at some point and the API even defines a future subclass of streams.


--
http://annevankesteren.nl/

[Takeshi Yoshino]

Can we assume that the Fetch API spec is already maintained considering use in non-SW world well enough so that it's fine to be implemented and exposed?

This line looks implying that:

> Window implements GlobalFetch

[Kentaro Hara]

From the perspective of modularization, I like the idea of moving XMLHttpRequest to modules/. It's nice to factor out more contents from core/ to modules/.

 

Currently core/inspector depends on core/xml/XMLHttpRequest*.
Is there any concern from inspector developers to remove the dependency?

tasak@ is now working on binding componentization, and once it's done, core/ cannot depend on modules/ basically. However, it's still possible for core/ to call methods in modules/ by using ModuleProxy (ModuleProxy is a hack to violate the dependency graciously). Thus, as long as the number of cross-dependency methods is limited, the dependency from core/ to modules/ is no problem.

On Mon, Jul 14, 2014 at 3:53 PM, Yutaka Hirano <yhi...@chromium.org> wrote:

--
Kentaro Hara, Tokyo, Japan

[Anne van Kesteren]

On Mon, Jul 14, 2014 at 9:40 AM, Takeshi Yoshino <tyos...@chromium.org> wrote:
> Can we assume that the Fetch API spec is already maintained considering use
> in non-SW world well enough so that it's fine to be implemented and exposed?

There are a couple of outstanding issues on exactly what kind of
headers we want to include. And whether caching should be automatic or
not, see the service workers repository. We can only get it more
stable by implementing and figuring out if the setup works I think. I
would prefer if we not ship for a couple of months to make sure we all
agree on the details of the API and what happens at the networking
layer, but getting something in the hands of developers would be
great.


--
http://annevankesteren.nl/

[Takeshi Yoshino]

Thanks. I think we should have less and less interfaces to do the same thing in the future. In that context, I like the idea of stopping adding new features to XHR. OTOH, people are waiting for streamed resource fetching, so we don't want to delay delivery of it for so long.

On Mon, Jul 14, 2014 at 4:52 PM, Anne van Kesteren <ann...@annevk.nl> wrote:

On Mon, Jul 14, 2014 at 9:40 AM, Takeshi Yoshino <tyos...@chromium.org> wrote:
> Can we assume that the Fetch API spec is already maintained considering use
> in non-SW world well enough so that it's fine to be implemented and exposed?

There are a couple of outstanding issues on exactly what kind of
headers we want to include. And whether caching should be automatic or
not, see the service workers repository. We can only get it more
stable by implementing and figuring out if the setup works I think. I

OK

 

would prefer if we not ship for a couple of months to make sure we all
agree on the details of the API and what happens at the networking
layer, but getting something in the hands of developers would be
great.

So, at some point in the future, we'll expose it to window, and to do so we'll move fetch(), Request and Response out of modules/serviceworkers. Maybe the new directory would be modules/fetch or something though it's a little confusing that we'll have core/fetch and modules/fetch.

EventSource is now living in core/page. Regardless of whether we'll extend XHR with Streams or not, it seems ideally we should have all of EventSource, XHR and Fetch API in the directory.

modules/fetch

- EventSource.*

- XMLHttpRequest.*

- Fetch.* except for FetchEvent

- Response

- Request

modules/streams

- Streams API implementation

modules/fetch depends on core/loader and modules/streams

modules/serviceworkers depends on modules/fetch.

[Adam Barth]

You question seems to boil down to whether we should have dependencies between modules.  We recently introduced our first inter-module dependency (that I'm aware of) so that push_messaging could depend on serviceworker.

I'm a little scared about having cross-module dependencies because one of the goals of modules is to make the system more loosely coupled.  Dependencies between modules can lead to tighter coupling.  IMHO, we should see how the push_messaging/serviceworker dependency works out before adding more cross-module dependencies.

From the dependencies you've listed, it sounds like |streams| should be in core because it's depended upon by both modules/fetch and modules/serviceworker.

w.r.t. inspector depending on XMLHttpRequest, we should likely generalize a lot of that code to handle other sorts of script-based network APIs (e.g., EventSource).  However, until we do that, you'll need to keep XMLHttpRequest in core other you'll introduce a core -> modules dependency that will prevent blink_core.dll from linking (once we create it).

Adam

[Takeshi Yoshino]

On Tue, Jul 15, 2014 at 1:02 AM, Adam Barth <aba...@chromium.org> wrote:

You question seems to boil down to whether we should have dependencies between modules.  We recently introduced our first inter-module dependency (that I'm aware of) so that push_messaging could depend on serviceworker.

I'm a little scared about having cross-module dependencies because one of the goals of modules is to make the system more loosely coupled.  Dependencies between modules can lead to tighter coupling.  IMHO, we should see how the push_messaging/serviceworker dependency works out before adding more cross-module dependencies.

From the dependencies you've listed, it sounds like |streams| should be in core because it's depended upon by both modules/fetch and modules/serviceworker.

I see. I think several other web platform features are also going to depend on |streams| in addition to them in the future. So, following the approach you suggested, I agree we should place |streams| in core/.

 

w.r.t. inspector depending on XMLHttpRequest, we should likely generalize a lot of that code to handle other sorts of script-based network APIs (e.g., EventSource).  However, until we do that, you'll need to keep XMLHttpRequest in core other you'll introduce a core -> modules dependency that will prevent blink_core.dll from linking (once we create it).

Yutaka is looking at the inspector -> xhr dependency. Yes, it seems we can generalize the hooks that are now defined specifically for XHR.

So, initially:

- We keep XHR in core/xml

- And add |streams| to core/streams

and later as refactoring is done and see the result of cross-module dependency, we could reorganize them.

One non trivial question is whether we really want to decouple fetch API code from modules/serviceworkers. I thought we want to and the right place would be some sub directory in core/ as we don't want to introduce cross-module dependencies (core/fetch is irrelative to the Fetch concept (http://src.chromium.org/viewvc/blink?view=revision&revision=155987), so some different name...). Thoughts?

[Adam Barth]

On Mon Jul 14 2014 at 8:58:51 PM, Takeshi Yoshino <tyos...@chromium.org> wrote:

On Tue, Jul 15, 2014 at 1:02 AM, Adam Barth <aba...@chromium.org> wrote:

You question seems to boil down to whether we should have dependencies between modules.  We recently introduced our first inter-module dependency (that I'm aware of) so that push_messaging could depend on serviceworker.

I'm a little scared about having cross-module dependencies because one of the goals of modules is to make the system more loosely coupled.  Dependencies between modules can lead to tighter coupling.  IMHO, we should see how the push_messaging/serviceworker dependency works out before adding more cross-module dependencies.

From the dependencies you've listed, it sounds like |streams| should be in core because it's depended upon by both modules/fetch and modules/serviceworker.

I see. I think several other web platform features are also going to depend on |streams| in addition to them in the future. So, following the approach you suggested, I agree we should place |streams| in core/.

Sounds like streams will become a core platform feature like Blob.

 

w.r.t. inspector depending on XMLHttpRequest, we should likely generalize a lot of that code to handle other sorts of script-based network APIs (e.g., EventSource).  However, until we do that, you'll need to keep XMLHttpRequest in core other you'll introduce a core -> modules dependency that will prevent blink_core.dll from linking (once we create it).

Yutaka is looking at the inspector -> xhr dependency. Yes, it seems we can generalize the hooks that are now defined specifically for XHR.

So, initially:

- We keep XHR in core/xml

- And add |streams| to core/streams

and later as refactoring is done and see the result of cross-module dependency, we could reorganize them.

One non trivial question is whether we really want to decouple fetch API code from modules/serviceworkers. I thought we want to and the right place would be some sub directory in core/ as we don't want to introduce cross-module dependencies (core/fetch is irrelative to the Fetch concept (http://src.chromium.org/viewvc/blink?view=revision&revision=155987), so some different name...). Thoughts?

You should be able to add APIs to serviceworkers without introducing a dependency from serviceworkers to the definition of the API, much in the same way you can add APIs to Window or Navigator without Window or Navigator depending on the API.

[Yutaka Hirano]

Thank you for comments!

I see. I will create core/streams.

If I can eliminate the dependency to XHR from core/inspector, I might move XHR classes to modules/fetch or modules/xmlhttprequest, but that doesn't block the Streams implementation.

[b.ke...@samsung.com]

On Monday, July 14, 2014 12:02:58 PM UTC-4, Adam Barth wrote:

You question seems to boil down to whether we should have dependencies between modules.  We recently introduced our first inter-module dependency (that I'm aware of) so that push_messaging could depend on serviceworker.

I'm a little scared about having cross-module dependencies because one of the goals of modules is to make the system more loosely coupled.  Dependencies between modules can lead to tighter coupling.  IMHO, we should see how the push_messaging/serviceworker dependency works out before adding more cross-module dependencies.

From the dependencies you've listed, it sounds like |streams| should be in core because it's depended upon by both modules/fetch and modules/serviceworker.

But than core cannot depend on modules, so isn't it rather a counter argument?

[Adam Barth]

On Tue Jul 15 2014 at 10:15:35 PM, <b.ke...@samsung.com> wrote:

On Monday, July 14, 2014 12:02:58 PM UTC-4, Adam Barth wrote:

You question seems to boil down to whether we should have dependencies between modules.  We recently introduced our first inter-module dependency (that I'm aware of) so that push_messaging could depend on serviceworker.

I'm a little scared about having cross-module dependencies because one of the goals of modules is to make the system more loosely coupled.  Dependencies between modules can lead to tighter coupling.  IMHO, we should see how the push_messaging/serviceworker dependency works out before adding more cross-module dependencies.

From the dependencies you've listed, it sounds like |streams| should be in core because it's depended upon by both modules/fetch and modules/serviceworker.

But than core cannot depend on modules, so isn't it rather a counter argument?

I'm not sure I understand.  If |streams| is in core, what causes a core -> modules dependency?

Adam

[b.ke...@samsung.com]

You said "|streams| should be in core because it's depended upon by both modules/fetch and modules/serviceworker".

So I wonder how will streams be able to depend on those modules if it's in core?

 

Adam

[Adam Barth]

Oh, I meant "depended upon by" to be the opposite.  Those modules depend on streams, not the other way around.

Adam

[b.ke...@samsung.com]

Got it, my bad.