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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 10031052: Excluding AppLaunchHandler on Android. (Closed)

Created:
8 years, 8 months ago by Jay Civelli
Modified:
8 years, 8 months ago
CC:
chromium-reviews, dbeam+watch-ntp_chromium.org, mihaip+watch_chromium.org, estade+watch_chromium.org
Visibility:
Public.

Description

Excluding AppLaunchHandler on Android. The AppLauncherHandler class is not used on Android and has several dependencies to the Browser object which is also not available on Android. So excluding them on Android. BUG=None TEST=unit_tests on Android should not report AppLaucherHandler symbols missing when compiled. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=132292

Patch Set 1 #

Total comments: 2

Patch Set 2 : Synced #

Total comments: 3

Patch Set 3 : Made ShouldShowApps() return false on Android #

Patch Set 4 : Rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -4 lines) Patch
M chrome/browser/extensions/extension_management_api.cc View 1 3 chunks +6 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/ntp/new_tab_ui.cc View 1 2 3 6 chunks +7 lines, -3 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 14 (0 generated)
Jesse Greenwald
https://chromiumcodereview.appspot.com/10031052/diff/1/chrome/browser/extensions/extension_management_api.cc File chrome/browser/extensions/extension_management_api.cc (right): https://chromiumcodereview.appspot.com/10031052/diff/1/chrome/browser/extensions/extension_management_api.cc#newcode23 chrome/browser/extensions/extension_management_api.cc:23: #if !defined(OS_ANDROID) Move to the end of the include ...
8 years, 8 months ago (2012-04-10 23:59:47 UTC) #1
Jesse Greenwald
https://chromiumcodereview.appspot.com/10031052/diff/1/chrome/chrome_browser.gypi File chrome/chrome_browser.gypi (right): https://chromiumcodereview.appspot.com/10031052/diff/1/chrome/chrome_browser.gypi#newcode4533 chrome/chrome_browser.gypi:4533: '../third_party/libusb/libusb.gyp:libusb', The libusb stuff should be removed before submitting.
8 years, 8 months ago (2012-04-11 00:00:44 UTC) #2
Jay Civelli
8 years, 8 months ago (2012-04-11 21:40:58 UTC) #3
Jay Civelli
8 years, 8 months ago (2012-04-12 17:54:11 UTC) #4
Aaron Boodman
lgtm
8 years, 8 months ago (2012-04-13 05:01:04 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jcivelli@chromium.org/10031052/4001
8 years, 8 months ago (2012-04-13 15:50:00 UTC) #6
commit-bot: I haz the power
Presubmit check for 10031052-4001 failed and returned exit status 1. Running presubmit commit checks ...
8 years, 8 months ago (2012-04-13 15:50:03 UTC) #7
asargent_no_longer_on_chrome
LGTM too; sorry for review latency.
8 years, 8 months ago (2012-04-13 16:15:03 UTC) #8
Jay Civelli
James, could you take a look at the webui part? I need a LGTM from ...
8 years, 8 months ago (2012-04-13 16:59:39 UTC) #9
Evan Stade
https://chromiumcodereview.appspot.com/10031052/diff/4001/chrome/browser/ui/webui/ntp/new_tab_ui.cc File chrome/browser/ui/webui/ntp/new_tab_ui.cc (right): https://chromiumcodereview.appspot.com/10031052/diff/4001/chrome/browser/ui/webui/ntp/new_tab_ui.cc#newcode134 chrome/browser/ui/webui/ntp/new_tab_ui.cc:134: #endif seems like you should change ShouldShowApps
8 years, 8 months ago (2012-04-13 19:01:10 UTC) #10
Evan Stade
lgtm https://chromiumcodereview.appspot.com/10031052/diff/4001/chrome/browser/ui/webui/ntp/new_tab_ui.cc File chrome/browser/ui/webui/ntp/new_tab_ui.cc (right): https://chromiumcodereview.appspot.com/10031052/diff/4001/chrome/browser/ui/webui/ntp/new_tab_ui.cc#newcode134 chrome/browser/ui/webui/ntp/new_tab_ui.cc:134: #endif On 2012/04/13 19:01:10, Evan Stade wrote: > ...
8 years, 8 months ago (2012-04-13 19:05:51 UTC) #11
Jay Civelli
https://chromiumcodereview.appspot.com/10031052/diff/4001/chrome/browser/ui/webui/ntp/new_tab_ui.cc File chrome/browser/ui/webui/ntp/new_tab_ui.cc (right): https://chromiumcodereview.appspot.com/10031052/diff/4001/chrome/browser/ui/webui/ntp/new_tab_ui.cc#newcode134 chrome/browser/ui/webui/ntp/new_tab_ui.cc:134: #endif On 2012/04/13 19:05:51, Evan Stade wrote: > On ...
8 years, 8 months ago (2012-04-13 20:59:39 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jcivelli@chromium.org/10031052/19001
8 years, 8 months ago (2012-04-13 20:59:46 UTC) #13
commit-bot: I haz the power
8 years, 8 months ago (2012-04-13 23:12:44 UTC) #14 
Change committed as 132292

Powered by Google App Engine
This is Rietveld 408576698