Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1151993003:
[2/5 blink] Shows the clients which are controlled by ServiceWorker in DevTools. (Closed)
|
Created:
5 years, 6 months ago by horo Modified:
5 years, 6 months ago Reviewers:
pfeldman CC:
blink-reviews, caseq+blink_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, sergeyv+blink_chromium.org, kozyatinskiy+blink_chromium.org Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Target Ref:
refs/heads/master Project:
blink Visibility:
Public. |
Description[2/5 blink] Shows the clients which are controlled by ServiceWorker in DevTools.
This cl changes protocol.json.
Screenshot: https://code.google.com/p/chromium/issues/detail?id=466871#c65
1/5 chromium: https://codereview.chromium.org/1160133002/
2/5 blink: This cl.
3/5 chromium: https://codereview.chromium.org/1149383004/
4/5 blink: https://codereview.chromium.org/1164583002/
5/5 chromium: https://codereview.chromium.org/1143363009/
BUG=466871
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196834
Patch Set 1 : #
Total comments: 3
Patch Set 2 : use DevToolsAgentHost ID for ClientId #
Total comments: 14
Patch Set 3 : incorporated pfeldman's comment #Patch Set 4 : s/focus/activate/ #Messages
Total messages: 17 (4 generated)
Patchset #1 (id:1) has been deleted
horo@chromium.org changed reviewers: + pfeldman@chromium.org
pfeldman@ Could you please review this?
https://codereview.chromium.org/1151993003/diff/20001/Source/devtools/protoco... File Source/devtools/protocol.json (right): https://codereview.chromium.org/1151993003/diff/20001/Source/devtools/protoco... Source/devtools/protocol.json:4124: { "name": "id", "type": "string" }, I think we should reuse the DevToolsAgentHost IDs here. That way we use only one way of referring to the targets in the protocol and for example, worker ids here would match the worker ids in workerCreated. We would also be able to relate this information with the 'discovery' domain that we always wanted to introduce. DevToolsAgentHost can be easily created for workers and pages, once it is created, it'll be there and will be accessible statically by its id. It also knows how to activate (focus) itself, how to close itself, has a url, title and favicon available. We could expose single Browser.getTargetInfo(id) method that would return all of it to you.
https://codereview.chromium.org/1151993003/diff/20001/Source/devtools/protoco... File Source/devtools/protocol.json (right): https://codereview.chromium.org/1151993003/diff/20001/Source/devtools/protoco... Source/devtools/protocol.json:4124: { "name": "id", "type": "string" }, On 2015/06/02 13:23:43, pfeldman wrote: > I think we should reuse the DevToolsAgentHost IDs here. That way we use only one > way of referring to the targets in the protocol and for example, worker ids here > would match the worker ids in workerCreated. We would also be able to relate > this information with the 'discovery' domain that we always wanted to introduce. > DevToolsAgentHost can be easily created for workers and pages, once it is > created, it'll be there and will be accessible statically by its id. It also > knows how to activate (focus) itself, how to close itself, has a url, title and > favicon available. We could expose single Browser.getTargetInfo(id) method that > would return all of it to you. Sorry, I don't understand well about the 'discovery' domain. When it will be available? Is there anyone who are implementing it?
You can either add it or add its methods into the SW domain for now so that it does not block you.
On 2015/06/03 12:06:15, pfeldman wrote: > You can either add it or add its methods into the SW domain for now so that it > does not block you. OK, then I will add them into the SW domain for now.
Patchset #2 (id:40001) has been deleted
https://codereview.chromium.org/1151993003/diff/20001/Source/devtools/protoco... File Source/devtools/protocol.json (right): https://codereview.chromium.org/1151993003/diff/20001/Source/devtools/protoco... Source/devtools/protocol.json:4124: { "name": "id", "type": "string" }, On 2015/06/03 10:30:53, horo wrote: > On 2015/06/02 13:23:43, pfeldman wrote: > > I think we should reuse the DevToolsAgentHost IDs here. That way we use only > one > > way of referring to the targets in the protocol and for example, worker ids > here > > would match the worker ids in workerCreated. We would also be able to relate > > this information with the 'discovery' domain that we always wanted to > introduce. > > DevToolsAgentHost can be easily created for workers and pages, once it is > > created, it'll be there and will be accessible statically by its id. It also > > knows how to activate (focus) itself, how to close itself, has a url, title > and > > favicon available. We could expose single Browser.getTargetInfo(id) method > that > > would return all of it to you. > > Sorry, I don't understand well about the 'discovery' domain. > When it will be available? Is there anyone who are implementing it? I updated CLs to use DevToolsAgentHost ID. Please review them again. Thank you.
https://codereview.chromium.org/1151993003/diff/60001/Source/devtools/protoco... File Source/devtools/protocol.json (right): https://codereview.chromium.org/1151993003/diff/60001/Source/devtools/protoco... Source/devtools/protocol.json:4127: { "name": "controlledClients", "type": "array", "optional": true, "items": { "type": "string" } } $ref TargetID https://codereview.chromium.org/1151993003/diff/60001/Source/devtools/protoco... Source/devtools/protocol.json:4149: "id": "ServiceWorkerClientInfo", TargetInfo https://codereview.chromium.org/1151993003/diff/60001/Source/devtools/protoco... Source/devtools/protocol.json:4153: { "name": "type", "$ref": "ServiceWorkerClientType" }, Keep type string for now (see DevToolsAgentHost for the list of actual types), also add the title: string. https://codereview.chromium.org/1151993003/diff/60001/Source/devtools/protoco... Source/devtools/protocol.json:4241: "name": "getClientInfo", getTargetInfo https://codereview.chromium.org/1151993003/diff/60001/Source/devtools/protoco... Source/devtools/protocol.json:4244: { "name": "clientId", "type": "string" } targetIdm, $ref "TargetID" (extends string) https://codereview.chromium.org/1151993003/diff/60001/Source/devtools/protoco... Source/devtools/protocol.json:4247: { "name": "clientInfo","$ref": "ServiceWorkerClientInfo" } targetId https://codereview.chromium.org/1151993003/diff/60001/Source/devtools/protoco... Source/devtools/protocol.json:4254: { "name": "clientId", "type": "string" } targetId
https://codereview.chromium.org/1151993003/diff/60001/Source/devtools/protoco... File Source/devtools/protocol.json (right): https://codereview.chromium.org/1151993003/diff/60001/Source/devtools/protoco... Source/devtools/protocol.json:4127: { "name": "controlledClients", "type": "array", "optional": true, "items": { "type": "string" } } On 2015/06/05 12:19:18, pfeldman wrote: > $ref TargetID Done. https://codereview.chromium.org/1151993003/diff/60001/Source/devtools/protoco... Source/devtools/protocol.json:4149: "id": "ServiceWorkerClientInfo", On 2015/06/05 12:19:18, pfeldman wrote: > TargetInfo Done. https://codereview.chromium.org/1151993003/diff/60001/Source/devtools/protoco... Source/devtools/protocol.json:4153: { "name": "type", "$ref": "ServiceWorkerClientType" }, On 2015/06/05 12:19:18, pfeldman wrote: > Keep type string for now (see DevToolsAgentHost for the list of actual types), > also add the title: string. Done. https://codereview.chromium.org/1151993003/diff/60001/Source/devtools/protoco... Source/devtools/protocol.json:4241: "name": "getClientInfo", On 2015/06/05 12:19:18, pfeldman wrote: > getTargetInfo Done. https://codereview.chromium.org/1151993003/diff/60001/Source/devtools/protoco... Source/devtools/protocol.json:4244: { "name": "clientId", "type": "string" } On 2015/06/05 12:19:18, pfeldman wrote: > targetIdm, $ref "TargetID" (extends string) Done. https://codereview.chromium.org/1151993003/diff/60001/Source/devtools/protoco... Source/devtools/protocol.json:4247: { "name": "clientInfo","$ref": "ServiceWorkerClientInfo" } On 2015/06/05 12:19:18, pfeldman wrote: > targetId changed to targetInfo. https://codereview.chromium.org/1151993003/diff/60001/Source/devtools/protoco... Source/devtools/protocol.json:4254: { "name": "clientId", "type": "string" } On 2015/06/05 12:19:18, pfeldman wrote: > targetId Done.
pfeldman@ Ping?
lgtm. sorry i thought i stamped it
The CQ bit was checked by horo@chromium.org
Thank you!
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1151993003/100001
Message was sent while issue was closed.
Committed patchset #4 (id:100001) as https://src.chromium.org/viewvc/blink?view=rev&revision=196834 |
