Skip to content

Commit

Permalink
Remove all the references of legacy camera app
Browse files Browse the repository at this point in the history
Since it has been 8 milestones since the camera app migrated to SWA, it
should be safe to remove them.

Bug: b:202082286
Test: Pass ash_unittests, unit_tests
Change-Id: I2d3b0a4b48654a1dcd93aec8546c4626763bbb1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3203996
Reviewed-by: Maria Petrisor <mpetrisor@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Wei Lee <wtlee@chromium.org>
Cr-Commit-Position: refs/heads/main@{#940170}
  • Loading branch information
Wei Lee authored and Chromium LUCI CQ committed Nov 10, 2021
1 parent d2339e6 commit 799de53
Show file tree
Hide file tree
Showing 17 changed files with 28 additions and 119 deletions.
2 changes: 0 additions & 2 deletions ash/metrics/demo_session_metrics_recorder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ DemoModeApp GetAppFromAppId(const std::string& app_id) {
return DemoModeApp::kScreensaver;
}

if (app_id == extension_misc::kCameraAppId)
return DemoModeApp::kCamera;
if (app_id == extension_misc::kChromeAppId)
return DemoModeApp::kBrowser;
if (app_id == extension_misc::kFilesManagerAppId)
Expand Down
40 changes: 20 additions & 20 deletions ash/metrics/demo_session_metrics_recorder_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ TEST_F(DemoSessionMetricsRecorderTest, BrowserWindows) {
TEST_F(DemoSessionMetricsRecorderTest, AppTypes) {
std::unique_ptr<aura::Window> browser_window = CreateBrowserWindow();
std::unique_ptr<aura::Window> chrome_app_window =
CreateChromeAppWindow(extension_misc::kCameraAppId);
CreateChromeAppWindow(extension_misc::kCalculatorAppId);
std::unique_ptr<aura::Window> hosted_app_browser_window =
CreateHostedAppBrowserWindow(extension_misc::kYoutubeAppId);
std::unique_ptr<aura::Window> arc_window =
Expand All @@ -218,8 +218,8 @@ TEST_F(DemoSessionMetricsRecorderTest, AppTypes) {
FireTimer();
SendUserActivity();
histogram_tester_->ExpectBucketCount(
"DemoMode.ActiveApp", DemoSessionMetricsRecorder::DemoModeApp::kCamera,
2);
"DemoMode.ActiveApp",
DemoSessionMetricsRecorder::DemoModeApp::kCalculator, 2);

wm::ActivateWindow(hosted_app_browser_window.get());
FireTimer();
Expand Down Expand Up @@ -294,7 +294,7 @@ TEST_F(DemoSessionMetricsRecorderTest, ActiveAppAfterDelayedArcPackageName) {
// Verify popup windows are categorized as kOtherWindow.
TEST_F(DemoSessionMetricsRecorderTest, PopupWindows) {
std::unique_ptr<aura::Window> chrome_app_window =
CreateChromeAppWindow(extension_misc::kCameraAppId);
CreateChromeAppWindow(extension_misc::kCalculatorAppId);
std::unique_ptr<aura::Window> popup_window = CreatePopupWindow();

wm::ActivateWindow(chrome_app_window.get());
Expand All @@ -308,8 +308,8 @@ TEST_F(DemoSessionMetricsRecorderTest, PopupWindows) {
SendUserActivity();

histogram_tester_->ExpectBucketCount(
"DemoMode.ActiveApp", DemoSessionMetricsRecorder::DemoModeApp::kCamera,
5);
"DemoMode.ActiveApp",
DemoSessionMetricsRecorder::DemoModeApp::kCalculator, 5);
histogram_tester_->ExpectBucketCount(
"DemoMode.ActiveApp",
DemoSessionMetricsRecorder::DemoModeApp::kOtherWindow, 3);
Expand Down Expand Up @@ -343,7 +343,7 @@ TEST_F(DemoSessionMetricsRecorderTest, OtherApps) {
// Verify samples are discarded after no user activity.
TEST_F(DemoSessionMetricsRecorderTest, DiscardAfterInactivity) {
std::unique_ptr<aura::Window> chrome_app_window =
CreateChromeAppWindow(extension_misc::kCameraAppId);
CreateChromeAppWindow(extension_misc::kCalculatorAppId);
std::unique_ptr<aura::Window> arc_window =
CreateChromeAppWindow("com.google.Photos");

Expand All @@ -354,8 +354,8 @@ TEST_F(DemoSessionMetricsRecorderTest, DiscardAfterInactivity) {
SendUserActivity();

histogram_tester_->ExpectUniqueSample(
"DemoMode.ActiveApp", DemoSessionMetricsRecorder::DemoModeApp::kCamera,
5);
"DemoMode.ActiveApp",
DemoSessionMetricsRecorder::DemoModeApp::kCalculator, 5);
ClearHistograms();

// Have no user activity for 20 seconds.
Expand All @@ -370,7 +370,7 @@ TEST_F(DemoSessionMetricsRecorderTest, DiscardAfterInactivity) {
// Verify sample collection resumes after user activity.
TEST_F(DemoSessionMetricsRecorderTest, ResumeAfterActivity) {
std::unique_ptr<aura::Window> chrome_app_window =
CreateChromeAppWindow(extension_misc::kCameraAppId);
CreateChromeAppWindow(extension_misc::kCalculatorAppId);

wm::ActivateWindow(chrome_app_window.get());

Expand All @@ -387,14 +387,14 @@ TEST_F(DemoSessionMetricsRecorderTest, ResumeAfterActivity) {
FireTimer();
SendUserActivity();
histogram_tester_->ExpectUniqueSample(
"DemoMode.ActiveApp", DemoSessionMetricsRecorder::DemoModeApp::kCamera,
5);
"DemoMode.ActiveApp",
DemoSessionMetricsRecorder::DemoModeApp::kCalculator, 5);
}

// Verify window activation during idle time doesn't trigger reporting.
TEST_F(DemoSessionMetricsRecorderTest, ActivateWindowWhenIdle) {
std::unique_ptr<aura::Window> chrome_app_window =
CreateChromeAppWindow(extension_misc::kCameraAppId);
CreateChromeAppWindow(extension_misc::kCalculatorAppId);
std::unique_ptr<aura::Window> chrome_app_window2 =
CreateChromeAppWindow(extension_misc::kGoogleKeepAppId);

Expand All @@ -414,7 +414,7 @@ TEST_F(DemoSessionMetricsRecorderTest, ActivateWindowWhenIdle) {

TEST_F(DemoSessionMetricsRecorderTest, RepeatedUserActivity) {
std::unique_ptr<aura::Window> chrome_app_window =
CreateChromeAppWindow(extension_misc::kCameraAppId);
CreateChromeAppWindow(extension_misc::kCalculatorAppId);
std::unique_ptr<aura::Window> arc_window =
CreateArcWindow("com.google.Photos");

Expand All @@ -440,8 +440,8 @@ TEST_F(DemoSessionMetricsRecorderTest, RepeatedUserActivity) {
SendUserActivity();

histogram_tester_->ExpectUniqueSample(
"DemoMode.ActiveApp", DemoSessionMetricsRecorder::DemoModeApp::kCamera,
3);
"DemoMode.ActiveApp",
DemoSessionMetricsRecorder::DemoModeApp::kCalculator, 3);
}

// Verify remaining samples are recorded on exit.
Expand Down Expand Up @@ -512,7 +512,7 @@ TEST_F(DemoSessionMetricsRecorderTest, UniqueAppsLaunchedOnDeletion) {
// Activate each window twice. Despite activating each twice,
// the count should only be incremented once per unique app.
std::unique_ptr<aura::Window> chrome_app_window =
CreateChromeAppWindow(extension_misc::kCameraAppId);
CreateChromeAppWindow(extension_misc::kCalculatorAppId);
wm::ActivateWindow(chrome_app_window.get());
wm::DeactivateWindow(chrome_app_window.get());
wm::ActivateWindow(chrome_app_window.get());
Expand Down Expand Up @@ -609,7 +609,7 @@ TEST_F(DemoSessionMetricsRecorderTest, AppLaunched) {

// Chrome apps
std::unique_ptr<aura::Window> chrome_app_window_1 =
CreateChromeAppWindow(extension_misc::kCameraAppId);
CreateChromeAppWindow(extension_misc::kCalculatorAppId);
wm::ActivateWindow(chrome_app_window_1.get());
wm::DeactivateWindow(chrome_app_window_1.get());
wm::ActivateWindow(chrome_app_window_1.get());
Expand Down Expand Up @@ -657,8 +657,8 @@ TEST_F(DemoSessionMetricsRecorderTest, AppLaunched) {
"DemoMode.AppLaunched", DemoSessionMetricsRecorder::DemoModeApp::kBrowser,
1);
histogram_tester_->ExpectBucketCount(
"DemoMode.AppLaunched", DemoSessionMetricsRecorder::DemoModeApp::kCamera,
1);
"DemoMode.AppLaunched",
DemoSessionMetricsRecorder::DemoModeApp::kCalculator, 1);
// We should see 2 "other chrome apps"
histogram_tester_->ExpectBucketCount(
"DemoMode.AppLaunched",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,6 @@ void RecordAppLaunch(const std::string& app_id,
RecordDefaultAppLaunch(DefaultAppName::kCalculator, launch_source);
} else if (app_id == web_app::kCanvasAppId) {
RecordDefaultAppLaunch(DefaultAppName::kChromeCanvas, launch_source);
} else if (app_id == extension_misc::kCameraAppId) {
RecordDefaultAppLaunch(DefaultAppName::kCamera, launch_source);
} else if (app_id == web_app::kCameraAppId) {
RecordDefaultAppLaunch(DefaultAppName::kCamera, launch_source);
} else if (app_id == web_app::kHelpAppId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -613,10 +613,6 @@ void ExtensionAppsChromeOs::OnSystemFeaturesPrefChanged() {
(is_pref_disabled_mode_hidden != is_disabled_apps_mode_hidden_);
is_disabled_apps_mode_hidden_ = is_pref_disabled_mode_hidden;

UpdateAppDisabledState(
disabled_system_features_pref, policy::SystemFeature::kCamera,
extension_misc::kCameraAppId, is_disabled_mode_changed);

UpdateAppDisabledState(disabled_system_features_pref,
policy::SystemFeature::kWebStore,
extensions::kWebStoreAppId, is_disabled_mode_changed);
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/chromeos/extensions/default_app_order.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ const char* const kDefaultAppOrder[] = {
extension_misc::kGooglePlayBooksAppId,
web_app::kPlayBooksAppId,

extension_misc::kCameraAppId,
web_app::kCameraAppId,

arc::kGooglePhotosAppId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

// Extensions that should not be attempted to be uninstalled and reinstalled.
const char* const kExemptExtensions[] = {
extension_misc::kCameraAppId,
extension_misc::kChromeAppId,
extension_misc::kLacrosAppId,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ bool IsComponentExtensionAllowlisted(const std::string& extension_id) {
#endif
#if BUILDFLAG(IS_CHROMEOS_ASH)
extension_misc::kAccessibilityCommonExtensionId,
extension_misc::kCameraAppId,
extension_misc::kChromeVoxExtensionId,
extension_misc::kEnhancedNetworkTtsExtensionId,
extension_misc::kEspeakSpeechSynthesisExtensionId,
Expand Down
4 changes: 3 additions & 1 deletion chrome/browser/extensions/external_provider_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ namespace {

#if BUILDFLAG(IS_CHROMEOS_ASH)

const char kCameraAppId[] = "hfhhnacclhffhdffklopdkcgdhifgngh";

// Certain pre-installed extensions are no longer needed on ARC devices as they
// were replaced by their ARC counterparts.
bool ShouldUninstallExtensionReplacedByArcApp(const std::string& extension_id) {
Expand Down Expand Up @@ -247,7 +249,7 @@ void ExternalProviderImpl::RetrieveExtensionsFromPrefs(
const base::DictionaryValue* extension_dict = nullptr;

#if BUILDFLAG(IS_CHROMEOS_ASH)
if (extension_id == extension_misc::kCameraAppId) {
if (extension_id == kCameraAppId) {
unsupported_extensions.insert(extension_id);
install_stage_tracker->ReportFailure(
extension_id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,7 @@ std::string
bool StandardManagementPolicyProvider::UserMayLoad(
const Extension* extension,
std::u16string* error) const {
// Component extensions are always allowed, besides the camera app that can be
// disabled by extension policy. This is a temporary solution until there's a
// dedicated policy to disable the camera, at which point the special check in
// the 'if' statement should be removed.
// TODO(http://crbug.com/1002935)
if (Manifest::IsComponentLocation(extension->location()) &&
extension->id() != extension_misc::kCameraAppId) {
if (Manifest::IsComponentLocation(extension->location())) {
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1479,7 +1479,7 @@ TEST_F(AppListSyncableServiceTest, TransferItem) {
EXPECT_FALSE(GetSyncItem(extension_misc::kYoutubeAppId));
// Attributes transfer from non-existing app fails.
EXPECT_FALSE(app_list_syncable_service()->TransferItemAttributes(
extension_misc::kCameraAppDevId, extension_misc::kYoutubeAppId));
"NonExistingId", extension_misc::kYoutubeAppId));

// Now Chrome app attributes match Webstore app.
EXPECT_TRUE(AreAllAppAtributesEqualInAppList(webstore_item, chrome_item));
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/ui/ash/chrome_new_window_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
#include "chrome/browser/web_applications/system_web_apps/system_web_app_manager.h"
#include "chrome/browser/web_applications/web_app_helpers.h"
#include "chrome/browser/web_applications/web_app_id.h"
#include "chrome/browser/web_applications/web_app_id_constants.h"
#include "chrome/browser/web_applications/web_app_provider.h"
#include "chrome/browser/web_applications/web_app_registrar.h"
#include "chrome/common/extensions/extension_constants.h"
Expand Down Expand Up @@ -824,12 +825,12 @@ void ChromeNewWindowClient::LaunchCameraApp(const std::string& queries,
DCHECK(IsCameraAppEnabled());
ChromeCameraAppUIDelegate::CameraAppDialog::ShowIntent(
queries, arc::GetArcWindow(task_id));
apps::RecordAppLaunch(extension_misc::kCameraAppId,
apps::RecordAppLaunch(web_app::kCameraAppId,
apps::mojom::LaunchSource::kFromArc);
}

void ChromeNewWindowClient::CloseCameraApp() {
const ash::ShelfID shelf_id(extension_misc::kCameraAppId);
const ash::ShelfID shelf_id(web_app::kCameraAppId);
AppWindowShelfItemController* const app_controller =
ChromeShelfController::instance()
->shelf_model()
Expand Down
30 changes: 0 additions & 30 deletions chrome/browser/ui/ash/shelf/chrome_shelf_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,6 @@ std::vector<ash::ShelfID> ChromeShelfPrefs::GetPinnedAppsFromSync(

if (ShouldPerformConsistencyMigrations()) {
needs_consistency_migrations_ = false;
MigrateLegacyCameraApp(syncable_service);
EnsureChromePinned(syncable_service);
}

Expand Down Expand Up @@ -534,35 +533,6 @@ void ChromeShelfPrefs::SkipPinnedAppsFromSyncForTest() {
skip_pinned_apps_from_sync_for_test = true;
}

void ChromeShelfPrefs::MigrateLegacyCameraApp(
app_list::AppListSyncableService* syncable_service) {
syncer::StringOrdinal legacy_camera_pinned_position;

for (const auto& sync_peer : syncable_service->sync_items()) {
if (IsLegacyCameraAppId(sync_peer.first) &&
!legacy_camera_pinned_position.IsValid()) {
legacy_camera_pinned_position = sync_peer.second->item_pin_ordinal;

// Wipe the position for legacy camera app.
syncable_service->SetPinPosition(sync_peer.first,
syncer::StringOrdinal());
}
}

bool has_camera_app =
syncable_service->GetSyncItem(extension_misc::kCameraAppId) != nullptr;
syncer::StringOrdinal camera_app_position =
syncable_service->GetPinPosition(extension_misc::kCameraAppId);
// If the camera app is in the sync list with no valid position and there is a
// legacy camera app which has valid position, use this position for the
// camera app.
if (has_camera_app && !camera_app_position.IsValid() &&
legacy_camera_pinned_position.IsValid()) {
syncable_service->SetPinPosition(extension_misc::kCameraAppId,
legacy_camera_pinned_position);
}
}

void ChromeShelfPrefs::EnsureChromePinned(
app_list::AppListSyncableService* syncable_service) {
syncer::StringOrdinal chrome_position =
Expand Down
6 changes: 0 additions & 6 deletions chrome/browser/ui/ash/shelf/chrome_shelf_prefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,6 @@ class ChromeShelfPrefs : public app_list::AppListSyncableService::Observer {
// https://crbug.com/1085597
static void SkipPinnedAppsFromSyncForTest();

// This is run once each time ash launches. This method unpins any of the
// legacy camera apps. and then uses their pinned position for the new camera
// app.
void MigrateLegacyCameraApp(
app_list::AppListSyncableService* syncable_service);

// This is run once each time ash launches. If the chrome app is not pinned
// then this creates a pin for the chrome app.
void EnsureChromePinned(app_list::AppListSyncableService* syncable_service);
Expand Down
25 changes: 0 additions & 25 deletions chrome/browser/ui/ash/shelf/chrome_shelf_prefs_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ std::unique_ptr<SyncItem> MakeSyncItem(
return item;
}

const char kLegacyCameraAppId[] = "ngmkobaiicipbagcngcmilfkhejlnfci";

// A fake for AppListSyncableService that allows easy modifications.
class AppListSyncableServiceFake : public app_list::AppListSyncableService {
public:
Expand Down Expand Up @@ -156,29 +154,6 @@ class ChromeShelfPrefsTest : public testing::Test {
std::unique_ptr<ChromeShelfPrefsFake> shelf_prefs_;
};

TEST_F(ChromeShelfPrefsTest, MigrateLegacyCameraApp) {
// Set up the initial ordinals.
syncer::StringOrdinal initial_ordinal =
syncer::StringOrdinal::CreateInitialOrdinal();
syncer::StringOrdinal next_ordinal = initial_ordinal.CreateAfter();
syncable_service_.item_map_[kLegacyCameraAppId] =
MakeSyncItem(kLegacyCameraAppId, next_ordinal);
syncable_service_.item_map_[extension_misc::kCameraAppId] =
MakeSyncItem(kLegacyCameraAppId, syncer::StringOrdinal());

// Migrate.
shelf_prefs_->MigrateLegacyCameraApp(&syncable_service_);

// Check that the legacy camera app now has an invalid ordinal.
EXPECT_FALSE(syncable_service_.item_map_[kLegacyCameraAppId]
->item_pin_ordinal.IsValid());

// Check that the new camera app has the ordinal of the legacy camera app.
auto& pin_ordinal = syncable_service_.item_map_[extension_misc::kCameraAppId]
->item_pin_ordinal;
EXPECT_TRUE(pin_ordinal.Equals(next_ordinal));
}

TEST_F(ChromeShelfPrefsTest, AddChromePinNoExistingOrdinal) {
shelf_prefs_->EnsureChromePinned(&syncable_service_);

Expand Down
7 changes: 0 additions & 7 deletions extensions/browser/extension_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1016,14 +1016,7 @@ void ExtensionPrefs::ClearInapplicableDisableReasonsForComponentExtension(
disable_reason::DISABLE_RELOAD |
disable_reason::DISABLE_UNSUPPORTED_REQUIREMENT |
disable_reason::DISABLE_CORRUPTED | disable_reason::DISABLE_REINSTALL;

// Allow the camera app to be disabled by extension policy. This is a
// temporary solution until there's a dedicated policy to disable the
// camera, at which point this should be removed.
// TODO(http://crbug.com/1002935)
int allowed_disable_reasons = kAllowDisableReasons;
if (component_extension_id == extension_misc::kCameraAppId)
allowed_disable_reasons |= disable_reason::DISABLE_BLOCKED_BY_POLICY;

// Some disable reasons incorrectly cause component extensions to never
// activate on load. See https://crbug.com/946839 for more details on why we
Expand Down
3 changes: 0 additions & 3 deletions extensions/common/constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,6 @@ const char kQuickOfficeInternalExtensionId[] =
const char kQuickOfficeExtensionId[] = "gbkeegbaiigmenfmjfclcdgdpimamgkj";
const char kMimeHandlerPrivateTestExtensionId[] =
"oickdpebdnfbgkcaoklfcdhjniefkcji";
const char kCameraAppId[] = "hfhhnacclhffhdffklopdkcgdhifgngh";
const char kCameraAppDevId[] = "flgnmkgjffmkephdokeeliiopbjaafpm";
const char kChromeAppId[] = "mgndgikekgjfcpckkfioiadnlibdjbkf";
// Generated by: echo "lacros-chrome" | sha256sum | head -c32 | tr 0-9a-f a-p
const char kLacrosAppId[] = "jaimifaeiicidiikhmjedcgdimealfbh";
Expand Down Expand Up @@ -174,7 +172,6 @@ const char kGuestModeTestExtensionId[] = "behllobkkfkfnphdnhnkndlbkcpglgmj";
bool IsSystemUIApp(base::StringPiece extension_id) {
static const char* const kApps[] = {
// clang-format off
kCameraAppId,
kChromeVoxExtensionId,
kFeedbackExtensionId,
kFilesManagerAppId,
Expand Down
6 changes: 0 additions & 6 deletions extensions/common/constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -243,12 +243,6 @@ extern const char kQuickOfficeExtensionId[];
// The extension id used for testing mimeHandlerPrivate.
extern const char kMimeHandlerPrivateTestExtensionId[];

// The extension id of the Camera application.
extern const char kCameraAppId[];

// The extension id of the devoloper version of Camera application.
extern const char kCameraAppDevId[];

// The extension id of the Chrome component application.
extern const char kChromeAppId[];

Expand Down

0 comments on commit 799de53

Please sign in to comment.