[WebLayer] Destroy Profile in WebLayer Shell

This CL has WebLayer Shell destroy its Profile at the point in the
shutdown process where Chrome destroys its Profile. This change is
concretely needed to enable the upcoming bringup of KeyedServices in
WebLayer; without this change, the KeyedServiceFactory singletons
DCHECK when they are destroyed at shutdown as they expect the context
shutdown (i.e., BrowserContext destruction) to have already occurred.

As part of this change we change session_service_browsertest.cc to
delete the Tab objects that were created by session restore. These
objects currently leak when not on Android, and a DCHECK deep in
//content goes off with this CL without this part of the change (that
DCHECK effectively expects that WebContents instances associated with
a BrowserContext instance are destroyed before the BrowserContext). I
have filed a bug for considering the long-term ownership model around
BrowserImpl::CreateTabForSessionRestore().

Bug: 1030692
Change-Id: I62bfd73e5e4d9d1099e18fc66cdd51d7380497c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2025367
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736294}

weblayer/app/jni_onload.cc

@@ -15,6 +15,7 @@ namespace weblayer {
 class MainDelegateImpl : public MainDelegate {
  public:
   void PreMainMessageLoopRun() override {}
+  void PostMainMessageLoopRun() override {}
   void SetMainMessageLoopQuitClosure(base::OnceClosure quit_closure) override {}
 };
 

weblayer/browser/browser_main_parts_impl.cc

@@ -126,6 +126,10 @@ bool BrowserMainPartsImpl::MainMessageLoopRun(int* result_code) {
   return !run_message_loop_;
 }
 
+void BrowserMainPartsImpl::PostMainMessageLoopRun() {
+  params_->delegate->PostMainMessageLoopRun();
+}
+
 void BrowserMainPartsImpl::PreDefaultMainMessageLoopRun(
     base::OnceClosure quit_closure) {
   // Wrap the method that stops the message loop so we can do other shutdown

weblayer/browser/browser_main_parts_impl.h

@@ -28,6 +28,7 @@ class BrowserMainPartsImpl : public content::BrowserMainParts {
   int PreEarlyInitialization() override;
   void PreMainMessageLoopStart() override;
   void PreMainMessageLoopRun() override;
+  void PostMainMessageLoopRun() override;
   bool MainMessageLoopRun(int* result_code) override;
   void PreDefaultMainMessageLoopRun(base::OnceClosure quit_closure) override;
 

weblayer/browser/session_service_browsertest.cc

@@ -7,6 +7,7 @@
 #include "base/files/file_path.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
+#include "build/build_config.h"
 #include "components/sessions/core/command_storage_manager_test_helper.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/url_loader_interceptor.h"
@@ -198,6 +199,12 @@ IN_PROC_BROWSER_TEST_F(SessionServiceTest, SingleTab) {
   EXPECT_EQ(1, browser->GetTabs()[0]
                    ->GetNavigationController()
                    ->GetNavigationListSize());
+
+  // A DCHECK goes off in //content if the Tab created via session restore
+  // leaks at shutdown.
+  // TODO(crbug.com/1046406): Rationalize the ownership model here.
+  Tab* restored_tab = browser->GetTabs()[0];
+  delete restored_tab;
 }
 
 IN_PROC_BROWSER_TEST_F(SessionServiceTest, TwoTabs) {
@@ -215,14 +222,15 @@ IN_PROC_BROWSER_TEST_F(SessionServiceTest, TwoTabs) {
   NavigateAndWaitForCompletion(url2, tab2.get());
   browser->SetActiveTab(tab2.get());
 
-  // Shutdown the service and run the assertions twice to ensure we handle
+  // Shut down the service.
+  ShutdownSessionServiceAndWait(browser.get());
+  tab1.reset();
+  tab2.reset();
+  browser.reset();
+
+  // Recreate the browser and run the assertions twice to ensure we handle
   // correctly storing state of tabs that need to be reloaded.
   for (int i = 0; i < 2; ++i) {
-    ShutdownSessionServiceAndWait(browser.get());
-    tab1.reset();
-    tab2.reset();
-    browser.reset();
-
     browser = CreateBrowser(GetProfile(), "x");
     // Should be no tabs while waiting for restore.
     EXPECT_TRUE(browser->GetTabs().empty()) << "iteration " << i;
@@ -244,6 +252,18 @@ IN_PROC_BROWSER_TEST_F(SessionServiceTest, TwoTabs) {
                      ->GetNavigationController()
                      ->GetNavigationListSize())
         << "iteration " << i;
+
+    ShutdownSessionServiceAndWait(browser.get());
+
+    // A DCHECK goes off in //content if the Tabs created via session restore
+    // leak at shutdown.
+    // TODO(crbug.com/1046406): Rationalize the ownership model here.
+    Tab* restored_tab_1 = browser->GetTabs()[0];
+    Tab* restored_tab_2 = browser->GetTabs()[1];
+    delete restored_tab_1;
+    delete restored_tab_2;
+
+    browser.reset();
   }
 }
 
@@ -305,6 +325,15 @@ IN_PROC_BROWSER_TEST_F(SessionServiceTest, MoveBetweenBrowsers) {
   EXPECT_TRUE(restored_tab_3->web_contents()->GetController().NeedsReload());
   restored_tab_3->web_contents()->GetController().LoadIfNecessary();
   content::WaitForLoadStop(restored_tab_3->web_contents());
+
+  // A DCHECK goes off in //content if the Tabs created via session restore
+  // leak at shutdown.
+  // TODO(crbug.com/1046406): Rationalize the ownership model here.
+  Tab* restored_tab_1 = browser1->GetTabs()[0];
+  Tab* restored_tab_2 = browser2->GetTabs()[1];
+  delete restored_tab_1;
+  delete restored_tab_2;
+  delete restored_tab_3;
 }
 
 }  // namespace weblayer

weblayer/public/main.h

@@ -20,6 +20,7 @@ namespace weblayer {
 class MainDelegate {
  public:
   virtual void PreMainMessageLoopRun() = 0;
+  virtual void PostMainMessageLoopRun() = 0;
   virtual void SetMainMessageLoopQuitClosure(
       base::OnceClosure quit_closure) = 0;
 };

weblayer/shell/app/shell_main_params.cc

@@ -49,24 +49,28 @@ GURL GetStartupURL() {
 class MainDelegateImpl : public MainDelegate {
  public:
   void PreMainMessageLoopRun() override {
-    InitializeProfiles();
+    InitializeProfile();
 
     Shell::Initialize();
 
     Shell::CreateNewWindow(profile_.get(), GetStartupURL(), gfx::Size());
   }
 
+  void PostMainMessageLoopRun() override { DestroyProfile(); }
+
   void SetMainMessageLoopQuitClosure(base::OnceClosure quit_closure) override {
     Shell::SetMainMessageLoopQuitClosure(std::move(quit_closure));
   }
 
  private:
-  void InitializeProfiles() {
+  void InitializeProfile() {
     profile_ = Profile::Create("web_shell");
 
     // TODO: create an incognito profile as well.
   }
 
+  void DestroyProfile() { profile_.reset(); }
+
   std::unique_ptr<Profile> profile_;
 };