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

Issue 2565543006: Revert of PlzNavigate: Fix for the PhishingDOMFeatureExtractorTest.SubFrames and SubframeRemoval tes (Closed)

Created:
4 years ago by jam
Modified:
4 years ago
CC:
chromium-reviews, grt+watch_chromium.org, darin-cc_chromium.org, Nathan Parker
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of PlzNavigate: Fix for the PhishingDOMFeatureExtractorTest.SubFrames and SubframeRemoval tests (patchset #6 id:100001 of https://codereview.chromium.org/2470413004/ ) Reason for revert: This isn't needed anymore after r437654 Original issue's description: > PlzNavigate: Fix for the PhishingDOMFeatureExtractorTest.SubFrames and SubframeRemoval tests > > The PhishingDOMFeatureExtractorTest tests instantiate the renderer side of the navigation > code namely RenderFrameImpl, RenderView, etc. However the browser side of the navigation > is not instantiated. The navigation code in RenderFrame checks for whether browser side > navigation is enabled and sends IPCs etc which don't work for the tests as written. > > Proposed fix is to provide a way to mock the IPCs being sent in the codepaths being > exercised in these tests. Specifically the FrameHostMsg_BeginNavigation and FrameMsg_CommitNavigation > IPCs. To achieve this we provide a way to subclass the ChromeMockRenderThread class which > is instantiated by the ChromeRenderViewTest class. This is achieved via a virtual function > CreateMockRenderThread(). This function is overridden by a new class PhishingMockRenderThread > which is instantiated for the duration of the PhishingDOMFeatureExtractorTest.* tests. > > The other alternative was to make these tests full fledged browser tests which would be a touch > difficult given the way these tests have been written. > > BUG=662268 > TBR=jam > > Committed: https://crrev.com/5fc8d8d462127a0f20651df4c8a8971926f21f50 > Cr-Commit-Position: refs/heads/master@{#430811} TBR=nparker@chromium.org,ananta@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=662268

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -67 lines) Patch
M chrome/renderer/chrome_mock_render_thread.h View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/renderer/safe_browsing/DEPS View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/renderer/safe_browsing/phishing_dom_feature_extractor_browsertest.cc View 4 chunks +0 lines, -52 lines 0 comments Download
M chrome/test/base/chrome_render_view_test.h View 1 chunk +0 lines, -6 lines 0 comments Download
M chrome/test/base/chrome_render_view_test.cc View 2 chunks +1 line, -5 lines 0 comments Download

Messages

Total messages: 6 (3 generated)
jam
Created Revert of PlzNavigate: Fix for the PhishingDOMFeatureExtractorTest.SubFrames and SubframeRemoval tests
4 years ago (2016-12-09 23:14:51 UTC) #2
Nathan Parker
4 years ago (2016-12-10 00:59:14 UTC) #5
Nathan Parker
4 years ago (2016-12-10 00:59:16 UTC) #6
Message was sent while issue was closed.

          

Powered by Google App Engine
This is Rietveld 408576698