Expose the BudgetService interface to Workers.

Expose the BudgetService interface to Workers.

The BudgetAPI proposed specification has the interface exposed in both document
and worker contexts. This patch adds worker exposure and makes the
document exposure explicit.

The patch adds testing for the new exposure, and also adds testing for all
interfaces in the budget implementation in service workers.

BUG=672111
Committed: https://crrev.com/85999851912f7605181b68fd4c13ffc1ed9a86b0
Cr-Commit-Position: refs/heads/master@{#438247}

[harkness, 2016-12-12 11:41:17]

harkness@chromium.org changed reviewers:
+ johnme@chromium.org

[harkness, 2016-12-12 12:58:11]

harkness@chromium.org changed reviewers:
+ peter@chromium.org
- johnme@chromium.org

[Peter Beverloo, 2016-12-12 14:28:43]

lgtm. Will you do BudgetState separately?

nit: please explicitly call it "the BudgetService interface". The feature itself
already was exposed to Service Workers.

https://codereview.chromium.org/2571453002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/budget/interfaces-in-service-worker.html
(right):

https://codereview.chromium.org/2571453002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/interfaces-in-service-worker.html:4:
<title>getCost succeeds from a Service Worker</title>
nit: use a proper title. on line 34 too

https://codereview.chromium.org/2571453002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/interfaces-in-service-worker.html:15:
var port;
var -> const/let

https://codereview.chromium.org/2571453002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/budget/resources/instrumentation-service-worker.js
(right):

https://codereview.chromium.org/2571453002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/resources/instrumentation-service-worker.js:62:
}
What exception do you expect to be thrown here? I don't think there is any.

The following would be much nicer:

const success = 'BudgetService' in self &&
                BudgetService.prototype.hasOwnProperty('getCost') &&
                BudgetService.prototype.hasOwnProperty('getBudget') &&
                BudgetService.prototype.hasOwnProperty('reserve') &&
                WorkerNavigator.prototype.hasOwnProperty('budget');

port.postMessage({ .. });

[harkness, 2016-12-12 16:00:34]

I completely forgot about BudgetState, I'll go ahead and land this and then do
that.

https://codereview.chromium.org/2571453002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/budget/interfaces-in-service-worker.html
(right):

https://codereview.chromium.org/2571453002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/interfaces-in-service-worker.html:4:
<title>getCost succeeds from a Service Worker</title>
On 2016/12/12 14:28:43, Peter Beverloo wrote:
> nit: use a proper title. on line 34 too

Done.

https://codereview.chromium.org/2571453002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/interfaces-in-service-worker.html:15:
var port;
On 2016/12/12 14:28:43, Peter Beverloo wrote:
> var -> const/let

Done.

https://codereview.chromium.org/2571453002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/budget/resources/instrumentation-service-worker.js
(right):

https://codereview.chromium.org/2571453002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/budget/resources/instrumentation-service-worker.js:62:
}
On 2016/12/12 14:28:43, Peter Beverloo wrote:
> What exception do you expect to be thrown here? I don't think there is any.
> 
> The following would be much nicer:
> 
> const success = 'BudgetService' in self &&
>                 BudgetService.prototype.hasOwnProperty('getCost') &&
>                 BudgetService.prototype.hasOwnProperty('getBudget') &&
>                 BudgetService.prototype.hasOwnProperty('reserve') &&
>                 WorkerNavigator.prototype.hasOwnProperty('budget');
> 
> port.postMessage({ .. });

Updated the code to remove the if statement, but left the try in. 

I used that a lot when I was testing because without that, the test just times
out without giving any information if there is an exception. While I was
testing, the exception was because I was using Navigator or navigator instead of
WorkerNavigator, but it could also happen if you accidentally deleted a
character in any of the names, and the timeout is very difficult to debug but
with the exception message, it's quick to track down.

[harkness, 2016-12-12 16:09:14]

The CQ bit was checked by harkness@chromium.org

[harkness, 2016-12-12 16:09:15]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org
Link to the patchset: https://codereview.chromium.org/2571453002/#ps20001
(title: "Code review comments")

[commit-bot: I haz the power, 2016-12-12 16:09:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-12 16:32:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-12 16:32:57]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[harkness, 2016-12-12 18:15:29]

Updated CL with changes to the expected global interfaces LayoutTests.

[harkness, 2016-12-12 18:16:50]

Description was changed from

==========
Expose the BudgetService to Workers.

The BudgetAPI proposed specification has the API exposed in both document
and worker contexts. This patch adds worker exposure and makes the
document exposure explicit.

The patch adds testing for the new exposure, and also adds testing for all
interfaces in the budget implementation in service workers.

BUG=672111
==========

to

==========
Expose the BudgetService interface to Workers.

The BudgetAPI proposed specification has the interface exposed in both document
and worker contexts. This patch adds worker exposure and makes the
document exposure explicit.

The patch adds testing for the new exposure, and also adds testing for all
interfaces in the budget implementation in service workers.

BUG=672111
==========

[harkness, 2016-12-12 18:30:04]

The CQ bit was checked by harkness@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-12 18:30:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-12 20:24:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-12 20:24:16]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[harkness, 2016-12-13 09:54:50]

The CQ bit was checked by harkness@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-13 09:55:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 11:36:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-13 11:36:36]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[harkness, 2016-12-13 13:38:01]

PTAL. Updated one more global interface LayoutTest.

[harkness, 2016-12-13 13:38:09]

The CQ bit was checked by harkness@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-13 13:38:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Peter Beverloo, 2016-12-13 13:42:29]

Still lgtm, that's just some virtual test suite.

[commit-bot: I haz the power, 2016-12-13 15:14:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-13 15:14:48]

Dry run: This issue passed the CQ dry run.

[harkness, 2016-12-13 15:18:08]

The CQ bit was checked by harkness@chromium.org

[commit-bot: I haz the power, 2016-12-13 15:18:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 15:23:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-13 15:23:32]

Failed to apply patch for
third_party/WebKit/Source/modules/budget/BudgetService.idl:
While running git apply --index -p1;
  error: patch failed:
third_party/WebKit/Source/modules/budget/BudgetService.idl:9
  error: third_party/WebKit/Source/modules/budget/BudgetService.idl: patch does
not apply

Patch:       third_party/WebKit/Source/modules/budget/BudgetService.idl
Index: third_party/WebKit/Source/modules/budget/BudgetService.idl
diff --git a/third_party/WebKit/Source/modules/budget/BudgetService.idl
b/third_party/WebKit/Source/modules/budget/BudgetService.idl
index
2087b9c6f19f8d37caa9156a3c8eb78d981a22ca..aa1d58d84ce295dab750c4780a42ef8ef211f1ed
100644
--- a/third_party/WebKit/Source/modules/budget/BudgetService.idl
+++ b/third_party/WebKit/Source/modules/budget/BudgetService.idl
@@ -9,7 +9,8 @@ enum OperationType {
 };
 
 [
-    RuntimeEnabled=Budget
+    RuntimeEnabled=Budget,
+    Exposed=(Window,Worker)
 ] interface BudgetService {
     [CallWith=ScriptState] Promise<double> getCost(OperationType operation);
     [CallWith=ScriptState] Promise<FrozenArray<BudgetState>> getBudget();

[harkness, 2016-12-13 17:34:09]

The CQ bit was checked by harkness@chromium.org

[harkness, 2016-12-13 17:34:09]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org
Link to the patchset: https://codereview.chromium.org/2571453002/#ps80001
(title: "Rebase")

[commit-bot: I haz the power, 2016-12-13 17:34:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 19:05:55]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1481650449145640,
"parent_rev": "d5ffa8478a56ac0e6560203e5b57b692072d1764", "commit_rev":
"ec83b4f0fe6bd4314068ce8f44b637c83c33d668"}

[commit-bot: I haz the power, 2016-12-13 19:06:22]

Description was changed from

==========
Expose the BudgetService interface to Workers.

The BudgetAPI proposed specification has the interface exposed in both document
and worker contexts. This patch adds worker exposure and makes the
document exposure explicit.

The patch adds testing for the new exposure, and also adds testing for all
interfaces in the budget implementation in service workers.

BUG=672111
==========

to

==========
Expose the BudgetService interface to Workers.

The BudgetAPI proposed specification has the interface exposed in both document
and worker contexts. This patch adds worker exposure and makes the
document exposure explicit.

The patch adds testing for the new exposure, and also adds testing for all
interfaces in the budget implementation in service workers.

BUG=672111

Review-Url: https://codereview.chromium.org/2571453002
==========

[commit-bot: I haz the power, 2016-12-13 19:06:24]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-12-13 19:10:12]

Description was changed from

==========
Expose the BudgetService interface to Workers.

The BudgetAPI proposed specification has the interface exposed in both document
and worker contexts. This patch adds worker exposure and makes the
document exposure explicit.

The patch adds testing for the new exposure, and also adds testing for all
interfaces in the budget implementation in service workers.

BUG=672111

Review-Url: https://codereview.chromium.org/2571453002
==========

to

==========
Expose the BudgetService interface to Workers.

The BudgetAPI proposed specification has the interface exposed in both document
and worker contexts. This patch adds worker exposure and makes the
document exposure explicit.

The patch adds testing for the new exposure, and also adds testing for all
interfaces in the budget implementation in service workers.

BUG=672111

Committed: https://crrev.com/85999851912f7605181b68fd4c13ffc1ed9a86b0
Cr-Commit-Position: refs/heads/master@{#438247}
==========

[commit-bot: I haz the power, 2016-12-13 19:10:13]

Patchset 5 (id:??) landed as
https://crrev.com/85999851912f7605181b68fd4c13ffc1ed9a86b0
Cr-Commit-Position: refs/heads/master@{#438247}