c***@googlecode.com
2015-08-10 14:55:38 UTC
Status: Started
Owner: ***@chromium.org
CC: ***@chromium.org
Labels: Type-Bug Pri-2 Cr-Blink-Loader OS-All
New issue 518855 by ***@chromium.org: Split the constructor of
ThreadableLoader into ctor and start() method
https://code.google.com/p/chromium/issues/detail?id=518855
Currently, ThreadableLoader::create() can return null or call didFail()
synchronously when something wrong (e.g. mixed content blocking).
However, this might be problematic because:
- Some call sites of create() lacks null checks.
- We usually do like:
m_loader = ThreadableLoader::create(executionContext, this, ...);
but if this->didFail() is called inside create(), m_loader is not yet set
at that point.
- Also it's not good to do much work in a constructor.
This issue resolves this problem by splitting the constructor into a new
trivial constructor and start():
- The constructor and create() do not call ThreadableLoaderClient methods.
- create() always return non-null.
CL by tyoshino@ is under review: https://codereview.chromium.org/1264453002/
Moving ThreadableLoaderClient* argument from the constructor/create() to
start() will be clearer, i.e. it explicitly states that
ThreadableLoaderClient methods are never called from create(), but it
requires code restructure, thus will be not included in the CL.
--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings
Owner: ***@chromium.org
CC: ***@chromium.org
Labels: Type-Bug Pri-2 Cr-Blink-Loader OS-All
New issue 518855 by ***@chromium.org: Split the constructor of
ThreadableLoader into ctor and start() method
https://code.google.com/p/chromium/issues/detail?id=518855
Currently, ThreadableLoader::create() can return null or call didFail()
synchronously when something wrong (e.g. mixed content blocking).
However, this might be problematic because:
- Some call sites of create() lacks null checks.
- We usually do like:
m_loader = ThreadableLoader::create(executionContext, this, ...);
but if this->didFail() is called inside create(), m_loader is not yet set
at that point.
- Also it's not good to do much work in a constructor.
This issue resolves this problem by splitting the constructor into a new
trivial constructor and start():
- The constructor and create() do not call ThreadableLoaderClient methods.
- create() always return non-null.
CL by tyoshino@ is under review: https://codereview.chromium.org/1264453002/
Moving ThreadableLoaderClient* argument from the constructor/create() to
start() will be clearer, i.e. it explicitly states that
ThreadableLoaderClient methods are never called from create(), but it
requires code restructure, thus will be not included in the CL.
--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings
--
--
Automated mail from issue updates at http://crbug.com/
Subscription options: http://groups.google.com/a/chromium.org/group/chromium-bugs
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-bugs+***@chromium.org.
--
Automated mail from issue updates at http://crbug.com/
Subscription options: http://groups.google.com/a/chromium.org/group/chromium-bugs
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-bugs+***@chromium.org.