Added budget_service.mojom

third_party/WebKit/Source/modules/budget/BudgetService.cpp

Index: third_party/WebKit/Source/modules/budget/BudgetService.cpp
diff --git a/third_party/WebKit/Source/modules/budget/BudgetService.cpp b/third_party/WebKit/Source/modules/budget/BudgetService.cpp
index 52ae315ddafb453f58e4b13fade28daa1d8c6a25..e82e4d703b751e7716bcf0f1533cb95f9f9dfdfe 100644
--- a/third_party/WebKit/Source/modules/budget/BudgetService.cpp
+++ b/third_party/WebKit/Source/modules/budget/BudgetService.cpp
@@ -9,21 +9,78 @@
 #include "bindings/core/v8/ScriptState.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/ExceptionCode.h"
+#include "modules/budget/BudgetChunk.h"
+#include "public/platform/InterfaceProvider.h"
+#include "public/platform/Platform.h"
+#include "public/platform/modules/budget_service/budget_service.mojom-blink.h"
 
 namespace blink {
 
-BudgetService::BudgetService() {}
+BudgetService::BudgetService()
+{
+    Platform::current()->interfaceProvider()->getInterface(mojo::GetProxy(&m_service));
+}
+
+BudgetService::~BudgetService()
+{
+}
+
+ScriptPromise BudgetService::getCost(ScriptState* scriptState, const AtomicString& /* actionType */)

[johnme, 2016/08/17 13:54:02]

operationType?

[harkness, 2016/08/18 10:23:26]

Actually, when it comes in to this function, it's action type and needs to be
mapped to operation type.

[johnme, 2016/08/18 13:04:31]

Oh, I was looking at https://beverloo.github.io/budget-api/ but I see that you
already called it actionType in BudgetService.idl. Presumably later you'll
rename it to match the spec in both places?

[harkness, 2016/08/22 08:57:47]

Yup, that's definitely on the roadmap.

+{
+    DCHECK(m_service);

[johnme, 2016/08/17 13:54:01]

Nit: Perhaps just DCHECK once in the constructor?

[harkness, 2016/08/18 10:23:26]

Done.

+    ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
+    ScriptPromise promise = resolver->promise();
+
+    // TODO(harkness): Map the actionType to OperationType.
+    // Get the cost for the action from the browser BudgetService.
+    mojom::blink::OperationType type = mojom::blink::OperationType::SILENT_PUSH;
+    m_service->GetCost(type, convertToBaseCallback(WTF::bind(&BudgetService::asyncGetCost, wrapPersistent(this), wrapPersistent(resolver))));
+    return promise;
+}
 
-ScriptPromise BudgetService::getCost(ScriptState* scriptState, const AtomicString& actionType)
+void BudgetService::asyncGetCost(ScriptPromiseResolver* resolver, double cost) const
 {
-    // TODO(harkness): Trigger the cost calculation.
-    return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(NotSupportedError, "Not yet implemented"));
+    resolver->resolve(cost);
 }
 
 ScriptPromise BudgetService::getBudget(ScriptState* scriptState)
 {
-    // TODO(harkness): Trigger the budget calculation.
-    return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(NotSupportedError, "Not yet implemented"));
+    DCHECK(m_service);
+    ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
+    ScriptPromise promise = resolver->promise();
+
+    // Get the budget from the browser BudgetService.
+    RefPtr<SecurityOrigin> origin(scriptState->getExecutionContext()->getSecurityOrigin());
+    m_service->GetBudget(origin, convertToBaseCallback(WTF::bind(&BudgetService::asyncGetBudget, wrapPersistent(this), PassRefPtr<ScriptState>(scriptState), wrapPersistent(resolver))));

[johnme, 2016/08/17 13:54:01]

1. Is it ok to hold a reference to the ScriptState, or could this cause a
reference cycle?

2. Should this be adoptRef(scriptState) instead?

3. Is it valid to adopt a raw pointer which may have existing refs elsewhere?

[harkness, 2016/08/18 10:23:26]

I didn't know about adoptRef, and if I'd known I probably should have used that.
However, in digging into this, it turns out that the scriptState is a member of
the resolver. So I can just get the scriptState from there rather than passing
it explicitly. 

+    return promise;
+}
+
+void BudgetService::asyncGetBudget(PassRefPtr<ScriptState> scriptState, ScriptPromiseResolver* resolver, const mojo::WTFArray<mojom::blink::BudgetStatePtr> expectations) const
+{
+
+    Vector<v8::Local<v8::Value>> budget;
+    if (!scriptState) {

[johnme, 2016/08/17 13:54:02]

How can this occur?

[harkness, 2016/08/18 10:23:26]

That was left over from when I was passing ScriptState as unretained, so I was
checking it. Given that I'm now getting scriptState from the resolver, I do need
this, but I combined it with the check for contextIsValid on line 65.

+        resolver->resolve(budget);
+        return;
+    }
+    if (!scriptState->contextIsValid()) {
+        resolver->resolve(budget);
+        return;
+    }
+
+    // Create a new scope for the object that is passed back to script.
+    ScriptState::Scope scope(scriptState.get());
+
+    // Copy the chunks into the budget array.
+    budget.grow(expectations.size());
+    for (uint i = 0; i < expectations.size(); i++) {
+        BudgetChunk chunk;
+        chunk.setAmount(expectations[i]->budget_at);
+        chunk.setExpiration(expectations[i]->time);
+        budget[i] = freezeV8Object(toV8(chunk, scriptState.get()), scriptState->isolate());

[johnme, 2016/08/17 13:54:01]

BudgetState objects shouldn't need to be individually frozen, since all their
attributes are readonly primitives (once you update BudgetChunk to match the
spec and stop being a dictionary). This might mean you no longer need to deal
with v8 directly, which would be nice.

[harkness, 2016/08/18 10:23:26]

I think I can get away without the extra freezeV8Object call, but I still think
I need the toV8, and that's where I needed the extra scope.

The difficulty here is that all other instances of FrozenArray returning an
object are returning a reference to a member object of the Blink class, so they
can rely on the class to control the lifetime. In my case, I need to explicitly
allocate a copy of the array on the v8 heap because after that method returns,
the memory that was passed from the browser will be deallocated.

[johnme, 2016/08/18 13:04:31]

It looks like ExtendableMessageEvent::ports() returns a HeapVector by value
(either a copy, or a new empty one). Either way, these are all refcounted, so
the lifetime of BudgetService shouldn't matter?

[harkness, 2016/08/22 08:57:47]

I hadn't realized that BudgetState (formerly BudgetChunk) had been switched from
a dictionary to an interface, so I updated for that. Once I did that, the
BudgetState was a refcounted object, so I could put it in a HeapVector and trust
to that to keep it alive.

+    }
+
+    resolver->resolve(budget);
 }
 
 } // namespace blink