New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 662268 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

PlzNavigate: PhishingDOMFeatureExtractorTest tests are broken

Project Member Reported by ananta@chromium.org, Nov 4 2016

Issue description

With PlzNavigate enabled the following tests don't run.
 PhishingDOMFeatureExtractorTest.SubFrames
 PhishingDOMFeatureExtractorTest.SubframeRemoval
 

Comment 1 Deleted

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5fc8d8d462127a0f20651df4c8a8971926f21f50

commit 5fc8d8d462127a0f20651df4c8a8971926f21f50
Author: ananta <ananta@chromium.org>
Date: Wed Nov 09 01:41:46 2016

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

Review-Url: https://codereview.chromium.org/2470413004
Cr-Commit-Position: refs/heads/master@{#430811}

[modify] https://crrev.com/5fc8d8d462127a0f20651df4c8a8971926f21f50/chrome/renderer/chrome_mock_render_thread.h
[modify] https://crrev.com/5fc8d8d462127a0f20651df4c8a8971926f21f50/chrome/renderer/safe_browsing/DEPS
[modify] https://crrev.com/5fc8d8d462127a0f20651df4c8a8971926f21f50/chrome/renderer/safe_browsing/phishing_dom_feature_extractor_browsertest.cc
[modify] https://crrev.com/5fc8d8d462127a0f20651df4c8a8971926f21f50/chrome/test/base/chrome_render_view_test.cc
[modify] https://crrev.com/5fc8d8d462127a0f20651df4c8a8971926f21f50/chrome/test/base/chrome_render_view_test.h

Comment 3 by clamy@chromium.org, Dec 6 2016

Labels: Proj-PlzNavigate-Blocking

Comment 4 by jam@chromium.org, Dec 12 2016

Status: Fixed (was: Assigned)

Sign in to add a comment