Change the FF CEEE to create its chrome frame instance initially hidden, and

make it visible only once the content is done loading.  This avoids any flicker
at startup, and fixes a bug where the chrome frame instance is removed after
a print preview.

Also added support to chrome frame to send an uninitialized ready state
when its being torn down.  Did some cleanup in the chrome frame tests which
were broke the first time this change was committed.

BUG=62979
TEST=Make sure that there is no UI flicker on chrome frame startup, and make
sure that the extenions works correct after showing print preview and closing
it.

Review URL: http://codereview.chromium.org/5022003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@66552 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 92 insertions, 90 deletions

diff --git a/chrome_frame/chrome_frame_npapi.cc b/chrome_frame/chrome_frame_npapi.cc
index ea7217f..f3144b1 100644
--- a/chrome_frame/chrome_frame_npapi.cc
+++ b/chrome_frame/chrome_frame_npapi.cc
@@ -290,9 +290,8 @@ bool ChromeFrameNPAPI::Initialize(NPMIMEType mime_type, NPP instance,
 }
 
 void ChromeFrameNPAPI::Uninitialize() {
-  // Don't call SetReadyState as it will end up calling FireEvent.
-  // We are in the context of NPP_DESTROY.
-  ready_state_ = READYSTATE_UNINITIALIZED;
+  if (ready_state_ != READYSTATE_UNINITIALIZED)
+    SetReadyState(READYSTATE_UNINITIALIZED);
 
   UnsubscribeFromFocusEvents();
 
@@ -851,7 +850,7 @@ void ChromeFrameNPAPI::OnMessageFromChromeFrame(int tab_handle,
     }
     DLOG_IF(WARNING, !invoke) << "InvokeDefault failed";
   } else {
-    NOTREACHED() << "CreateMessageEvent";
+    DLOG(WARNING) << "CreateMessageEvent failed, probably exiting";
   }
 }
 
@@ -1055,7 +1054,7 @@ void ChromeFrameNPAPI::DispatchEvent(NPObject* event) {
         npapi::GetStringIdentifier("dispatchEvent"), &param, 1, &result);
     DLOG_IF(WARNING, !invoke) << "dispatchEvent failed";
   } else {
-    NOTREACHED() << "NPNVPluginElementNPObject";
+    DLOG(WARNING) << "ChromeFrameNPAPI::DispatchEvent failed, probably exiting";
   }
 }
 
@@ -1440,7 +1439,7 @@ NpProxyService* ChromeFrameNPAPI::CreatePrefService() {
 }
 
 NPObject* ChromeFrameNPAPI::GetWindowObject() const {
-  if (!window_object_.get()) {
+  if (!window_object_.get() && instance_) {
     NPError ret = npapi::GetValue(instance_, NPNVWindowNPObject,
         window_object_.Receive());
     DLOG_IF(ERROR, ret != NPERR_NO_ERROR) << "NPNVWindowNPObject failed";
diff --git a/chrome_frame/chrome_frame_npapi_unittest.cc b/chrome_frame/chrome_frame_npapi_unittest.cc
index 34194440..f209a86 100644
--- a/chrome_frame/chrome_frame_npapi_unittest.cc
+++ b/chrome_frame/chrome_frame_npapi_unittest.cc
@@ -68,9 +68,6 @@ class MockNPAPI: public ChromeFrameNPAPI {
     ChromeFrameNPAPI::OnAutomationServerReady();
   }
 
-  // Neuter this (or it dchecks during testing).
-  void SetReadyState(READYSTATE new_state) {}
-
   ChromeFrameAutomationClient* CreateAutomationClient() {
     return mock_automation_client_;
   }
@@ -100,13 +97,88 @@ MATCHER_P4(LaunchParamEq, version_check, extra, incognito, widget,
          arg->incognito() == incognito,
          arg->widget_mode() == widget;
 }
+
+
+static const NPIdentifier kOnPrivateMessageId =
+    reinterpret_cast<NPIdentifier>(0x100);
+static const NPIdentifier kPostPrivateMessageId =
+    reinterpret_cast<NPIdentifier>(0x100);
 }
 
+class MockNetscapeFuncs {
+ public:
+  MockNetscapeFuncs() {
+    CHECK(NULL == current_);
+    current_ = this;
+  }
+
+  ~MockNetscapeFuncs() {
+    CHECK(this == current_);
+    current_ = NULL;
+  }
+
+  MOCK_METHOD3(GetValue, NPError(NPP, NPNVariable, void *));
+  MOCK_METHOD3(GetStringIdentifiers, void(const NPUTF8 **,
+                                          int32_t,
+                                          NPIdentifier *));  // NOLINT
+  MOCK_METHOD1(RetainObject, NPObject*(NPObject*));  // NOLINT
+  MOCK_METHOD1(ReleaseObject, void(NPObject*));  // NOLINT
+
+
+  void GetPrivilegedStringIdentifiers(const NPUTF8 **names,
+                                      int32_t name_count,
+                                      NPIdentifier *identifiers) {
+    for (int32_t i = 0; i < name_count; ++i) {
+      if (0 == strcmp(names[i], "onprivatemessage")) {
+        identifiers[i] = kOnPrivateMessageId;
+      } else if (0 == strcmp(names[i], "postPrivateMessage")) {
+        identifiers[i] = kPostPrivateMessageId;
+      } else {
+        identifiers[i] = 0;
+      }
+    }
+  }
+
+  static const NPNetscapeFuncs* netscape_funcs() {
+    return &netscape_funcs_;
+  }
+
+ private:
+  static NPError MockGetValue(NPP instance,
+                              NPNVariable variable,
+                              void *ret_value) {
+    DCHECK(current_);
+    return current_->GetValue(instance, variable, ret_value);
+  }
+
+  static void MockGetStringIdentifiers(const NPUTF8 **names,
+                                       int32_t name_count,
+                                       NPIdentifier *identifiers) {
+    DCHECK(current_);
+    return current_->GetStringIdentifiers(names, name_count, identifiers);
+  }
+
+  static NPObject* MockRetainObject(NPObject* obj) {
+    DCHECK(current_);
+    return current_->RetainObject(obj);
+  }
+
+  static void MockReleaseObject(NPObject* obj) {
+    DCHECK(current_);
+    current_->ReleaseObject(obj);
+  }
+
+  static MockNetscapeFuncs* current_;
+  static NPNetscapeFuncs netscape_funcs_;
+};
+
 // Test fixture to allow testing the privileged NPAPI APIs
 class TestNPAPIPrivilegedApi: public ::testing::Test {
  public:
   virtual void SetUp() {
     memset(&instance, 0, sizeof(instance));
+    npapi::InitializeBrowserFunctions(
+        const_cast<NPNetscapeFuncs*>(mock_funcs.netscape_funcs()));
 
     // Gets owned & destroyed by mock_api (in the
     // ChromeFramePlugin<T>::Uninitialize() function).
@@ -119,6 +191,11 @@ class TestNPAPIPrivilegedApi: public ::testing::Test {
   }
 
   virtual void TearDown() {
+    // Make sure to uninitialize the mock NPAPI before we uninitialize the
+    // browser function, otherwise we will get a DCHECK in the NPAPI dtor
+    // when it tries to perform the uninitialize there.
+    mock_api.Uninitialize();
+    npapi::UninitializeBrowserFunctions();
   }
 
   void SetupPrivilegeTest(bool is_incognito,
@@ -154,6 +231,7 @@ class TestNPAPIPrivilegedApi: public ::testing::Test {
   }
 
  public:
+  MockNetscapeFuncs mock_funcs;
   MockNPAPI mock_api;
   MockAutomationClient* mock_automation;
   MockProxyService* mock_proxy;
@@ -264,79 +342,6 @@ TEST_F(TestNPAPIPrivilegedApi, PrivilegedAllowsArgsAndProfile) {
 
 namespace {
 
-static const NPIdentifier kOnPrivateMessageId =
-    reinterpret_cast<NPIdentifier>(0x100);
-static const NPIdentifier kPostPrivateMessageId =
-    reinterpret_cast<NPIdentifier>(0x100);
-
-
-class MockNetscapeFuncs {
- public:
-  MockNetscapeFuncs() {
-    CHECK(NULL == current_);
-    current_ = this;
-  }
-
-  ~MockNetscapeFuncs() {
-    CHECK(this == current_);
-    current_ = NULL;
-  }
-
-  MOCK_METHOD3(GetValue, NPError(NPP, NPNVariable, void *));
-  MOCK_METHOD3(GetStringIdentifiers, void(const NPUTF8 **,
-                                          int32_t,
-                                          NPIdentifier *));  // NOLINT
-  MOCK_METHOD1(RetainObject, NPObject*(NPObject*));  // NOLINT
-  MOCK_METHOD1(ReleaseObject, void(NPObject*));  // NOLINT
-
-
-  void GetPrivilegedStringIdentifiers(const NPUTF8 **names,
-                                      int32_t name_count,
-                                      NPIdentifier *identifiers) {
-    for (int32_t i = 0; i < name_count; ++i) {
-      if (0 == strcmp(names[i], "onprivatemessage")) {
-        identifiers[i] = kOnPrivateMessageId;
-      } else if (0 == strcmp(names[i], "postPrivateMessage")) {
-        identifiers[i] = kPostPrivateMessageId;
-      } else {
-        identifiers[i] = 0;
-      }
-    }
-  }
-
-  static const NPNetscapeFuncs* netscape_funcs() {
-    return &netscape_funcs_;
-  }
-
- private:
-  static NPError MockGetValue(NPP instance,
-                              NPNVariable variable,
-                              void *ret_value) {
-    DCHECK(current_);
-    return current_->GetValue(instance, variable, ret_value);
-  }
-
-  static void MockGetStringIdentifiers(const NPUTF8 **names,
-                                       int32_t name_count,
-                                       NPIdentifier *identifiers) {
-    DCHECK(current_);
-    return current_->GetStringIdentifiers(names, name_count, identifiers);
-  }
-
-  static NPObject* MockRetainObject(NPObject* obj) {
-    DCHECK(current_);
-    return current_->RetainObject(obj);
-  }
-
-  static void MockReleaseObject(NPObject* obj) {
-    DCHECK(current_);
-    current_->ReleaseObject(obj);
-  }
-
-  static MockNetscapeFuncs* current_;
-  static NPNetscapeFuncs netscape_funcs_;
-};
-
 MockNetscapeFuncs* MockNetscapeFuncs::current_ = NULL;
 NPNetscapeFuncs MockNetscapeFuncs::netscape_funcs_ = {
   0,   // size
@@ -394,8 +399,6 @@ class TestNPAPIPrivilegedProperty: public TestNPAPIPrivilegedApi {
  public:
   virtual void SetUp() {
     TestNPAPIPrivilegedApi::SetUp();
-    npapi::InitializeBrowserFunctions(
-        const_cast<NPNetscapeFuncs*>(mock_funcs.netscape_funcs()));
 
     // Expect calls to release and retain objects.
     EXPECT_CALL(mock_funcs, RetainObject(kMockNPObject))
@@ -417,12 +420,8 @@ class TestNPAPIPrivilegedProperty: public TestNPAPIPrivilegedApi {
   }
 
   virtual void TearDown() {
-    npapi::UninitializeBrowserFunctions();
     TestNPAPIPrivilegedApi::TearDown();
   }
-
- public:
-  MockNetscapeFuncs mock_funcs;
 };
 
 
diff --git a/chrome_frame/np_event_listener.cc b/chrome_frame/np_event_listener.cc
index 937ca1d..cfbaded 100644
--- a/chrome_frame/np_event_listener.cc
+++ b/chrome_frame/np_event_listener.cc
@@ -172,9 +172,13 @@ NPObject* NPObjectEventListener::GetObjectElement(NPP instance) {
         // We've got the <embed> element but we really want
         // the <object> element.
         if (npapi::GetProperty(instance, object, ids[PARENT_ELEMENT], &var)) {
-          DCHECK(NPVARIANT_IS_OBJECT(var));
           npapi::ReleaseObject(object);
-          object = NPVARIANT_TO_OBJECT(var);
+          object = NULL;
+          if (NPVARIANT_IS_OBJECT(var)) {
+            object = NPVARIANT_TO_OBJECT(var);
+          } else {
+            DVLOG(1) << __FUNCTION__ << " Could not find our parent";
+          }
         }
       } else {
         DVLOG(1) << __FUNCTION__ << " got " << tag;