Split the constructor of ThreadableLoader into two methods (ctor and start())

third_party/WebKit/Source/core/loader/ThreadableLoader.h

 /*
  * Copyright (C) 2009 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
@@ skipping 115 matching lines @@
 //
 // - ResourceLoaderOptions argument will be passed to the FetchRequest
 //   that this ThreadableLoader creates. It can be altered e.g. when
 //   redirect happens.
 class CORE_EXPORT ThreadableLoader : public RefCounted<ThreadableLoader> {
     WTF_MAKE_NONCOPYABLE(ThreadableLoader);
 public:
     // ThreadableLoaderClient methods may not destroy the ThreadableLoader
     // instance in them.
     static void loadResourceSynchronously(ExecutionContext&, const ResourceRequest&, ThreadableLoaderClient&, const ThreadableLoaderOptions&, const ResourceLoaderOptions&);
+
+    // This method never returns nullptr.
+    //
+    // This method must always be followed by |start()| call.
+    // |ThreadableLoaderClient| methods are never called before |start()| call.
+    //
+    // The async loading feature is separated into the |create()| method and
+    // and the |start()| method in order to:
+    // - reduce work done in a constructor
+    // - not to ask the users to handle fail in the constructor and other async

[hiroshige, 2016/01/26 08:44:50]

nit: s/ fail / failures / ?

[tyoshino (SeeGerritForStatus), 2016/01/29 12:36:52]

Right! Fixed.

+    //   failures separately
+    //
     // Loading completes when one of the following methods are called:
     // - didFinishLoading()
     // - didFail()
     // - didFailAccessControlCheck()
     // - didFailRedirectCheck()
-    // After any of these methods is called, no ThreadableLoaderClient method
-    // will be called.
+    // After any of these methods is called, the loader won't call any of the
+    // |ThreadableLoaderClient| methods.

[hiroshige, 2016/01/26 08:44:50]

nit: I'm not sure about usage of |'s. Is there any guidelines how to use |'s?
Should we use |'s for function names with () and class/type names?
(I think I've got No for this question in a review comment but I cannot find the
CL.)

[tyoshino (SeeGerritForStatus), 2016/01/29 12:36:52]

IIRC, we've never discussed this seriously and formally. As it's clear that
they're identifiers in the code as long as they have "()" or letters
capitalized, I'm ok with removing them. Fixed.

     //
-    // When ThreadableLoader is destructed, ThreadableLoaderClient methods are
-    // NOT called in response to the destruction synchronously or after
-    // destruction.
+    // When a |ThreadableLoader| is destructed, any of the
+    // |ThreadableLoaderClient| methods is NOT called in response to the
+    // destruction either synchronously or after destruction.
     //
-    // When ThreadableLoader::cancel() is called,
-    // ThreadableLoaderClient::didFail() is called with ResourceError with
-    // isCancellation() returning true, if any of didFinishLoading() or
-    // didFail.*() methods have not been called yet. (didFail() may be called
-    // with a ResourceError with isCancellation() returning true also for
-    // cancellation happened inside the loader.)
+    // When |ThreadableLoader::cancel()| is called,
+    // |ThreadableLoaderClient::didFail()| is called with a |ResourceError|
+    // with |isCancellation()| returning true, if any of |didFinishLoading()|
+    // or didFail.*() methods have not been called yet. (didFail() may be
+    // called with a |ResourceError| with |isCancellation()| returning true
+    // also for cancellation happened inside the loader.)
     //
-    // ThreadableLoaderClient methods:
-    // - may call cancel()
-    // - can destroy the ThreadableLoader instance in them (by clearing
-    //   RefPtr<ThreadableLoader>).
-    static PassRefPtr<ThreadableLoader> create(ExecutionContext&, ThreadableLoaderClient*, const ResourceRequest&, const ThreadableLoaderOptions&, const ResourceLoaderOptions&);
+    // |ThreadableLoaderClient| methods:
+    // - may call |cancel()|
+    // - can destroy the |ThreadableLoader| instance in them (by clearing
+    //   |RefPtr<ThreadableLoader>|).
+    static PassRefPtr<ThreadableLoader> create(ExecutionContext&, ThreadableLoaderClient*, const ThreadableLoaderOptions&, const ResourceLoaderOptions&);
+
+    // The methods on the |ThreadableLoaderClient| passed on |create()| call
+    // may be called synchronous to |start()| call.
+    virtual void start(const ResourceRequest&) = 0;
 
     // A ThreadableLoader may have a timeout specified. It is possible, in some cases, for
     // the timeout to be overridden after the request is sent (for example, XMLHttpRequests
     // may override their timeout setting after sending).
     //
     // Set a new timeout relative to the time the request started, in milliseconds.
     virtual void overrideTimeout(unsigned long timeoutMilliseconds) = 0;
 
     virtual void cancel() = 0;
 
     virtual ~ThreadableLoader() { }
 
 protected:
     ThreadableLoader() { }
 };
 
 } // namespace blink
 
 #endif // ThreadableLoader_h