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

Issue 608443 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

Unaddressable access in ntp_snippets::NTPSnippetsFetcher::FetchSnippets

Project Member Reported by reillyg@chromium.org, May 2 2016

Issue description

The Dr. Memory bots report an unaddressable access in ntp_snippets::NTPSnippetsFetcher::FetchSnippets:

UNADDRESSABLE ACCESS: reading 0x00000000-0x00000004 4 byte(s)
# 0 ntp_snippets::NTPSnippetsFetcher::FetchSnippets                            [components\ntp_snippets\ntp_snippets_fetcher.cc:95]
# 1 ntp_snippets::`anonymous namespace'::NTPSnippetsFetcherTest_ShouldFetchSuccessfully_Test::TestBody [components\ntp_snippets\ntp_snippets_fetcher_unittest.cc:79]
# 2 testing::internal::HandleExceptionsInMethodIfSupported<>                   [testing\gtest\src\gtest.cc:2458]
Note: @0:02:57.916 in thread 7316
Note: instruction: mov    (%ecx) -> %eax
Suppression (error hash=#FC94A3B5E50D171F#):
For more info on using suppressions see http://dev.chromium.org/developers/how-tos/using-drmemory#TOC-Suppressing-error-reports-from-the-
{
UNADDRESSABLE ACCESS
name=<insert_a_suppression_name_here>
*!ntp_snippets::NTPSnippetsFetcher::FetchSnippets
*!ntp_snippets::`anonymous namespace'::NTPSnippetsFetcherTest_ShouldFetchSuccessfully_Test::TestBody
*!testing::internal::HandleExceptionsInMethodIfSupported<>
}

https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%29/builds/4757

These tests were added in https://codereview.chromium.org/1940843002.
 
Status: Assigned (was: Untriaged)
This patch has been reverted for breaking the iOS build (https://codereview.chromium.org/1942913002). Please investigate this issue as well before relanding the patch.
Project Member

Comment 2 by bugdroid1@chromium.org, May 3 2016

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

commit 7bef8e81ca49e36e1f90dc50aa84a45a465eb3f6
Author: mastiz <mastiz@chromium.org>
Date: Tue May 03 12:52:47 2016

[NTP Snippets] Add unit tests for NTPSnippetsFetcher

Second attempt after https://codereview.chromium.org/1940843002/ was
reverted in https://codereview.chromium.org/1940843002/

The class had almost no coverage given that, besides not having unit
tests, the ones for NTPSnippetsService don't actually exercise the
fetcher.

The proposed tests make use of FakeURLFetcherFactory to exercise basic
success and failure cases and verify how callbacks are triggered.
FakeURLFetcherFactory doesn't however support verifying features like
the uploaded POST data for selected hosts which is left outside the
scope of this patch.

A custom fetcher factory (FailingFakeURLFetcherFactory) is introduced
to handle cases where no baked in response exists, which otherwise
leads to null pointers returned by URLFetcher::Create() and later
dereferenced.

BUG= 584428 , 608443 

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

[modify] https://crrev.com/7bef8e81ca49e36e1f90dc50aa84a45a465eb3f6/components/components_tests.gyp
[modify] https://crrev.com/7bef8e81ca49e36e1f90dc50aa84a45a465eb3f6/components/ntp_snippets/BUILD.gn
[modify] https://crrev.com/7bef8e81ca49e36e1f90dc50aa84a45a465eb3f6/components/ntp_snippets/ntp_snippets_fetcher.cc
[modify] https://crrev.com/7bef8e81ca49e36e1f90dc50aa84a45a465eb3f6/components/ntp_snippets/ntp_snippets_fetcher.h
[add] https://crrev.com/7bef8e81ca49e36e1f90dc50aa84a45a465eb3f6/components/ntp_snippets/ntp_snippets_fetcher_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment