Reland "Removed BrowserView::ViewHierarchChanged, moved most code to...

Reland "Removed BrowserView::ViewHierarchChanged, moved most code to constructor and widget-specfic code to AddedToWidget."

This is a reland of f3616c26

Original change's description:
> Removed BrowserView::ViewHierarchChanged, moved most code to constructor and widget-specfic code to AddedToWidget.
> 
> Bug: 245409
> Change-Id: I1e606548df4361199862c561ad86f4d40cbaa7df
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1979098
> Commit-Queue: Allen Bauer <kylixrd@chromium.org>
> Reviewed-by: Peter Kasting <pkasting@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#728998}

Bug: 245409
Change-Id: Id5317849e130ec8715200f0947bc01f2bc932842
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1991945
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729914}

chrome/browser/ui/views/frame/browser_non_client_frame_view.cc

@@ -46,7 +46,10 @@ BrowserNonClientFrameView::BrowserNonClientFrameView(BrowserFrame* frame,
     g_browser_process->profile_manager()->
         GetProfileAttributesStorage().AddObserver(this);
   }
-  MaybeObserveTabstrip();
+  if (browser_view_->tabstrip()) {
+    DCHECK(!tab_strip_observer_.IsObserving(browser_view_->tabstrip()));
+    tab_strip_observer_.Add(browser_view_->tabstrip());
+  }
 }
 
 BrowserNonClientFrameView::~BrowserNonClientFrameView() {
@@ -58,7 +61,6 @@ BrowserNonClientFrameView::~BrowserNonClientFrameView() {
 }
 
 void BrowserNonClientFrameView::OnBrowserViewInitViewsComplete() {
-  MaybeObserveTabstrip();
   UpdateMinimumSize();
 }
 
@@ -385,13 +387,6 @@ int BrowserNonClientFrameView::GetSystemMenuY() const {
 }
 #endif
 
-void BrowserNonClientFrameView::MaybeObserveTabstrip() {
-  if (browser_view_->tabstrip()) {
-    DCHECK(!tab_strip_observer_.IsObserving(browser_view_->tabstrip()));
-    tab_strip_observer_.Add(browser_view_->tabstrip());
-  }
-}
-
 const ui::ThemeProvider*
 BrowserNonClientFrameView::GetThemeProviderForProfile() const {
   // Because the frame's accessor reads the ThemeProvider from the profile and

chrome/browser/ui/views/frame/browser_non_client_frame_view.h

@@ -177,8 +177,6 @@ class BrowserNonClientFrameView : public views::NonClientFrameView,
   int GetSystemMenuY() const override;
 #endif
 
-  void MaybeObserveTabstrip();
-
   // Gets a theme provider that should be non-null even before we're added to a
   // view hierarchy.
   const ui::ThemeProvider* GetThemeProviderForProfile() const;

chrome/browser/ui/views/frame/browser_view.cc

@@ -430,6 +430,70 @@ BrowserView::BrowserView(std::unique_ptr<Browser> browser)
   browser_->tab_strip_model()->AddObserver(this);
   immersive_mode_controller_ = chrome::CreateImmersiveModeController();
   md_observer_.Add(ui::MaterialDesignController::GetInstance());
+
+  // Top container holds tab strip region and toolbar and lives at the front of
+  // the view hierarchy.
+  top_container_ = AddChildView(std::make_unique<TopContainerView>(this));
+  tab_strip_region_view_ =
+      top_container_->AddChildView(std::make_unique<TabStripRegionView>());
+
+  // TabStrip takes ownership of the controller.
+  auto tabstrip_controller = std::make_unique<BrowserTabStripController>(
+      browser_->tab_strip_model(), this);
+  BrowserTabStripController* tabstrip_controller_ptr =
+      tabstrip_controller.get();
+  tabstrip_ = tab_strip_region_view_->AddChildView(std::make_unique<TabStrip>(
+      std::move(tabstrip_controller)));  // Takes ownership.
+  tabstrip_controller_ptr->InitFromModel(tabstrip_);
+
+  // Create WebViews early so |webui_tab_strip_| can observe their size.
+  auto devtools_web_view =
+      std::make_unique<views::WebView>(browser_->profile());
+  devtools_web_view->SetID(VIEW_ID_DEV_TOOLS_DOCKED);
+  devtools_web_view->SetVisible(false);
+
+  auto contents_web_view =
+      std::make_unique<ContentsWebView>(browser_->profile());
+  contents_web_view->SetID(VIEW_ID_TAB_CONTAINER);
+  contents_web_view->SetEmbedFullscreenWidgetMode(true);
+
+  auto contents_container = std::make_unique<views::View>();
+  devtools_web_view_ =
+      contents_container->AddChildView(std::move(devtools_web_view));
+  contents_web_view_ =
+      contents_container->AddChildView(std::move(contents_web_view));
+  contents_container->SetLayoutManager(std::make_unique<ContentsLayoutManager>(
+      devtools_web_view_, contents_web_view_));
+
+  toolbar_ = top_container_->AddChildView(
+      std::make_unique<ToolbarView>(browser_.get(), this));
+
+  contents_separator_ =
+      top_container_->AddChildView(std::make_unique<ContentsSeparator>());
+
+  web_contents_close_handler_ =
+      std::make_unique<WebContentsCloseHandler>(contents_web_view_);
+
+  contents_container_ = AddChildView(std::move(contents_container));
+  set_contents_view(contents_container_);
+
+  // InfoBarContainer needs to be added as a child here for drop-shadow, but
+  // needs to come after toolbar in focus order (see EnsureFocusOrder()).
+  infobar_container_ =
+      AddChildView(std::make_unique<InfoBarContainerView>(this));
+
+  InitStatusBubble();
+
+  // Create do-nothing view for the sake of controlling the z-order of the find
+  // bar widget.
+  find_bar_host_view_ = AddChildView(std::make_unique<View>());
+
+#if defined(OS_WIN)
+  // Create a custom JumpList and add it to an observer of TabRestoreService
+  // so we can update the custom JumpList when a tab is added or removed.
+  if (JumpList::Enabled())
+    load_complete_listener_ = std::make_unique<LoadCompleteListener>(this);
+#endif
 }
 
 BrowserView::~BrowserView() {
@@ -2479,19 +2543,80 @@ void BrowserView::OnGestureEvent(ui::GestureEvent* event) {
 
 void BrowserView::ViewHierarchyChanged(
     const views::ViewHierarchyChangedDetails& details) {
-  if (details.child != this)
-    return;
+  // Override here in order to suppress the call to
+  // views::ClientView::ViewHierarchyChanged();
+}
 
-  // On removal, this class may not have a widget anymore, so go to the parent.
-  auto* widget = details.is_add ? GetWidget() : details.parent->GetWidget();
-  if (!widget)
-    return;
+void BrowserView::AddedToWidget() {
+  views::ClientView::AddedToWidget();
 
-  bool init = !initialized_ && details.is_add;
-  if (init) {
-    InitViews();
-    initialized_ = true;
+#if defined(OS_CHROMEOS)
+  // TopControlsSlideController must be initialized here in AddedToWidget()
+  // rather than Init() as it depends on the browser frame being ready.
+  if (IsBrowserTypeNormal()) {
+    DCHECK(frame_);
+    top_controls_slide_controller_ =
+        std::make_unique<TopControlsSlideControllerChromeOS>(this);
   }
+#endif
+
+  GetWidget()->AddObserver(this);
+
+  // Stow a pointer to this object onto the window handle so that we can get at
+  // it later when all we have is a native view.
+  GetWidget()->SetNativeWindowProperty(kBrowserViewKey, this);
+
+  // Stow a pointer to the browser's profile onto the window handle so that we
+  // can get it later when all we have is a native view.
+  GetWidget()->SetNativeWindowProperty(Profile::kProfileKey,
+                                       browser_->profile());
+
+#if defined(USE_AURA)
+  // Stow a pointer to the browser's profile onto the window handle so
+  // that windows will be styled with the appropriate NativeTheme.
+  SetThemeProfileForWindow(GetNativeWindow(), browser_->profile());
+#endif
+
+  toolbar_->Init();
+
+  LoadAccelerators();
+
+  // |immersive_mode_controller_| may depend on the presence of a Widget, so it
+  // is initialized here.
+  immersive_mode_controller_->Init(this);
+  immersive_mode_controller_->AddObserver(this);
+
+  // See https://crbug.com/993502.
+  views::View* web_footer_experiment = nullptr;
+  if (base::FeatureList::IsEnabled(features::kWebFooterExperiment)) {
+    web_footer_experiment = AddChildView(
+        std::make_unique<WebFooterExperimentView>(browser_->profile()));
+  }
+
+  // TODO(https://crbug.com/1036519): Remove BrowserViewLayout dependence on
+  // Widget and move to the constructor.
+  auto browser_view_layout = std::make_unique<BrowserViewLayout>(
+      std::make_unique<BrowserViewLayoutDelegateImpl>(this),
+      GetWidget()->GetNativeView(), this, top_container_,
+      tab_strip_region_view_, tabstrip_, toolbar_, infobar_container_,
+      contents_container_, immersive_mode_controller_.get(),
+      web_footer_experiment, contents_separator_);
+  SetLayoutManager(std::move(browser_view_layout));
+
+  EnsureFocusOrder();
+
+  // This browser view may already have a custom button provider set (e.g the
+  // hosted app frame).
+  if (!toolbar_button_provider_)
+    SetToolbarButtonProvider(toolbar_);
+
+  frame_->OnBrowserViewInitViewsComplete();
+  frame_->GetFrameView()->UpdateMinimumSize();
+  using_native_frame_ = frame_->ShouldUseNativeFrame();
+
+  MaybeInitializeWebUITabStrip();
+
+  initialized_ = true;
 }
 
 void BrowserView::PaintChildren(const views::PaintInfo& paint_info) {
@@ -2570,131 +2695,6 @@ void BrowserView::InfoBarContainerStateChanged(bool is_animating) {
   ToolbarSizeChanged(is_animating);
 }
 
-void BrowserView::InitViews() {
-  // TopControlsSlideController must be initialized here in InitViews() rather
-  // than Init() as it depends on the browser frame being ready.
-#if defined(OS_CHROMEOS)
-  if (IsBrowserTypeNormal()) {
-    DCHECK(frame_);
-    top_controls_slide_controller_ =
-        std::make_unique<TopControlsSlideControllerChromeOS>(this);
-  }
-#endif
-
-  GetWidget()->AddObserver(this);
-
-  // Stow a pointer to this object onto the window handle so that we can get at
-  // it later when all we have is a native view.
-  GetWidget()->SetNativeWindowProperty(kBrowserViewKey, this);
-
-  // Stow a pointer to the browser's profile onto the window handle so that we
-  // can get it later when all we have is a native view.
-  GetWidget()->SetNativeWindowProperty(Profile::kProfileKey,
-                                       browser_->profile());
-
-#if defined(USE_AURA)
-  // Stow a pointer to the browser's profile onto the window handle so
-  // that windows will be styled with the appropriate NativeTheme.
-  SetThemeProfileForWindow(GetNativeWindow(), browser_->profile());
-#endif
-
-  LoadAccelerators();
-
-  // Top container holds tab strip region and toolbar and lives at the front of
-  // the view hierarchy.
-  top_container_ = new TopContainerView(this);
-  AddChildView(top_container_);
-  tab_strip_region_view_ = new TabStripRegionView();
-  top_container_->AddChildView(tab_strip_region_view_);
-
-  // TabStrip takes ownership of the controller.
-  BrowserTabStripController* tabstrip_controller =
-      new BrowserTabStripController(browser_->tab_strip_model(), this);
-  tabstrip_ =
-      new TabStrip(std::unique_ptr<TabStripController>(tabstrip_controller));
-  tab_strip_region_view_->AddChildView(tabstrip_);  // Takes ownership.
-  tabstrip_controller->InitFromModel(tabstrip_);
-
-  // Create WebViews early so |webui_tab_strip_| can observe their size.
-  devtools_web_view_ = new views::WebView(browser_->profile());
-  devtools_web_view_->SetID(VIEW_ID_DEV_TOOLS_DOCKED);
-  devtools_web_view_->SetVisible(false);
-
-  contents_web_view_ = new ContentsWebView(browser_->profile());
-  contents_web_view_->SetID(VIEW_ID_TAB_CONTAINER);
-  contents_web_view_->SetEmbedFullscreenWidgetMode(true);
-
-  contents_container_ = new views::View();
-  contents_container_->AddChildView(devtools_web_view_);
-  contents_container_->AddChildView(contents_web_view_);
-  contents_container_->SetLayoutManager(std::make_unique<ContentsLayoutManager>(
-      devtools_web_view_, contents_web_view_));
-
-  toolbar_ = top_container_->AddChildView(
-      std::make_unique<ToolbarView>(browser_.get(), this));
-  toolbar_->Init();
-
-  contents_separator_ =
-      top_container_->AddChildView(std::make_unique<ContentsSeparator>());
-
-  // This browser view may already have a custom button provider set (e.g the
-  // hosted app frame).
-  if (!toolbar_button_provider_)
-    SetToolbarButtonProvider(toolbar_);
-
-  web_contents_close_handler_.reset(
-      new WebContentsCloseHandler(contents_web_view_));
-
-  AddChildView(contents_container_);
-  set_contents_view(contents_container_);
-
-  // InfoBarContainer needs to be added as a child here for drop-shadow, but
-  // needs to come after toolbar in focus order (see EnsureFocusOrder()).
-  infobar_container_ = new InfoBarContainerView(this);
-  AddChildView(infobar_container_);
-
-  InitStatusBubble();
-
-  // Create do-nothing view for the sake of controlling the z-order of the find
-  // bar widget.
-  find_bar_host_view_ = new View();
-  AddChildView(find_bar_host_view_);
-
-  immersive_mode_controller_->Init(this);
-  immersive_mode_controller_->AddObserver(this);
-
-  // See https://crbug.com/993502.
-  views::View* web_footer_experiment = nullptr;
-  if (base::FeatureList::IsEnabled(features::kWebFooterExperiment)) {
-    web_footer_experiment = AddChildView(
-        std::make_unique<WebFooterExperimentView>(browser_->profile()));
-  }
-
-  auto browser_view_layout = std::make_unique<BrowserViewLayout>(
-      std::make_unique<BrowserViewLayoutDelegateImpl>(this),
-      GetWidget()->GetNativeView(), this, top_container_,
-      tab_strip_region_view_, tabstrip_, toolbar_, infobar_container_,
-      contents_container_, immersive_mode_controller_.get(),
-      web_footer_experiment, contents_separator_);
-  SetLayoutManager(std::move(browser_view_layout));
-
-  EnsureFocusOrder();
-
-#if defined(OS_WIN)
-  // Create a custom JumpList and add it to an observer of TabRestoreService
-  // so we can update the custom JumpList when a tab is added or removed.
-  if (JumpList::Enabled()) {
-    load_complete_listener_ = std::make_unique<LoadCompleteListener>(this);
-  }
-#endif
-
-  frame_->OnBrowserViewInitViewsComplete();
-  frame_->GetFrameView()->UpdateMinimumSize();
-  using_native_frame_ = frame_->ShouldUseNativeFrame();
-
-  MaybeInitializeWebUITabStrip();
-}
-
 void BrowserView::MaybeInitializeWebUITabStrip() {
 #if BUILDFLAG(ENABLE_WEBUI_TAB_STRIP)
   if (browser_->SupportsWindowFeature(Browser::FEATURE_TABSTRIP) &&

chrome/browser/ui/views/frame/browser_view.h

@@ -498,6 +498,7 @@ class BrowserView : public BrowserWindow,
   void OnGestureEvent(ui::GestureEvent* event) override;
   void ViewHierarchyChanged(
       const views::ViewHierarchyChangedDetails& details) override;
+  void AddedToWidget() override;
   void PaintChildren(const views::PaintInfo& paint_info) override;
   void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
   void ChildPreferredSizeChanged(View* child) override;
@@ -579,9 +580,6 @@ class BrowserView : public BrowserWindow,
   // affected.
   void RevealTabStripIfNeeded();
 
-  // Constructs and initializes the child views.
-  void InitViews();
-
   // Make sure the WebUI tab strip exists if it should.
   void MaybeInitializeWebUITabStrip();