Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upLink entry pipelines #15566
Link entry pipelines #15566
Conversation
highfive
commented
Feb 15, 2017
|
Heads up! This PR modifies the following files:
|
|
r? @asajeffrey |
| None => { | ||
| let pipeline_id = match entry.pipeline_id() { | ||
| Some(pipeline_id) => pipeline_id, | ||
| None => { |
This comment has been minimized.
This comment has been minimized.
| match exit_mode { | ||
| ExitPipelineMode::Normal => pipeline.exit(dbc), | ||
| ExitPipelineMode::Force => pipeline.force_exit(dbc), | ||
| } | ||
| debug!("Closed pipeline {:?}.", pipeline_id); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
cbrewster
force-pushed the
cbrewster:link_entry_pipelines
branch
from
e79ab89
to
2170b0d
cbrewster
force-pushed the
cbrewster:link_entry_pipelines
branch
from
2170b0d
to
558843b
|
|
cbrewster
force-pushed the
cbrewster:link_entry_pipelines
branch
from
558843b
to
e64556b
|
I'm not in love with Rc'ing a PipelineId; I wonder if we can use the same sort of weak Rc that we used for event loops? That is, a |
| /// The URL for the current session history entry | ||
| pub url: ServoUrl, | ||
| /// The current session history entry. | ||
| pub current: FrameState, |
This comment has been minimized.
This comment has been minimized.
asajeffrey
Feb 21, 2017
Member
The reason for having instant, pipeline_id and url as separate fields was to ensure that the current pipeline id is never None. This is in the constellation, so doing some hoop-jumping to avoid panic is probablyu worth it.
This comment has been minimized.
This comment has been minimized.
cbrewster
Feb 22, 2017
Author
Member
Fair enough, Although this does become hard to maintain once we add more info on FrameState. Maybe we could have an info struct that contains all the frame state info other than the pipeline id?
| extern crate msg; | ||
| extern crate servo_url; | ||
|
|
||
| #[cfg(test)] mod frame; |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
cbrewster
Feb 22, 2017
Author
Member
Probably not, but all of the other unit test crates follow this pattern.
| // Discard should work now that current is no longer linked to this entry. | ||
| let evicted_id = frame.discard_entry(&frame.prev[0]); | ||
| assert_eq!(evicted_id, Some(pipeline_id)); | ||
| } |
This comment has been minimized.
This comment has been minimized.
|
Using |
|
True, I wonder if there's an API which is like RecoverableWeak<T> {
...
fn upgrade_or(&self, default: T) -> Rc<T> { ... }
fn upgrade_or_else(&self, factory: F) -> Rc<T> where F: FnOnce() -> T { ... }
} |
|
I think we could implement that using |
|
|
|
I believe we are going to take this in a different direction |
cbrewster commentedFeb 15, 2017
•
edited by larsbergstrom
Soon we will be allowing multiple entries to share pipelines. This causes issues when handling how we discard pipelines in distant history entries. This change uses an
Rcto make all entries that share the same pipeline id actually point to the same pipeline id. Once one of the entries is reloaded, the pipeline is now automatically used in all other entries that previously shared the discarded pipeline.A link is broken when a navigation occurs or when a navigation with replacement enabled occurs.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is