From 6f88af792329af13d4bddbbfde6473a984fb7605 Mon Sep 17 00:00:00 2001 From: John Mayhew Date: Wed, 28 Mar 2018 17:30:15 +0000 Subject: [PATCH] Change CefFrame::GetName() to return assigned name if it is non-empty before returning unique name (issue #2403) --- libcef/renderer/browser_impl.cc | 7 +- libcef/renderer/frame_impl.cc | 2 +- libcef/renderer/render_frame_util.cc | 9 ++- libcef/renderer/render_frame_util.h | 2 +- tests/ceftests/frame_unittest.cc | 115 ++++++++++++++++++++++----- 5 files changed, 108 insertions(+), 27 deletions(-) diff --git a/libcef/renderer/browser_impl.cc b/libcef/renderer/browser_impl.cc index 70895af18..f14fcdb94 100644 --- a/libcef/renderer/browser_impl.cc +++ b/libcef/renderer/browser_impl.cc @@ -214,7 +214,7 @@ CefRefPtr CefBrowserImpl::GetFrame(const CefString& name) { for (WebFrame* cur_frame = web_view->MainFrame(); cur_frame; cur_frame = cur_frame->TraverseNext()) { if (cur_frame->IsWebLocalFrame() && - render_frame_util::GetUniqueName(cur_frame->ToWebLocalFrame()) == + render_frame_util::GetName(cur_frame->ToWebLocalFrame()) == searchname) { frame = cur_frame; break; @@ -269,8 +269,7 @@ void CefBrowserImpl::GetFrameNames(std::vector& names) { for (WebFrame* frame = render_view()->GetWebView()->MainFrame(); frame; frame = frame->TraverseNext()) { if (frame->IsWebLocalFrame()) - names.push_back( - render_frame_util::GetUniqueName(frame->ToWebLocalFrame())); + names.push_back(render_frame_util::GetName(frame->ToWebLocalFrame())); } } } @@ -356,7 +355,7 @@ CefRefPtr CefBrowserImpl::GetWebFrameImpl( frame->Parent()->ToWebLocalFrame()) : webkit_glue::kInvalidFrameId; const base::string16& name = - base::UTF8ToUTF16(render_frame_util::GetUniqueName(frame)); + base::UTF8ToUTF16(render_frame_util::GetName(frame)); // Notify the browser that the frame has been identified. Send(new CefHostMsg_FrameIdentified(routing_id(), frame_id, parent_id, name)); diff --git a/libcef/renderer/frame_impl.cc b/libcef/renderer/frame_impl.cc index 93a87c604..1283432ad 100644 --- a/libcef/renderer/frame_impl.cc +++ b/libcef/renderer/frame_impl.cc @@ -188,7 +188,7 @@ CefString CefFrameImpl::GetName() { CEF_REQUIRE_RT_RETURN(name); if (frame_) - name = render_frame_util::GetUniqueName(frame_); + name = render_frame_util::GetName(frame_); return name; } diff --git a/libcef/renderer/render_frame_util.cc b/libcef/renderer/render_frame_util.cc index 98984b3bc..48690dee8 100644 --- a/libcef/renderer/render_frame_util.cc +++ b/libcef/renderer/render_frame_util.cc @@ -24,7 +24,14 @@ int64_t GetIdentifier(blink::WebLocalFrame* frame) { return webkit_glue::kInvalidFrameId; } -std::string GetUniqueName(blink::WebLocalFrame* frame) { +std::string GetName(blink::WebLocalFrame* frame) { + DCHECK(frame); + // Return the assigned name if it is non-empty. This represents the name + // property on the frame DOM element. If the assigned name is empty, revert to + // the internal unique name. + if (frame->AssignedName().length() > 0) { + return frame->AssignedName().Utf8(); + } content::RenderFrameImpl* render_frame = content::RenderFrameImpl::FromWebFrame(frame); DCHECK(render_frame); diff --git a/libcef/renderer/render_frame_util.h b/libcef/renderer/render_frame_util.h index 54a259c2a..ee451e514 100644 --- a/libcef/renderer/render_frame_util.h +++ b/libcef/renderer/render_frame_util.h @@ -17,7 +17,7 @@ class WebLocalFrame; namespace render_frame_util { int64_t GetIdentifier(blink::WebLocalFrame* frame); -std::string GetUniqueName(blink::WebLocalFrame* frame); +std::string GetName(blink::WebLocalFrame* frame); } // namespace render_frame_util diff --git a/tests/ceftests/frame_unittest.cc b/tests/ceftests/frame_unittest.cc index 44a0b6d7e..9ec9eb465 100644 --- a/tests/ceftests/frame_unittest.cc +++ b/tests/ceftests/frame_unittest.cc @@ -95,11 +95,12 @@ const char kFrameNavMsg[] = "FrameTest.Navigation"; const char kFrameNavOrigin0[] = "http://tests-framenav0.com/"; const char kFrameNavOrigin1[] = "http://tests-framenav1.com/"; const char kFrameNavOrigin2[] = "http://tests-framenav2.com/"; +const char kFrameNavOrigin3[] = "http://tests-framenav3.com/"; // Maximum number of navigations. Should be kept synchronized with the number // of kFrameNavOrigin* values. Don't modify this value without checking the // below use cases. -const int kMaxMultiNavNavigations = 3; +const int kMaxMultiNavNavigations = 4; // Global variables identifying the currently running test. bool g_frame_nav_test = false; @@ -163,7 +164,7 @@ class FrameNavExpectations { CompletionCallback completion_callback_; }; -// Browser process expectations abstact base class. +// Browser process expectations abstract base class. class FrameNavExpectationsBrowser : public FrameNavExpectations { public: explicit FrameNavExpectationsBrowser(int nav) @@ -427,7 +428,7 @@ class FrameNavTestHandler : public TestHandler { CreateBrowser(expectations_->GetMainURL()); // Time out the test after a reasonable period of time. - SetTestTimeout(); + SetTestTimeout(15000); } // Transition to the next navigation. @@ -1665,6 +1666,7 @@ namespace { const char kFrame0Name[] = ""; const char kFrame1Name[] = "nav2"; const char kFrame2Name[] = "-->"; +const char kFrame3Name[] = "nav3"; bool VerifyBrowserIframe(CefRefPtr browser, CefRefPtr frame, @@ -1672,11 +1674,11 @@ bool VerifyBrowserIframe(CefRefPtr browser, int frame_number) { V_DECLARE(); - // frame0 contains frame1 contains frame2. - CefRefPtr frame0, frame1, frame2; - CefRefPtr frame0b, frame1b, frame2b; - int64 frame0id, frame1id, frame2id; - std::string frame0url, frame1url, frame2url; + // frame0 contains frame1 contains frame2, contains frame3. + CefRefPtr frame0, frame1, frame2, frame3; + CefRefPtr frame0b, frame1b, frame2b, frame3b; + int64 frame0id, frame1id, frame2id, frame3id; + std::string frame0url, frame1url, frame2url, frame3url; // Find frames by name. frame0 = browser->GetFrame(kFrame0Name); @@ -1685,6 +1687,8 @@ bool VerifyBrowserIframe(CefRefPtr browser, V_EXPECT_TRUE(frame1.get()); frame2 = browser->GetFrame(kFrame2Name); V_EXPECT_TRUE(frame2.get()); + frame3 = browser->GetFrame(kFrame3Name); + V_EXPECT_TRUE(frame3.get()); // Verify that the name matches. V_EXPECT_TRUE(frame0->GetName().ToString() == kFrame0Name) @@ -1696,6 +1700,9 @@ bool VerifyBrowserIframe(CefRefPtr browser, V_EXPECT_TRUE(frame2->GetName().ToString() == kFrame2Name) << "expected: " << kFrame2Name << " actual: " << frame2->GetName().ToString(); + V_EXPECT_TRUE(frame3->GetName().ToString() == kFrame3Name) + << "expected: " << kFrame3Name + << " actual: " << frame3->GetName().ToString(); // Verify that the URL matches. frame0url = GetMultiNavURL(origin, 0); @@ -1710,6 +1717,10 @@ bool VerifyBrowserIframe(CefRefPtr browser, V_EXPECT_TRUE(frame2->GetURL() == frame2url) << "expected: " << frame2url << " actual: " << frame2->GetURL().ToString(); + frame3url = GetMultiNavURL(origin, 3); + V_EXPECT_TRUE(frame3->GetURL() == frame3url) + << "expected: " << frame3url + << " actual: " << frame3->GetURL().ToString(); // Verify that the frame id is valid. frame0id = frame0->GetIdentifier(); @@ -1718,6 +1729,8 @@ bool VerifyBrowserIframe(CefRefPtr browser, V_EXPECT_TRUE(frame1id > 0) << "actual: " << frame1id; frame2id = frame2->GetIdentifier(); V_EXPECT_TRUE(frame2id > 0) << "actual: " << frame2id; + frame3id = frame3->GetIdentifier(); + V_EXPECT_TRUE(frame3id > 0) << "actual: " << frame3id; // Verify that the current frame has the correct id. if (frame_number == 0) { @@ -1729,6 +1742,9 @@ bool VerifyBrowserIframe(CefRefPtr browser, } else if (frame_number == 2) { V_EXPECT_TRUE(frame->GetIdentifier() == frame2id) << "expected: " << frame2id << " actual: " << frame->GetIdentifier(); + } else if (frame_number == 3) { + V_EXPECT_TRUE(frame->GetIdentifier() == frame3id) + << "expected: " << frame3id << " actual: " << frame->GetIdentifier(); } // Find frames by id. @@ -1738,6 +1754,8 @@ bool VerifyBrowserIframe(CefRefPtr browser, V_EXPECT_TRUE(frame1b.get()); frame2b = browser->GetFrame(frame2->GetIdentifier()); V_EXPECT_TRUE(frame2b.get()); + frame3b = browser->GetFrame(frame3->GetIdentifier()); + V_EXPECT_TRUE(frame3b.get()); // Verify that the id matches. V_EXPECT_TRUE(frame0b->GetIdentifier() == frame0id) @@ -1746,31 +1764,37 @@ bool VerifyBrowserIframe(CefRefPtr browser, << "expected: " << frame1id << " actual: " << frame1b->GetIdentifier(); V_EXPECT_TRUE(frame2b->GetIdentifier() == frame2id) << "expected: " << frame2id << " actual: " << frame2b->GetIdentifier(); + V_EXPECT_TRUE(frame3b->GetIdentifier() == frame3id) + << "expected: " << frame3id << " actual: " << frame3b->GetIdentifier(); size_t frame_count = browser->GetFrameCount(); - V_EXPECT_TRUE(frame_count == 3U) << "actual: " << frame_count; + V_EXPECT_TRUE(frame_count == 4U) << "actual: " << frame_count; // Verify the GetFrameNames result. std::vector names; browser->GetFrameNames(names); - V_EXPECT_TRUE(names.size() == 3U) << "actual: " << names.size(); + V_EXPECT_TRUE(names.size() == 4U) << "actual: " << names.size(); V_EXPECT_TRUE(names[0].ToString() == kFrame0Name) << "expected: " << kFrame0Name << " actual: " << names[0].ToString(); V_EXPECT_TRUE(names[1].ToString() == kFrame1Name) << "expected: " << kFrame1Name << " actual: " << names[1].ToString(); V_EXPECT_TRUE(names[2].ToString() == kFrame2Name) << "expected: " << kFrame2Name << " actual: " << names[2].ToString(); + V_EXPECT_TRUE(names[3].ToString() == kFrame3Name) + << "expected: " << kFrame3Name << " actual: " << names[3].ToString(); // Verify the GetFrameIdentifiers result. std::vector idents; browser->GetFrameIdentifiers(idents); - V_EXPECT_TRUE(idents.size() == 3U) << "actual: " << idents.size(); + V_EXPECT_TRUE(idents.size() == 4U) << "actual: " << idents.size(); V_EXPECT_TRUE(idents[0] == frame0id) << "expected: " << frame0id << " actual: " << idents[0]; V_EXPECT_TRUE(idents[1] == frame1id) << "expected: " << frame1id << " actual: " << idents[1]; V_EXPECT_TRUE(idents[2] == frame2id) << "expected: " << frame2id << " actual: " << idents[2]; + V_EXPECT_TRUE(idents[3] == frame3id) + << "expected: " << frame3id << " actual: " << idents[3]; // Verify parent hierarchy. V_EXPECT_FALSE(frame0->GetParent().get()); @@ -1780,6 +1804,9 @@ bool VerifyBrowserIframe(CefRefPtr browser, V_EXPECT_TRUE(frame2->GetParent()->GetIdentifier() == frame1id) << "expected: " << frame1id << " actual: " << frame2->GetParent()->GetIdentifier(); + V_EXPECT_TRUE(frame3->GetParent()->GetIdentifier() == frame2id) + << "expected: " << frame2id + << " actual: " << frame3->GetParent()->GetIdentifier(); V_RETURN(); } @@ -1807,6 +1834,9 @@ class FrameNavExpectationsBrowserTestNestedIframes case 2: origin_ = kFrameNavOrigin2; break; + case 3: + origin_ = kFrameNavOrigin3; + break; default: EXPECT_TRUE(false); // Not reached. break; @@ -1830,9 +1860,21 @@ class FrameNavExpectationsBrowserTestNestedIframes // Frame 1. Contains an unnamed iframe. return "Nav2