New issue
Advanced search Search tips

Issue 910426 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

page_set should not be needed in initializer for a page

Project Member Reported by crouleau@chromium.org, Nov 30

Issue description

This is both confusing and painful to use.

if a page really needs a page_set reference, then we should be able to add it in StorySet's AddStory method. It would just be great to not have it in the initializer.
 
This is a good direction, though to be warned that it will not be trivial. The key reason why page needs a pointer to page_set is because the mapping from page to wpr recording is stored in page_set object (https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry/story/story_set.py?rcl=ee85992c5c619df6e291ba6dc2e86654f344a492&l=24)
Could we just move it to not be in the initializer? I guess that would have the issue of if someone added a single page object to multiple page sets then we would end up with the single page instance having only a reference to the pageset that it was most recently added to. :( 
Note for posterity that this idea was inspired by the hoops that I needed to jump through for https://chromium-review.googlesource.com/c/chromium/src/+/1356320
When I looked at this problem back then, the fix I come up with is to remove the need of the map from page to wpr recording. Instead, every page instance must specify the location of their wpr record.
That solution makes sense to me.

Sign in to add a comment