Skip to content

Fix a logic bug in Browser::create_browser_context. #315

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

Open
chanxuehong wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix a logic bug in Browser::create_browser_context. #315

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
29 changes: 16 additions & 13 deletions src/browser/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,10 +330,6 @@ impl Browser {
.create_browser_context(CreateBrowserContextParams::default())
.await?;
self.browser_context = BrowserContext::from(browser_context_id);
self.sender
.clone()
.send(HandlerMessage::InsertContext(self.browser_context.clone()))
.await?;
}

Ok(self)
Expand All @@ -346,11 +342,7 @@ impl Browser {
/// incognito context.
pub async fn quit_incognito_context(&mut self) -> Result<&mut Self> {
if let Some(id) = self.browser_context.take() {
self.dispose_browser_context(id.clone()).await?;
self.sender
.clone()
.send(HandlerMessage::DisposeContext(BrowserContext::from(id)))
.await?;
self.dispose_browser_context(id).await?;
}
Ok(self)
}
Expand Down Expand Up @@ -458,19 +450,30 @@ impl Browser {
&self,
params: CreateBrowserContextParams,
) -> Result<BrowserContextId> {
let response = self.execute(params).await?;
Ok(response.result.browser_context_id)
let (tx, rx) = oneshot_channel();

self.sender
.clone()
.send(HandlerMessage::CreateBrowserContext(params, tx))
.await?;

rx.await?
}

/// Deletes a browser context.
pub async fn dispose_browser_context(
&self,
browser_context_id: impl Into<BrowserContextId>,
) -> Result<()> {
self.execute(DisposeBrowserContextParams::new(browser_context_id))
let (tx, rx) = oneshot_channel();

let params = DisposeBrowserContextParams::new(browser_context_id);
self.sender
.clone()
.send(HandlerMessage::DisposeBrowserContext(params, tx))
.await?;

Ok(())
rx.await?
}

/// Clears cookies.
Expand Down
106 changes: 100 additions & 6 deletions src/handler/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,30 @@ impl Handler {
}
}
}
PendingRequest::CreateBrowserContext(tx) => {
match to_command_response::<CreateBrowserContextParams>(resp, method) {
Ok(resp) => {
self.browser_contexts
.insert(BrowserContext::from(resp.browser_context_id.clone()));
let _ = tx.send(Ok(resp.browser_context_id.clone())).ok();
}
Err(err) => {
let _ = tx.send(Err(err)).ok();
}
}
}
PendingRequest::DisposeBrowserContext(browser_context_id, tx) => {
match to_command_response::<DisposeBrowserContextParams>(resp, method) {
Ok(_resp) => {
self.browser_contexts
.remove(&BrowserContext::from(browser_context_id));
let _ = tx.send(Ok(())).ok();
}
Err(err) => {
let _ = tx.send(Err(err)).ok();
}
}
}
PendingRequest::Navigate(id) => {
self.on_navigation_response(id, resp);
}
Expand Down Expand Up @@ -395,6 +419,63 @@ impl Handler {
}
}

fn create_browser_context(
&mut self,
params: CreateBrowserContextParams,
tx: OneshotSender<Result<BrowserContextId>>,
) {
let method = params.identifier();
match serde_json::to_value(params) {
Ok(params) => match self.conn.submit_command(method.clone(), None, params) {
Ok(call_id) => {
self.pending_commands.insert(
call_id,
(
PendingRequest::CreateBrowserContext(tx),
method,
Instant::now(),
),
);
}
Err(err) => {
let _ = tx.send(Err(err.into())).ok();
}
},
Err(err) => {
let _ = tx.send(Err(err.into())).ok();
}
}
}

fn dispose_browser_context(
&mut self,
params: DisposeBrowserContextParams,
tx: OneshotSender<Result<()>>,
) {
let method = params.identifier();
let browser_context_id = params.browser_context_id.clone();
match serde_json::to_value(params) {
Ok(params) => match self.conn.submit_command(method.clone(), None, params) {
Ok(call_id) => {
self.pending_commands.insert(
call_id,
(
PendingRequest::DisposeBrowserContext(browser_context_id, tx),
method,
Instant::now(),
),
);
}
Err(err) => {
let _ = tx.send(Err(err.into())).ok();
}
},
Err(err) => {
let _ = tx.send(Err(err.into())).ok();
}
}
}

/// Process an incoming event read from the websocket
fn on_event(&mut self, event: CdpEventMessage) {
if let Some(ref session_id) = event.session_id {
Expand Down Expand Up @@ -495,6 +576,12 @@ impl Handler {
PendingRequest::GetTargets(tx) => {
let _ = tx.send(Err(CdpError::Timeout));
}
PendingRequest::CreateBrowserContext(tx) => {
let _ = tx.send(Err(CdpError::Timeout));
}
PendingRequest::DisposeBrowserContext(_browser_context_id, tx) => {
let _ = tx.send(Err(CdpError::Timeout));
}
PendingRequest::Navigate(nav) => {
if let Some(nav) = self.navigations.remove(&nav) {
match nav {
Expand Down Expand Up @@ -556,11 +643,11 @@ impl Stream for Handler {
.collect();
let _ = tx.send(pages);
}
HandlerMessage::InsertContext(ctx) => {
pin.browser_contexts.insert(ctx);
HandlerMessage::CreateBrowserContext(params, tx) => {
pin.create_browser_context(params, tx);
}
HandlerMessage::DisposeContext(ctx) => {
pin.browser_contexts.remove(&ctx);
HandlerMessage::DisposeBrowserContext(params, tx) => {
pin.dispose_browser_context(params, tx);
}
HandlerMessage::GetPage(target_id, tx) => {
let page = pin
Expand Down Expand Up @@ -733,6 +820,10 @@ enum PendingRequest {
CreateTarget(OneshotSender<Result<Page>>),
/// A Request to fetch old `Target`s created before connection
GetTargets(OneshotSender<Result<Vec<TargetInfo>>>),
/// A Request to create a new `BrowserContext`.
CreateBrowserContext(OneshotSender<Result<BrowserContextId>>),
/// A Request to dispose an old `BrowserContext`.
DisposeBrowserContext(BrowserContextId, OneshotSender<Result<()>>),
/// A Request to navigate a specific `Target`.
///
/// Navigation requests are not automatically completed once the response to
Expand All @@ -756,8 +847,11 @@ enum PendingRequest {
pub(crate) enum HandlerMessage {
CreatePage(CreateTargetParams, OneshotSender<Result<Page>>),
FetchTargets(OneshotSender<Result<Vec<TargetInfo>>>),
InsertContext(BrowserContext),
DisposeContext(BrowserContext),
CreateBrowserContext(
CreateBrowserContextParams,
OneshotSender<Result<BrowserContextId>>,
),
DisposeBrowserContext(DisposeBrowserContextParams, OneshotSender<Result<()>>),
GetPages(OneshotSender<Vec<Page>>),
Command(CommandMessage),
GetPage(TargetId, OneshotSender<Option<Page>>),
Expand Down