Add new FrameTest.* unit tests and fix discovered CefFrame-related bugs.

- Allow empty |name| argument to CefBrowser::GetFrame. This will return the main frame.
- Modify CefBrowser::GetFrame to search both assigned and unique frame names.
- Calling CefFrame::IsFocused on the main frame should return true when there are no other frames.
- Fix CefBrowser::GetFrameIdentifiers and GetFrameNames to return correct values in the renderer process (issue #1236).
- Delete NavigationTest.FrameNameIdent which is now obsoleted by the new FrameTests.

git-svn-id: https://chromiumembedded.googlecode.com/svn/trunk@1842 5089003a-bbd8-11dd-ad1f-f1f9622dbc98

cef.gyp

@@ -403,6 +403,7 @@
         'tests/unittests/display_unittest.cc',
         'tests/unittests/dom_unittest.cc',
         'tests/unittests/download_unittest.cc',
+        'tests/unittests/frame_unittest.cc',
         'tests/unittests/geolocation_unittest.cc',
         'tests/unittests/jsdialog_unittest.cc',
         'tests/unittests/life_span_unittest.cc',
@@ -1478,6 +1479,7 @@
             'tests/unittests/client_app_delegates.cc',
             'tests/unittests/cookie_unittest.cc',
             'tests/unittests/dom_unittest.cc',
+            'tests/unittests/frame_unittest.cc',
             'tests/unittests/message_router_unittest.cc',
             'tests/unittests/navigation_unittest.cc',
             'tests/unittests/process_message_unittest.cc',

include/cef_browser.h

@@ -159,7 +159,7 @@ class CefBrowser : public virtual CefBase {
   ///
   // Returns the frame with the specified name, or NULL if not found.
   ///
-  /*--cef()--*/
+  /*--cef(optional_param=name)--*/
   virtual CefRefPtr<CefFrame> GetFrame(const CefString& name) =0;
 
   ///

libcef/browser/frame_host_impl.cc

@@ -56,7 +56,7 @@ CefFrameHostImpl::CefFrameHostImpl(CefBrowserHostImpl* browser,
     : frame_id_(frame_id),
       is_main_frame_(is_main_frame),
       browser_(browser),
-      is_focused_(false),
+      is_focused_(is_main_frame_),  // The main frame always starts focused.
       url_(url),
       name_(name),
       parent_frame_id_(parent_frame_id == kUnspecifiedFrameId ?

libcef/renderer/browser_impl.cc

@@ -190,9 +190,17 @@ CefRefPtr<CefFrame> CefBrowserImpl::GetFrame(int64 identifier) {
 CefRefPtr<CefFrame> CefBrowserImpl::GetFrame(const CefString& name) {
   CEF_REQUIRE_RT_RETURN(NULL);
 
-  if (render_view()->GetWebView()) {
+  blink::WebView* web_view = render_view()->GetWebView();
-    WebFrame* frame =
+  if (web_view) {
-        render_view()->GetWebView()->findFrameByName(name.ToString16());
+    const blink::WebString& frame_name = name.ToString16();
+    // Search by assigned frame name (Frame::name).
+    WebFrame* frame = web_view->findFrameByName(frame_name,
+                                                web_view->mainFrame());
+    if (!frame) {
+      // Search by unique frame name (Frame::uniqueName).
+      frame = webkit_glue::FindFrameByUniqueName(frame_name,
+                                                 web_view->mainFrame());
+    }
     if (frame)
       return GetWebFrameImpl(frame).get();
   }
@@ -222,6 +230,9 @@ size_t CefBrowserImpl::GetFrameCount() {
 void CefBrowserImpl::GetFrameIdentifiers(std::vector<int64>& identifiers) {
   CEF_REQUIRE_RT_RETURN_VOID();
 
+  if (identifiers.size() > 0)
+    identifiers.clear();
+
   if (render_view()->GetWebView()) {
     WebFrame* main_frame = render_view()->GetWebView()->mainFrame();
     if (main_frame) {
@@ -237,6 +248,9 @@ void CefBrowserImpl::GetFrameIdentifiers(std::vector<int64>& identifiers) {
 void CefBrowserImpl::GetFrameNames(std::vector<CefString>& names) {
   CEF_REQUIRE_RT_RETURN_VOID();
 
+  if (names.size() > 0)
+    names.clear();
+
   if (render_view()->GetWebView()) {
     WebFrame* main_frame = render_view()->GetWebView()->mainFrame();
     if (main_frame) {

libcef/renderer/webkit_glue.cc

@@ -26,6 +26,7 @@ MSVC_PUSH_WARNING_LEVEL(0);
 #include "third_party/WebKit/public/web/WebViewClient.h"
 
 #include "third_party/WebKit/Source/core/dom/Node.h"
+#include "third_party/WebKit/Source/web/WebLocalFrameImpl.h"
 #include "third_party/WebKit/Source/web/WebViewImpl.h"
 MSVC_POP_WARNING();
 #undef LOG
@@ -94,4 +95,30 @@ int64 GetIdentifier(blink::WebFrame* frame) {
   return kInvalidFrameId;
 }
 
+// Based on WebViewImpl::findFrameByName and FrameTree::find.
+blink::WebFrame* FindFrameByUniqueName(const blink::WebString& unique_name,
+                                       blink::WebFrame* relative_to_frame) {
+  blink::Frame* start_frame = toWebLocalFrameImpl(relative_to_frame)->frame();
+  if (!start_frame)
+    return NULL;
+
+  const AtomicString& atomic_name = unique_name;
+  blink::Frame* found_frame = NULL;
+
+  // Search the subtree starting with |start_frame|.
+  for (blink::Frame* frame = start_frame;
+       frame;
+       frame = frame->tree().traverseNext(start_frame)) {
+    if (frame->tree().uniqueName() == atomic_name) {
+      found_frame = frame;
+      break;
+    }
+  }
+
+  if (found_frame && found_frame->isLocalFrame())
+    return blink::WebLocalFrameImpl::fromFrame(toLocalFrame(found_frame));
+
+  return NULL;
+}
+
 }  // webkit_glue

libcef/renderer/webkit_glue.h

@@ -38,6 +38,11 @@ bool SetNodeValue(blink::WebNode& node, const blink::WebString& value);
 
 int64 GetIdentifier(blink::WebFrame* frame);
 
+// Find the frame with the specified |unique_name| relative to
+// |relative_to_frame| in the frame hierarchy.
+blink::WebFrame* FindFrameByUniqueName(const blink::WebString& unique_name,
+                                       blink::WebFrame* relative_to_frame);
+
 }  // webkit_glue
 
 #endif  // CEF_LIBCEF_RENDERER_WEBKIT_GLUE_H_

libcef_dll/cpptoc/browser_cpptoc.cc

@@ -247,10 +247,7 @@ struct _cef_frame_t* CEF_CALLBACK browser_get_frame(struct _cef_browser_t* self,
   DCHECK(self);
   if (!self)
     return NULL;
-  // Verify param: name; type: string_byref_const
+  // Unverified params: name
-  DCHECK(name);
-  if (!name)
-    return NULL;
 
   // Execute
   CefRefPtr<CefFrame> _retval = CefBrowserCppToC::Get(self)->GetFrame(

libcef_dll/ctocpp/browser_ctocpp.cc

@@ -226,10 +226,7 @@ CefRefPtr<CefFrame> CefBrowserCToCpp::GetFrame(const CefString& name) {
 
   // AUTO-GENERATED CONTENT - DELETE THIS COMMENT BEFORE MODIFYING
 
-  // Verify param: name; type: string_byref_const
+  // Unverified params: name
-  DCHECK(!name.empty());
-  if (name.empty())
-    return NULL;
 
   // Execute
   cef_frame_t* _retval = struct_->get_frame(struct_,

tests/unittests/client_app_delegates.cc

@@ -6,9 +6,9 @@
 
 // static
 void ClientApp::CreateBrowserDelegates(BrowserDelegateSet& delegates) {
-  // Bring in the V8 tests.
+  // Bring in the Frame tests.
-  extern void CreateV8BrowserTests(BrowserDelegateSet& delegates);
+  extern void CreateFrameBrowserTests(BrowserDelegateSet& delegates);
-  CreateV8BrowserTests(delegates);
+  CreateFrameBrowserTests(delegates);
 
   // Bring in the Navigation tests.
   extern void CreateNavigationBrowserTests(BrowserDelegateSet& delegates);
@@ -17,48 +17,56 @@ void ClientApp::CreateBrowserDelegates(BrowserDelegateSet& delegates) {
   // Bring in the RequestHandler tests.
   extern void CreateRequestHandlerBrowserTests(BrowserDelegateSet& delegates);
   CreateRequestHandlerBrowserTests(delegates);
+
+  // Bring in the V8 tests.
+  extern void CreateV8BrowserTests(BrowserDelegateSet& delegates);
+  CreateV8BrowserTests(delegates);
 }
 
 // static
 void ClientApp::CreateRenderDelegates(RenderDelegateSet& delegates) {
-  // Bring in the routing test handler delegate.
+  // Bring in the Frame tests.
-  extern void CreateRoutingTestHandlerDelegate(
+  extern void CreateFrameRendererTests(RenderDelegateSet& delegates);
-      ClientApp::RenderDelegateSet& delegates);
+  CreateFrameRendererTests(delegates);
-  CreateRoutingTestHandlerDelegate(delegates);
+
+  // Bring in the DOM tests.
+  extern void CreateDOMRendererTests(RenderDelegateSet& delegates);
+  CreateDOMRendererTests(delegates);
 
   // Bring in the message router tests.
   extern void CreateMessageRouterRendererTests(
       ClientApp::RenderDelegateSet& delegates);
   CreateMessageRouterRendererTests(delegates);
 
+  // Bring in the Navigation tests.
+  extern void CreateNavigationRendererTests(RenderDelegateSet& delegates);
+  CreateNavigationRendererTests(delegates);
+
   // Bring in the process message tests.
   extern void CreateProcessMessageRendererTests(
       ClientApp::RenderDelegateSet& delegates);
   CreateProcessMessageRendererTests(delegates);
 
-  // Bring in the V8 tests.
+  // Bring in the RequestHandler tests.
-  extern void CreateV8RendererTests(RenderDelegateSet& delegates);
+  extern void CreateRequestHandlerRendererTests(RenderDelegateSet& delegates);
-  CreateV8RendererTests(delegates);
+  CreateRequestHandlerRendererTests(delegates);
-
-  // Bring in the DOM tests.
-  extern void CreateDOMRendererTests(RenderDelegateSet& delegates);
-  CreateDOMRendererTests(delegates);
-
-  // Bring in the URLRequest tests.
-  extern void CreateURLRequestRendererTests(RenderDelegateSet& delegates);
-  CreateURLRequestRendererTests(delegates);
-
-  // Bring in the Navigation tests.
-  extern void CreateNavigationRendererTests(RenderDelegateSet& delegates);
-  CreateNavigationRendererTests(delegates);
 
   // Bring in the Request tests.
   extern void CreateRequestRendererTests(RenderDelegateSet& delegates);
   CreateRequestRendererTests(delegates);
 
-  // Bring in the RequestHandler tests.
+  // Bring in the routing test handler delegate.
-  extern void CreateRequestHandlerRendererTests(RenderDelegateSet& delegates);
+  extern void CreateRoutingTestHandlerDelegate(
-  CreateRequestHandlerRendererTests(delegates);
+      ClientApp::RenderDelegateSet& delegates);
+  CreateRoutingTestHandlerDelegate(delegates);
+
+  // Bring in the URLRequest tests.
+  extern void CreateURLRequestRendererTests(RenderDelegateSet& delegates);
+  CreateURLRequestRendererTests(delegates);
+
+  // Bring in the V8 tests.
+  extern void CreateV8RendererTests(RenderDelegateSet& delegates);
+  CreateV8RendererTests(delegates);
 }
 
 // static

tests/unittests/navigation_unittest.cc

@@ -493,174 +493,6 @@ TEST(NavigationTest, History) {
 }
 
 
-namespace {
-
-const char kFNav1[] = "http://tests/nav1.html";
-const char kFNav2[] = "http://tests/nav2.html";
-const char kFNav3[] = "http://tests/nav3.html";
-
-class FrameNameIdentNavTestHandler : public TestHandler {
- public:
-  FrameNameIdentNavTestHandler() : browse_ct_(0) {}
-
-  virtual void RunTest() OVERRIDE {
-    // Add the frame resources.
-    std::stringstream ss;
-
-    // Page with named frame
-    ss << "<html>Nav1<iframe src=\"" << kFNav2 << "\" name=\"nav2\"></html>";
-    AddResource(kFNav1, ss.str(), "text/html");
-    ss.str("");
-
-    // Page with unnamed frame
-    ss << "<html>Nav2<iframe src=\"" << kFNav3 << "\"></html>";
-    AddResource(kFNav2, ss.str(), "text/html");
-    ss.str("");
-
-    AddResource(kFNav3, "<html>Nav3</html>", "text/html");
-
-    // Create the browser.
-    CreateBrowser(kFNav1);
-  }
-
-  virtual bool OnBeforeResourceLoad(CefRefPtr<CefBrowser> browser,
-                                    CefRefPtr<CefFrame> frame,
-                                    CefRefPtr<CefRequest> request) OVERRIDE {
-    std::string name = frame->GetName();
-    CefRefPtr<CefFrame> parent = frame->GetParent();
-
-    std::string url = request->GetURL();
-    if (url == kFNav1) {
-      EXPECT_EQ(RT_MAIN_FRAME, request->GetResourceType());
-      EXPECT_EQ(TT_EXPLICIT, request->GetTransitionType());
-
-      frame1_ident_ = frame->GetIdentifier();
-      if (name == "") {
-        frame1_name_ = name;
-        got_frame1_name_.yes();
-      }
-      if (!parent.get())
-        got_frame1_ident_parent_before_.yes();
-    } else if (url == kFNav2) {
-      EXPECT_EQ(RT_SUB_FRAME, request->GetResourceType());
-      EXPECT_EQ(TT_LINK, request->GetTransitionType());
-
-      frame2_ident_ = frame->GetIdentifier();
-      if (name == "nav2") {
-        frame2_name_ = name;
-        got_frame2_name_.yes();
-      }
-      if (parent.get() && frame1_ident_ == parent->GetIdentifier())
-        got_frame2_ident_parent_before_.yes();
-    } else if (url == kFNav3) {
-      EXPECT_EQ(RT_SUB_FRAME, request->GetResourceType());
-      EXPECT_EQ(TT_LINK, request->GetTransitionType());
-
-      frame3_ident_ = frame->GetIdentifier();
-      if (name == "<!--framePath //nav2/<!--frame0-->-->") {
-        frame3_name_ = name;
-        got_frame3_name_.yes();
-      }
-       if (parent.get() && frame2_ident_ == parent->GetIdentifier())
-        got_frame3_ident_parent_before_.yes();
-    }
-
-    return false;
-  }
-
-  virtual void OnLoadEnd(CefRefPtr<CefBrowser> browser,
-                         CefRefPtr<CefFrame> frame,
-                         int httpStatusCode) OVERRIDE {
-    std::string url = frame->GetURL();
-    CefRefPtr<CefFrame> parent = frame->GetParent();
-
-    if (url == kFNav1) {
-      if (frame1_ident_ == frame->GetIdentifier())
-        got_frame1_ident_.yes();
-      if (!parent.get())
-        got_frame1_ident_parent_after_.yes();
-    } else if (url == kFNav2) {
-      if (frame2_ident_ == frame->GetIdentifier())
-        got_frame2_ident_.yes();
-      if (parent.get() && frame1_ident_ == parent->GetIdentifier())
-        got_frame2_ident_parent_after_.yes();
-    } else if (url == kFNav3) {
-      if (frame3_ident_ == frame->GetIdentifier())
-        got_frame3_ident_.yes();
-      if (parent.get() && frame2_ident_ == parent->GetIdentifier())
-        got_frame3_ident_parent_after_.yes();
-    }
-
-    if (++browse_ct_ == 3) {
-      // Test GetFrameNames
-      std::vector<CefString> names;
-      browser->GetFrameNames(names);
-      EXPECT_EQ((size_t)3, names.size());
-      EXPECT_STREQ(frame1_name_.c_str(), names[0].ToString().c_str());
-      EXPECT_STREQ(frame2_name_.c_str(), names[1].ToString().c_str());
-      EXPECT_STREQ(frame3_name_.c_str(), names[2].ToString().c_str());
-
-      // Test GetFrameIdentifiers
-      std::vector<int64> idents;
-      browser->GetFrameIdentifiers(idents);
-      EXPECT_EQ((size_t)3, idents.size());
-      EXPECT_EQ(frame1_ident_, idents[0]);
-      EXPECT_EQ(frame2_ident_, idents[1]);
-      EXPECT_EQ(frame3_ident_, idents[2]);
-
-      DestroyTest();
-    }
-  }
-
-  int browse_ct_;
-
-  int64 frame1_ident_;
-  std::string frame1_name_;
-  int64 frame2_ident_;
-  std::string frame2_name_;
-  int64 frame3_ident_;
-  std::string frame3_name_;
-
-  TrackCallback got_frame1_name_;
-  TrackCallback got_frame2_name_;
-  TrackCallback got_frame3_name_;
-  TrackCallback got_frame1_ident_;
-  TrackCallback got_frame2_ident_;
-  TrackCallback got_frame3_ident_;
-  TrackCallback got_frame1_ident_parent_before_;
-  TrackCallback got_frame2_ident_parent_before_;
-  TrackCallback got_frame3_ident_parent_before_;
-  TrackCallback got_frame1_ident_parent_after_;
-  TrackCallback got_frame2_ident_parent_after_;
-  TrackCallback got_frame3_ident_parent_after_;
-};
-
-}  // namespace
-
-// Verify frame names and identifiers.
-TEST(NavigationTest, FrameNameIdent) {
-  CefRefPtr<FrameNameIdentNavTestHandler> handler =
-      new FrameNameIdentNavTestHandler();
-  handler->ExecuteTest();
-
-  ASSERT_GT(handler->frame1_ident_, 0);
-  ASSERT_GT(handler->frame2_ident_, 0);
-  ASSERT_GT(handler->frame3_ident_, 0);
-  ASSERT_TRUE(handler->got_frame1_name_);
-  ASSERT_TRUE(handler->got_frame2_name_);
-  ASSERT_TRUE(handler->got_frame3_name_);
-  ASSERT_TRUE(handler->got_frame1_ident_);
-  ASSERT_TRUE(handler->got_frame2_ident_);
-  ASSERT_TRUE(handler->got_frame3_ident_);
-  ASSERT_TRUE(handler->got_frame1_ident_parent_before_);
-  ASSERT_TRUE(handler->got_frame2_ident_parent_before_);
-  ASSERT_TRUE(handler->got_frame3_ident_parent_before_);
-  ASSERT_TRUE(handler->got_frame1_ident_parent_after_);
-  ASSERT_TRUE(handler->got_frame2_ident_parent_after_);
-  ASSERT_TRUE(handler->got_frame3_ident_parent_after_);
-}
-
-
 namespace {
 
 const char kRNav1[] = "http://tests/nav1.html";

tests/unittests/test_handler.h

@@ -251,4 +251,35 @@ bool TestFailed();
 #define EXPECT_FILE_THREAD()     EXPECT_TRUE(CefCurrentlyOn(TID_FILE));
 #define EXPECT_RENDERER_THREAD() EXPECT_TRUE(CefCurrentlyOn(TID_RENDERER));
 
+// Helper macros for executing checks in a method with a boolean return value.
+// For example:
+//
+// bool VerifyVals(bool a, bool b) {
+//   V_DECLARE();
+//   V_EXPECT_TRUE(a);
+//   V_EXPECT_FALSE(b);
+//   V_RETURN();
+// }
+//
+// EXPECT_TRUE(VerifyVals(true, false));
+
+#define V_DECLARE() \
+    bool __verify = true; \
+    bool __result
+
+#define V_RETURN() \
+    return __verify
+
+#define V_EXPECT_TRUE(condition) \
+    __result = !!(condition); \
+    __verify &= __result; \
+    GTEST_TEST_BOOLEAN_(__result, #condition, false, true, \
+                        GTEST_NONFATAL_FAILURE_)
+
+#define V_EXPECT_FALSE(condition) \
+    __result = !!(condition); \
+    __verify &= !__result; \
+    GTEST_TEST_BOOLEAN_(!(__result), #condition, true, false, \
+                        GTEST_NONFATAL_FAILURE_)
+
 #endif  // CEF_TESTS_UNITTESTS_TEST_HANDLER_H_