Add auto-retry on connection failure.

[bemasc]

Partially fixes #1825

Looks like this:

@lucyhe @jpevarnek

[salomegeo]

maybe remove this from messages.json too?

[bemasc]

I added this back in.

[lucyhe]

Review status: 0 of 5 files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

---

^{src/generic_ui/polymer/root.html, line 383 [r2] (raw file):}
Personal preference is "Reconnecting" would look better if text were smaller

---

^{src/generic_ui/scripts/ui.ts, line 372 [r2] (raw file):}
Is this to block failures which time out after a new attempt succeeds?

---

^{src/generic_ui/scripts/ui.ts, line 588 [r2] (raw file):}
If we keep this, I feel like this should be this.stopGettingFromInstance(data.instanceId); and in the else if below there should be this.stopGettingFromInstance(this.instanceGettingAccessFrom_);

Longer, optional suggestion: since .browserApi.stopUsingProxy was moved out and this function to only called after we have already terminated the connection (by calling stopGettingFromInstance) or if we received a disconnect message from the core, I think this function could be changed to "stoppedGetting". This would require one more change in root.ts's revertProxySettings, which would need to set disconnectedWhileProxying to false and update the icon (which is private right now though). I feel like this is better than having revertProxySettings call stopGettingInUiAndConfig when the "AndConfig" part is now handled by stopUsingProxy, and the "stopGettingInUi" part has already happened (i.e. getting status is already gone and icon is now an error icon). (Hope this made sense)

---

Comments from the review on Reviewable.io

[bemasc]

Review status: 0 of 7 files reviewed at latest revision, 3 unresolved discussions, some commit checks pending.

---

^{src/generic_ui/polymer/root.html, line 383 [r2] (raw file):}
Done. (See updated screenshots.)

---

^{src/generic_ui/scripts/ui.ts, line 372 [r2] (raw file):}
I added a comment to try to explain what this is for.

---

^{src/generic_ui/scripts/ui.ts, line 589 [r2] (raw file):}
Done! Instead of making updateIcon_ public, I just made sure to call it everywhere that changes icon-relevant state.

---

Comments from the review on Reviewable.io

[lucyhe]

Awesome! 👍

---

Review status: 0 of 7 files reviewed at latest revision, all discussions resolved, some commit checks broke.

---

Comments from the review on Reviewable.io

[salomegeo]

Review status: 0 of 7 files reviewed at latest revision, 3 unresolved discussions, some commit checks broke.

---

^{src/generic_ui/scripts/ui.ts, line 593 [r3] (raw file):}
Why did you remove this? Was it redundant?

---

^{src/generic_ui/scripts/ui.ts, line 597 [r3] (raw file):}
Is there a case when this is not set?

---

^{src/generic_ui/scripts/ui.ts, line 617 [r3] (raw file):}
I don't really understand why you need this, but in case you do, this will be false if you close and open a popup window.

---

Comments from the review on Reviewable.io

[bemasc]

Review status: 0 of 8 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

---

^{src/generic_ui/scripts/ui.ts, line 593 [r3] (raw file):}
Yes. disconnectedWhileProxying is set to true here if there is an error, and then is set to false if the connection succeeds or is canceled.

---

^{src/generic_ui/scripts/ui.ts, line 597 [r3] (raw file):}
I think you're right; this was redundant. Removed.

---

^{src/generic_ui/scripts/ui.ts, line 617 [r3] (raw file):}
Good point! I've removed proxySet_ entirely, and also stopped relying on instanceGettingAccessFrom_, which is cleared when the window is closed and opened. Instead, I've changed core.disconnectedWhileProxying to include the ID of the instance from which we were disconnected. This also required fixing the state synchronization for instanceTryingToGetAccessFrom, which was getting lost across window close/open.

---

Comments from the review on Reviewable.io

[salomegeo]

👍

---

Review status: 0 of 8 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

---

Comments from the review on Reviewable.io