[Nav Bug] Only cache session when disabling web usage and add inttest.

This is a logic bug introduced in crrev.com/c/789791. If
SetWebUsageEnabled(true) is called twice consecutively, the session at
the time of the first call will incorrectly be restored during the
second call. The navigation history between the two calls will be lost.

This was exposed in TabUsageRecorderTestCase/testColdLaunchReloadCount
egtest.

Bug: 781916
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I6f7226fb42ba7d647b9229514a574afc57703ae3
Reviewed-on: https://chromium-review.googlesource.com/803740Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Danyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521123}

ios/web/web_state/navigation_and_load_callbacks_inttest.mm

@@ -421,6 +421,34 @@ TEST_F(NavigationAndLoadCallbacksTest, NewPageNavigation) {
   LoadUrl(url);
 }
 
+// Tests that if web usage is already enabled, enabling it again would not cause
+// any page loads (related to restoring cached session). This is a regression
+// test for crbug.com/781916.
+TEST_F(NavigationAndLoadCallbacksTest, EnableWebUsageTwice) {
+  const GURL url = HttpServer::MakeUrl("http://chromium.test");
+  std::map<GURL, std::string> responses;
+  responses[url] = "Chromium Test";
+  web::test::SetUpSimpleHttpServer(responses);
+
+  // Only expect one set of load events from the first LoadUrl(), not subsequent
+  // SetWebUsageEnabled(true) calls. Web usage is already enabled, so the
+  // subsequent calls should be noops.
+  NavigationContext* context = nullptr;
+  EXPECT_CALL(observer_, DidStartLoading(web_state()));
+  EXPECT_CALL(*decider_, ShouldAllowRequest(_, _)).WillOnce(Return(true));
+  EXPECT_CALL(observer_, DidStartNavigation(web_state(), _))
+      .WillOnce(VerifyNewPageStartedContext(web_state(), url, &context));
+  EXPECT_CALL(*decider_, ShouldAllowResponse(_, /*for_main_frame=*/true))
+      .WillOnce(Return(true));
+  EXPECT_CALL(observer_, DidFinishNavigation(web_state(), _))
+      .WillOnce(VerifyNewPageFinishedContext(web_state(), url, &context));
+  EXPECT_CALL(observer_, DidStopLoading(web_state()));
+
+  LoadUrl(url);
+  web_state()->SetWebUsageEnabled(true);
+  web_state()->SetWebUsageEnabled(true);
+}
+
 // Tests failed navigation to a new page.
 TEST_F(NavigationAndLoadCallbacksTest, FailedNavigation) {
   const GURL url = HttpServer::MakeUrl("unsupported://chromium.test");

ios/web/web_state/web_state_impl.mm

@@ -551,8 +551,10 @@ void WebStateImpl::SetWebUsageEnabled(bool enabled) {
   // the newly created WKWebView.
   // TODO(crbug.com/557963): don't destroy WKWebView to clear browser data.
   if (web::GetWebClient()->IsSlimNavigationManagerEnabled()) {
-    if (enabled && cached_session_storage_) {
-      RestoreSessionStorage(cached_session_storage_.get());
+    if (enabled) {
+      if (cached_session_storage_) {
+        RestoreSessionStorage(cached_session_storage_.get());
+      }
       cached_session_storage_.reset();
     } else {
       cached_session_storage_.reset(BuildSessionStorage());