Show LastModified and ResponseTime of ServiceWorker script in DevTools (issue 1048763002 by horo@chromium.org)

0 views
Skip to first unread message

ho...@chromium.org

unread,
Mar 30, 2015, 4:23:41 AM3/30/15
to dgo...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, loislo...@chromium.org, eustas...@chromium.org, malch...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, aandre...@chromium.org, kozyatins...@chromium.org
Reviewers: dgozman,

Message:
dgozman@
Could you please review this?

Description:
Show LastModified and ResponseTime of ServiceWorker script in DevTools

This cl depends on https://codereview.chromium.org/1037933002

BUG=466871

Please review this at https://codereview.chromium.org/1048763002/

Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Affected files (+59, -5 lines):
M
LayoutTests/http/tests/inspector/service-workers/service-workers-view.html
M
LayoutTests/http/tests/inspector/service-workers/service-workers-view-expected.txt
M Source/devtools/front_end/resources/ServiceWorkersView.js
M Source/devtools/front_end/resources/serviceWorkersView.css
M Source/devtools/front_end/sdk/ServiceWorkerManager.js
M Source/devtools/protocol.json


Index:
LayoutTests/http/tests/inspector/service-workers/service-workers-view-expected.txt
diff --git
a/LayoutTests/http/tests/inspector/service-workers/service-workers-view-expected.txt
b/LayoutTests/http/tests/inspector/service-workers/service-workers-view-expected.txt
index
8d028097ec17440cb290b9b7b1241e8b5ebab702..758708325ac1c230260ce45d9626d88121251ec8
100644
---
a/LayoutTests/http/tests/inspector/service-workers/service-workers-view-expected.txt
+++
b/LayoutTests/http/tests/inspector/service-workers/service-workers-view-expected.txt
@@ -16,6 +16,8 @@ activated
running
/inspector/service-workers/resources/service-worker-empty.js
inspect
+LAST-MODIFIED
+RESPONSE-TIME


============================
@@ -30,6 +32,8 @@ activated
running
/inspector/service-workers/resources/service-worker-empty.js
inspect
+LAST-MODIFIED
+RESPONSE-TIME


Scope: /inspector/service-workers/resources/scope2/
@@ -39,6 +43,8 @@ activated
running
/inspector/service-workers/resources/service-worker-empty.js
inspect
+LAST-MODIFIED
+RESPONSE-TIME


============================
@@ -54,6 +60,8 @@ redundant
running
/inspector/service-workers/resources/service-worker-empty.js
inspect
+LAST-MODIFIED
+RESPONSE-TIME

Scope: /inspector/service-workers/resources/scope2/

@@ -62,6 +70,8 @@ activated
running
/inspector/service-workers/resources/service-worker-empty.js
inspect
+LAST-MODIFIED
+RESPONSE-TIME


============================
Index:
LayoutTests/http/tests/inspector/service-workers/service-workers-view.html
diff --git
a/LayoutTests/http/tests/inspector/service-workers/service-workers-view.html
b/LayoutTests/http/tests/inspector/service-workers/service-workers-view.html
index
3e5343ff71937a395562e5f6d9652a1bdba8d6f6..3ea8e7e275f945436e23747e1e90e6a4a24b6fce
100644
---
a/LayoutTests/http/tests/inspector/service-workers/service-workers-view.html
+++
b/LayoutTests/http/tests/inspector/service-workers/service-workers-view.html
@@ -15,9 +15,22 @@ function test()
var step = 0;


InspectorTest.addSniffer(WebInspector.ServiceWorkersView.prototype, "_registrationUpdated",
registrationUpdated, true);
+ function replaceInnerTextAll(rootElement, selectors, replacementString)
+ {
+ var elements = rootElement.querySelectorAll(selectors);
+ for (var i = 0; i < elements.length; i++)
+ elements[i].innerText = replacementString;
+ }
+ function modifyTestUnfriendlyText(rootElement)
+ {
+
replaceInnerTextAll(rootElement, ".service-worker-script-last-modified", "LAST-MODIFIED");
+
replaceInnerTextAll(rootElement, ".service-worker-script-response-time", "RESPONSE-TIME");
+ }
function dumpServiceWorkersView()
{
var swView = WebInspector.panels.resources.visibleView;
+
+ modifyTestUnfriendlyText(swView._root);
InspectorTest.addResult("==== ServiceWorkersView ====");

InspectorTest.addResult(swView._root.querySelector(".service-workers-origin-title").innerText);
var registrationElements =
swView._root.querySelectorAll(".service-workers-registration");
Index: Source/devtools/front_end/resources/ServiceWorkersView.js
diff --git a/Source/devtools/front_end/resources/ServiceWorkersView.js
b/Source/devtools/front_end/resources/ServiceWorkersView.js
index
eec898427161a22f3e1aff240945e0e72381c694..e3095b8e7095f4cf42c25b76a7243d2aa7037ad4
100644
--- a/Source/devtools/front_end/resources/ServiceWorkersView.js
+++ b/Source/devtools/front_end/resources/ServiceWorkersView.js
@@ -321,6 +321,20 @@ WebInspector.SWRegistrationElement.prototype = {

inspectButton.createTextChild(WebInspector.UIString("inspect"));
inspectButton.addEventListener("click",
this._inspectButtonClicked.bind(this, version.id), false);
}
+
+ if (version.scriptLastModified) {
+ var scriptLastModifiedDiv =
scriptURLDiv.createChild("div", "service-workers-info");
+ var scriptLastModifiedText =
WebInspector.UIString("Last-Modified: ") + (new
Date(version.scriptLastModified * 1000)).toConsoleTime();
+
scriptLastModifiedDiv.createChild("div", "service-workers-info-icon");
+
scriptLastModifiedDiv.createChild("div", "service-workers-info-message
service-worker-script-last-modified").createTextChild(scriptLastModifiedText);
+ }
+ if (version.scriptResponseTime) {
+ var scriptResponseTimeDiv =
scriptURLDiv.createChild("div", "service-workers-info");
+ var scriptResponseTimeText =
WebInspector.UIString("Response time: ") + (new
Date(version.scriptResponseTime * 1000)).toConsoleTime();
+
scriptResponseTimeDiv.createChild("div", "service-workers-info-icon");
+
scriptResponseTimeDiv.createChild("div", "service-workers-info-message
service-worker-script-response-time").createTextChild(scriptResponseTimeText);
+ }
+
var errorMessages = version.errorMessages;
for (var index = 0; index < errorMessages.length; ++index) {
var errorDiv =
scriptURLDiv.createChild("div", "service-workers-error");
@@ -334,6 +348,7 @@ WebInspector.SWRegistrationElement.prototype = {
script_path = String.sprintf("%s:%d", script_path,
errorMessages[index].lineNumber);

errorDiv.createChild("div", "service-workers-error-line").createTextChild(script_path);
}
+
}
if (!versions.length) {
var stateRowElement =
versionsElement.createChild("div", "service-workers-version-row");
Index: Source/devtools/front_end/resources/serviceWorkersView.css
diff --git a/Source/devtools/front_end/resources/serviceWorkersView.css
b/Source/devtools/front_end/resources/serviceWorkersView.css
index
3fefbd9b2a87c1ff9f3012399a39d15f939e8568..6ef9bead512e061fe23427c652fde9f877ff2504
100644
--- a/Source/devtools/front_end/resources/serviceWorkersView.css
+++ b/Source/devtools/front_end/resources/serviceWorkersView.css
@@ -187,10 +187,10 @@
opacity: 0.8;
}

-.service-workers-error-icon {
+.service-workers-error-icon,
+.service-workers-info-icon {
background-image: url(Images/statusbarButtonGlyphs.png);
background-size: 320px 144px;
- background-position: -213px -96px;
height: 10px;
width: 10px;
border: 0;
@@ -200,11 +200,21 @@
}

@media (-webkit-min-device-pixel-ratio: 1.5) {
-.service-workers-error-icon {
+.service-workers-error-icon,
+.service-workers-info-icon {
background-image: url(Images/statusbarButtonGlyphs_2x.png);
}
} /* media */

+
+.service-workers-error-icon {
+ background-position: -213px -96px;
+}
+
+.service-workers-info-icon {
+ background-position: -213px -107px;
+}
+
.service-workers-error {
display: flex;
}
@@ -213,6 +223,10 @@
font-weight: bold;
}

+.service-workers-info {
+ display: flex;
+}
+
.service-workers-color-0 {
background-color: #003366;
}
Index: Source/devtools/front_end/sdk/ServiceWorkerManager.js
diff --git a/Source/devtools/front_end/sdk/ServiceWorkerManager.js
b/Source/devtools/front_end/sdk/ServiceWorkerManager.js
index
205f9de7414127757465b1e1ab53c3fe1a012680..0f2df9d33d57fe51e4a60dbea155076e8fcbe11f
100644
--- a/Source/devtools/front_end/sdk/ServiceWorkerManager.js
+++ b/Source/devtools/front_end/sdk/ServiceWorkerManager.js
@@ -456,7 +456,8 @@ WebInspector.ServiceWorkerVersion.prototype = {
this.scriptURL = payload.scriptURL;
this.runningStatus = payload.runningStatus;
this.status = payload.status;
- this.updateTime = payload.updateTime;
+ this.scriptLastModified = payload.scriptLastModified;
+ this.scriptResponseTime = payload.scriptResponseTime;
},

/**
Index: Source/devtools/protocol.json
diff --git a/Source/devtools/protocol.json b/Source/devtools/protocol.json
index
78344e8f3b0ad5d504f6a258c59e2d1e0cd1ebf9..47ee5e270d2697b9ef83d49c00414f57bfcfeeb9
100644
--- a/Source/devtools/protocol.json
+++ b/Source/devtools/protocol.json
@@ -4106,7 +4106,8 @@
{ "name": "scriptURL", "type": "string" },

{ "name": "runningStatus", "$ref": "ServiceWorkerVersionRunningStatus" },

{ "name": "status", "$ref": "ServiceWorkerVersionStatus" },
- { "name": "updateTime", "type": "number", "optional":
true}
+
{ "name": "scriptLastModified", "type": "number", "optional": true},
+
{ "name": "scriptResponseTime", "type": "number", "optional": true}
]
},
{


dgo...@chromium.org

unread,
Mar 30, 2015, 8:54:23 AM3/30/15
to ho...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, loislo...@chromium.org, eustas...@chromium.org, malch...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, aandre...@chromium.org, kozyatins...@chromium.org
This patch includes https://codereview.chromium.org/1039983003/. Should we
close
the latter?


https://codereview.chromium.org/1048763002/diff/60001/Source/devtools/front_end/resources/ServiceWorkersView.js
File Source/devtools/front_end/resources/ServiceWorkersView.js (right):

https://codereview.chromium.org/1048763002/diff/60001/Source/devtools/front_end/resources/ServiceWorkersView.js#newcode327
Source/devtools/front_end/resources/ServiceWorkersView.js:327: var
scriptLastModifiedText = WebInspector.UIString("Last-Modified: ") + (new
Date(version.scriptLastModified * 1000)).toConsoleTime();
WI.UIString("Last-Modified: %s", <last modified value>)

https://codereview.chromium.org/1048763002/diff/60001/Source/devtools/front_end/resources/ServiceWorkersView.js#newcode331
Source/devtools/front_end/resources/ServiceWorkersView.js:331: if
(version.scriptResponseTime) {
As a user, I don't understand the "Response time" value. What does it
mean? Let's provide explanation in title, or even legend for the whole
SW view.

https://codereview.chromium.org/1048763002/diff/60001/Source/devtools/front_end/resources/ServiceWorkersView.js#newcode333
Source/devtools/front_end/resources/ServiceWorkersView.js:333: var
scriptResponseTimeText = WebInspector.UIString("Response time: ") + (new
Date(version.scriptResponseTime * 1000)).toConsoleTime();
ditto

https://codereview.chromium.org/1048763002/diff/60001/Source/devtools/front_end/resources/serviceWorkersView.css
File Source/devtools/front_end/resources/serviceWorkersView.css (right):

https://codereview.chromium.org/1048763002/diff/60001/Source/devtools/front_end/resources/serviceWorkersView.css#newcode190
Source/devtools/front_end/resources/serviceWorkersView.css:190:
.service-workers-error-icon,
Instead of these styles, just use custom element "dt-icon-label" and set
it's type. See smallIcon.css for available types.

https://codereview.chromium.org/1048763002/

ho...@chromium.org

unread,
Mar 30, 2015, 11:05:40 AM3/30/15
to dgo...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, loislo...@chromium.org, eustas...@chromium.org, malch...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, aandre...@chromium.org, kozyatins...@chromium.org
> This patch includes https://codereview.chromium.org/1039983003/. Should we
close
> the latter?

Sorry, I wrongly uploaded Patch Set 3.
I need to land 3 patches in this order.
1. blink: https://codereview.chromium.org/1039983003/
2. chromium: https://codereview.chromium.org/1037933002/
3. blink: https://codereview.chromium.org/1048763002/



https://codereview.chromium.org/1048763002/diff/60001/Source/devtools/front_end/resources/ServiceWorkersView.js
File Source/devtools/front_end/resources/ServiceWorkersView.js (right):

https://codereview.chromium.org/1048763002/diff/60001/Source/devtools/front_end/resources/ServiceWorkersView.js#newcode327
Source/devtools/front_end/resources/ServiceWorkersView.js:327: var
scriptLastModifiedText = WebInspector.UIString("Last-Modified: ") + (new
Date(version.scriptLastModified * 1000)).toConsoleTime();
On 2015/03/30 12:54:23, dgozman wrote:
> WI.UIString("Last-Modified: %s", <last modified value>)

Done.

https://codereview.chromium.org/1048763002/diff/60001/Source/devtools/front_end/resources/ServiceWorkersView.js#newcode331
Source/devtools/front_end/resources/ServiceWorkersView.js:331: if
(version.scriptResponseTime) {
On 2015/03/30 12:54:23, dgozman wrote:
> As a user, I don't understand the "Response time" value. What does it
mean?
> Let's provide explanation in title, or even legend for the whole SW
view.

Changed to "Server response time: "

https://codereview.chromium.org/1048763002/diff/60001/Source/devtools/front_end/resources/ServiceWorkersView.js#newcode333
Source/devtools/front_end/resources/ServiceWorkersView.js:333: var
scriptResponseTimeText = WebInspector.UIString("Response time: ") + (new
Date(version.scriptResponseTime * 1000)).toConsoleTime();
On 2015/03/30 12:54:23, dgozman wrote:
> ditto

Done.

https://codereview.chromium.org/1048763002/diff/60001/Source/devtools/front_end/resources/serviceWorkersView.css
File Source/devtools/front_end/resources/serviceWorkersView.css (right):

https://codereview.chromium.org/1048763002/diff/60001/Source/devtools/front_end/resources/serviceWorkersView.css#newcode190
Source/devtools/front_end/resources/serviceWorkersView.css:190:
.service-workers-error-icon,
On 2015/03/30 12:54:23, dgozman wrote:
> Instead of these styles, just use custom element "dt-icon-label" and
set it's
> type. See smallIcon.css for available types.

Done.

https://codereview.chromium.org/1048763002/

dgo...@chromium.org

unread,
Mar 30, 2015, 11:22:16 AM3/30/15
to ho...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, loislo...@chromium.org, eustas...@chromium.org, malch...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, aandre...@chromium.org, kozyatins...@chromium.org

https://codereview.chromium.org/1048763002/diff/80001/Source/devtools/front_end/resources/ServiceWorkersView.js
File Source/devtools/front_end/resources/ServiceWorkersView.js (right):

https://codereview.chromium.org/1048763002/diff/80001/Source/devtools/front_end/resources/ServiceWorkersView.js#newcode326
Source/devtools/front_end/resources/ServiceWorkersView.js:326: var
scriptLastModifiedDiv = scriptURLDiv.createChild("div",
"service-workers-info");
You can just create "dt-icon-label" in scriptURLDiv, set it's type and
createTextChild inside.

https://codereview.chromium.org/1048763002/diff/80001/Source/devtools/front_end/resources/ServiceWorkersView.js#newcode329
Source/devtools/front_end/resources/ServiceWorkersView.js:329:
scriptLastModifiedDiv.createChild("div", "service-workers-info-message
service-worker-script-last-modified").createTextChild(scriptLastModifiedText);
I don't see where you use these classes.

https://codereview.chromium.org/1048763002/diff/80001/Source/devtools/front_end/resources/ServiceWorkersView.js#newcode335
Source/devtools/front_end/resources/ServiceWorkersView.js:335:
scriptResponseTimeDiv.createChild("div", "service-workers-info-message
service-worker-script-response-time").createTextChild(scriptResponseTimeText);
ditto

https://codereview.chromium.org/1048763002/

ho...@chromium.org

unread,
Mar 30, 2015, 11:39:52 AM3/30/15
to dgo...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, loislo...@chromium.org, eustas...@chromium.org, malch...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, aandre...@chromium.org, kozyatins...@chromium.org

https://codereview.chromium.org/1048763002/diff/80001/Source/devtools/front_end/resources/ServiceWorkersView.js
File Source/devtools/front_end/resources/ServiceWorkersView.js (right):

https://codereview.chromium.org/1048763002/diff/80001/Source/devtools/front_end/resources/ServiceWorkersView.js#newcode326
Source/devtools/front_end/resources/ServiceWorkersView.js:326: var
scriptLastModifiedDiv = scriptURLDiv.createChild("div",
"service-workers-info");
On 2015/03/30 15:22:16, dgozman wrote:
> You can just create "dt-icon-label" in scriptURLDiv, set it's type and
> createTextChild inside.

Done.

https://codereview.chromium.org/1048763002/diff/80001/Source/devtools/front_end/resources/ServiceWorkersView.js#newcode329
Source/devtools/front_end/resources/ServiceWorkersView.js:329:
scriptLastModifiedDiv.createChild("div", "service-workers-info-message
service-worker-script-last-modified").createTextChild(scriptLastModifiedText);
On 2015/03/30 15:22:15, dgozman wrote:
> I don't see where you use these classes.

Removed service-workers-info-message.
service-worker-script-last-modified is used in LayouTest.

https://codereview.chromium.org/1048763002/diff/80001/Source/devtools/front_end/resources/ServiceWorkersView.js#newcode335
Source/devtools/front_end/resources/ServiceWorkersView.js:335:
scriptResponseTimeDiv.createChild("div", "service-workers-info-message
service-worker-script-response-time").createTextChild(scriptResponseTimeText);
On 2015/03/30 15:22:16, dgozman wrote:
> ditto

Removed service-workers-info-message.
service-worker-script-response-time is used in LayouTest.

https://codereview.chromium.org/1048763002/

dgo...@chromium.org

unread,
Mar 30, 2015, 11:53:33 AM3/30/15
to ho...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, loislo...@chromium.org, eustas...@chromium.org, malch...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, aandre...@chromium.org, kozyatins...@chromium.org
lgtm



https://codereview.chromium.org/1048763002/

commi...@chromium.org

unread,
Mar 31, 2015, 2:29:40 AM3/31/15
to ho...@chromium.org, dgo...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, loislo...@chromium.org, eustas...@chromium.org, malch...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, aandre...@chromium.org, kozyatins...@chromium.org
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1048763002/100001


https://codereview.chromium.org/1048763002/

commi...@chromium.org

unread,
Mar 31, 2015, 3:16:17 AM3/31/15
to ho...@chromium.org, dgo...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, loislo...@chromium.org, eustas...@chromium.org, malch...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, aandre...@chromium.org, kozyatins...@chromium.org
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1048763002/120001


https://codereview.chromium.org/1048763002/

commi...@chromium.org

unread,
Mar 31, 2015, 5:23:50 AM3/31/15
to ho...@chromium.org, dgo...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, loislo...@chromium.org, eustas...@chromium.org, malch...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, aandre...@chromium.org, kozyatins...@chromium.org
Committed patchset #5 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=192818

https://codereview.chromium.org/1048763002/
Reply all
Reply to author
Forward
0 new messages