Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(105)

Issue 515073002: Revert of Re-enable SitePerProcessBrowserTest.CrossSiteIframe (Closed)

Created:
6 years, 3 months ago by benwells
Modified:
6 years, 3 months ago
CC:
chromium-reviews, darin-cc_chromium.org, nasko+codewatch_chromium.org, jam, creis+watch_chromium.org, site-isolation-reviews_chromium.org
Project:
chromium
Visibility:
Public.

Description

Revert of Re-enable SitePerProcessBrowserTest.CrossSiteIframe (patchset #10 of https://codereview.chromium.org/479403004/) Reason for revert: This is causing unaddressable memory errors on the DrMemory full bot. Build where the error was introduced: http://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Content%20Browser%20%28DrMemory%20full%29%20%284%29/builds/945 (error hash: 25DFC5E3C30FC6B7) Sample log output: UNADDRESSABLE ACCESS of freed memory: reading 0x034348e0-0x034348e4 4 byte(s) # 0 blink_web.dll!blink::toCoreFrame [third_party\webkit\source\web\webframe.cpp:24] # 1 blink_web.dll!blink::RemoteFrameClient::firstChild [third_party\webkit\source\web\remoteframeclient.cpp:49] # 2 blink_web.dll!blink::WebRemoteFrameImpl::~WebRemoteFrameImpl [third_party\webkit\source\web\webremoteframeimpl.cpp:106] # 3 blink_web.dll!blink::WebRemoteFrameImpl::close [third_party\webkit\source\web\webremoteframeimpl.cpp:131] # 4 content.dll!content::RenderFrameProxy::~RenderFrameProxy [content\renderer\render_frame_proxy.cc:120] # 5 content.dll!content::RenderFrameProxy::`vector deleting destructor' # 6 content.dll!content::RenderFrameProxy::OnDeleteProxy [content\renderer\render_frame_proxy.cc:180] # 7 content.dll!content::MessageRouter::RouteMessage [content\common\message_router.cc:54] # 8 content.dll!content::MessageRouter::OnMessageReceived [content\common\message_router.cc:46] # 9 content.dll!content::ChildThread::OnMessageReceived [content\child\child_thread.cc:494] #10 ipc.dll!IPC::ChannelProxy::Context::OnDispatchMessage [ipc\ipc_channel_proxy.cc:273] #11 ipc.dll!base::internal::Invoker<>::Run [base\bind_internal.h:1253] #12 base.dll!base::debug::TaskAnnotator::RunTask [base\debug\task_annotator.cc:62] #13 base.dll!base::MessageLoop::RunTask [base\message_loop\message_loop.cc:436] #14 base.dll!base::MessageLoop::DeferOrRunPendingTask [base\message_loop\message_loop.cc:445] #15 base.dll!base::MessageLoop::DoWork [base\message_loop\message_loop.cc:552] #16 base.dll!base::MessagePumpDefault::Run [base\message_loop\message_pump_default.cc:32] #17 base.dll!base::MessageLoop::RunHandler [base\message_loop\message_loop.cc:408] #18 content.dll!content::RendererMain [content\renderer\renderer_main.cc:227] #19 content.dll!content::RunNamedProcessTypeMain [content\app\content_main_runner.cc:415] #20 content.dll!content::ContentMainRunnerImpl::Run [content\app\content_main_runner.cc:764] #21 content.dll!content::ContentMain [content\app\content_main.cc:19] #22 content::LaunchTests [content\public\test\test_launcher.cc:475] #23 main [content\test\content_test_launcher.cc:123] BTW IMHO the CL summary for this change could be better, this change did way more than re-enalbe a test. Ideally the summary would capture the entirety of the change. Original issue's description: > Re-enable SitePerProcessBrowserTest.CrossSiteIframe > > This CL fixes a few issues in the cross-process subframe navigation and re-enables the CrossSiteIframe test on most platforms. There are still some issues on ChromeOS and Android, which will be investigated and fixed in follow-up CL. > > BUG=399775, 357747 > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=291437 TBR=creis@chromium.org,avi@chromium.org,nasko@chromium.org NOTREECHECKS=true NOTRY=true BUG=399775, 357747 Committed: https://crrev.com/471be39ef0b50e63cb4861f19016ae9fd3f8273b Cr-Commit-Position: refs/heads/master@{#292336}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+65 lines, -47 lines) Patch
M chrome/browser/ui/cocoa/applescript/tab_applescript.mm View 1 chunk +9 lines, -1 line 0 comments Download
M content/browser/frame_host/render_frame_host_impl.h View 1 chunk +0 lines, -3 lines 0 comments Download
M content/browser/frame_host/render_frame_host_impl.cc View 2 chunks +1 line, -5 lines 0 comments Download
M content/browser/frame_host/render_frame_host_manager.cc View 2 chunks +6 lines, -5 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_impl.h View 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_impl.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M content/browser/site_per_process_browsertest.cc View 1 chunk +3 lines, -8 lines 0 comments Download
M content/browser/web_contents/web_contents_impl_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/common/frame_messages.h View 1 chunk +0 lines, -3 lines 0 comments Download
M content/common/view_messages.h View 1 chunk +2 lines, -0 lines 0 comments Download
M content/public/browser/render_widget_host.h View 1 chunk +3 lines, -0 lines 0 comments Download
M content/renderer/render_frame_impl.h View 2 chunks +3 lines, -1 line 0 comments Download
M content/renderer/render_frame_impl.cc View 8 chunks +12 lines, -16 lines 0 comments Download
M content/renderer/render_frame_proxy.cc View 1 chunk +10 lines, -4 lines 0 comments Download
M content/renderer/render_view_impl.h View 1 chunk +1 line, -0 lines 0 comments Download
M content/renderer/render_view_impl.cc View 2 chunks +9 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
benwells
Created Revert of Re-enable SitePerProcessBrowserTest.CrossSiteIframe
6 years, 3 months ago (2014-08-28 07:02:31 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/515073002/1
6 years, 3 months ago (2014-08-28 07:02:51 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1) as 85475c16ce647592792752837be96cca01c6878e
6 years, 3 months ago (2014-08-28 07:03:36 UTC) #3
commit-bot: I haz the power
6 years, 3 months ago (2014-09-10 02:58:05 UTC) #4
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/471be39ef0b50e63cb4861f19016ae9fd3f8273b
Cr-Commit-Position: refs/heads/master@{#292336}

Powered by Google App Engine
This is Rietveld 408576698