PaymentRequest should be a ContextLifecycleObserver

third_party/WebKit/Source/modules/payments/PaymentRequest.cpp

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "modules/payments/PaymentRequest.h"
 
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/JSONValuesForV8.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "bindings/core/v8/ScriptState.h"
@@ skipping 183 matching lines @@
     m_paymentProvider->Abort();
 }
 
 const AtomicString& PaymentRequest::interfaceName() const
 {
     return EventTargetNames::PaymentRequest;
 }
 
 ExecutionContext* PaymentRequest::getExecutionContext() const
 {
-    return m_scriptState->getExecutionContext();
-}
-
-ScriptPromise PaymentRequest::complete(ScriptState* scriptState, bool success)
-{
-    if (m_completeResolver)
-        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(InvalidStateError, "Already called complete() once"));
-
-    m_completeResolver = ScriptPromiseResolver::create(scriptState);
-    m_paymentProvider->Complete(success);
-
-    return m_completeResolver->promise();
+    return ContextLifecycleObserver::getExecutionContext();
 }
 
 DEFINE_TRACE(PaymentRequest)
 {
     visitor->trace(m_details);
     visitor->trace(m_options);
     visitor->trace(m_shippingAddress);
     visitor->trace(m_showResolver);
     visitor->trace(m_completeResolver);
     EventTargetWithInlineData::trace(visitor);
+    ContextLifecycleObserver::trace(visitor);
 }
 
 PaymentRequest::PaymentRequest(ScriptState* scriptState, const Vector<String>& supportedMethods, const PaymentDetails& details, const PaymentOptions& options, const ScriptValue& data, ExceptionState& exceptionState)
-    : m_scriptState(scriptState)
+    : ContextLifecycleObserver(scriptState->getExecutionContext())
     , m_supportedMethods(supportedMethods)
     , m_details(details)
     , m_options(options)
     , m_clientBinding(this)
 {
     // TODO(rouslan): Also check for a top-level browsing context.
     // https://github.com/w3c/browser-payment-api/issues/2
     if (!scriptState->getExecutionContext()->isSecureContext()) {
         exceptionState.throwSecurityError("Must be in a secure context");
         return;
@@ skipping 50 matching lines @@
 
             m_stringifiedData = jsonData->toJSONString();
         }
     }
 
     // Set the currently selected option if only one option was passed.
     if (details.hasShippingOptions() && details.shippingOptions().size() == 1)
         m_shippingOption = details.shippingOptions().begin()->id();
 }
 
+ScriptPromise PaymentRequest::complete(ScriptState* scriptState, bool success)

[please use gerrit instead, 2016/04/13 22:56:46]

Let's keep the order of methods in PaymentRequest.cpp and PaymentRequest.h the
same. If you did not move "complete()" method in PaymentRequest.h, please do not
move the "complete()" method in PaymentRequest.cpp.

[rwlbuis, 2016/04/14 00:28:33]

Done.

+{
+    if (m_completeResolver)
+        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(InvalidStateError, "Already called complete() once"));
+
+    m_completeResolver = ScriptPromiseResolver::create(scriptState);
+    m_paymentProvider->Complete(success);
+
+    return m_completeResolver->promise();
+}
+
+void PaymentRequest::contextDestroyed()
+{
+    cleanUp();
+}
+
 void PaymentRequest::OnShippingAddressChange(mojom::wtf::ShippingAddressPtr address)
 {
     DCHECK(m_showResolver);
     DCHECK(!m_completeResolver);
 
     String errorMessage;
     if (!PaymentsValidators::isValidRegionCodeFormat(address->region_code, &errorMessage)
         || !PaymentsValidators::isValidLanguageCodeFormat(address->language_code, &errorMessage)
         || !PaymentsValidators::isValidScriptCodeFormat(address->script_code, &errorMessage)) {
         m_showResolver->reject(DOMException::create(SyntaxError, errorMessage));
@@ skipping 43 matching lines @@
 {
     DCHECK(m_completeResolver);
     m_completeResolver->resolve();
     cleanUp();
 }
 
 void PaymentRequest::cleanUp()
 {
     m_completeResolver.clear();
     m_showResolver.clear();
     m_clientBinding.Close();

[please use gerrit instead, 2016/04/13 22:56:46]

I recommend checking Binding::is_bound() in here instead of inside of Close().
This line should change to:

if (m_clientBinding.is_bound())
    m_clientBinding.Close();

[rwlbuis, 2016/04/14 00:28:33]

Done.

     m_paymentProvider.reset();
 }
 
 } // namespace blink