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

Issue 10861006: Fix the failed case AndroidProviderBackendTest.UpdateFavicon on Android (Closed)

Created:
8 years, 4 months ago by yongsheng
Modified:
8 years, 4 months ago
Reviewers:
michaelbai, pkotwicz
CC:
chromium-reviews, brettw-cc_chromium.org
Visibility:
Public.

Description

Fix the failed case AndroidProviderBackendTest.UpdateFavicon on Android It's caused by issue 10831341 which uses scoped_refptr<base::RefCountedMemory>. The content of the 'data' is swapped so need to reset it. BUG= TEST=run_tests.py -s unit_tests --gtest_filter=AndroidProviderBackendTest.UpdateFavicon TBR=sky

Patch Set 1 #

Total comments: 1

Patch Set 2 : Changes according to the comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -2 lines) Patch
M chrome/browser/history/android/android_provider_backend_unittest.cc View 1 2 chunks +5 lines, -2 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
yongsheng
@michaelbai and @pkotwicz, could you please help review it?
8 years, 4 months ago (2012-08-20 09:58:10 UTC) #1
pkotwicz
LGTM with nit
8 years, 4 months ago (2012-08-20 14:17:22 UTC) #2
pkotwicz
http://codereview.chromium.org/10861006/diff/1/chrome/browser/history/android/android_provider_backend_unittest.cc File chrome/browser/history/android/android_provider_backend_unittest.cc (right): http://codereview.chromium.org/10861006/diff/1/chrome/browser/history/android/android_provider_backend_unittest.cc#newcode1144 chrome/browser/history/android/android_provider_backend_unittest.cc:1144: data.push_back('1'); You can actually use new RefCountedBytes(data) instead.
8 years, 4 months ago (2012-08-20 14:17:29 UTC) #3
michaelbai
lgtm
8 years, 4 months ago (2012-08-20 22:07:42 UTC) #4
michaelbai
On 2012/08/20 22:07:42, michaelbai wrote: > lgtm Please TRB=sky
8 years, 4 months ago (2012-08-20 22:08:32 UTC) #5
yongsheng
On 2012/08/20 22:08:32, michaelbai wrote: > On 2012/08/20 22:07:42, michaelbai wrote: > > lgtm > ...
8 years, 4 months ago (2012-08-21 05:50:20 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yongsheng.zhu@intel.com/10861006/7001
8 years, 4 months ago (2012-08-22 08:56:10 UTC) #7
commit-bot: I haz the power
Failed to apply patch for chrome/browser/history/android/android_provider_backend_unittest.cc: While running patch -p1 --forward --force; patching file chrome/browser/history/android/android_provider_backend_unittest.cc ...
8 years, 4 months ago (2012-08-22 08:56:12 UTC) #8
yongsheng
8 years, 4 months ago (2012-08-22 09:00:34 UTC) #9
On 2012/08/21 05:50:20, yongsheng wrote:
> On 2012/08/20 22:08:32, michaelbai wrote:
> > On 2012/08/20 22:07:42, michaelbai wrote:
> > > lgtm
> > 
> > Please TRB=sky
> All done. thanks two. I'll commit it.

pkotwicz changed the code for this case. won't need it anymore. close it.

Powered by Google App Engine
This is Rietveld 408576698