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.

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

Make PipelineId a pair of PipelineId and Option<SubpageId> #5235

Closed
jdm opened this issue Mar 17, 2015 · 11 comments
Closed

Make PipelineId a pair of PipelineId and Option<SubpageId> #5235

jdm opened this issue Mar 17, 2015 · 11 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Mar 17, 2015

This would conceivably allow us to move all subpage-related modification out of script and into the constellation, where it arguably belongs.

@jdm
Copy link
Member Author

@jdm jdm commented Mar 17, 2015

Blocks #5236.

@frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Mar 17, 2015

I'll take a stab at this right now

@glennw
Copy link
Member

@glennw glennw commented Mar 17, 2015

I was thinking something like:

struct PipelineId {
    window_id: WindowId,
    subpage_id: Option<SubpageId>,
};

And then have the HTMLIFrameElement code be the one that generates the unique pipeline id, since that would simplify a number of other things in the future.

@jdm does that sound reasonable to you? It might hit an unforeseen roadblock, but it sounds like that could work.

@jdm jdm added the C-assigned label Mar 17, 2015
@jdm
Copy link
Member Author

@jdm jdm commented Mar 17, 2015

@glennw So the iframe would get the window id from the window, and create the new subpage id? That sounds reasonable to me.

@glennw
Copy link
Member

@glennw glennw commented Mar 17, 2015

Yes, the window_id would be set once on each window creation, and the window would contain the subpage counter (as it does now) which the iframe would query/increment.

frewsxcv added a commit to frewsxcv/servo that referenced this issue Mar 17, 2015
frewsxcv added a commit to frewsxcv/servo that referenced this issue Mar 17, 2015
frewsxcv added a commit to frewsxcv/servo that referenced this issue Mar 17, 2015
frewsxcv added a commit to frewsxcv/servo that referenced this issue Mar 17, 2015
frewsxcv added a commit to frewsxcv/servo that referenced this issue Mar 17, 2015
frewsxcv added a commit to frewsxcv/servo that referenced this issue Mar 17, 2015
frewsxcv added a commit to frewsxcv/servo that referenced this issue Mar 18, 2015
frewsxcv added a commit to frewsxcv/servo that referenced this issue Mar 18, 2015
frewsxcv added a commit to frewsxcv/servo that referenced this issue Mar 18, 2015
@frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Mar 22, 2015

FYI, I didn't feel I have the knowledge to complete this right now, so I'm no longer working on it

@jdm jdm removed the C-assigned label Mar 22, 2015
@KiChjang
Copy link
Member

@KiChjang KiChjang commented Jun 7, 2015

I can take a look at this one.

@jdm jdm added the C-assigned label Jun 8, 2015
@KiChjang
Copy link
Member

@KiChjang KiChjang commented Jun 9, 2015

I just took a look at the previous PR, and I came across with the usage of the new WindowId struct. How and where is it going to be used and created?

@benschulz
Copy link
Contributor

@benschulz benschulz commented Sep 4, 2015

In its current form this issue seems to contradict #5241.

As far as @KiChjang's question is concerned, my interpretation is as follows. The constellation manages a (bijective) mapping from PipelineId to WindowId by incrementing a counter for each new PipelineId it receives from ScriptLoadedURLInIFrame messages. Then it sends the generated WindowId as part of the AttachLayout message. However, I'm not 100% certain that's right.

Assuming you get an answer to your question, will you continue working on this, @KiChjang?

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Sep 7, 2015

You can go ahead and work on this issue, I have plenty of other stuff on my plate right now.

@glennw
Copy link
Member

@glennw glennw commented Nov 16, 2015

This is handled a different way now - iframes are able to generate their own PipelineId.

@glennw glennw closed this Nov 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants
You can’t perform that action at this time.