Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix extraction of background image URLs. #576

Merged
merged 1 commit into from
Oct 12, 2022
Merged

Fix extraction of background image URLs. #576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion internal/process_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ def process_chrome_timings(self):
if 'name' in event and 'ts' in event and 'args' in event and 'frame' in event['args'] and \
event['args']['frame'] in main_frames and \
(event['ts'] >= start_time or 'value' in event['args']) and \
event['name'].lower().find('largest') and \
event['name'].lower().find('largest') >= 0 and \
'data' in event['args'] and 'size' in event['args']['data']:
name = event['name']
if name not in largest or event['args']['data']['size'] > largest[name]['args']['data']['size']:
Expand All @@ -379,6 +379,10 @@ def process_chrome_timings(self):
paint_event['element'] = event['args']['data']['element']
if 'type' in event['args']['data']:
paint_event['type'] = event['args']['data']['type']
if 'imageUrl' in event['args']['data'] and len(event['args']['data']['imageUrl']):
paint_event['imageUrl'] = event['args']['data']['imageUrl']
if 'url' in event['args']['data'] and len(event['args']['data']['url']):
paint_event['url'] = event['args']['data']['url']
if 'largestPaints' not in page_data:
page_data['largestPaints'] = []
page_data['largestPaints'].append(paint_event)
Expand Down Expand Up @@ -489,6 +493,8 @@ def process_chrome_timings(self):
if matches:
page_data['LargestContentfulPaintType'] = 'background-image'
page_data['LargestContentfulPaintImageURL'] = matches.group(1)
if 'imageUrl' in paint_event:
page_data['LargestContentfulPaintImageURL'] = paint_event['imageUrl']
elif 'largestPaints' in page_data:
for paint_event in page_data['largestPaints']:
if paint_event['event'] == 'LargestTextPaint' and paint_event['time'] == event['time']:
Expand Down
67 changes: 35 additions & 32 deletions internal/support/chrome/inject.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,37 +1,40 @@
new PerformanceObserver((entryList) => {
for (const entry of entryList.getEntries()) {
try {
let event = {
name: entry.name,
entryType: entry.entryType,
startTime: entry['startTime'],
size: entry['size'],
url: entry['url'],
id: entry['id'],
loadTime: entry['loadTime'],
renderTime: entry['renderTime'],
};
if (entry['element']) {
event['element'] = {
nodeName: entry.element['nodeName'],
boundingRect: entry.element.getBoundingClientRect(),
outerHTML: entry.element.outerHTML,
}
if (entry.element['src']) {
event.element['src'] = entry.element.src;
}
if (entry.element['currentSrc']) {
event.element['currentSrc'] = entry.element.currentSrc;
}
try {
let style = window.getComputedStyle(object[key]);
if (style.backgroundImage && style.backgroundImage != 'none') {
contents[key]['background-image'] = style.backgroundImage;
}
} catch (err) {
}
}
console.debug('wptagent_message:' + JSON.stringify({'name': 'perfentry', 'data': event}));
try {
let event = {
name: entry.name,
entryType: entry.entryType,
startTime: entry['startTime'],
size: entry['size'],
url: entry['url'],
id: entry['id'],
loadTime: entry['loadTime'],
renderTime: entry['renderTime'],
};
if (entry['element']) {
event['element'] = {
nodeName: entry.element['nodeName'],
boundingRect: entry.element.getBoundingClientRect(),
outerHTML: entry.element.outerHTML,
}
if (entry.element['src']) {
event.element['src'] = entry.element.src;
}
if (entry.element['currentSrc']) {
event.element['currentSrc'] = entry.element.currentSrc;
}
try {
let style = window.getComputedStyle(entry.element);
if (style.backgroundImage && style.backgroundImage != 'none') {
event.element['background-image'] = style.backgroundImage;
}
if (style.content && style.content != 'none') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious how do we use this? Because it can be a whole bunch of stuff other than none, I wonder why we filter out none

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'none' is the default value that gets populated when it's not explicitly set. https://developer.mozilla.org/en-US/docs/Web/CSS/background-image#syntax

event.element['content'] = style.content;
}
} catch (err) {
}
}
console.debug('wptagent_message:' + JSON.stringify({'name': 'perfentry', 'data': event}));
} catch (err) {
}
}
Expand Down
2 changes: 2 additions & 0 deletions internal/support/trace_parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,8 @@ def post_process_user_timing(self, dom_tree, performance_timing):
for perf_entry in performance_timing:
if 'entryType' in perf_entry and perf_entry['entryType'] == 'largest-contentful-paint' and 'size' in perf_entry and perf_entry['size'] == event['args']['data']['size'] and 'consumed' not in perf_entry:
perf_entry['consumed'] = True
if 'url' in perf_entry and len(perf_entry['url']) and 'url' not in event['args']['data']:
event['args']['data']['url'] = perf_entry['url']
if 'element' in perf_entry:
event['args']['data']['element'] = perf_entry['element']
elif 'score' in event['args']['data'] and event['name'].startswith('LayoutShift'):
Expand Down
12 changes: 6 additions & 6 deletions internal/webpagetest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1566,6 +1566,12 @@ def upload_task_result(self, task):
self.update_browser_viewport(task)
if task['run'] == 1 and not task['cached']:
self.collect_crux_data(task)
# Post-process the given test run
try:
from internal.process_test import ProcessTest
ProcessTest(self.options, self.job, task)
except Exception:
logging.exception('Error post-processing test')
# Stop logging to the file
if self.log_handler is not None:
try:
Expand All @@ -1586,12 +1592,6 @@ def upload_task_result(self, task):
if self.job['warmup'] > 0:
logging.debug('Discarding warmup run')
else:
# Post-process the given test run
try:
from internal.process_test import ProcessTest
ProcessTest(self.options, self.job, task)
except Exception:
logging.exception('Error post-processing test')
# Continue with the upload
if 'page_data' in task and 'fullyLoadedCPUpct' in task['page_data']:
self.cpu_pct = task['page_data']['fullyLoadedCPUpct']
Expand Down