Mac: Attach Extension NSViews to the view hierarchy before creating renderers

chrome/browser/ui/cocoa/extensions/extension_popup_controller.mm

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "chrome/browser/ui/cocoa/extensions/extension_popup_controller.h"
 
 #include <algorithm>
 
 #include "base/callback.h"
 #include "chrome/browser/chrome_notification_types.h"
@@ skipping 28 matching lines @@
 
 // Given a value and a rage, clamp the value into the range.
 CGFloat Clamp(CGFloat value, CGFloat min, CGFloat max) {
   return std::max(min, std::min(max, value));
 }
 
 }  // namespace
 
 @interface ExtensionPopupController (Private)
 // Callers should be using the public static method for initialization.
-// NOTE: This takes ownership of |host|.
-- (id)initWithHost:(extensions::ExtensionViewHost*)host
-      parentWindow:(NSWindow*)parentWindow
-        anchoredAt:(NSPoint)anchoredAt
-     arrowLocation:(info_bubble::BubbleArrowLocation)arrowLocation
-           devMode:(BOOL)devMode;
+- (id)initWithParentWindow:(NSWindow*)parentWindow
+                anchoredAt:(NSPoint)anchoredAt
+             arrowLocation:(info_bubble::BubbleArrowLocation)arrowLocation
+                   devMode:(BOOL)devMode;
+
+// Set the ExtensionViewHost, taking ownership.
+- (void)setExtensionViewHost:(scoped_ptr<extensions::ExtensionViewHost>)host;
 
 // Called when the extension's hosted NSView has been resized.
 - (void)extensionViewFrameChanged;
 
 // Called when the extension's size changes.
 - (void)onSizeChanged:(NSSize)newSize;
 
 // Called when the extension view is shown.
 - (void)onViewDidShow;
 
@@ skipping 78 matching lines @@
   // know what it is for notifications, but our ExtensionViewHost may not be
   // valid.
   WebContents* web_contents_;
   base::Callback<void(content::DevToolsAgentHost*, bool)> devtools_callback_;
 };
 
 @implementation ExtensionPopupController
 
 @synthesize extensionId = extensionId_;
 
-- (id)initWithHost:(extensions::ExtensionViewHost*)host
-      parentWindow:(NSWindow*)parentWindow
-        anchoredAt:(NSPoint)anchoredAt
-     arrowLocation:(info_bubble::BubbleArrowLocation)arrowLocation
-           devMode:(BOOL)devMode {
+- (id)initWithParentWindow:(NSWindow*)parentWindow
+                anchoredAt:(NSPoint)anchoredAt
+             arrowLocation:(info_bubble::BubbleArrowLocation)arrowLocation
+                   devMode:(BOOL)devMode {
   base::scoped_nsobject<InfoBubbleWindow> window([[InfoBubbleWindow alloc]
       initWithContentRect:ui::kWindowSizeDeterminedLater
                 styleMask:NSBorderlessWindowMask
                   backing:NSBackingStoreBuffered
                     defer:YES]);
   if (!window.get())
     return nil;
 
   anchoredAt = [parentWindow convertBaseToScreen:anchoredAt];
   if ((self = [super initWithWindow:window
                        parentWindow:parentWindow
                          anchoredAt:anchoredAt])) {
-    host_.reset(host);
-    extensionId_ = host_->extension_id();
     beingInspected_ = devMode;
     ignoreWindowDidResignKey_ = NO;
-
-    InfoBubbleView* view = self.bubble;
-    [view setArrowLocation:arrowLocation];
-
-    extensionView_ = host->view()->GetNativeView();
-    container_.reset(new ExtensionPopupContainer(self));
-    static_cast<ExtensionViewMac*>(host->view())
-        ->set_container(container_.get());
-
-    NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
-    [center addObserver:self
-               selector:@selector(extensionViewFrameChanged)
-                   name:NSViewFrameDidChangeNotification
-                 object:extensionView_];
-
-    [view addSubview:extensionView_];

[tapted, 2015/02/18 06:00:11]

note this line is now moved to ExtensionViewMac::CreateWidgetHostViewIn

-
-    notificationBridge_.reset(new DevtoolsNotificationBridge(self));
-    registrar_.reset(new content::NotificationRegistrar);
-    if (beingInspected_) {
-      // Listen for the extension to finish loading so the dev tools can be
-      // opened.
-      registrar_->Add(notificationBridge_.get(),
-                      extensions::NOTIFICATION_EXTENSION_HOST_DID_STOP_LOADING,
-                      content::Source<BrowserContext>(host->browser_context()));
-    }
+    [[self bubble] setArrowLocation:arrowLocation];
   }
   return self;
 }
 
+- (void)setExtensionViewHost:(scoped_ptr<extensions::ExtensionViewHost>)host {

[Robert Sesek, 2015/02/18 14:56:20]

This method should be down around line 320.

[tapted, 2015/02/18 22:35:28]

Done.

+  DCHECK(!host_);
+  DCHECK(host);
+  host_.swap(host);
+
+  extensionId_ = host_->extension_id();
+  container_.reset(new ExtensionPopupContainer(self));
+  ExtensionViewMac* hostView = static_cast<ExtensionViewMac*>(host_->view());
+  hostView->set_container(container_.get());
+  hostView->CreateWidgetHostViewIn([self bubble]);

[tapted, 2015/02/18 06:00:11]

note this line is new

+
+  extensionView_ = hostView->GetNativeView();
+
+  NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
+  [center addObserver:self
+             selector:@selector(extensionViewFrameChanged)
+                 name:NSViewFrameDidChangeNotification
+               object:extensionView_];
+
+  notificationBridge_.reset(new DevtoolsNotificationBridge(self));
+  registrar_.reset(new content::NotificationRegistrar);
+  if (beingInspected_) {
+    // Listen for the extension to finish loading so the dev tools can be
+    // opened.
+    registrar_->Add(notificationBridge_.get(),
+                    extensions::NOTIFICATION_EXTENSION_HOST_DID_STOP_LOADING,
+                    content::Source<BrowserContext>(host_->browser_context()));
+  }
+}
+
 - (void)dealloc {
   [[NSNotificationCenter defaultCenter] removeObserver:self];
   [super dealloc];
 }
 
 - (void)showDevTools {
   DevToolsWindow::OpenDevToolsWindow(host_->host_contents());
 }
 
 - (void)close {
@@ skipping 69 matching lines @@
 
   // If we click the browser/page action again, we should close the popup.
   // Make Mac behavior the same with Windows and others.
   if (gPopup) {
     std::string extension_id = url.host();
     extensions::ExtensionViewHost* host = [gPopup extensionViewHost];
     if (extension_id == host->extension_id()) {
       [gPopup close];
       return nil;
     }
+
+    [gPopup close];

[Robert Sesek, 2015/02/18 14:56:20]

Move this above line 292 and then delete 293.

[tapted, 2015/02/18 22:35:28]

Done.

   }
 
-  extensions::ExtensionViewHost* host =
-      extensions::ExtensionViewHostFactory::CreatePopupHost(url, browser);
+  // Create the popup first. This establishes an initially hidden NSWindow so
+  // that the renderer is able to gather correct screen metrics for the initial
+  // paint.
+  gPopup = [[ExtensionPopupController alloc]
+      initWithParentWindow:browser->window()->GetNativeWindow()
+                anchoredAt:anchoredAt
+             arrowLocation:arrowLocation
+                   devMode:devMode];
+
+  scoped_ptr<extensions::ExtensionViewHost> host(
+      extensions::ExtensionViewHostFactory::CreatePopupHost(url, browser));
   DCHECK(host);
-  if (!host)
-    return nil;
-
-  [gPopup close];
-
-  // Takes ownership of |host|. Also will autorelease itself when the popup is
-  // closed, so no need to do that here.
-  gPopup = [[ExtensionPopupController alloc]
-      initWithHost:host
-      parentWindow:browser->window()->GetNativeWindow()
-        anchoredAt:anchoredAt
-     arrowLocation:arrowLocation
-           devMode:devMode];
+  [gPopup setExtensionViewHost:host.Pass()];
   return gPopup;
 }
 
 + (ExtensionPopupController*)popup {
   return gPopup;
 }
 
 - (void)extensionViewFrameChanged {
   // If there are no changes in the width or height of the frame, then ignore.
   if (NSEqualSizes([extensionView_ frame].size, extensionFrame_.size))
@@ skipping 54 matching lines @@
   // the notification is fired (and consequently the view contents have loaded).
   if (![window isVisible]) {
     [self showWindow:self];
   }
 }
 
 - (void)onSizeChanged:(NSSize)newSize {
   // When we update the size, the window will become visible. Stay hidden until
   // the host is loaded.
   pendingSize_ = newSize;
-  if (!host_->did_stop_loading())
+  if (!host_ || !host_->did_stop_loading())
     return;
 
   // No need to use CA here, our caller calls us repeatedly to animate the
   // resizing.
   NSRect frame = [extensionView_ frame];
   frame.size = newSize;
 
   // |new_size| is in pixels. Convert to view units.
   frame.size = [extensionView_ convertSize:frame.size fromView:nil];
 
@@ skipping 37 matching lines @@
   return minSize;
 }
 
 // Private (TestingAPI)
 + (NSSize)maxPopupSize {
   NSSize maxSize = {ExtensionViewMac::kMaxWidth, ExtensionViewMac::kMaxHeight};
   return maxSize;
 }
 
 @end