Update from chromium https://crrev.com/301725/ This updates DEPS to reflect changes in 301725 / 90a7c4e3fdeb82a18e17f24e56345b9086a8308b, imports changes, and adds a patch file for our ui/gl/gl_surface modifications. Review URL: https://codereview.chromium.org/669813003
diff --git a/BUILD.gn b/BUILD.gn index d94b440..84beb15 100644 --- a/BUILD.gn +++ b/BUILD.gn
@@ -57,6 +57,7 @@ "android/jni_weak_ref.h", "android/library_loader/library_loader_hooks.cc", "android/library_loader/library_loader_hooks.h", + "android/library_loader/library_load_from_apk_status_codes.h", "android/memory_pressure_listener_android.cc", "android/memory_pressure_listener_android.h", "android/java_handler_thread.cc", @@ -507,10 +508,12 @@ "process/process_metrics_win.cc", "process/process_posix.cc", "process/process_win.cc", - "profiler/scoped_profile.cc", - "profiler/scoped_profile.h", "profiler/alternate_timer.cc", "profiler/alternate_timer.h", + "profiler/scoped_profile.cc", + "profiler/scoped_profile.h", + "profiler/scoped_tracker.cc", + "profiler/scoped_tracker.h", "profiler/tracked_time.cc", "profiler/tracked_time.h", "rand_util.cc", @@ -1340,7 +1343,6 @@ "//base/test:run_all_unittests", "//base/test:test_support", "//base/third_party/dynamic_annotations", - "//base/third_party/nspr", "//testing/gmock", "//testing/gtest", "//third_party/icu", @@ -1464,14 +1466,17 @@ } # GYP: //base.gyp:base_java_application_state + # GYP: //base.gyp:base_java_library_load_from_apk_status_codes # GYP: //base.gyp:base_java_memory_pressure_level java_cpp_enum("base_android_java_enums_srcjar") { sources = [ "android/application_status_listener.h", + "android/library_loader/library_load_from_apk_status_codes.h", "memory/memory_pressure_listener.h", ] outputs = [ "org/chromium/base/ApplicationState.java", + "org/chromium/base/library_loader/LibraryLoadFromApkStatusCodes.java", "org/chromium/base/MemoryPressureLevel.java", ] }
diff --git a/OWNERS b/OWNERS index a014c74..92844b6 100644 --- a/OWNERS +++ b/OWNERS
@@ -9,6 +9,19 @@ # On extended leave. willchan@chromium.org +# Chromium is a very mature project, most things that are generally useful are +# already here, and that things not here aren't generally useful. +# +# Base is pulled into many projects. For example, various ChromeOS daemons. So +# the bar for adding stuff is that it must have demonstrated wide +# applicability. Prefer to add things closer to where they're used (i.e. "not +# base"), and pull into base only when needed. In a project our size, +# sometimes even duplication is OK and inevitable. +# +# Adding a new logging macro DPVELOG_NE is not more clear than just +# writing the stuff you want to log in a regular logging statement, even +# if it makes your calling code longer. Just add it to your own code. + per-file *.isolate=csharp@chromium.org per-file *.isolate=maruel@chromium.org per-file bind.h=ajwong@chromium.org
diff --git a/android/java/src/org/chromium/base/ResourceExtractor.java b/android/java/src/org/chromium/base/ResourceExtractor.java index 4cf155c..37fea6c 100644 --- a/android/java/src/org/chromium/base/ResourceExtractor.java +++ b/android/java/src/org/chromium/base/ResourceExtractor.java
@@ -36,6 +36,8 @@ private static final String LAST_LANGUAGE = "Last language"; private static final String PAK_FILENAMES = "Pak filenames"; private static final String ICU_DATA_FILENAME = "icudtl.dat"; + private static final String V8_NATIVES_DATA_FILENAME = "natives_blob.bin"; + private static final String V8_SNAPSHOT_DATA_FILENAME = "snapshot_blob.bin"; private static String[] sMandatoryPaks = null; @@ -111,8 +113,11 @@ if (!paksToInstall.matcher(file).matches()) { continue; } - boolean isICUData = file.equals(ICU_DATA_FILENAME); - File output = new File(isICUData ? getAppDataDir() : outputDir, file); + boolean isAppDataFile = file.equals(ICU_DATA_FILENAME) + || file.equals(V8_NATIVES_DATA_FILENAME) + || file.equals(V8_SNAPSHOT_DATA_FILENAME); + File output = new File(isAppDataFile + ? getAppDataDir() : outputDir, file); if (output.exists()) { continue; } @@ -138,10 +143,11 @@ throw new IOException(file + " extracted with 0 length!"); } - if (!isICUData) { + if (!isAppDataFile) { filenames.add(file); } else { - // icudata needs to be accessed by a renderer process. + // icu and V8 data need to be accessed by a renderer + // process. output.setReadable(true, false); } } finally { @@ -274,18 +280,21 @@ * running the tests. */ @VisibleForTesting - public void setExtractAllPaksForTesting() { - List<String> pakFileAssets = new ArrayList<String>(); + public void setExtractAllPaksAndV8SnapshotForTesting() { + List<String> pakAndSnapshotFileAssets = new ArrayList<String>(); AssetManager manager = mContext.getResources().getAssets(); try { String[] files = manager.list(""); for (String file : files) { - if (file.endsWith(".pak")) pakFileAssets.add(file); + if (file.endsWith(".pak")) pakAndSnapshotFileAssets.add(file); } } catch (IOException e) { Log.w(LOGTAG, "Exception while accessing assets: " + e.getMessage(), e); } - setMandatoryPaksToExtract(pakFileAssets.toArray(new String[pakFileAssets.size()])); + pakAndSnapshotFileAssets.add("natives_blob.bin"); + pakAndSnapshotFileAssets.add("snapshot_blob.bin"); + setMandatoryPaksToExtract(pakAndSnapshotFileAssets.toArray( + new String[pakAndSnapshotFileAssets.size()])); } private ResourceExtractor(Context context) { @@ -340,15 +349,27 @@ /** * Pak files (UI strings and other resources) should be updated along with * Chrome. A version mismatch can lead to a rather broken user experience. - * The ICU data (icudtl.dat) is less version-sensitive, but still can - * lead to malfunction/UX misbehavior. So, we regard failing to update them - * as an error. + * Failing to update the V8 snapshot files will lead to a version mismatch + * between V8 and the loaded snapshot which will cause V8 to crash, so this + * is treated as an error. The ICU data (icudtl.dat) is less + * version-sensitive, but still can lead to malfunction/UX misbehavior. So, + * we regard failing to update them as an error. */ private void deleteFiles() { File icudata = new File(getAppDataDir(), ICU_DATA_FILENAME); if (icudata.exists() && !icudata.delete()) { Log.e(LOGTAG, "Unable to remove the icudata " + icudata.getName()); } + File v8_natives = new File(getAppDataDir(), V8_NATIVES_DATA_FILENAME); + if (v8_natives.exists() && !v8_natives.delete()) { + Log.e(LOGTAG, + "Unable to remove the v8 data " + v8_natives.getName()); + } + File v8_snapshot = new File(getAppDataDir(), V8_SNAPSHOT_DATA_FILENAME); + if (v8_snapshot.exists() && !v8_snapshot.delete()) { + Log.e(LOGTAG, + "Unable to remove the v8 data " + v8_snapshot.getName()); + } File dir = getOutputDir(); if (dir.exists()) { File[] files = dir.listFiles();
diff --git a/android/java/src/org/chromium/base/SysUtils.java b/android/java/src/org/chromium/base/SysUtils.java index eabef61..1c8378c 100644 --- a/android/java/src/org/chromium/base/SysUtils.java +++ b/android/java/src/org/chromium/base/SysUtils.java
@@ -103,13 +103,12 @@ } private static boolean detectLowEndDevice() { - if (CommandLine.isInitialized()) { - if (CommandLine.getInstance().hasSwitch(BaseSwitches.LOW_END_DEVICE_MODE)) { - int mode = Integer.parseInt(CommandLine.getInstance().getSwitchValue( - BaseSwitches.LOW_END_DEVICE_MODE)); - if (mode == 1) return true; - if (mode == 0) return false; - } + assert CommandLine.isInitialized(); + if (CommandLine.getInstance().hasSwitch(BaseSwitches.LOW_END_DEVICE_MODE)) { + int mode = Integer.parseInt(CommandLine.getInstance().getSwitchValue( + BaseSwitches.LOW_END_DEVICE_MODE)); + if (mode == 1) return true; + if (mode == 0) return false; } if (Build.VERSION.SDK_INT <= ANDROID_LOW_MEMORY_ANDROID_SDK_THRESHOLD) {
diff --git a/android/java/src/org/chromium/base/library_loader/LibraryLoader.java b/android/java/src/org/chromium/base/library_loader/LibraryLoader.java index dcf0754..c56d76c 100644 --- a/android/java/src/org/chromium/base/library_loader/LibraryLoader.java +++ b/android/java/src/org/chromium/base/library_loader/LibraryLoader.java
@@ -50,9 +50,9 @@ private static boolean sIsUsingBrowserSharedRelros = false; private static boolean sLoadAtFixedAddressFailed = false; - // One-way switch recording whether the device supports memory mapping - // APK files with executable permissions. Only used in the browser. - private static boolean sLibraryLoadFromApkSupported = false; + // One-way switch becomes true if the library was loaded from the APK file + // directly. + private static boolean sLibraryWasLoadedFromApk = false; // One-way switch becomes true if the system library loading failed, // and the right native library was found and loaded by the hack. @@ -164,17 +164,10 @@ // Load libraries using the Chromium linker. Linker.prepareLibraryLoad(); - // Check if the device supports loading a library directly from the APK file. - String apkfile = context.getApplicationInfo().sourceDir; - if (Linker.isInBrowserProcess()) { - sLibraryLoadFromApkSupported = Linker.checkLibraryLoadFromApkSupport( - apkfile); - } - for (String library : NativeLibraries.LIBRARIES) { String zipfile = null; if (Linker.isInZipFile()) { - zipfile = apkfile; + zipfile = context.getApplicationInfo().sourceDir; Log.i(TAG, "Loading " + library + " from within " + zipfile); } else { Log.i(TAG, "Loading: " + library); @@ -186,6 +179,7 @@ try { if (zipfile != null) { Linker.loadLibraryInZipFile(zipfile, library); + sLibraryWasLoadedFromApk = true; } else { Linker.loadLibrary(library); } @@ -200,6 +194,7 @@ if (!isLoaded) { if (zipfile != null) { Linker.loadLibraryInZipFile(zipfile, library); + sLibraryWasLoadedFromApk = true; } else { Linker.loadLibrary(library); } @@ -308,27 +303,39 @@ // Called after all native initializations are complete. public static void onNativeInitializationComplete(Context context) { - onNativeInitializationComplete(); - } - - // Called after all native initializations are complete. - @Deprecated - public static void onNativeInitializationComplete() { - recordBrowserProcessHistogram(); + recordBrowserProcessHistogram(context); nativeRecordNativeLibraryHack(sNativeLibraryHackWasUsed); } // Record Chromium linker histogram state for the main browser process. Called from // onNativeInitializationComplete(). - private static void recordBrowserProcessHistogram() { + private static void recordBrowserProcessHistogram(Context context) { if (Linker.isUsed()) { - assert Linker.isInBrowserProcess(); nativeRecordChromiumAndroidLinkerBrowserHistogram(sIsUsingBrowserSharedRelros, sLoadAtFixedAddressFailed, - sLibraryLoadFromApkSupported); + getLibraryLoadFromApkStatus(context)); } } + // Returns the device's status for loading a library directly from the APK file. + // This method can only be called when the Chromium linker is used. + private static int getLibraryLoadFromApkStatus(Context context) { + assert Linker.isUsed(); + + if (sLibraryWasLoadedFromApk) { + return LibraryLoadFromApkStatusCodes.SUCCESSFUL; + } + + if (context == null) { + Log.w(TAG, "Unknown APK filename due to null context"); + return LibraryLoadFromApkStatusCodes.UNKNOWN; + } + + return Linker.checkLibraryLoadFromApkSupport(context.getApplicationInfo().sourceDir) ? + LibraryLoadFromApkStatusCodes.SUPPORTED : + LibraryLoadFromApkStatusCodes.NOT_SUPPORTED; + } + // Register pending Chromium linker histogram state for renderer processes. This cannot be // recorded as a histogram immediately because histograms and IPC are not ready at the // time it are captured. This function stores a pending value, so that a later call to @@ -354,11 +361,11 @@ // Method called to record statistics about the Chromium linker operation for the main // browser process. Indicates whether the linker attempted relro sharing for the browser, // and if it did, whether the library failed to load at a fixed address. Also records - // support for memory mapping APK files with executable permissions. + // support for loading a library directly from the APK file. private static native void nativeRecordChromiumAndroidLinkerBrowserHistogram( boolean isUsingBrowserSharedRelros, boolean loadAtFixedAddressFailed, - boolean apkMemoryMappingSupported); + int libraryLoadFromApkStatus); // Method called to register (for later recording) statistics about the Chromium linker // operation for a renderer process. Indicates whether the linker attempted relro sharing,
diff --git a/android/java/src/org/chromium/base/library_loader/Linker.java b/android/java/src/org/chromium/base/library_loader/Linker.java index 41104f0..997b67c 100644 --- a/android/java/src/org/chromium/base/library_loader/Linker.java +++ b/android/java/src/org/chromium/base/library_loader/Linker.java
@@ -196,6 +196,8 @@ private static boolean sRelroSharingSupported = false; // Set to true if this runs in the browser process. Disabled by initServiceProcess(). + // TODO(petrcermak): This flag can be incorrectly set to false (even though this might run in + // the browser process) on low-memory devices. private static boolean sInBrowserProcess = true; // Becomes true to indicate this process needs to wait for a shared RELRO in @@ -230,6 +232,7 @@ System.loadLibrary(TAG); } catch (UnsatisfiedLinkError e) { // In a component build, the ".cr" suffix is added to each library name. + Log.w(TAG, "Couldn't load lib" + TAG + ".so, trying lib" + TAG + ".cr.so"); System.loadLibrary(TAG + ".cr"); } sRelroSharingSupported = nativeCanUseSharedRelro(); @@ -382,19 +385,6 @@ } /** - * Call this method to determine if the linker is running in the browser - * process. - * - * @return true if the linker is running in the browser process. - */ - public static boolean isInBrowserProcess() { - synchronized (Linker.class) { - ensureInitializedLocked(); - return sInBrowserProcess; - } - } - - /** * Call this method to determine if the chromium project must load * the library directly from the zip file. */ @@ -838,6 +828,7 @@ * @return true if supported. */ public static boolean checkLibraryLoadFromApkSupport(String apkFile) { + assert apkFile != null; synchronized (Linker.class) { ensureInitializedLocked();
diff --git a/android/library_loader/library_load_from_apk_status_codes.h b/android/library_loader/library_load_from_apk_status_codes.h new file mode 100644 index 0000000..73ddbd5 --- /dev/null +++ b/android/library_loader/library_load_from_apk_status_codes.h
@@ -0,0 +1,36 @@ +// Copyright 2014 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. + +#ifndef BASE_ANDROID_LIBRARY_LOAD_FROM_APK_STATUS_CODES_H_ +#define BASE_ANDROID_LIBRARY_LOAD_FROM_APK_STATUS_CODES_H_ + +namespace base { +namespace android { + +namespace { + +// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.base.library_loader +enum LibraryLoadFromApkStatusCodes { + // The loader was unable to determine whether the functionality is supported. + LIBRARY_LOAD_FROM_APK_STATUS_CODES_UNKNOWN = 0, + + // The device does not support loading a library directly from the APK file. + LIBRARY_LOAD_FROM_APK_STATUS_CODES_NOT_SUPPORTED = 1, + + // The device supports loading a library directly from the APK file. + LIBRARY_LOAD_FROM_APK_STATUS_CODES_SUPPORTED = 2, + + // A library was successfully loaded directly from the APK file. + LIBRARY_LOAD_FROM_APK_STATUS_CODES_SUCCESSFUL = 3, + + // End sentinel. + LIBRARY_LOAD_FROM_APK_STATUS_CODES_MAX = 4, +}; + +} // namespace + +} // namespace android +} // namespace base + +#endif // BASE_ANDROID_LIBRARY_LOAD_FROM_APK_STATUS_CODES_H_
diff --git a/android/library_loader/library_loader_hooks.cc b/android/library_loader/library_loader_hooks.cc index 819fe3d..809275d 100644 --- a/android/library_loader/library_loader_hooks.cc +++ b/android/library_loader/library_loader_hooks.cc
@@ -6,6 +6,7 @@ #include "base/android/command_line_android.h" #include "base/android/jni_string.h" +#include "base/android/library_loader/library_load_from_apk_status_codes.h" #include "base/at_exit.h" #include "base/metrics/histogram.h" #include "jni/LibraryLoader_jni.h" @@ -46,14 +47,6 @@ RendererHistogramCode g_renderer_histogram_code = NO_PENDING_HISTOGRAM_CODE; -enum LibraryLoadFromApkSupportCode { - // The device's support for loading a library directly from the APK file. - NOT_SUPPORTED = 0, - SUPPORTED = 1, - - MAX_LIBRARY_LOAD_FROM_APK_SUPPORT_CODE = 2, -}; - } // namespace static void RegisterChromiumAndroidLinkerRendererHistogram( @@ -85,7 +78,7 @@ jclass clazz, jboolean is_using_browser_shared_relros, jboolean load_at_fixed_address_failed, - jboolean library_load_from_apk_supported) { + jint library_load_from_apk_status) { // For low-memory devices, record whether or not we successfully loaded the // browser at a fixed address. Otherwise just record a normal invocation. BrowserHistogramCode histogram_code; @@ -99,12 +92,10 @@ histogram_code, MAX_BROWSER_HISTOGRAM_CODE); - // Record whether the device supports loading a library directly from the APK - // file. - UMA_HISTOGRAM_ENUMERATION("ChromiumAndroidLinker.LibraryLoadFromApkSupported", - library_load_from_apk_supported ? - SUPPORTED : NOT_SUPPORTED, - MAX_LIBRARY_LOAD_FROM_APK_SUPPORT_CODE); + // Record the device support for loading a library directly from the APK file. + UMA_HISTOGRAM_ENUMERATION("ChromiumAndroidLinker.LibraryLoadFromApkStatus", + library_load_from_apk_status, + LIBRARY_LOAD_FROM_APK_STATUS_CODES_MAX); } void SetLibraryLoadedHook(LibraryLoadedHook* func) {
diff --git a/base.gyp b/base.gyp index beb8995..42e270f 100644 --- a/base.gyp +++ b/base.gyp
@@ -1344,6 +1344,7 @@ }, 'dependencies': [ 'base_java_application_state', + 'base_java_library_load_from_apk_status_codes', 'base_java_memory_pressure_level', 'base_native_libraries_gen', ], @@ -1379,6 +1380,16 @@ }, { # GN: //base:base_android_java_enums_srcjar + 'target_name': 'base_java_library_load_from_apk_status_codes', + 'toolsets': ['host', 'target'], + 'type': 'none', + 'variables': { + 'source_file': 'android/library_loader/library_load_from_apk_status_codes.h' + }, + 'includes': [ '../build/android/java_cpp_enum.gypi' ], + }, + { + # GN: //base:base_android_java_enums_srcjar 'target_name': 'base_java_memory_pressure_level', 'type': 'none', 'variables': {
diff --git a/base.gypi b/base.gypi index 9542529..416a7e5 100644 --- a/base.gypi +++ b/base.gypi
@@ -63,6 +63,7 @@ 'android/jni_weak_ref.h', 'android/library_loader/library_loader_hooks.cc', 'android/library_loader/library_loader_hooks.h', + 'android/library_loader/library_load_from_apk_status_codes.h', 'android/memory_pressure_listener_android.cc', 'android/memory_pressure_listener_android.h', 'android/java_handler_thread.cc', @@ -496,10 +497,12 @@ 'process/process_metrics_win.cc', 'process/process_posix.cc', 'process/process_win.cc', - 'profiler/scoped_profile.cc', - 'profiler/scoped_profile.h', 'profiler/alternate_timer.cc', 'profiler/alternate_timer.h', + 'profiler/scoped_profile.cc', + 'profiler/scoped_profile.h', + 'profiler/scoped_tracker.cc', + 'profiler/scoped_tracker.h', 'profiler/tracked_time.cc', 'profiler/tracked_time.h', 'rand_util.cc',
diff --git a/base_paths_win.cc b/base_paths_win.cc index a9b31c7..5bef310 100644 --- a/base_paths_win.cc +++ b/base_paths_win.cc
@@ -95,16 +95,6 @@ return false; cur = FilePath(system_buffer); break; - case base::DIR_LOCAL_APP_DATA_LOW: - if (win::GetVersion() < win::VERSION_VISTA) - return false; - - // TODO(nsylvain): We should use SHGetKnownFolderPath instead. Bug 1281128 - if (FAILED(SHGetFolderPath(NULL, CSIDL_APPDATA, NULL, SHGFP_TYPE_CURRENT, - system_buffer))) - return false; - cur = FilePath(system_buffer).DirName().AppendASCII("LocalLow"); - break; case base::DIR_LOCAL_APP_DATA: if (FAILED(SHGetFolderPath(NULL, CSIDL_LOCAL_APPDATA, NULL, SHGFP_TYPE_CURRENT, system_buffer)))
diff --git a/base_paths_win.h b/base_paths_win.h index 4620171..032de34 100644 --- a/base_paths_win.h +++ b/base_paths_win.h
@@ -25,7 +25,6 @@ DIR_START_MENU, // Usually "C:\Documents and Settings\<user>\ // Start Menu\Programs" DIR_APP_DATA, // Application Data directory under the user profile. - DIR_LOCAL_APP_DATA_LOW, // Local AppData directory for low integrity level. DIR_LOCAL_APP_DATA, // "Local Settings\Application Data" directory under // the user profile. DIR_COMMON_APP_DATA, // W2K, XP, W2K3: "C:\Documents and Settings\
diff --git a/callback_internal.h b/callback_internal.h index 9d7761c..b85973d 100644 --- a/callback_internal.h +++ b/callback_internal.h
@@ -77,7 +77,7 @@ template <typename U> static NoType Test(...); - static const bool value = sizeof(Test<T>(0)) == sizeof(YesType) && + static const bool value = sizeof((Test<T>(0))) == sizeof(YesType) && !is_const<T>::value; };
diff --git a/debug/task_annotator.cc b/debug/task_annotator.cc index f2e4b73..ae2d797 100644 --- a/debug/task_annotator.cc +++ b/debug/task_annotator.cc
@@ -30,6 +30,7 @@ const PendingTask& pending_task) { tracked_objects::ThreadData::PrepareForStartOfRun(pending_task.birth_tally); tracked_objects::TaskStopwatch stopwatch; + stopwatch.Start(); tracked_objects::Duration queue_duration = stopwatch.StartTime() - pending_task.EffectiveTimePosted();
diff --git a/debug/trace_event_impl.h b/debug/trace_event_impl.h index 79bdc97..6075e2d 100644 --- a/debug/trace_event_impl.h +++ b/debug/trace_event_impl.h
@@ -23,7 +23,6 @@ #include "base/synchronization/lock.h" #include "base/threading/thread.h" #include "base/threading/thread_local.h" -#include "base/timer/timer.h" // Older style trace macros with explicit id and extra data // Only these macros result in publishing data to ETW as currently implemented.
diff --git a/files/file_util_posix.cc b/files/file_util_posix.cc index 5a94cef..d86d9bc 100644 --- a/files/file_util_posix.cc +++ b/files/file_util_posix.cc
@@ -322,7 +322,9 @@ } if (S_ISDIR(from_stat.st_mode)) { - if (mkdir(target_path.value().c_str(), from_stat.st_mode & 01777) != 0 && + if (mkdir(target_path.value().c_str(), + (from_stat.st_mode & 01777) | S_IRUSR | S_IXUSR | S_IWUSR) != + 0 && errno != EEXIST) { DLOG(ERROR) << "CopyDirectory() couldn't create directory: " << target_path.value() << " errno = " << errno;
diff --git a/files/file_util_unittest.cc b/files/file_util_unittest.cc index bd4839a..08c9587 100644 --- a/files/file_util_unittest.cc +++ b/files/file_util_unittest.cc
@@ -1379,24 +1379,34 @@ } // Sets the source file to read-only. -void SetReadOnly(const FilePath& path) { +void SetReadOnly(const FilePath& path, bool read_only) { #if defined(OS_WIN) - // On Windows, it involves setting a bit. + // On Windows, it involves setting/removing the 'readonly' bit. DWORD attrs = GetFileAttributes(path.value().c_str()); ASSERT_NE(INVALID_FILE_ATTRIBUTES, attrs); ASSERT_TRUE(SetFileAttributes( - path.value().c_str(), attrs | FILE_ATTRIBUTE_READONLY)); - attrs = GetFileAttributes(path.value().c_str()); + path.value().c_str(), + read_only ? (attrs | FILE_ATTRIBUTE_READONLY) : + (attrs & ~FILE_ATTRIBUTE_READONLY))); // Files in the temporary directory should not be indexed ever. If this // assumption change, fix this unit test accordingly. // FILE_ATTRIBUTE_NOT_CONTENT_INDEXED doesn't exist on XP. - DWORD expected = FILE_ATTRIBUTE_ARCHIVE | FILE_ATTRIBUTE_READONLY; + DWORD expected = read_only ? + ((attrs & (FILE_ATTRIBUTE_ARCHIVE | FILE_ATTRIBUTE_DIRECTORY)) | + FILE_ATTRIBUTE_READONLY) : + (attrs & (FILE_ATTRIBUTE_ARCHIVE | FILE_ATTRIBUTE_DIRECTORY)); + // TODO(ripp@yandex-team.ru): this seems out of place here. If we really think + // it is important to verify that temp files are not indexed there should be + // a dedicated test for that (create a file, inspect the attributes) if (win::GetVersion() >= win::VERSION_VISTA) expected |= FILE_ATTRIBUTE_NOT_CONTENT_INDEXED; + attrs = GetFileAttributes(path.value().c_str()); ASSERT_EQ(expected, attrs); #else - // On all other platforms, it involves removing the write bit. - EXPECT_TRUE(SetPosixFilePermissions(path, S_IRUSR)); + // On all other platforms, it involves removing/setting the write bit. + mode_t mode = read_only ? S_IRUSR : (S_IRUSR | S_IWUSR); + EXPECT_TRUE(SetPosixFilePermissions( + path, DirectoryExists(path) ? (mode | S_IXUSR) : mode)); #endif } @@ -1413,23 +1423,34 @@ } TEST_F(FileUtilTest, CopyDirectoryACL) { - // Create a directory. + // Create source directories. FilePath src = temp_dir_.path().Append(FILE_PATH_LITERAL("src")); - CreateDirectory(src); - ASSERT_TRUE(PathExists(src)); + FilePath src_subdir = src.Append(FILE_PATH_LITERAL("subdir")); + CreateDirectory(src_subdir); + ASSERT_TRUE(PathExists(src_subdir)); // Create a file under the directory. FilePath src_file = src.Append(FILE_PATH_LITERAL("src.txt")); CreateTextFile(src_file, L"Gooooooooooooooooooooogle"); - SetReadOnly(src_file); + SetReadOnly(src_file, true); ASSERT_TRUE(IsReadOnly(src_file)); + // Make directory read-only. + SetReadOnly(src_subdir, true); + ASSERT_TRUE(IsReadOnly(src_subdir)); + // Copy the directory recursively. FilePath dst = temp_dir_.path().Append(FILE_PATH_LITERAL("dst")); FilePath dst_file = dst.Append(FILE_PATH_LITERAL("src.txt")); EXPECT_TRUE(CopyDirectory(src, dst, true)); + FilePath dst_subdir = dst.Append(FILE_PATH_LITERAL("subdir")); + ASSERT_FALSE(IsReadOnly(dst_subdir)); ASSERT_FALSE(IsReadOnly(dst_file)); + + // Give write permissions to allow deletion. + SetReadOnly(src_subdir, false); + ASSERT_FALSE(IsReadOnly(src_subdir)); } TEST_F(FileUtilTest, CopyFile) { @@ -1480,7 +1501,7 @@ // Set the source file to read-only. ASSERT_FALSE(IsReadOnly(src)); - SetReadOnly(src); + SetReadOnly(src, true); ASSERT_TRUE(IsReadOnly(src)); // Copy the file.
diff --git a/logging.h b/logging.h index 54ff579..6a1df76 100644 --- a/logging.h +++ b/logging.h
@@ -701,7 +701,7 @@ #define DCHECK_GT(val1, val2) DCHECK_OP(GT, > , val1, val2) #define DCHECK_IMPLIES(val1, val2) DCHECK(!(val1) || (val2)) -#if defined(NDEBUG) && defined(OS_CHROMEOS) +#if !DCHECK_IS_ON && defined(OS_CHROMEOS) #define NOTREACHED() LOG(ERROR) << "NOTREACHED() hit in " << \ __FUNCTION__ << ". " #else
diff --git a/path_service_unittest.cc b/path_service_unittest.cc index c6cc0e6..543deb6 100644 --- a/path_service_unittest.cc +++ b/path_service_unittest.cc
@@ -98,18 +98,8 @@ #if defined(OS_WIN) for (int key = base::PATH_WIN_START + 1; key < base::PATH_WIN_END; ++key) { bool valid = true; - switch(key) { - case base::DIR_LOCAL_APP_DATA_LOW: - // DIR_LOCAL_APP_DATA_LOW is not supported prior Vista and is expected - // to fail. - valid = base::win::GetVersion() >= base::win::VERSION_VISTA; - break; - case base::DIR_APP_SHORTCUTS: - // DIR_APP_SHORTCUTS is not supported prior Windows 8 and is expected to - // fail. - valid = base::win::GetVersion() >= base::win::VERSION_WIN8; - break; - } + if (key == base::DIR_APP_SHORTCUTS) + valid = base::win::GetVersion() >= base::win::VERSION_WIN8; if (valid) EXPECT_TRUE(ReturnsValidPath(key)) << key;
diff --git a/posix/eintr_wrapper.h b/posix/eintr_wrapper.h index 854c43a..5a5dc75 100644 --- a/posix/eintr_wrapper.h +++ b/posix/eintr_wrapper.h
@@ -25,7 +25,7 @@ #if defined(NDEBUG) #define HANDLE_EINTR(x) ({ \ - typeof(x) eintr_wrapper_result; \ + decltype(x) eintr_wrapper_result; \ do { \ eintr_wrapper_result = (x); \ } while (eintr_wrapper_result == -1 && errno == EINTR); \ @@ -36,7 +36,7 @@ #define HANDLE_EINTR(x) ({ \ int eintr_wrapper_counter = 0; \ - typeof(x) eintr_wrapper_result; \ + decltype(x) eintr_wrapper_result; \ do { \ eintr_wrapper_result = (x); \ } while (eintr_wrapper_result == -1 && errno == EINTR && \ @@ -47,7 +47,7 @@ #endif // NDEBUG #define IGNORE_EINTR(x) ({ \ - typeof(x) eintr_wrapper_result; \ + decltype(x) eintr_wrapper_result; \ do { \ eintr_wrapper_result = (x); \ if (eintr_wrapper_result == -1 && errno == EINTR) { \
diff --git a/profiler/scoped_profile.cc b/profiler/scoped_profile.cc index e1edc97..8b0ae59 100644 --- a/profiler/scoped_profile.cc +++ b/profiler/scoped_profile.cc
@@ -13,20 +13,32 @@ ScopedProfile::ScopedProfile(const Location& location) : birth_(ThreadData::TallyABirthIfActive(location)) { + if (!birth_) + return; + ThreadData::PrepareForStartOfRun(birth_); + stopwatch_.Start(); +} + +ScopedProfile::ScopedProfile(const Location& location, Mode mode) + : birth_(NULL) { + if (mode == DISABLED) + return; + + birth_ = ThreadData::TallyABirthIfActive(location); + if (!birth_) + return; + + ThreadData::PrepareForStartOfRun(birth_); + stopwatch_.Start(); } ScopedProfile::~ScopedProfile() { - StopClockAndTally(); -} - -void ScopedProfile::StopClockAndTally() { - stopwatch_.Stop(); - if (!birth_) return; + + stopwatch_.Stop(); ThreadData::TallyRunInAScopedRegionIfTracking(birth_, stopwatch_); - birth_ = NULL; } } // namespace tracked_objects
diff --git a/profiler/scoped_profile.h b/profiler/scoped_profile.h index 7290908..6a76486 100644 --- a/profiler/scoped_profile.h +++ b/profiler/scoped_profile.h
@@ -35,11 +35,17 @@ class BASE_EXPORT ScopedProfile { public: - explicit ScopedProfile(const Location& location); - ~ScopedProfile(); + // Mode of operation. Specifies whether ScopedProfile should be a no-op or + // needs to create and tally a task. + enum Mode { + DISABLED, // Do nothing. + ENABLED // Create and tally a task. + }; - // Stop tracing prior to the end destruction of the instance. - void StopClockAndTally(); + // TODO(vadimt): Remove this constructor. + explicit ScopedProfile(const Location& location); + ScopedProfile(const Location& location, Mode mode); + ~ScopedProfile(); private: Births* birth_; // Place in code where tracking started.
diff --git a/profiler/scoped_tracker.cc b/profiler/scoped_tracker.cc new file mode 100644 index 0000000..5cd0bca --- /dev/null +++ b/profiler/scoped_tracker.cc
@@ -0,0 +1,24 @@ +// Copyright 2014 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. + +#include "base/profiler/scoped_tracker.h" + +namespace tracked_objects { + +namespace { + +ScopedProfile::Mode g_scoped_profile_mode = ScopedProfile::DISABLED; + +} // namespace + +// static +void ScopedTracker::Enable() { + g_scoped_profile_mode = ScopedProfile::ENABLED; +} + +ScopedTracker::ScopedTracker(const Location& location) + : scoped_profile_(location, g_scoped_profile_mode) { +} + +} // namespace tracked_objects
diff --git a/profiler/scoped_tracker.h b/profiler/scoped_tracker.h new file mode 100644 index 0000000..fbd7309 --- /dev/null +++ b/profiler/scoped_tracker.h
@@ -0,0 +1,37 @@ +// Copyright 2014 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. + +#ifndef BASE_PROFILER_SCOPED_TRACKER_H_ +#define BASE_PROFILER_SCOPED_TRACKER_H_ + +//------------------------------------------------------------------------------ +// Utilities for temporarily instrumenting code to dig into issues that were +// found using profiler data. + +#include "base/base_export.h" +#include "base/location.h" +#include "base/profiler/scoped_profile.h" + +namespace tracked_objects { + +// ScopedTracker instruments a region within the code if the instrumentation is +// enabled. It can be used, for example, to find out if a source of jankiness is +// inside the instrumented code region. +class BASE_EXPORT ScopedTracker { + public: + ScopedTracker(const Location& location); + + // Enables instrumentation for the remainder of the current process' life. If + // this function is not called, all profiler instrumentations are no-ops. + static void Enable(); + + private: + const ScopedProfile scoped_profile_; + + DISALLOW_COPY_AND_ASSIGN(ScopedTracker); +}; + +} // namespace tracked_objects + +#endif // BASE_PROFILER_SCOPED_TRACKER_H_
diff --git a/run_loop.cc b/run_loop.cc index e92e110..2aa4def 100644 --- a/run_loop.cc +++ b/run_loop.cc
@@ -51,6 +51,7 @@ // Use task stopwatch to exclude the loop run time from the current task, if // any. tracked_objects::TaskStopwatch stopwatch; + stopwatch.Start(); loop_->RunHandler(); stopwatch.Stop();
diff --git a/scoped_observer.h b/scoped_observer.h index 3754ed5..5b0d533 100644 --- a/scoped_observer.h +++ b/scoped_observer.h
@@ -9,6 +9,7 @@ #include <vector> #include "base/basictypes.h" +#include "base/logging.h" // ScopedObserver is used to keep track of the set of sources an object has // attached itself to as an observer. When ScopedObserver is destroyed it @@ -30,7 +31,9 @@ // Remove the object passed to the constructor as an observer from |source|. void Remove(Source* source) { - sources_.erase(std::find(sources_.begin(), sources_.end(), source)); + auto it = std::find(sources_.begin(), sources_.end(), source); + DCHECK(it != sources_.end()); + sources_.erase(it); source->RemoveObserver(observer_); }
diff --git a/test/trace_to_file.cc b/test/trace_to_file.cc index 423f65c..e157c85 100644 --- a/test/trace_to_file.cc +++ b/test/trace_to_file.cc
@@ -5,6 +5,7 @@ #include "base/test/trace_to_file.h" #include "base/base_switches.h" +#include "base/bind.h" #include "base/command_line.h" #include "base/debug/trace_event_impl.h" #include "base/files/file_util.h"
diff --git a/third_party/nspr/BUILD.gn b/third_party/nspr/BUILD.gn index ddbcbc1..516ca1f 100644 --- a/third_party/nspr/BUILD.gn +++ b/third_party/nspr/BUILD.gn
@@ -3,7 +3,7 @@ # found in the LICENSE file. source_set("nspr") { - visibility = [ "//base/*" ] + visibility = [ "//base" ] sources = [ "prtime.cc", "prtime.h",
diff --git a/threading/sequenced_worker_pool.cc b/threading/sequenced_worker_pool.cc index 08ef5f0..4c37320 100644 --- a/threading/sequenced_worker_pool.cc +++ b/threading/sequenced_worker_pool.cc
@@ -756,6 +756,7 @@ tracked_objects::ThreadData::PrepareForStartOfRun(task.birth_tally); tracked_objects::TaskStopwatch stopwatch; + stopwatch.Start(); task.task.Run(); stopwatch.Stop();
diff --git a/threading/worker_pool_posix.cc b/threading/worker_pool_posix.cc index f54b7ec..cd3c9dc 100644 --- a/threading/worker_pool_posix.cc +++ b/threading/worker_pool_posix.cc
@@ -97,6 +97,7 @@ tracked_objects::ThreadData::PrepareForStartOfRun(pending_task.birth_tally); tracked_objects::TaskStopwatch stopwatch; + stopwatch.Start(); pending_task.task.Run(); stopwatch.Stop();
diff --git a/threading/worker_pool_win.cc b/threading/worker_pool_win.cc index 8fc7324..ff3cc83 100644 --- a/threading/worker_pool_win.cc +++ b/threading/worker_pool_win.cc
@@ -30,6 +30,7 @@ g_worker_pool_running_on_this_thread.Get().Set(true); tracked_objects::TaskStopwatch stopwatch; + stopwatch.Start(); pending_task->task.Run(); stopwatch.Stop();
diff --git a/tracked_objects.cc b/tracked_objects.cc index 659d421..4fe8851 100644 --- a/tracked_objects.cc +++ b/tracked_objects.cc
@@ -854,16 +854,32 @@ //------------------------------------------------------------------------------ TaskStopwatch::TaskStopwatch() - : start_time_(ThreadData::Now()), - current_thread_data_(ThreadData::Get()), + : wallclock_duration_ms_(0), + current_thread_data_(NULL), excluded_duration_ms_(0), parent_(NULL) { #if DCHECK_IS_ON - state_ = RUNNING; + state_ = CREATED; child_ = NULL; #endif +} - wallclock_duration_ms_ = 0; +TaskStopwatch::~TaskStopwatch() { +#if DCHECK_IS_ON + DCHECK(state_ != RUNNING); + DCHECK(child_ == NULL); +#endif +} + +void TaskStopwatch::Start() { +#if DCHECK_IS_ON + DCHECK(state_ == CREATED); + state_ = RUNNING; +#endif + + start_time_ = ThreadData::Now(); + + current_thread_data_ = ThreadData::Get(); if (!current_thread_data_) return; @@ -878,13 +894,6 @@ current_thread_data_->current_stopwatch_ = this; } -TaskStopwatch::~TaskStopwatch() { -#if DCHECK_IS_ON - DCHECK(state_ != RUNNING); - DCHECK(child_ == NULL); -#endif -} - void TaskStopwatch::Stop() { const TrackedTime end_time = ThreadData::Now(); #if DCHECK_IS_ON @@ -910,12 +919,15 @@ DCHECK(parent_->child_ == this); parent_->child_ = NULL; #endif - parent_->excluded_duration_ms_ += - wallclock_duration_ms_; + parent_->excluded_duration_ms_ += wallclock_duration_ms_; parent_ = NULL; } TrackedTime TaskStopwatch::StartTime() const { +#if DCHECK_IS_ON + DCHECK(state_ != CREATED); +#endif + return start_time_; } @@ -928,6 +940,10 @@ } ThreadData* TaskStopwatch::GetThreadData() const { +#if DCHECK_IS_ON + DCHECK(state_ != CREATED); +#endif + return current_thread_data_; }
diff --git a/tracked_objects.h b/tracked_objects.h index 055cf19..222f581 100644 --- a/tracked_objects.h +++ b/tracked_objects.h
@@ -709,6 +709,9 @@ TaskStopwatch(); ~TaskStopwatch(); + // Starts stopwatch. + void Start(); + // Stops stopwatch. void Stop(); @@ -744,12 +747,9 @@ TaskStopwatch* parent_; #if DCHECK_IS_ON - // State of the stopwatch. Stopwatch is first constructed in a running state, - // then stopped, then destructed. - enum { - RUNNING, - STOPPED - } state_; + // State of the stopwatch. Stopwatch is first constructed in a created state + // state, then is optionally started/stopped, then destructed. + enum { CREATED, RUNNING, STOPPED } state_; // Currently running stopwatch that is directly nested in this one, if such // stopwatch exists. NULL otherwise.
diff --git a/tracked_objects_unittest.cc b/tracked_objects_unittest.cc index 3ca7d74..f19ba7b 100644 --- a/tracked_objects_unittest.cc +++ b/tracked_objects_unittest.cc
@@ -111,6 +111,16 @@ // static unsigned int TrackedObjectsTest::test_time_; +TEST_F(TrackedObjectsTest, TaskStopwatchNoStartStop) { + if (!ThreadData::InitializeAndSetTrackingStatus( + ThreadData::PROFILING_CHILDREN_ACTIVE)) { + return; + } + + // Check that creating and destroying a stopwatch without starting it doesn't + // crash. + TaskStopwatch stopwatch; +} TEST_F(TrackedObjectsTest, MinimalStartupShutdown) { // Minimal test doesn't even create any tasks. @@ -190,6 +200,7 @@ base::TrackingInfo pending_task(location, kBogusBirthTime); SetTestTime(1); TaskStopwatch stopwatch; + stopwatch.Start(); // Finally conclude the outer run. const int32 time_elapsed = 1000; SetTestTime(start_time + time_elapsed); @@ -382,6 +393,7 @@ const unsigned int kEndOfRun = 7; SetTestTime(kStartOfRun); TaskStopwatch stopwatch; + stopwatch.Start(); SetTestTime(kEndOfRun); stopwatch.Stop(); @@ -422,6 +434,7 @@ const unsigned int kEndOfRun = 7; SetTestTime(kStartOfRun); TaskStopwatch stopwatch; + stopwatch.Start(); SetTestTime(kEndOfRun); stopwatch.Stop(); @@ -456,6 +469,7 @@ const unsigned int kEndOfRun = 7; SetTestTime(kStartOfRun); TaskStopwatch stopwatch; + stopwatch.Start(); SetTestTime(kEndOfRun); stopwatch.Stop(); @@ -485,6 +499,7 @@ const unsigned int kEndOfRun = 7; SetTestTime(kStartOfRun); TaskStopwatch stopwatch; + stopwatch.Start(); SetTestTime(kEndOfRun); stopwatch.Stop(); @@ -552,6 +567,7 @@ const unsigned int kEndOfRun = 7; SetTestTime(kStartOfRun); TaskStopwatch stopwatch; + stopwatch.Start(); SetTestTime(kEndOfRun); stopwatch.Stop(); @@ -562,6 +578,7 @@ pending_task2.time_posted = kTimePosted; // Overwrite implied Now(). SetTestTime(kStartOfRun); TaskStopwatch stopwatch2; + stopwatch2.Start(); SetTestTime(kEndOfRun); stopwatch2.Stop(); @@ -595,6 +612,7 @@ const unsigned int kEndOfRun = 7; SetTestTime(kStartOfRun); TaskStopwatch stopwatch; + stopwatch.Start(); SetTestTime(kEndOfRun); stopwatch.Stop(); @@ -659,9 +677,11 @@ SetTestTime(5); TaskStopwatch task_stopwatch; + task_stopwatch.Start(); { SetTestTime(8); TaskStopwatch exclusion_stopwatch; + exclusion_stopwatch.Start(); SetTestTime(12); exclusion_stopwatch.Stop(); } @@ -695,14 +715,17 @@ SetTestTime(5); TaskStopwatch task_stopwatch; + task_stopwatch.Start(); { SetTestTime(8); TaskStopwatch exclusion_stopwatch; + exclusion_stopwatch.Start(); SetTestTime(12); exclusion_stopwatch.Stop(); SetTestTime(15); TaskStopwatch exclusion_stopwatch2; + exclusion_stopwatch2.Start(); SetTestTime(18); exclusion_stopwatch2.Stop(); } @@ -739,9 +762,11 @@ SetTestTime(5); TaskStopwatch task_stopwatch; + task_stopwatch.Start(); { SetTestTime(8); TaskStopwatch exclusion_stopwatch; + exclusion_stopwatch.Start(); { Location second_location(kFunction, kFile, kSecondFakeLineNumber, NULL); base::TrackingInfo nested_task(second_location, kDelayedStartTime); @@ -750,6 +775,7 @@ base::TimeTicks() + base::TimeDelta::FromMilliseconds(8); SetTestTime(9); TaskStopwatch nested_task_stopwatch; + nested_task_stopwatch.Start(); SetTestTime(11); nested_task_stopwatch.Stop(); ThreadData::TallyRunOnNamedThreadIfTracking(