[WebLayer] PageInfo no longer uses ui in its constructor.

Several classes inherit from PageInfoUI and create a PageInfo object,
which needs a pointer to a PageInfoUI object in turn to be constructed.

This CL removes all usage of PageInfoUI from PageInfo's constructor
by adding a SetUiAndInit method on PageInfo.

This eliminates a potential source of crashes wherein PageInfo calls into
PageInfoUI (concretely, the PageInfoUI subclass) from its constructor,
the latter tries to access its PageInfo member, and that member is null
precisely because it is in the process of being set.

There is no behavioral change, as all creation sites of PageInfo now
call its InitializeUIState() method immediately after constructing it.

One detail: Prior to this change, |security_level_| wasn't set in the
PageInfo constructor's initialization list but was rather set via the
call to ComputeUIInputs() that occurs in the constructor. With this CL
|security_level_| *needs* to be set in the initialization list as that
passed-in value is used as input to ComputeUIInputs(), which is now
called from InitializeUIState().

Bug: 1052375
Change-Id: I61143f3bdc4e02097d4c27db7f46bfab2355f934
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2105296
Commit-Queue: Mugdha Lakhani <nator@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Carlos IL <carlosil@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752426}

chrome/browser/android/vr/vr_shell.cc

@@ -1205,12 +1205,14 @@ std::unique_ptr<PageInfo> VrShell::CreatePageInfo() {
 
   SecurityStateTabHelper* helper =
       SecurityStateTabHelper::FromWebContents(web_contents_);
-  return std::make_unique<PageInfo>(
-      this, Profile::FromBrowserContext(web_contents_->GetBrowserContext()),
+  auto page_info = std::make_unique<PageInfo>(
+      Profile::FromBrowserContext(web_contents_->GetBrowserContext()),
       std::make_unique<ChromePageInfoDelegate>(web_contents_),
       TabSpecificContentSettings::FromWebContents(web_contents_), web_contents_,
       entry->GetVirtualURL(), helper->GetSecurityLevel(),
       *helper->GetVisibleSecurityState());
+  page_info->InitializeUiState(this);
+  return page_info;
 }
 
 gfx::AcceleratedWidget VrShell::GetRenderSurface() {

chrome/browser/ui/android/page_info/connection_info_popup_android.cc

@@ -65,11 +65,12 @@ ConnectionInfoPopupAndroid::ConnectionInfoPopupAndroid(
   // |TabSpecificContentSettings| and need to create one; otherwise, noop.
   TabSpecificContentSettings::CreateForWebContents(web_contents);
   presenter_ = std::make_unique<PageInfo>(
-      this, Profile::FromBrowserContext(web_contents->GetBrowserContext()),
+      Profile::FromBrowserContext(web_contents->GetBrowserContext()),
       std::make_unique<ChromePageInfoDelegate>(web_contents),
       TabSpecificContentSettings::FromWebContents(web_contents), web_contents,
       nav_entry->GetURL(), helper->GetSecurityLevel(),
       *helper->GetVisibleSecurityState());
+  presenter_->InitializeUiState(this);
 }
 
 ConnectionInfoPopupAndroid::~ConnectionInfoPopupAndroid() {

chrome/browser/ui/android/page_info/page_info_controller_android.cc

@@ -68,11 +68,12 @@ PageInfoControllerAndroid::PageInfoControllerAndroid(
   // |TabSpecificContentSettings| and need to create one; otherwise, noop.
   TabSpecificContentSettings::CreateForWebContents(web_contents);
   presenter_ = std::make_unique<PageInfo>(
-      this, Profile::FromBrowserContext(web_contents->GetBrowserContext()),
+      Profile::FromBrowserContext(web_contents->GetBrowserContext()),
       std::make_unique<ChromePageInfoDelegate>(web_contents),
       TabSpecificContentSettings::FromWebContents(web_contents), web_contents,
       nav_entry->GetURL(), helper->GetSecurityLevel(),
       *helper->GetVisibleSecurityState());
+  presenter_->InitializeUiState(this);
 }
 
 PageInfoControllerAndroid::~PageInfoControllerAndroid() {}

chrome/browser/ui/page_info/page_info.cc

@@ -348,7 +348,6 @@ const char kPageInfoTimeNoActionPrefix[] =
 }  // namespace
 
 PageInfo::PageInfo(
-    PageInfoUI* ui,
     Profile* profile,
     std::unique_ptr<PageInfoDelegate> delegate,
     TabSpecificContentSettings* tab_specific_content_settings,
@@ -357,7 +356,6 @@ PageInfo::PageInfo(
     security_state::SecurityLevel security_level,
     const security_state::VisibleSecurityState& visible_security_state)
     : content::WebContentsObserver(web_contents),
-      ui_(ui),
       delegate_(std::move(delegate)),
       show_info_bar_(false),
       site_url_(url),
@@ -372,25 +370,11 @@ PageInfo::PageInfo(
       tab_specific_content_settings_(tab_specific_content_settings),
       did_revoke_user_ssl_decisions_(false),
       profile_(profile),
-      security_level_(security_state::NONE),
+      security_level_(security_level),
       visible_security_state_for_metrics_(visible_security_state),
       show_change_password_buttons_(false),
       did_perform_action_(false) {
   DCHECK(delegate_);
-  ComputeUIInputs(url, security_level, visible_security_state);
-
-  PresentSitePermissions();
-  PresentSiteIdentity();
-  PresentSiteData();
-  PresentPageFeatureInfo();
-
-  // Every time the Page Info UI is opened a |PageInfo| object is
-  // created. So this counts how ofter the Page Info UI is opened.
-  RecordPageInfoAction(PAGE_INFO_OPENED);
-
-  // Record the time when the Page Info UI is opened so the total time it is
-  // open can be measured.
-  start_time_ = base::TimeTicks::Now();
 }
 
 PageInfo::~PageInfo() {
@@ -464,6 +448,26 @@ PageInfo::~PageInfo() {
   }
 }
 
+void PageInfo::InitializeUiState(PageInfoUI* ui) {
+  ui_ = ui;
+  DCHECK(ui_);
+
+  ComputeUIInputs(site_url_, security_level_,
+                  visible_security_state_for_metrics_);
+  PresentSitePermissions();
+  PresentSiteIdentity();
+  PresentSiteData();
+  PresentPageFeatureInfo();
+
+  // Every time the Page Info UI is opened, this method is called.
+  // So this counts how often the Page Info UI is opened.
+  RecordPageInfoAction(PAGE_INFO_OPENED);
+
+  // Record the time when the Page Info UI is opened so the total time it is
+  // open can be measured.
+  start_time_ = base::TimeTicks::Now();
+}
+
 void PageInfo::UpdateSecurityState(
     security_state::SecurityLevel security_level,
     const security_state::VisibleSecurityState& visible_security_state) {

chrome/browser/ui/page_info/page_info.h

@@ -144,10 +144,8 @@ class PageInfo : public content::WebContentsObserver {
   };
 
   // Creates a PageInfo for the passed |url| using the given |ssl| status
-  // object to determine the status of the site's connection. The
-  // |PageInfo| takes ownership of the |ui|.
-  PageInfo(PageInfoUI* ui,
-           Profile* profile,
+  // object to determine the status of the site's connection.
+  PageInfo(Profile* profile,
            std::unique_ptr<PageInfoDelegate> delegate,
            TabSpecificContentSettings* tab_specific_content_settings,
            content::WebContents* web_contents,
@@ -156,6 +154,15 @@ class PageInfo : public content::WebContentsObserver {
            const security_state::VisibleSecurityState& visible_security_state);
   ~PageInfo() override;
 
+  // Initializes UI state that is dependent on having access to the PageInfoUI
+  // object associated with this object. This explicit post-construction
+  // initialization step is necessary as PageInfoUI subclasses create this
+  // object and also may invoke it as part of the initialization flow that
+  // occurs in this method. If this initialization flow was done as part of
+  // PageInfo's constructor, those subclasses would not have their PageInfo
+  // member set and crashes would ensue.
+  void InitializeUiState(PageInfoUI* ui);
+
   // This method is called to update the presenter's security state and forwards
   // that change on to the UI to be redrawn.
   void UpdateSecurityState(

chrome/browser/ui/page_info/page_info_unittest.cc

@@ -221,10 +221,10 @@ class PageInfoTest : public ChromeRenderViewHostTestHarness {
   PageInfo* page_info() {
     if (!page_info_.get()) {
       page_info_ = std::make_unique<PageInfo>(
-          mock_ui(), profile(),
-          std::make_unique<ChromePageInfoDelegate>(web_contents()),
+          profile(), std::make_unique<ChromePageInfoDelegate>(web_contents()),
           tab_specific_content_settings(), web_contents(), url(),
           security_level(), visible_security_state());
+      page_info_->InitializeUiState(mock_ui());
     }
     return page_info_.get();
   }

chrome/browser/ui/views/page_info/page_info_bubble_view.cc

@@ -520,9 +520,10 @@ PageInfoBubbleView::PageInfoBubbleView(
   // |TabSpecificContentSettings| and need to create one; otherwise, noop.
   TabSpecificContentSettings::CreateForWebContents(web_contents);
   presenter_ = std::make_unique<PageInfo>(
-      this, profile, std::make_unique<ChromePageInfoDelegate>(web_contents),
+      profile, std::make_unique<ChromePageInfoDelegate>(web_contents),
       TabSpecificContentSettings::FromWebContents(web_contents), web_contents,
       url, security_level, visible_security_state);
+  presenter_->InitializeUiState(this);
 }
 
 void PageInfoBubbleView::WebContentsDestroyed() {