Delete session on last window

[andreastt]

This implements step 4 of the Close Window steps that closes the session.

When the window handle array sent back from the CloseWindow command is empty, delete the session.

It also changes the WebDriverError’s API slightly by publishing the delete_session field instead of using the hard-to-understand set_delete_session and delete_session getter/setters.

---

This change is 

[andreastt]

cc @whimboo

[jgraham]

Reviewed 1 of 1 files at r1, 1 of 1 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, 1 of 1 files at r5.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

src/error.rs, line 108 at r4 (raw file):

    pub error: ErrorStatus,
    pub message: Cow<'static, str>,
    pub delete_session: bool,

I'm not really convinced this API change is worth the breakage. I mean it's probably better, but not lots better.

---

src/server.rs, line 74 at r5 (raw file):

                        }
                        Ok(WebDriverResponse::CloseWindow(ValueResponse { ref value })) => {
                            let windows = value.as_array().unwrap();

Don't use unwrap here. Either change the code to return an error in this case, or perhaps preferably, change the response type to ensure that we only have an array earlier.

---

Comments from Reviewable

[andreastt]

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

src/error.rs, line 108 at r4 (raw file):

Previously, jgraham wrote…

I'm not really convinced this API change is worth the breakage. I mean it's probably better, but not lots better.

Sure, would you be more comfortable with this change if I drop this specific commit?

---

Comments from Reviewable

[andreastt]

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

src/server.rs, line 74 at r5 (raw file):

Previously, jgraham wrote…

Don't use unwrap here. Either change the code to return an error in this case, or perhaps preferably, change the response type to ensure that we only have an array earlier.

I can do something like this to make sure it always works?

let empty = vec![];
let windows = value.as_array().unwrap_or(empty.as_ref());

---

Comments from Reviewable

[jgraham]

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

src/error.rs, line 108 at r4 (raw file):

Previously, andreastt (Andreas Tolfsen) wrote…

Sure, would you be more comfortable with this change if I drop this specific commit?

Well I guess now I think of it adding an extra response type will probably also break consumers, so I guess I'm fine with this.

---

Comments from Reviewable

[jgraham]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

src/server.rs, line 74 at r5 (raw file):

Previously, andreastt (Andreas Tolfsen) wrote…

I can do something like this to make sure it always works?

let empty = vec![];
let windows = value.as_array().unwrap_or(empty.as_ref());

Well you can, but that would hide problems. Can't we make a new CloseWindowResponse type that takes an array? Then if we can't construct that from the actual response we'll return an error.

---

Comments from Reviewable

[andreastt]

Review status: 1 of 3 files reviewed at latest revision, 1 unresolved discussion.

---

src/server.rs, line 74 at r5 (raw file):

Previously, jgraham wrote…

Well you can, but that would hide problems. Can't we make a new CloseWindowResponse type that takes an array? Then if we can't construct that from the actual response we'll return an error.

That’s a great idea! I’ve made it so.

---

Comments from Reviewable

[andreastt]

Review status: 1 of 3 files reviewed at latest revision, 1 unresolved discussion.

---

src/server.rs, line 74 at r5 (raw file):

Previously, andreastt (Andreas Tolfsen) wrote…

That’s a great idea! I’ve made it so.

Done.

---

Comments from Reviewable

[jgraham]

Reviewed 1 of 2 files at r6, 1 of 1 files at r7.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable