Commit 31eadf08 authored by Samuel Huang's avatar Samuel Huang Committed by Commit Bot

[DevUI DFM] Add comments related to Native Resource Splitting for DFM, with minor cleanup.

Native Resource Splitting for DFM (on Android) changes the dynamics of
PAK file usage. For context, key changes are:
* Creating split resource PAK for DFMs
* Downloading split resources on on-demand DFM install.
* Loading split resource PAK *on use*.
* Serving resource data bytes from split resources only in code that's
  deemed safe to do so (e.g., by "gating" code so its reachable only
  when the associated DFM is installed).

Also important are what's *not* done for split resource PAKs (but are
still done for the main resources.pak, which is installed in the base
module).
* Loading split resource PAKs on startup.
* Sharing {file descriptor, memory mapped file region} of split
  resource PAKs with child processes (i.e., split resource PACK are
  accessible from UI thread only).
* Retrieving {file descriptor, memory mapped file region} of split
  resource PAKs from child processes (complements the above).

This CL adds comments to code sites related to loading (at startup),
sharing, and retrieving operations for resources.pak. The comments
state that in general, these are not done for DFM split resources.

The CL also performs some minor cleanup.

Bug: 927131
Change-Id: Ib087b02087355043e6a0983c85345da14e64b4d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1774661
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691286}
parent 82c4e858
...@@ -118,7 +118,7 @@ ...@@ -118,7 +118,7 @@
#include "base/android/java_exception_reporter.h" #include "base/android/java_exception_reporter.h"
#include "chrome/browser/android/crash/pure_java_exception_handler.h" #include "chrome/browser/android/crash/pure_java_exception_handler.h"
#include "chrome/common/chrome_descriptors.h" #include "chrome/common/chrome_descriptors.h"
#else #else // defined(OS_ANDROID)
// Diagnostics is only available on non-android platforms. // Diagnostics is only available on non-android platforms.
#include "chrome/browser/diagnostics/diagnostics_controller.h" #include "chrome/browser/diagnostics/diagnostics_controller.h"
#include "chrome/browser/diagnostics/diagnostics_writer.h" #include "chrome/browser/diagnostics/diagnostics_writer.h"
...@@ -957,13 +957,17 @@ void ChromeMainDelegate::PreSandboxStartup() { ...@@ -957,13 +957,17 @@ void ChromeMainDelegate::PreSandboxStartup() {
kAndroidChrome100PercentPakDescriptor, kAndroidChrome100PercentPakDescriptor,
kAndroidUIResourcesPakDescriptor, kAndroidUIResourcesPakDescriptor,
}; };
for (size_t i = 0; i < base::size(extra_pak_keys); ++i) { for (int extra_pak_key : extra_pak_keys) {
pak_fd = global_descriptors->Get(extra_pak_keys[i]); pak_fd = global_descriptors->Get(extra_pak_key);
pak_region = global_descriptors->GetRegion(extra_pak_keys[i]); pak_region = global_descriptors->GetRegion(extra_pak_key);
ui::ResourceBundle::GetSharedInstance().AddDataPackFromFileRegion( ui::ResourceBundle::GetSharedInstance().AddDataPackFromFileRegion(
base::File(pak_fd), pak_region, ui::SCALE_FACTOR_100P); base::File(pak_fd), pak_region, ui::SCALE_FACTOR_100P);
} }
// For Android: Native resources for DFMs should only be used by the browser
// process. Their file descriptors and memory mapped file region are not
// passed to child processes, and are therefore not loaded here.
base::i18n::SetICUDefaultLocale(locale); base::i18n::SetICUDefaultLocale(locale);
const std::string loaded_locale = locale; const std::string loaded_locale = locale;
#else #else
......
...@@ -3576,6 +3576,10 @@ void ChromeContentBrowserClient::GetAdditionalMappedFilesForChildProcess( ...@@ -3576,6 +3576,10 @@ void ChromeContentBrowserClient::GetAdditionalMappedFilesForChildProcess(
int fd = ui::GetMainAndroidPackFd(&region); int fd = ui::GetMainAndroidPackFd(&region);
mappings->ShareWithRegion(kAndroidUIResourcesPakDescriptor, fd, region); mappings->ShareWithRegion(kAndroidUIResourcesPakDescriptor, fd, region);
// For Android: Native resources for DFMs should only be used by the browser
// process. Their file descriptors and memory mapped file regions are not
// passed to child processes.
fd = ui::GetCommonResourcesPackFd(&region); fd = ui::GetCommonResourcesPackFd(&region);
mappings->ShareWithRegion(kAndroidChrome100PercentPakDescriptor, fd, region); mappings->ShareWithRegion(kAndroidChrome100PercentPakDescriptor, fd, region);
......
...@@ -99,6 +99,9 @@ std::string InitResourceBundleAndDetermineLocale(PrefService* local_state, ...@@ -99,6 +99,9 @@ std::string InitResourceBundleAndDetermineLocale(PrefService* local_state,
base::PathService::Get(chrome::FILE_RESOURCES_PACK, &resources_pack_path); base::PathService::Get(chrome::FILE_RESOURCES_PACK, &resources_pack_path);
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
ui::LoadMainAndroidPackFile("assets/resources.pak", resources_pack_path); ui::LoadMainAndroidPackFile("assets/resources.pak", resources_pack_path);
// Avoid loading DFM native resources here, to keep startup lean. These
// resources are loaded on-use, when an already-installed DFM loads.
#else #else
ui::ResourceBundle::GetSharedInstance().AddDataPackFromPath( ui::ResourceBundle::GetSharedInstance().AddDataPackFromPath(
resources_pack_path, ui::SCALE_FACTOR_NONE); resources_pack_path, ui::SCALE_FACTOR_NONE);
......
...@@ -14,6 +14,8 @@ enum { ...@@ -14,6 +14,8 @@ enum {
kAndroidSecondaryLocalePakDescriptor, kAndroidSecondaryLocalePakDescriptor,
kAndroidChrome100PercentPakDescriptor, kAndroidChrome100PercentPakDescriptor,
kAndroidUIResourcesPakDescriptor, kAndroidUIResourcesPakDescriptor,
// DFMs with native resources typically do not share file descriptors with
// child processes. Hence no corresponding *PakDescriptor is defined.
kAndroidMinidumpDescriptor, kAndroidMinidumpDescriptor,
#endif #endif
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment