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

Issue 341083008: [AiS] Fix ownership issue for image service observers (Closed)

Created:
6 years, 6 months ago by groby-ooo-7-16
Modified:
6 years, 6 months ago
Reviewers:
Ted C, sky
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@img-bridge
Project:
chromium
Visibility:
Public.

Description

[AiS] Fix ownership issue for image service observers The Java bridge erroneously did a "delete this" on the observer when request completion was signaled. That is not necessary, since the BitmapFetcherService takes ownership of all observers. As a result of this, there is also no need for the observer to signal request completion. If clients are interested in taking action on completion, they can do so in the Observer's destructor. TBR=sky@chromium.org, tedchoc@chromium.org BUG=380916 NOTRY=true Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=278869

Patch Set 1 #

Patch Set 2 : Fix compile error. #

Patch Set 3 : Fix unit tests. #

Patch Set 4 : git cl format #

Patch Set 5 : Re-upload. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -22 lines) Patch
M chrome/browser/android/omnibox/answers_image_bridge.cc View 1 1 chunk +1 line, -5 lines 0 comments Download
M chrome/browser/bitmap_fetcher/bitmap_fetcher_service.cc View 1 2 3 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/bitmap_fetcher/bitmap_fetcher_service_unittest.cc View 1 2 3 3 chunks +15 lines, -13 lines 0 comments Download

Messages

Total messages: 21 (0 generated)
groby-ooo-7-16
Ted, Sky: Just a heads-up. Small bug fix that's TBR'd, but waiting for trybots.
6 years, 6 months ago (2014-06-20 01:05:50 UTC) #1
Ted C
lgtm
6 years, 6 months ago (2014-06-20 01:12:12 UTC) #2
sky
LGTM
6 years, 6 months ago (2014-06-20 01:38:54 UTC) #3
groby-ooo-7-16
The CQ bit was checked by groby@chromium.org
6 years, 6 months ago (2014-06-20 01:48:45 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/groby@chromium.org/341083008/20001
6 years, 6 months ago (2014-06-20 01:52:03 UTC) #5
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_dbg_triggered_tests on tryserver.chromium ...
6 years, 6 months ago (2014-06-20 12:58:26 UTC) #6
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 6 months ago (2014-06-20 14:01:36 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/builds/44186)
6 years, 6 months ago (2014-06-20 14:01:37 UTC) #8
groby-ooo-7-16
Heads up - fixed the broken test.
6 years, 6 months ago (2014-06-20 18:04:44 UTC) #9
groby-ooo-7-16
The CQ bit was checked by groby@chromium.org
6 years, 6 months ago (2014-06-20 18:05:33 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/groby@chromium.org/341083008/60001
6 years, 6 months ago (2014-06-20 18:06:39 UTC) #11
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_aosp on tryserver.chromium ...
6 years, 6 months ago (2014-06-20 22:27:40 UTC) #12
groby-ooo-7-16
Marked as NOTRY since AOSP build keeps exploding on missing JSON for steps.
6 years, 6 months ago (2014-06-20 22:29:15 UTC) #13
groby-ooo-7-16
The CQ bit was unchecked by groby@chromium.org
6 years, 6 months ago (2014-06-20 22:29:20 UTC) #14
groby-ooo-7-16
The CQ bit was checked by groby@chromium.org
6 years, 6 months ago (2014-06-20 22:29:27 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/groby@chromium.org/341083008/60001
6 years, 6 months ago (2014-06-20 22:30:44 UTC) #16
commit-bot: I haz the power
Commit queue failed due to new patchset.
6 years, 6 months ago (2014-06-20 22:42:50 UTC) #17
groby-ooo-7-16
The CQ bit was unchecked by groby@chromium.org
6 years, 6 months ago (2014-06-20 22:46:35 UTC) #18
groby-ooo-7-16
The CQ bit was checked by groby@chromium.org
6 years, 6 months ago (2014-06-20 22:46:36 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/groby@chromium.org/341083008/80001
6 years, 6 months ago (2014-06-20 22:47:41 UTC) #20
commit-bot: I haz the power
6 years, 6 months ago (2014-06-20 23:51:46 UTC) #21
Message was sent while issue was closed.
Change committed as 278869

Powered by Google App Engine
This is Rietveld 408576698