Commit c5a4d782 authored by Qiaofei Ye's avatar Qiaofei Ye Committed by Commit Bot

Remove containerName truncation, switch to use atomicString

As mentioned in Bug 1030396, the containerName attribute was first added in https://codereview.chromium.org/2515993002/ and it was truncated to 100 chars. The spec does not mention this truncation anywhere, see https://w3c.github.io/longtasks/.

In this PR, we removed undocumented containerName truncation, and changed member variables to use
atomicString instead of String.

Bug: 1030396
Change-Id: Ie1e7e6d7d1878ca16091a5f4db316d9c59368be0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2276680
Commit-Queue: Qiaofei Ye <qiaye@microsoft.com>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Reviewed-by: default avatarNicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786484}
parent 1c02ee80
...@@ -35,15 +35,15 @@ AtomicString TaskAttributionTiming::containerType() const { ...@@ -35,15 +35,15 @@ AtomicString TaskAttributionTiming::containerType() const {
return container_type_; return container_type_;
} }
String TaskAttributionTiming::containerSrc() const { AtomicString TaskAttributionTiming::containerSrc() const {
return container_src_; return container_src_;
} }
String TaskAttributionTiming::containerId() const { AtomicString TaskAttributionTiming::containerId() const {
return container_id_; return container_id_;
} }
String TaskAttributionTiming::containerName() const { AtomicString TaskAttributionTiming::containerName() const {
return container_name_; return container_name_;
} }
......
...@@ -20,9 +20,9 @@ class TaskAttributionTiming final : public PerformanceEntry { ...@@ -20,9 +20,9 @@ class TaskAttributionTiming final : public PerformanceEntry {
PerformanceEntryType EntryTypeEnum() const override; PerformanceEntryType EntryTypeEnum() const override;
AtomicString containerType() const; AtomicString containerType() const;
String containerSrc() const; AtomicString containerSrc() const;
String containerId() const; AtomicString containerId() const;
String containerName() const; AtomicString containerName() const;
void Trace(Visitor*) const override; void Trace(Visitor*) const override;
...@@ -37,10 +37,9 @@ class TaskAttributionTiming final : public PerformanceEntry { ...@@ -37,10 +37,9 @@ class TaskAttributionTiming final : public PerformanceEntry {
void BuildJSONValue(V8ObjectBuilder&) const override; void BuildJSONValue(V8ObjectBuilder&) const override;
AtomicString container_type_; AtomicString container_type_;
// TODO(crbug.com/1030396): change the members below to AtomicString. AtomicString container_src_;
String container_src_; AtomicString container_id_;
String container_id_; AtomicString container_name_;
String container_name_;
}; };
} // namespace blink } // namespace blink
......
...@@ -63,14 +63,11 @@ namespace blink { ...@@ -63,14 +63,11 @@ namespace blink {
namespace { namespace {
String GetFrameAttribute(HTMLFrameOwnerElement* frame_owner, AtomicString GetFrameAttribute(HTMLFrameOwnerElement* frame_owner,
const QualifiedName& attr_name, const QualifiedName& attr_name) {
bool truncate) { AtomicString attr_value;
String attr_value;
if (frame_owner->hasAttribute(attr_name)) { if (frame_owner->hasAttribute(attr_name)) {
attr_value = frame_owner->getAttribute(attr_name); attr_value = frame_owner->getAttribute(attr_name);
if (truncate && attr_value.length() > 100)
attr_value = attr_value.Substring(0, 100); // Truncate to 100 chars
} }
return attr_value; return attr_value;
} }
...@@ -94,12 +91,12 @@ AtomicString GetFrameOwnerType(HTMLFrameOwnerElement* frame_owner) { ...@@ -94,12 +91,12 @@ AtomicString GetFrameOwnerType(HTMLFrameOwnerElement* frame_owner) {
return ""; return "";
} }
String GetFrameSrc(HTMLFrameOwnerElement* frame_owner) { AtomicString GetFrameSrc(HTMLFrameOwnerElement* frame_owner) {
switch (frame_owner->OwnerType()) { switch (frame_owner->OwnerType()) {
case mojom::blink::FrameOwnerElementType::kObject: case mojom::blink::FrameOwnerElementType::kObject:
return GetFrameAttribute(frame_owner, html_names::kDataAttr, false); return GetFrameAttribute(frame_owner, html_names::kDataAttr);
default: default:
return GetFrameAttribute(frame_owner, html_names::kSrcAttr, false); return GetFrameAttribute(frame_owner, html_names::kSrcAttr);
} }
} }
...@@ -312,11 +309,10 @@ void WindowPerformance::ReportLongTask(base::TimeTicks start_time, ...@@ -312,11 +309,10 @@ void WindowPerformance::ReportLongTask(base::TimeTicks start_time,
} else { } else {
HTMLFrameOwnerElement* frame_owner = HTMLFrameOwnerElement* frame_owner =
culprit_dom_window->GetFrame()->DeprecatedLocalOwner(); culprit_dom_window->GetFrame()->DeprecatedLocalOwner();
AddLongTaskTiming( AddLongTaskTiming(start_time, end_time, attribution.first,
start_time, end_time, attribution.first, GetFrameOwnerType(frame_owner), GetFrameOwnerType(frame_owner), GetFrameSrc(frame_owner),
GetFrameSrc(frame_owner), GetFrameAttribute(frame_owner, html_names::kIdAttr),
GetFrameAttribute(frame_owner, html_names::kIdAttr, false), GetFrameAttribute(frame_owner, html_names::kNameAttr));
GetFrameAttribute(frame_owner, html_names::kNameAttr, true));
} }
} }
......
<!DOCTYPE HTML>
<meta charset=utf-8>
<title>LongTask Timing: long tasks in long-name iframe containers</title>
<body>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<h1>Longtasks in iframe</h1>
<div id="log"></div>
<script>
const longContainerName = 'iframeWithLongNameMoreThan100CharactersSpaceHolderSpaceHolderSpaceHolderSpaceHolderSpaceHolderSpaceHolder';
promise_test(async t => {
assert_implements(window.PerformanceLongTaskTiming, 'Longtasks are not supported.');
const initialTime = performance.now();
return new Promise(resolve => {
const observer = new PerformanceObserver(t.step_func(entryList => {
const entries = entryList.getEntries();
assert_equals(entries.length, 1,
'Exactly one entry is expected.');
const longtask = entries[0];
assert_equals(longtask.entryType, 'longtask');
if (longtask.name == 'self' ||
longtask.name == 'multiple-contexts' ||
longtask.name == 'unknown')
return;
assert_equals(longtask.name, 'same-origin-descendant');
assert_greater_than(longtask.duration, 50);
assert_greater_than_equal(longtask.startTime, initialTime);
const currentTime = performance.now();
assert_less_than_equal(longtask.startTime, currentTime);
// Assert the TaskAttributionTiming entry in attribution.
assert_equals(longtask.attribution.length, 1,
'Exactly one attribution entry is expected');
const attribution = longtask.attribution[0];
assert_equals(attribution.entryType, 'taskattribution');
assert_equals(attribution.name, 'unknown');
assert_equals(attribution.duration, 0);
assert_equals(attribution.startTime, 0);
assert_equals(attribution.containerId, longContainerName + '-id');
assert_equals(attribution.containerName, longContainerName + '-name');
assert_equals(attribution.containerSrc, 'resources/subframe-with-longtask.html');
observer.disconnect();
resolve();
}));
observer.observe({entryTypes: ['longtask']});
const iframe = document.createElement('iframe');
iframe.id = longContainerName + '-id';
iframe.name = longContainerName + '-name';
iframe.src = 'resources/subframe-with-longtask.html';
document.body.appendChild(iframe);
});
}, `Performance longtask entries in ${longContainerName} are observable in parent.`);
</script>
</body>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment