[iOS] Ensure PolicyDecisionStateTracker's callback is called

This changes PolicyDecisionStateTracker's destructor to invoke
the state tracker's callback if it hasn't already been invoked.
Destroying the callback without ever invoking it can result in
a WKWebView-provided callback never getting invoked, which
triggers an NSException causing a crash.

In the linked bug, the PolicyDecisionStateTracker is destroyed
as a result of MainFrameUrlQuery::operator= implicitly
releasing its old response_callback, which owns the state
tracker. An alternative fix would be to change that method
to invoke the old response_callback. However, the change made
by this CL is a more general fix for the problem, and should
prevent similar issues in the future.

Bug: 1079471
Change-Id: Ib8782b2278cace0f24692dbe951d498792111095
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2229201
Commit-Queue: Ali Juma <ajuma@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774772}

ios/web/web_state/policy_decision_state_tracker.h

@@ -19,7 +19,9 @@ namespace web {
 // also Allow. If at least one decision is PolicyDecision::Cancel, the final
 // result is Cancel. Otherwise, if at least one decision is
 // CancelAndDisplayError, the final result is also CancelAndDisplayError, with
-// the error associated with the first such decision.
+// the error associated with the first such decision. If this is destroyed
+// before all decisions have been received and the callback has not yet been
+// invoked, the callback is invoked with PolicyDecision::Cancel.
 class PolicyDecisionStateTracker
     : public base::SupportsWeakPtr<PolicyDecisionStateTracker> {
  public:

ios/web/web_state/policy_decision_state_tracker.mm

@@ -16,7 +16,11 @@ PolicyDecisionStateTracker::PolicyDecisionStateTracker(
     base::OnceCallback<void(WebStatePolicyDecider::PolicyDecision)> callback)
     : callback_(std::move(callback)) {}
 
-PolicyDecisionStateTracker::~PolicyDecisionStateTracker() = default;
+PolicyDecisionStateTracker::~PolicyDecisionStateTracker() {
+  if (!callback_.is_null()) {
+    std::move(callback_).Run(WebStatePolicyDecider::PolicyDecision::Cancel());
+  }
+}
 
 void PolicyDecisionStateTracker::OnSinglePolicyDecisionReceived(
     WebStatePolicyDecider::PolicyDecision decision) {

ios/web/web_state/policy_decision_state_tracker_unittest.mm

@@ -282,4 +282,64 @@ TEST_F(PolicyDecisionStateTrackerTest, ShowErrorMixed) {
   EXPECT_EQ(policy_decision_->GetDisplayError().code, error1.code);
 }
 
+// Test fixture that supports destroying its PolicyDecisionStateTracker,
+// allowing destructor behavior to be tested.
+class PolicyDecisionStateTrackerDestructionTest : public PlatformTest {
+ public:
+  PolicyDecisionStateTrackerDestructionTest()
+      : policy_decision_state_tracker_(
+            std::make_unique<PolicyDecisionStateTracker>(
+                base::BindOnce(&PolicyDecisionStateTrackerDestructionTest::
+                                   OnDecisionDetermined,
+                               base::Unretained(this)))) {}
+
+  void OnDecisionDetermined(
+      WebStatePolicyDecider::PolicyDecision policy_decision) {
+    policy_decision_ = policy_decision;
+  }
+
+  void DestroyPolicyDecisionStateTracker() {
+    policy_decision_state_tracker_.reset();
+  }
+
+  std::unique_ptr<PolicyDecisionStateTracker> policy_decision_state_tracker_;
+  base::Optional<WebStatePolicyDecider::PolicyDecision> policy_decision_;
+};
+
+// Tests the case where no decisions have been received by the time the
+// PolicyDecisionStateTracker is destroyed.
+TEST_F(PolicyDecisionStateTrackerDestructionTest, NoDecisionsReceived) {
+  int num_decisions_requested = 3;
+  policy_decision_state_tracker_->FinishedRequestingDecisions(
+      num_decisions_requested);
+
+  DestroyPolicyDecisionStateTracker();
+
+  EXPECT_TRUE(policy_decision_);
+  EXPECT_TRUE(policy_decision_->ShouldCancelNavigation());
+}
+
+// Tests the case where some but not all decisions have been received by the
+// time the PolicyDecisionStateTracker is destroyed.
+TEST_F(PolicyDecisionStateTrackerDestructionTest, OnlySomeDecisionsReceived) {
+  int num_decisions_requested = 3;
+  policy_decision_state_tracker_->FinishedRequestingDecisions(
+      num_decisions_requested);
+
+  policy_decision_state_tracker_->OnSinglePolicyDecisionReceived(
+      WebStatePolicyDecider::PolicyDecision::Allow());
+  EXPECT_FALSE(policy_decision_state_tracker_->DeterminedFinalResult());
+  EXPECT_FALSE(policy_decision_);
+
+  policy_decision_state_tracker_->OnSinglePolicyDecisionReceived(
+      WebStatePolicyDecider::PolicyDecision::Allow());
+  EXPECT_FALSE(policy_decision_state_tracker_->DeterminedFinalResult());
+  EXPECT_FALSE(policy_decision_);
+
+  DestroyPolicyDecisionStateTracker();
+
+  EXPECT_TRUE(policy_decision_);
+  EXPECT_TRUE(policy_decision_->ShouldCancelNavigation());
+}
+
 }  // namespace web