Bug 39204 – schedule/unscheduleAll API in ResourceHandleMac

• Component: WebCore Misc.
• Status: RESOLVED INVALID

[Philippe Normand]

For the GStreamer support in mac os, the WebKitWebSource element needs to pause/unpause the HTTP download depending on the status of buffered/consumed data in the pipeline. So we'd need 2 new methods in ResourceHandleMac to schedule or unschedule all the current scheduledPairs at once.

[Philippe Normand]

Created attachment 56220 [details]
proposed patch

[Darin Adler]

Comment on attachment 56220 [details]
proposed patch

It's a very bad idea to add a Page pointer to the ResourceHandle class. This goes against the layering, where the resource handle is the low level mechanism in the platform layer that is not tied to the page loading mechanism. We should look for another way to do this. I need to know more about the big picture here.

[Philippe Normand]

Thanks Darin for looking at the patch.

Would it be ok to pass the Page pointer in parameter of the new methods then? This would work for my use-case as well.

About the big picture, I tried to explain it in the bug description but it wasn't probably clear enough :)

The gstreamer framework is based on the concept of pipelines where data flow between elements (source elements, demuxers, decoders, sink elements,...). Our pipeline uses a custom source element that does the HTTP data downloading with the ResourceHandle "low level" API. It has a custom ResourceHandleClient. The element is using appsrc [1]. Appsrc has an internal queue that manages the data buffering. When it's full appsrc will ask the application (ResourceHandle) to 
pause the download because it has enough data for now to feed the pipeline. When the queue reaches a low level appsrc will ask ResourceHandle to resume the download so the queue can fill again.

The source code of this element is in WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp

[1] http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/gst-plugins-base-libs-appsrc.html

[Philippe Normand]

Darin can you please reply to comment 3? Thanks!

[Darin Adler]

(In reply to comment #3)
> Would it be ok to pass the Page pointer in parameter of the new methods then? This would work for my use-case as well.

Not really OK. The layering issue is that things in the platform directory can't know about things at a higher level at all. So they can’t even mention Page in their definitions.

We have to think about how to structure this so the higher level loader can set up these resource handles appropriately, but the whole point of the ResourceHandle abstraction is a low level one that does not involve things like Frame and Page.

I don’t immediately know what the right solution is. Maciej Stachoiwak might have some ideas. He’s currently on vacation. Since the platform directory was originally conceived in its current form by Hyatt, he might have ideas too.

[Philippe Normand]

Is Maciej back? Hyatt? ;)

Another thing I can do without touching the ResourceHandleMac code is to get the SchedulePairs and internal connection instance from my side and do the scheduling/unscheduling there. This feels nasty but could maybe be acceptable in the short term. Thoughts?

[Nikolas Zimmermann]

Comment on attachment 56220 [details]
proposed patch

r-, because of the layering violation.

I have no clue about this particular part of the code, but you could avoid passing Page, by just querying Page* from the call site, and pass in a SchedulePairHashSet* pointer to your scheduleAll/unscheduleAll functions.
Not sure if this design is good at all, other reviewers could comment better than me.

[Philippe Normand]

It seems using  ResourceHandle::setDefersLoading in the WebKitWebSource gstreamer element is a better approach. See bug 44157