Service Worker: Correct WindowClient.idl

Service Worker: Correct WindowClient.idl

- Remove WindowClient.frameType attribute
  . This is already defined in Client and we don't need to redefine. This change
doesn't affect any behavior.
- Change the promise resolution value type of WindowClient.focus() from boolean
  to WindowClient.
  . A promise resolution value type is just an annotation in WebIDL, so this 
change does not affect actual implementation. We already resolve with 
WindowClient, see http/tests/serviceworker/chromium/windowclient-focus.html.

Spec: https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#window-client-interface

BUG=506736
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=198269

[jungkees, 2015-07-03 02:29:36]

jungkee.song@samsung.com changed reviewers:
+ jinho.bang@samsung.com, l.gombos@samsung.com

[jungkees, 2015-07-03 02:29:50]

PTAL

[zino, 2015-07-03 02:38:57]

Looks good to me.

I think it is better to add the spec link to CL description.

https://codereview.chromium.org/1214733006/diff/1/Source/modules/serviceworke...
File Source/modules/serviceworkers/WindowClient.idl (left):

https://codereview.chromium.org/1214733006/diff/1/Source/modules/serviceworke...
Source/modules/serviceworkers/WindowClient.idl:23: readonly attribute
ContextFrameType frameType;
Clarify: This is already defined in Client and we don't need to redefine.
            This change doesn't affect any behavior.

[jungkees, 2015-07-03 02:44:11]

jungkee.song@samsung.com changed reviewers:
+ falken@chromium.org, kinuko@chromium.org, michaeln@chromium.org,
nhiroki@chromium.org

[jungkees, 2015-07-03 02:45:25]

Thanks for review, zino@. I added the spec link in the CL description.
OWNERS, PTAL

[nhiroki, 2015-07-03 03:18:08]

https://codereview.chromium.org/1214733006/diff/1/Source/modules/serviceworke...
File Source/modules/serviceworkers/WindowClient.idl (right):

https://codereview.chromium.org/1214733006/diff/1/Source/modules/serviceworke...
Source/modules/serviceworkers/WindowClient.idl:23: [CallWith=ScriptState]
Promise<WindowClient> focus();
A promise resolution value type is just an annotation in WebIDL, so this change
does not affect actual implementation. OK, LGTM.

[falken, 2015-07-03 03:22:49]

falken@chromium.org changed reviewers:
+ mlamouri@chromium.org

[falken, 2015-07-03 03:22:49]

+mlamouri who implemented focus()

https://codereview.chromium.org/1214733006/diff/1/Source/modules/serviceworke...
File Source/modules/serviceworkers/WindowClient.idl (right):

https://codereview.chromium.org/1214733006/diff/1/Source/modules/serviceworke...
Source/modules/serviceworkers/WindowClient.idl:23: [CallWith=ScriptState]
Promise<WindowClient> focus();
Doesn't the C++ code need updating too? I assume we're resolving with a boolean
now.

And we don't have tests that test the return value of focus()? Can you add them?

[falken, 2015-07-03 03:23:38]

On 2015/07/03 03:18:08, nhiroki wrote:
>
https://codereview.chromium.org/1214733006/diff/1/Source/modules/serviceworke...
> File Source/modules/serviceworkers/WindowClient.idl (right):
> 
>
https://codereview.chromium.org/1214733006/diff/1/Source/modules/serviceworke...
> Source/modules/serviceworkers/WindowClient.idl:23: [CallWith=ScriptState]
> Promise<WindowClient> focus();
> A promise resolution value type is just an annotation in WebIDL, so this
change
> does not affect actual implementation. OK, LGTM.

Ah, didn't realize that. But is our current code resolving with a boolean or
WindowClient?

[nhiroki, 2015-07-03 03:31:01]

On 2015/07/03 03:23:38, falken wrote:
> On 2015/07/03 03:18:08, nhiroki wrote:
> >
>
https://codereview.chromium.org/1214733006/diff/1/Source/modules/serviceworke...
> > File Source/modules/serviceworkers/WindowClient.idl (right):
> > 
> >
>
https://codereview.chromium.org/1214733006/diff/1/Source/modules/serviceworke...
> > Source/modules/serviceworkers/WindowClient.idl:23: [CallWith=ScriptState]
> > Promise<WindowClient> focus();
> > A promise resolution value type is just an annotation in WebIDL, so this
> change
> > does not affect actual implementation. OK, LGTM.
> 
> Ah, didn't realize that. But is our current code resolving with a boolean or
> WindowClient?

A focus promise is resolved with WindowClient:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

Tests:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[falken, 2015-07-03 03:38:19]

OK thanks for the links. jungkee: Can you edit the CL description to be clear
that this isn't changing behavior? 

for removal of frameType (courtesy of zino):
"This is already defined in Client and we don't need to redefine. This change
doesn't affect any behavior." 

for changing the focus promise (courtesy of nhiroki):
"A promise resolution value type is just an annotation in WebIDL, so this change
does not affect actual implementation. We already resolve with WindowClient, see
http/tests/serviceworker/chromium/windowclient-focus.html

with that lgtm

[jungkees, 2015-07-03 04:02:56]

Yes I had double-checked the test is running okay. Thanks all for the review and
the clarification.

[jungkees, 2015-07-03 04:03:04]

The CQ bit was checked by jungkee.song@samsung.com

[commit-bot: I haz the power, 2015-07-03 04:03:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1214733006/1

[commit-bot: I haz the power, 2015-07-03 05:09:52]

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

[commit-bot: I haz the power, 2015-07-03 05:09:54]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/69167)

(exceeded global retry quota)

[jungkees, 2015-07-03 05:37:02]

Rebased onto ToT and updated
global-interface-listing-service-worker-expected.txt.

[jungkees, 2015-07-03 05:37:09]

The CQ bit was checked by jungkee.song@samsung.com

[jungkees, 2015-07-03 05:37:10]

The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org,
nhiroki@chromium.org
Link to the patchset: https://codereview.chromium.org/1214733006/#ps20001
(title: "Rebase; Update webexposed interface expected list.")

[commit-bot: I haz the power, 2015-07-03 05:37:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1214733006/20001

[commit-bot: I haz the power, 2015-07-03 07:05:17]

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

[commit-bot: I haz the power, 2015-07-03 07:05:18]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/6...)

[jungkees, 2015-07-03 07:31:17]

The CQ bit was checked by jungkee.song@samsung.com

[jungkees, 2015-07-03 07:31:18]

The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org,
nhiroki@chromium.org
Link to the patchset: https://codereview.chromium.org/1214733006/#ps40001
(title: "Don't trim trailing white space in the original file.")

[commit-bot: I haz the power, 2015-07-03 07:31:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1214733006/40001

[commit-bot: I haz the power, 2015-07-03 08:24:21]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=198269