Skip to content

Commit

Permalink
Revert "[Files app] Separate chrome-untrusted resources"
Browse files Browse the repository at this point in the history
This reverts commit 2288a6b.

Reason for revert: causes compilation failures on bots e.g. https://ci.chromium.org/ui/p/chromium/builders/try/linux-chromeos-rel/1005808/overview

Original change's description:
> [Files app] Separate chrome-untrusted resources
>
> The resources in u/f/file_manager/foreground/elements/sandboxed/
> are meant to be loaded in a safe environment with restricted
> permissions (either <webview> or an iframed chrome-untrusted page).
>
> Separate these resources from the main ui/file_manager code to better
> reflect the fact that chrome-untrusted://file-manager is now its own
> separate page with its own resources.
>
> Legacy Files app now "borrows" these resources from the chrome-untrusted
> WebUI, to load them in <webview> tags. This code is meant to be removed
> once Files app fully migrates to a SWA.
>
> Test: browser_tests --gtest_filter=QuickView/FilesAppBrowserTest.Test/*
> Bug: b:192894681, b:201363705, b:197601728
> Change-Id: I3490672beb5a72d814cd5fb8c7d908fe8678472d
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3189577
> Reviewed-by: calamity <calamity@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
> Reviewed-by: Bo Majewski <majewski@chromium.org>
> Commit-Queue: Jeremie Boulic <jboulic@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#930868}

Bug: b:192894681, b:201363705, b:197601728
Change-Id: I7b507f1eb486a76149c59cd078e783adc99e4cbd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3218725
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Nic Hollingum <hollingum@google.com>
Reviewed-by: Chih-Yu Huang <akahuang@chromium.org>
Owners-Override: Nic Hollingum <hollingum@google.com>
Commit-Queue: Chih-Yu Huang <akahuang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#930896}
  • Loading branch information
Nic Hollingum authored and Chromium LUCI CQ committed Oct 13, 2021
1 parent b59602c commit 7d11744
Show file tree
Hide file tree
Showing 25 changed files with 97 additions and 101 deletions.
1 change: 0 additions & 1 deletion ash/webui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ group("closure_compile") {
deps = [
"//ash/webui/diagnostics_ui:closure_compile",
"//ash/webui/file_manager/resources:closure_compile",
"//ash/webui/file_manager/untrusted_resources:closure_compile",
"//ash/webui/help_app_ui:closure_compile",
"//ash/webui/media_app_ui:closure_compile",
"//ash/webui/os_feedback_ui:closure_compile",
Expand Down
2 changes: 1 addition & 1 deletion ash/webui/file_manager/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ static_library("file_manager_untrusted_ui") {
"url_constants.h",
]
deps = [
"//ash/webui/file_manager/untrusted_resources:file_manager_untrusted_resources",
"//ash/webui/file_manager/resources:file_manager_untrusted_resources",
"//base",
"//content/public/browser",
"//content/public/common",
Expand Down
2 changes: 1 addition & 1 deletion ash/webui/file_manager/resource_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ void AddFilesAppResources(content::WebUIDataSource* source,
std::string path(entries[i].path);
// Only load resources for Files app.
if (base::StartsWith(path, "file_manager/") &&
path.find("untrusted_resources/") == std::string::npos) {
path.find("sandboxed/") == std::string::npos) {
// Files app UI has all paths relative to //ui/file_manager/file_manager/
// so we remove the leading file_manager/ to match the existing paths.
base::ReplaceFirstSubstringAfterOffset(&path, 0, "file_manager/", "");
Expand Down
12 changes: 6 additions & 6 deletions ash/webui/file_manager/resource_loader_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ TEST_F(ResourceLoaderTest, AddFilesAppResources) {
const webui::ResourcePath kTestResources[] = {
{"file_manager/images/icon192.png", 8},
{"file_manager_fakes.js", 9},
{"file_manager/untrusted_resources_files_img_content.css", 10},
{"file_manager/untrusted_resources/files_img_content.css", 11},
{"file_manager/sandboxed_files_img_content.css", 10},
{"file_manager/sandboxed/files_img_content.css", 11},
};

const size_t kTestResourcesSize = base::size(kTestResources);
Expand All @@ -43,10 +43,10 @@ TEST_F(ResourceLoaderTest, AddFilesAppResources) {

EXPECT_EQ(8, source()->PathToIdrOrDefault("images/icon192.png"));
EXPECT_EQ(-1, source()->PathToIdrOrDefault("file_manager_fakes.js"));
EXPECT_EQ(10, source()->PathToIdrOrDefault(
"untrusted_resources_files_img_content.css"));
EXPECT_EQ(-1, source()->PathToIdrOrDefault(
"untrusted_resources/files_img_content.css"));
EXPECT_EQ(10,
source()->PathToIdrOrDefault("sandboxed_files_img_content.css"));
EXPECT_EQ(-1,
source()->PathToIdrOrDefault("sandboxed/files_img_content.css"));
}

} // namespace file_manager
Expand Down
50 changes: 50 additions & 0 deletions ash/webui/file_manager/resources/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -203,3 +203,53 @@ grit("file_manager_swa_resources") {
]
output_dir = target_gen_dir
}

generated_untrusted_grd = "$target_gen_dir/file_manager_untrusted_resources.grd"

generate_grd("build_file_manager_untrusted_grd") {
grd_prefix = "file_manager_untrusted"
out_grd = generated_untrusted_grd

input_files_base_dir =
rebase_path("//ui/file_manager/file_manager/foreground/elements/", "//")
input_files = [
"files_quick_view.css",
"sandboxed/files_audio_content.css",
"sandboxed/files_audio_content.html",
"sandboxed/files_text_content.css",
"sandboxed/files_text_content.html",
"sandboxed/files_img_content.css",
"sandboxed/files_img_content.html",
"sandboxed/files_media_content.js",
"sandboxed/files_video_content.css",
"sandboxed/files_video_content.html",
]
resource_path_rewrites = [
"sandboxed/files_audio_content.css|files_audio_content.css",
"sandboxed/files_audio_content.html|files_audio_content.html",
"sandboxed/files_text_content.css|files_text_content.css",
"sandboxed/files_text_content.html|files_text_content.html",
"sandboxed/files_img_content.css|files_img_content.css",
"sandboxed/files_img_content.html|files_img_content.html",
"sandboxed/files_media_content.js|files_media_content.js",
"sandboxed/files_video_content.css|files_video_content.css",
"sandboxed/files_video_content.html|files_video_content.html",
]
}

# Resources used by chrome-untrusted://file-manager.
grit("file_manager_untrusted_resources") {
# These arguments are needed since the grd is generated at build time.
enable_input_discovery_for_gn_analyze = false

source = generated_untrusted_grd
deps = [ ":build_file_manager_untrusted_grd" ]

outputs = [
"grit/file_manager_untrusted_resources.h",
"grit/file_manager_untrusted_resources_map.cc",
"grit/file_manager_untrusted_resources_map.h",
"file_manager_untrusted_resources.pak",
]
output_dir = target_gen_dir
}
63 changes: 0 additions & 63 deletions ash/webui/file_manager/untrusted_resources/BUILD.gn

This file was deleted.

2 changes: 1 addition & 1 deletion chrome/browser/chromeos/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ source_set("chromeos") {
"//ash/webui/file_manager:file_manager_ui",
"//ash/webui/file_manager:file_manager_untrusted_ui",
"//ash/webui/file_manager/resources:file_manager_swa_resources_grit",
"//ash/webui/file_manager/untrusted_resources:file_manager_untrusted_resources_grit",
"//ash/webui/file_manager/resources:file_manager_untrusted_resources_grit",
"//ash/webui/help_app_ui",
"//ash/webui/media_app_ui",
"//ash/webui/os_feedback_ui",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@

#if BUILDFLAG(IS_CHROMEOS_ASH)
#include "ash/keyboard/ui/resources/keyboard_resource_util.h"
#include "ash/webui/file_manager/resources/grit/file_manager_untrusted_resources_map.h"
#include "base/command_line.h"
#include "chrome/browser/ash/file_manager/file_manager_string_util.h"
#include "chrome/browser/browser_process.h"
Expand Down Expand Up @@ -112,19 +111,6 @@ ChromeComponentExtensionResourceManager::Data::Data() {
AddComponentResourceEntries(kFileManagerGenResources,
kFileManagerGenResourcesSize);

// Add Files app resources to display untrusted content in <webview> frames.
// Files app extension's resource paths need to be prefixed by
// "file_manager/".
for (size_t i = 0; i < kFileManagerUntrustedResourcesSize; ++i) {
base::FilePath resource_path =
base::FilePath("file_manager")
.AppendASCII(kFileManagerUntrustedResources[i].path);
resource_path = resource_path.NormalizePathSeparators();

DCHECK(!base::Contains(path_to_resource_id_, resource_path));
path_to_resource_id_[resource_path] = kFileManagerUntrustedResources[i].id;
}

// ResourceBundle and g_browser_process are not always initialized in unit
// tests.
if (ui::ResourceBundle::HasSharedInstance() && g_browser_process) {
Expand Down
4 changes: 2 additions & 2 deletions chrome/chrome_paks.gni
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ template("chrome_extra_paks") {
"$root_gen_dir/ash/ash_shortcut_customization_app_resources.pak",
"$root_gen_dir/ash/public/cpp/resources/ash_public_unscaled_resources.pak",
"$root_gen_dir/ash/webui/file_manager/resources/file_manager_swa_resources.pak",
"$root_gen_dir/ash/webui/file_manager/untrusted_resources/file_manager_untrusted_resources.pak",
"$root_gen_dir/ash/webui/file_manager/resources/file_manager_untrusted_resources.pak",
"$root_gen_dir/chrome/audio_resources.pak",
"$root_gen_dir/chrome/bluetooth_pairing_dialog_resources.pak",
"$root_gen_dir/chrome/browser/supervised_user/supervised_user_unscaled_resources.pak",
Expand Down Expand Up @@ -243,7 +243,7 @@ template("chrome_extra_paks") {
deps += [
"//ash/public/cpp/resources:ash_public_unscaled_resources",
"//ash/webui/file_manager/resources:file_manager_swa_resources",
"//ash/webui/file_manager/untrusted_resources:file_manager_untrusted_resources",
"//ash/webui/file_manager/resources:file_manager_untrusted_resources",
"//ash/webui/resources:diagnostics_app_resources",
"//ash/webui/resources:help_app_bundle_resources",
"//ash/webui/resources:help_app_kids_magazine_bundle_resources",
Expand Down
2 changes: 1 addition & 1 deletion tools/gritsettings/resource_ids.spec
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@
"META": {"sizes": {"includes": [100]}},
"includes": [3000],
},
"<(SHARED_INTERMEDIATE_DIR)/ash/webui/file_manager/untrusted_resources/file_manager_untrusted_resources.grd": {
"<(SHARED_INTERMEDIATE_DIR)/ash/webui/file_manager/resources/file_manager_untrusted_resources.grd": {
"META": {"sizes": {"includes": [10]}},
"includes": [3020],
},
Expand Down
1 change: 1 addition & 0 deletions ui/file_manager/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ group("closure_compile") {
"file_manager/background/js:closure_compile",
"file_manager/common/js:closure_compile",
"file_manager/foreground/elements:closure_compile",
"file_manager/foreground/elements/sandboxed:closure_compile",
"file_manager/foreground/js:closure_compile",
"file_manager/foreground/js/metadata:closure_compile",
"file_manager/foreground/js/ui:closure_compile",
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Copyright 2021 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

import("//third_party/closure_compiler/compile_js.gni")

js_library("files_media_content") {
externs_list = [ "//ui/file_manager/file_manager/externs/quick_view.js" ]
}

js_type_check("closure_compile") {
deps = [ ":files_media_content" ]
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions ui/file_manager/file_manager/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@
"partitions": [{
"name": "trusted",
"accessible_resources": [
"untrusted_resources/files_audio_content.*",
"untrusted_resources/files_img_content.*",
"untrusted_resources/files_media_content.js",
"untrusted_resources/files_text_content.*",
"untrusted_resources/files_video_content.*"
"foreground/elements/sandboxed/files_audio_content.*",
"foreground/elements/sandboxed/files_img_content.*",
"foreground/elements/sandboxed/files_media_content.js",
"foreground/elements/sandboxed/files_text_content.*",
"foreground/elements/sandboxed/files_video_content.*"
]
}]
},
Expand Down
9 changes: 9 additions & 0 deletions ui/file_manager/file_manager_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,15 @@
<include name="IDR_FILE_MANAGER_BACKGROUND_HTML" file="file_manager/background.html" type="BINDATA" />
<!-- Polymer elements -->
<include name="IDR_FILE_MANAGER_ELEMENTS_FILES_QUICK_PREVIEW_CSS" file="file_manager/foreground/elements/files_quick_view.css" type="BINDATA" />
<include name="IDR_FILE_MANAGER_SANDBOXED_FILES_AUDIO_CONTENT_CSS" file="file_manager/foreground/elements/sandboxed/files_audio_content.css" type="BINDATA" />
<include name="IDR_FILE_MANAGER_SANDBOXED_FILES_AUDIO_CONTENT_HTML" file="file_manager/foreground/elements/sandboxed/files_audio_content.html" type="BINDATA" />
<include name="IDR_FILE_MANAGER_SANDBOXED_FILES_TEXT_CONTENT_CSS" file="file_manager/foreground/elements/sandboxed/files_text_content.css" type="BINDATA" />
<include name="IDR_FILE_MANAGER_SANDBOXED_FILES_TEXT_CONTENT_HTML" file="file_manager/foreground/elements/sandboxed/files_text_content.html" type="BINDATA" />
<include name="IDR_FILE_MANAGER_SANDBOXED_FILES_IMG_CONTENT_CSS" file="file_manager/foreground/elements/sandboxed/files_img_content.css" type="BINDATA" />
<include name="IDR_FILE_MANAGER_SANDBOXED_FILES_IMG_CONTENT_HTML" file="file_manager/foreground/elements/sandboxed/files_img_content.html" type="BINDATA" />
<include name="IDR_FILE_MANAGER_SANDBOXED_FILES_MEDIA_CONTENT_JS" file="file_manager/foreground/elements/sandboxed/files_media_content.js" type="BINDATA" />
<include name="IDR_FILE_MANAGER_SANDBOXED_FILES_VIDEO_CONTENT_CSS" file="file_manager/foreground/elements/sandboxed/files_video_content.css" type="BINDATA" />
<include name="IDR_FILE_MANAGER_SANDBOXED_FILES_VIDEO_CONTENT_HTML" file="file_manager/foreground/elements/sandboxed/files_video_content.html" type="BINDATA" />

<!-- Scripts required by the metadata parser worker. -->

Expand Down

0 comments on commit 7d11744

Please sign in to comment.