Reland of Clear LifecycleObserver::m_context when...

Reland of Clear LifecycleObserver::m_context when LifecycleObserver::contextDestroyed gets called (patchset #1 id:1 of https://codereview.chromium.org/2365463004/ )

Reason for revert:
Relanding this CL because the fix to the original crash has been landed in https://codereview.chromium.org/2364693005/

Original issue's description:
> Revert of Clear LifecycleObserver::m_context when LifecycleObserver::contextDestroyed gets called (patchset #11 id:200001 of https://codereview.chromium.org/2317483005/ )
>
> Reason for revert:
> Speculatively reverting for https://crbug.com/649272
>
> Original issue's description:
> > Clear LifecycleObserver::m_context when LifecycleObserver::contextDestroyed gets called
> >
> > This CL clears LifecycleObserver::m_context when LifecycleObserver::contextDestroyed gets called.
> > This means that LifecycleObserver::context() starts returning false after the context gets destroyed.
> >
> > The new behavior makes a lot of more sense, but the problem is that some existing code is assuming that
> > LifecycleObserver::context() keeps returning the context even after the context gets destroyed.
> > This CL fixed the problematic code to make all layout tests pass, but I'm not sure if I've updated all of the problematic code
> > (because the coverage of the layout tests is not sufficient).
> > However, the worst thing that can happen is that code that had been assuming that LifecycleObserver::context()
> > returns a context even after the context gets destroyed will stop working (it may cause a null-deref crash).
> > I think the risk is low.
> >
> > BUG=610176
> >
> > Committed: https://crrev.com/b729a998b05ec916731171d59de95e0aea31bbac
> > Cr-Commit-Position: refs/heads/master@{#419951}
>
> TBR=haraken@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=610176,649272
> NOTRY=true
>
> Committed: https://crrev.com/04b1826356a37de2a322e7e6e0e80f07d05f210f
> Cr-Commit-Position: refs/heads/master@{#420498}

TBR=wfh@chromium.org,dcheng@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=610176,649272

Review-Url: https://codereview.chromium.org/2367753002
Cr-Commit-Position: refs/heads/master@{#420608}

third_party/WebKit/LayoutTests/fast/files/file-reader-detached-no-crash-expected.txt

@@ -3,7 +3,7 @@ Accessing FileReader when in a detached state.
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS fileReader.readAsArrayBuffer(new File(['hello'], 'world.html')) threw exception AbortError: Failed to execute 'readAsArrayBuffer' on 'FileReader': Reading from a Document-detached FileReader is not supported..
+PASS fileReader.readAsArrayBuffer(new File(['hello'], 'world.html')) threw exception AbortError: Failed to execute 'readAsArrayBuffer' on 'FileReader': Reading from a detached FileReader is not supported..
 PASS successfullyParsed is true
 
 TEST COMPLETE

third_party/WebKit/LayoutTests/fast/xmlhttprequest/xmlhttprequest-detached-mixedcontent-no-crash-expected.txt

@@ -4,7 +4,7 @@ Performing mixed content checks in a detached setting shouldn't crash.
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS xhr.send() threw exception NetworkError: Failed to execute 'send' on 'XMLHttpRequest': Failed to load 'http://user:pass@127.0.0.1:8000/doesNotExist'..
+PASS xhr.send() threw exception NetworkError: Failed to execute 'send' on 'XMLHttpRequest': Failed to load 'http://user:pass@127.0.0.1:8000/doesNotExist': Document is already detached..
 PASS successfullyParsed is true
 
 TEST COMPLETE

third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp

@@ -295,6 +295,7 @@ void XMLHttpRequest::initResponseDocument()
     bool isHTML = responseIsHTML();
     if ((m_response.isHTTP() && !responseIsXML() && !isHTML)
         || (isHTML && m_responseTypeCode == ResponseTypeDefault)
+        || !getExecutionContext()
         || getExecutionContext()->isWorkerGlobalScope()) {
         m_responseDocument = nullptr;
         return;
@@ -410,7 +411,7 @@ void XMLHttpRequest::setTimeout(unsigned timeout, ExceptionState& exceptionState
 {
     // FIXME: Need to trigger or update the timeout Timer here, if needed. http://webkit.org/b/98156
     // XHR2 spec, 4.7.3. "This implies that the timeout attribute can be set while fetching is in progress. If that occurs it will still be measured relative to the start of fetching."
-    if (getExecutionContext()->isDocument() && !m_async) {
+    if (getExecutionContext() && getExecutionContext()->isDocument() && !m_async) {
         exceptionState.throwDOMException(InvalidAccessError, "Timeouts cannot be set for synchronous requests made from a document.");
         return;
     }
@@ -435,7 +436,7 @@ void XMLHttpRequest::setResponseType(const String& responseType, ExceptionState&
 
     // Newer functionality is not available to synchronous requests in window contexts, as a spec-mandated
     // attempt to discourage synchronous XHR use. responseType is one such piece of functionality.
-    if (!m_async && getExecutionContext()->isDocument()) {
+    if (getExecutionContext() && getExecutionContext()->isDocument() && !m_async) {
         exceptionState.throwDOMException(InvalidAccessError, "The response type cannot be changed for synchronous requests made from a document.");
         return;
     }
@@ -556,6 +557,9 @@ void XMLHttpRequest::setWithCredentials(bool value, ExceptionState& exceptionSta
 
 void XMLHttpRequest::open(const AtomicString& method, const String& urlString, ExceptionState& exceptionState)
 {
+    if (!getExecutionContext())
+        return;
+
     KURL url(getExecutionContext()->completeURL(urlString));
     if (!validateOpenArguments(method, url, exceptionState))
         return;
@@ -565,6 +569,9 @@ void XMLHttpRequest::open(const AtomicString& method, const String& urlString, E
 
 void XMLHttpRequest::open(const AtomicString& method, const String& urlString, bool async, const String& username, const String& password, ExceptionState& exceptionState)
 {
+    if (!getExecutionContext())
+        return;
+
     KURL url(getExecutionContext()->completeURL(urlString));
     if (!validateOpenArguments(method, url, exceptionState))
         return;
@@ -642,8 +649,11 @@ void XMLHttpRequest::open(const AtomicString& method, const KURL& url, bool asyn
 
 bool XMLHttpRequest::initSend(ExceptionState& exceptionState)
 {
-    if (!getExecutionContext())
+    if (!getExecutionContext()) {
+        handleNetworkError();
+        throwForLoadFailureIfNeeded(exceptionState, "Document is already detached.");
         return false;
+    }
 
     if (m_state != kOpened || m_loader) {
         exceptionState.throwDOMException(InvalidStateError, "The object's state must be OPENED.");
@@ -876,7 +886,7 @@ void XMLHttpRequest::createRequest(PassRefPtr<EncodedFormData> httpBody, Excepti
     }
 
     DCHECK(getExecutionContext());
-    ExecutionContext& executionContext = *this->getExecutionContext();
+    ExecutionContext& executionContext = *getExecutionContext();
 
     // The presence of upload event listeners forces us to use preflighting because POSTing to an URL that does not
     // permit cross origin requests should look exactly like POSTing to an URL that does not respond at all.
@@ -1504,7 +1514,7 @@ void XMLHttpRequest::endLoading()
 
     changeState(kDone);
 
-    if (!getExecutionContext()->isDocument() || !document() || !document()->frame() || !document()->frame()->page())
+    if (!getExecutionContext() || !getExecutionContext()->isDocument() || !document() || !document()->frame() || !document()->frame()->page())
         return;
 
     if (status() >= 200 && status() < 300) {

third_party/WebKit/Source/modules/fetch/Body.cpp

@@ -211,7 +211,7 @@ bool Body::isBodyLocked()
 
 bool Body::hasPendingActivity() const
 {
-    if (getExecutionContext()->activeDOMObjectsAreStopped())
+    if (!getExecutionContext() || getExecutionContext()->activeDOMObjectsAreStopped())
         return false;
     if (!bodyBuffer())
         return false;

third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerContainerClient.cpp

@@ -35,6 +35,8 @@ const char* ServiceWorkerContainerClient::supplementName()
 
 ServiceWorkerContainerClient* ServiceWorkerContainerClient::from(ExecutionContext* context)
 {
+    if (!context)
+        return nullptr;
     if (context->isWorkerGlobalScope()) {
         WorkerClients* clients = toWorkerGlobalScope(context)->clients();
         ASSERT(clients);

third_party/WebKit/Source/modules/websockets/DOMWebSocket.cpp

@@ -243,6 +243,7 @@ DOMWebSocket::~DOMWebSocket()
 
 void DOMWebSocket::logError(const String& message)
 {
+    if (getExecutionContext())
         getExecutionContext()->addConsoleMessage(ConsoleMessage::create(JSMessageSource, ErrorMessageLevel, message));
 }
 

third_party/WebKit/Source/platform/LifecycleNotifier.h

@@ -61,9 +61,7 @@ protected:
     {
     }
 
-#if DCHECK_IS_ON()
     T* context() { return static_cast<T*>(this); }
-#endif
 
     using ObserverSet = HeapHashSet<WeakMember<Observer>>;
 
@@ -98,8 +96,9 @@ inline void LifecycleNotifier<T, Observer>::notifyContextDestroyed()
     ObserverSet observers;
     m_observers.swap(observers);
     for (Observer* observer : observers) {
-        ASSERT(observer->lifecycleContext() == context());
+        DCHECK(observer->lifecycleContext() == context());
         observer->contextDestroyed();
+        observer->clearContext();
     }
 }
 

third_party/WebKit/Source/platform/LifecycleObserver.h

@@ -43,6 +43,11 @@ public:
 
     Context* lifecycleContext() const { return m_lifecycleContext; }
 
+    void clearContext()
+    {
+        setContext(nullptr);
+    }
+
 protected:
     explicit LifecycleObserver(Context* context)
         : m_lifecycleContext(nullptr)
@@ -52,11 +57,6 @@ protected:
 
     void setContext(Context*);
 
-    void clearContext()
-    {
-        setContext(nullptr);
-    }
-
 private:
     WeakMember<Context> m_lifecycleContext;
 };