Redirect HTML resource bytes directly to parser thread (Blink side CL)

public/platform/WebThreadedResourceProvider.h

 /*
- * Copyright (C) 2013 Google Inc.  All rights reserved.
+ * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
  * copyright notice, this list of conditions and the following disclaimer
  * in the documentation and/or other materials provided with the
  * distribution.
  *     * Neither the name of Google Inc. nor the names of its
  * contributors may be used to endorse or promote products derived from
  * this software without specific prior written permission.
  *
  * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
  * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
  * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
  * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
  * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
  * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
  * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
  * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
  * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef HTMLParserThread_h
-#define HTMLParserThread_h
+#ifndef WebThreadedResourceProvider_h
+#define WebThreadedResourceProvider_h
 
-#include "wtf/Functional.h"
-#include "wtf/OwnPtr.h"
-#include "public/platform/WebThread.h"
+namespace blink {
 
-namespace WebCore {
+class WebThread;
 
-class HTMLParserThread {
+class WebThreadedResourceBackgroundClient {

[darin (slow to review), 2014/02/16 07:12:27]

nit: The rule for the Blink public API is one file per type.

 public:

[darin (slow to review), 2014/02/16 07:12:27]

this interface should have a virtual destructor too?

-    static void init();
-    static void shutdown();
-
-    // It is an error to call shared() before init() or after shutdown();
-    static HTMLParserThread* shared();
-
-    void postTask(const Closure&);
-
-private:
-    HTMLParserThread();
-    ~HTMLParserThread();
-
-    blink::WebThread& ensureThread();
-
-    OwnPtr<blink::WebThread> m_thread;
+    virtual void didReceivedData(const char* data, size_t length) = 0;
 };
 
-} // namespace WebCore
+class WebThreadedResourceForegroundClient {
+public:
+    virtual void didSwitchedToBackgroundClient() = 0;

[darin (slow to review), 2014/02/16 07:12:27]

This function could also be part of WebURLLoaderClient. In other words, maybe it
would be better to recast this CL as a mechanism to allow the WebURLLoader user
to attach a data sink that should be fed on a user specified WebThread. that
suggests that communication on the main thread can remain on WebURLLoaderClient,
and then all you really need to spec is the equivalent of
WebThreadedResourceBackgroundClient. I suspect you don't really even need a
WebThreadedResourceProvider interface.

We might even consider a generic interface like this:

  class WebDataReceiver {
   public:
    virtual void acceptData(const char* data, int dataLength) = 0;
  };

(^^^ note: the above uses |int| instead of |size_t| for consistency with
WebURLLoaderClient::didReceiveData, but I could see the case for switching all
of these byte count parameters to size_t. That might be better done in a
different CL.)

Then, you could probably just go with a method on WebURLLoader to set a
WebDataReceiver. I'd argue that you should prescribe that a WebDataReceiver may
only be set prior to the first call to didReceiveData.

[oystein (OOO til 10th of July), 2014/02/20 18:43:08]

Folding this into WebURLLoaderClient to make it more generic makes sense to me;
I'm taking a stab at this now.
I think the last part there is something we should work towards, but it may be a
stretch to do it as part of this CL as so much of the setup we do of the parsing
and document writing is done on-demand when the first data packet arrives,
rather than as a result of the HTTP response (as far as I can tell). It does
mean we have to do a bit of a song'n'dance to make sure we process the data in
the right order on the background thread (like this CL does in
BackgroundHTMLParser.cpp), but maybe I can move that around so that the logic
there is contained somewhere centrally and doesn't need to be re-done for each
other resource type which would reuse this system. Sounds reasonable?

 
-#endif // HTMLParserThread_h
+    virtual ~WebThreadedResourceForegroundClient() { }
+};
+
+class WebThreadedResourceProvider {

[darin (slow to review), 2014/02/16 07:12:27]

I think it would be helpful to provide some documentation about what a
foreground and background client is. It is quite mysterious if you are only
reading this header file.

+public:
+    virtual void setBackgroundClient(WebThreadedResourceBackgroundClient*) = 0;
+    virtual void setForegroundClient(WebThreadedResourceForegroundClient*) = 0;
+    virtual WebThread* resourceProviderThread() = 0;

[darin (slow to review), 2014/02/16 07:12:27]

I have a feeling that the resourceProviderThread is something that doesn't vary
much, yet it is a virtual function, which means it can vary. Maybe it should be
pushed to Blink instead of being queried by Blink? Or, maybe Blink could
allocate it using Platform::createThread?

+
+    virtual ~WebThreadedResourceProvider() { }
+};
+
+} // namespace blink
+
+#endif