Add null checks in navigator.serviceWorker access to fix possible crash

Source/modules/serviceworkers/ServiceWorkerContainer.cpp

 /*
  * 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
@@ skipping 75 matching lines @@
     m_provider->registerServiceWorker(patternURL, scriptURL, new CallbackPromiseAdapter<ServiceWorker, ServiceWorkerError>(resolver, executionContext));
     return promise;
 }
 
 ScriptPromise ServiceWorkerContainer::unregisterServiceWorker(ExecutionContext* executionContext, const String& pattern)
 {
     ASSERT(RuntimeEnabledFeatures::serviceWorkerEnabled());
     RefPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(executionContext);
     ScriptPromise promise = resolver->promise();
 
+    if (!m_provider) {
+        resolver->reject(DOMError::create(NotSupportedError, "No provider is available"));

[dominicc (has gone to gerrit), 2014/03/31 04:52:09]

It would be great to see a W3C-style test for this so that we can check that the
assert(s) in the test line up with language in the spec.

[kinuko, 2014/03/31 12:33:05]

Yes, but if we can have a test code that can repro this situation 100%.

+        return promise;
+    }
+
     RefPtr<SecurityOrigin> documentOrigin = executionContext->securityOrigin();
     KURL patternURL = executionContext->completeURL(pattern);
     if (!pattern.isEmpty() && !documentOrigin->canRequest(patternURL)) {
         resolver->reject(DOMError::create(SecurityError, "Can only unregister for patterns in the document's origin."));
-
         return promise;
     }
 
     m_provider->unregisterServiceWorker(patternURL, new CallbackPromiseAdapter<ServiceWorker, ServiceWorkerError>(resolver, executionContext));
     return promise;
 }
 
 ServiceWorkerContainer::ServiceWorkerContainer(ExecutionContext* executionContext)
-    : m_provider(ServiceWorkerContainerClient::from(executionContext)->provider())
+    : m_provider(0)

[dominicc (has gone to gerrit), 2014/03/31 04:52:09]

Why use nullptr sometimes and 0 other times?

[kinuko, 2014/03/31 12:33:05]

Some compilers don't accept assigning nullptr to a raw ptr. (While for RefPtr* /
OwnPtr* we always use nullptr now)

 {
     ScriptWrappable::init(this);
-    m_provider->setClient(this);
+
+    if (ServiceWorkerContainerClient::from(executionContext)) {

[dominicc (has gone to gerrit), 2014/03/31 04:52:09]

More idiomatic Blink would be something like:

if (Foo* bar = ServiceWorkerContainerClient::from(executionContext)) {
  m_provider = bar->provider();
  ...

etc.

[kinuko, 2014/03/31 12:33:05]

Done.

+        m_provider = ServiceWorkerContainerClient::from(executionContext)->provider();
+        if (m_provider)
+            m_provider->setClient(this);
+    }
 }
 
 } // namespace WebCore