Update from https://crrev.com/329939 Includes an update to build_v8.patch. https://crrev.com/329939 includes mac->android cross compilation fixes. TBR=cstout@chromium.org Review URL: https://codereview.chromium.org/1141793003
diff --git a/BUILD.gn b/BUILD.gn index e7985ef..2170520 100644 --- a/BUILD.gn +++ b/BUILD.gn
@@ -216,6 +216,8 @@ "files/file_posix.cc", "files/file_proxy.cc", "files/file_proxy.h", + "files/file_tracing.cc", + "files/file_tracing.h", "files/file_util.cc", "files/file_util.h", "files/file_util_android.cc", @@ -269,6 +271,8 @@ "mac/bundle_locations.h", "mac/bundle_locations.mm", "mac/cocoa_protocols.h", + "mac/dispatch_source_mach.cc", + "mac/dispatch_source_mach.h", "mac/foundation_util.h", "mac/foundation_util.mm", "mac/launch_services_util.cc", @@ -599,6 +603,8 @@ "win/iat_patch_function.h", "win/iunknown_impl.cc", "win/iunknown_impl.h", + "win/memory_pressure_monitor.cc", + "win/memory_pressure_monitor.h", "win/message_window.cc", "win/message_window.h", "win/metro.cc", @@ -1146,6 +1152,7 @@ "lazy_instance_unittest.cc", "logging_unittest.cc", "mac/bind_objc_block_unittest.mm", + "mac/dispatch_source_mach_unittest.cc", "mac/foundation_util_unittest.mm", "mac/libdispatch_task_runner_unittest.cc", "mac/mac_util_unittest.mm", @@ -1282,6 +1289,7 @@ "win/event_trace_provider_unittest.cc", "win/i18n_unittest.cc", "win/iunknown_impl_unittest.cc", + "win/memory_pressure_monitor_unittest.cc", "win/message_window_unittest.cc", "win/object_watcher_unittest.cc", "win/pe_image_unittest.cc",
diff --git a/android/build_info.cc b/android/build_info.cc index 11202a0..4d3cd55 100644 --- a/android/build_info.cc +++ b/android/build_info.cc
@@ -68,11 +68,16 @@ return Singleton<BuildInfo, BuildInfoSingletonTraits >::get(); } -void BuildInfo::set_java_exception_info(const std::string& info) { +void BuildInfo::SetJavaExceptionInfo(const std::string& info) { DCHECK(!java_exception_info_) << "info should be set only once."; java_exception_info_ = strndup(info.c_str(), 4096); } +void BuildInfo::ClearJavaExceptionInfo() { + delete java_exception_info_; + java_exception_info_ = nullptr; +} + // static bool BuildInfo::RegisterBindings(JNIEnv* env) { return RegisterNativesImpl(env);
diff --git a/android/build_info.h b/android/build_info.h index 9d73bdb..d6155b9 100644 --- a/android/build_info.h +++ b/android/build_info.h
@@ -99,7 +99,9 @@ return java_exception_info_; } - void set_java_exception_info(const std::string& info); + void SetJavaExceptionInfo(const std::string& info); + + void ClearJavaExceptionInfo(); static bool RegisterBindings(JNIEnv* env);
diff --git a/android/java/src/org/chromium/base/ApiCompatibilityUtils.java b/android/java/src/org/chromium/base/ApiCompatibilityUtils.java index 3df7b92..76d7396 100644 --- a/android/java/src/org/chromium/base/ApiCompatibilityUtils.java +++ b/android/java/src/org/chromium/base/ApiCompatibilityUtils.java
@@ -349,6 +349,7 @@ /** * @see android.view.Window#setStatusBarColor(int color). + * TODO(ianwen): remove this method after downstream rolling. */ public static void setStatusBarColor(Activity activity, int statusBarColor) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { @@ -365,6 +366,22 @@ } /** + * @see android.view.Window#setStatusBarColor(int color). + */ + public static void setStatusBarColor(Window window, int statusBarColor) { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { + // If both system bars are black, we can remove these from our layout, + // removing or shrinking the SurfaceFlinger overlay required for our views. + if (statusBarColor == Color.BLACK && window.getNavigationBarColor() == Color.BLACK) { + window.clearFlags(WindowManager.LayoutParams.FLAG_DRAWS_SYSTEM_BAR_BACKGROUNDS); + } else { + window.addFlags(WindowManager.LayoutParams.FLAG_DRAWS_SYSTEM_BAR_BACKGROUNDS); + } + window.setStatusBarColor(statusBarColor); + } + } + + /** * @see android.content.res.Resources#getDrawable(int id). */ @SuppressWarnings("deprecation")
diff --git a/android/java/src/org/chromium/base/PathUtils.java b/android/java/src/org/chromium/base/PathUtils.java index 0b3ed20..e46fc30 100644 --- a/android/java/src/org/chromium/base/PathUtils.java +++ b/android/java/src/org/chromium/base/PathUtils.java
@@ -47,7 +47,7 @@ } return paths; } - }.executeOnExecutor(AsyncTask.SERIAL_EXECUTOR, suffix); + }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR, suffix); } /**
diff --git a/android/java_runtime.h b/android/java_runtime.h index bde4c5c..4ca889e 100644 --- a/android/java_runtime.h +++ b/android/java_runtime.h
@@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef BASE_ANDROID_JAVA_RUNTIME_H -#define BASE_ANDROID_JAVA_RUNTIME_H +#ifndef BASE_ANDROID_JAVA_RUNTIME_H_ +#define BASE_ANDROID_JAVA_RUNTIME_H_ #include "base/android/scoped_java_ref.h" #include "base/base_export.h" @@ -25,4 +25,4 @@ } // namespace android } // namespace base -#endif // BASE_ANDROID_JAVA_RUNTIME_H +#endif // BASE_ANDROID_JAVA_RUNTIME_H_
diff --git a/android/jni_android.cc b/android/jni_android.cc index 1b715dc..5416be3 100644 --- a/android/jni_android.cc +++ b/android/jni_android.cc
@@ -28,52 +28,6 @@ g_class_loader = LAZY_INSTANCE_INITIALIZER; jmethodID g_class_loader_load_class_method_id = 0; -std::string GetJavaExceptionInfo(JNIEnv* env, jthrowable java_throwable) { - ScopedJavaLocalRef<jclass> throwable_clazz = - GetClass(env, "java/lang/Throwable"); - jmethodID throwable_printstacktrace = - MethodID::Get<MethodID::TYPE_INSTANCE>( - env, throwable_clazz.obj(), "printStackTrace", - "(Ljava/io/PrintStream;)V"); - - // Create an instance of ByteArrayOutputStream. - ScopedJavaLocalRef<jclass> bytearray_output_stream_clazz = - GetClass(env, "java/io/ByteArrayOutputStream"); - jmethodID bytearray_output_stream_constructor = - MethodID::Get<MethodID::TYPE_INSTANCE>( - env, bytearray_output_stream_clazz.obj(), "<init>", "()V"); - jmethodID bytearray_output_stream_tostring = - MethodID::Get<MethodID::TYPE_INSTANCE>( - env, bytearray_output_stream_clazz.obj(), "toString", - "()Ljava/lang/String;"); - ScopedJavaLocalRef<jobject> bytearray_output_stream(env, - env->NewObject(bytearray_output_stream_clazz.obj(), - bytearray_output_stream_constructor)); - - // Create an instance of PrintStream. - ScopedJavaLocalRef<jclass> printstream_clazz = - GetClass(env, "java/io/PrintStream"); - jmethodID printstream_constructor = - MethodID::Get<MethodID::TYPE_INSTANCE>( - env, printstream_clazz.obj(), "<init>", - "(Ljava/io/OutputStream;)V"); - ScopedJavaLocalRef<jobject> printstream(env, - env->NewObject(printstream_clazz.obj(), printstream_constructor, - bytearray_output_stream.obj())); - - // Call Throwable.printStackTrace(PrintStream) - env->CallVoidMethod(java_throwable, throwable_printstacktrace, - printstream.obj()); - - // Call ByteArrayOutputStream.toString() - ScopedJavaLocalRef<jstring> exception_string( - env, static_cast<jstring>( - env->CallObjectMethod(bytearray_output_stream.obj(), - bytearray_output_stream_tostring))); - - return ConvertJavaStringToUTF8(exception_string); -} - } // namespace namespace base { @@ -287,7 +241,7 @@ // Set the exception_string in BuildInfo so that breakpad can read it. // RVO should avoid any extra copies of the exception string. - base::android::BuildInfo::GetInstance()->set_java_exception_info( + base::android::BuildInfo::GetInstance()->SetJavaExceptionInfo( GetJavaExceptionInfo(env, java_throwable)); } @@ -295,5 +249,52 @@ CHECK(false) << "Please include Java exception stack in crash report"; } +std::string GetJavaExceptionInfo(JNIEnv* env, jthrowable java_throwable) { + ScopedJavaLocalRef<jclass> throwable_clazz = + GetClass(env, "java/lang/Throwable"); + jmethodID throwable_printstacktrace = + MethodID::Get<MethodID::TYPE_INSTANCE>( + env, throwable_clazz.obj(), "printStackTrace", + "(Ljava/io/PrintStream;)V"); + + // Create an instance of ByteArrayOutputStream. + ScopedJavaLocalRef<jclass> bytearray_output_stream_clazz = + GetClass(env, "java/io/ByteArrayOutputStream"); + jmethodID bytearray_output_stream_constructor = + MethodID::Get<MethodID::TYPE_INSTANCE>( + env, bytearray_output_stream_clazz.obj(), "<init>", "()V"); + jmethodID bytearray_output_stream_tostring = + MethodID::Get<MethodID::TYPE_INSTANCE>( + env, bytearray_output_stream_clazz.obj(), "toString", + "()Ljava/lang/String;"); + ScopedJavaLocalRef<jobject> bytearray_output_stream(env, + env->NewObject(bytearray_output_stream_clazz.obj(), + bytearray_output_stream_constructor)); + + // Create an instance of PrintStream. + ScopedJavaLocalRef<jclass> printstream_clazz = + GetClass(env, "java/io/PrintStream"); + jmethodID printstream_constructor = + MethodID::Get<MethodID::TYPE_INSTANCE>( + env, printstream_clazz.obj(), "<init>", + "(Ljava/io/OutputStream;)V"); + ScopedJavaLocalRef<jobject> printstream(env, + env->NewObject(printstream_clazz.obj(), printstream_constructor, + bytearray_output_stream.obj())); + + // Call Throwable.printStackTrace(PrintStream) + env->CallVoidMethod(java_throwable, throwable_printstacktrace, + printstream.obj()); + + // Call ByteArrayOutputStream.toString() + ScopedJavaLocalRef<jstring> exception_string( + env, static_cast<jstring>( + env->CallObjectMethod(bytearray_output_stream.obj(), + bytearray_output_stream_tostring))); + + return ConvertJavaStringToUTF8(exception_string); +} + + } // namespace android } // namespace base
diff --git a/android/jni_android.h b/android/jni_android.h index 504eb85..9df9041 100644 --- a/android/jni_android.h +++ b/android/jni_android.h
@@ -130,6 +130,10 @@ // This function will call CHECK() macro if there's any pending exception. BASE_EXPORT void CheckException(JNIEnv* env); +// This returns a string representation of the java stack trace. +BASE_EXPORT std::string GetJavaExceptionInfo(JNIEnv* env, + jthrowable java_throwable); + } // namespace android } // namespace base
diff --git a/barrier_closure.cc b/barrier_closure.cc index 1b77429..1dcc502 100644 --- a/barrier_closure.cc +++ b/barrier_closure.cc
@@ -28,8 +28,9 @@ void BarrierInfo::Run() { DCHECK(!base::AtomicRefCountIsZero(&num_callbacks_left_)); if (!base::AtomicRefCountDec(&num_callbacks_left_)) { - done_closure_.Run(); + base::Closure done_closure = done_closure_; done_closure_.Reset(); + done_closure.Run(); } }
diff --git a/barrier_closure_unittest.cc b/barrier_closure_unittest.cc index ab05cb8..dcea09f 100644 --- a/barrier_closure_unittest.cc +++ b/barrier_closure_unittest.cc
@@ -13,24 +13,68 @@ TEST(BarrierClosureTest, RunImmediatelyForZeroClosures) { int count = 0; - base::Closure doneClosure(base::Bind(&Increment, base::Unretained(&count))); + base::Closure done_closure(base::Bind(&Increment, base::Unretained(&count))); - base::Closure barrierClosure = base::BarrierClosure(0, doneClosure); + base::Closure barrier_closure = base::BarrierClosure(0, done_closure); EXPECT_EQ(1, count); } TEST(BarrierClosureTest, RunAfterNumClosures) { int count = 0; - base::Closure doneClosure(base::Bind(&Increment, base::Unretained(&count))); + base::Closure done_closure(base::Bind(&Increment, base::Unretained(&count))); - base::Closure barrierClosure = base::BarrierClosure(2, doneClosure); + base::Closure barrier_closure = base::BarrierClosure(2, done_closure); EXPECT_EQ(0, count); - barrierClosure.Run(); + barrier_closure.Run(); EXPECT_EQ(0, count); - barrierClosure.Run(); + barrier_closure.Run(); EXPECT_EQ(1, count); } +class DestructionIndicator { + public: + // Sets |*destructed| to true in destructor. + DestructionIndicator(bool* destructed) : destructed_(destructed) { + *destructed_ = false; + } + + ~DestructionIndicator() { *destructed_ = true; } + + void DoNothing() {} + + private: + bool* destructed_; +}; + +TEST(BarrierClosureTest, ReleasesDoneClosureWhenDone) { + bool done_destructed = false; + base::Closure barrier_closure = base::BarrierClosure( + 1, base::Bind(&DestructionIndicator::DoNothing, + base::Owned(new DestructionIndicator(&done_destructed)))); + EXPECT_FALSE(done_destructed); + barrier_closure.Run(); + EXPECT_TRUE(done_destructed); +} + +void ResetBarrierClosure(base::Closure* closure) { + *closure = base::Closure(); +} + +// Tests a case when |done_closure| resets a |barrier_closure|. +// |barrier_closure| is a Closure holding the |done_closure|. |done_closure| +// holds a pointer back to the |barrier_closure|. When |barrier_closure| is +// Run() it calls ResetBarrierClosure() which erases the |barrier_closure| while +// still inside of its Run(). The Run() implementation (in base::BarrierClosure) +// must not try use itself after executing ResetBarrierClosure() or this test +// would crash inside Run(). +TEST(BarrierClosureTest, KeepingClosureAliveUntilDone) { + base::Closure barrier_closure; + base::Closure done_closure = + base::Bind(ResetBarrierClosure, &barrier_closure); + barrier_closure = base::BarrierClosure(1, done_closure); + barrier_closure.Run(); +} + } // namespace
diff --git a/base.gyp b/base.gyp index 80c3f10..05ff45e 100644 --- a/base.gyp +++ b/base.gyp
@@ -514,6 +514,7 @@ 'lazy_instance_unittest.cc', 'logging_unittest.cc', 'mac/bind_objc_block_unittest.mm', + 'mac/dispatch_source_mach_unittest.cc', 'mac/foundation_util_unittest.mm', 'mac/libdispatch_task_runner_unittest.cc', 'mac/mac_util_unittest.mm', @@ -654,6 +655,7 @@ 'win/event_trace_provider_unittest.cc', 'win/i18n_unittest.cc', 'win/iunknown_impl_unittest.cc', + 'win/memory_pressure_monitor_unittest.cc', 'win/message_window_unittest.cc', 'win/object_watcher_unittest.cc', 'win/pe_image_unittest.cc',
diff --git a/base.gypi b/base.gypi index 4a8cd17..05ad049 100644 --- a/base.gypi +++ b/base.gypi
@@ -209,6 +209,8 @@ 'files/file_posix.cc', 'files/file_proxy.cc', 'files/file_proxy.h', + 'files/file_tracing.cc', + 'files/file_tracing.h', 'files/file_util.cc', 'files/file_util.h', 'files/file_util_android.cc', @@ -278,6 +280,8 @@ 'mac/bundle_locations.mm', 'mac/close_nocancel.cc', 'mac/cocoa_protocols.h', + 'mac/dispatch_source_mach.cc', + 'mac/dispatch_source_mach.h', 'mac/foundation_util.h', 'mac/foundation_util.mm', 'mac/launch_services_util.cc', @@ -705,6 +709,8 @@ 'win/iat_patch_function.h', 'win/iunknown_impl.cc', 'win/iunknown_impl.h', + 'win/memory_pressure_monitor.cc', + 'win/memory_pressure_monitor.h', 'win/message_window.cc', 'win/message_window.h', 'win/metro.cc',
diff --git a/base64.h b/base64.h index def9b67..dd72c39 100644 --- a/base64.h +++ b/base64.h
@@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef BASE_BASE64_H__ -#define BASE_BASE64_H__ +#ifndef BASE_BASE64_H_ +#define BASE_BASE64_H_ #include <string> @@ -22,4 +22,4 @@ } // namespace base -#endif // BASE_BASE64_H__ +#endif // BASE_BASE64_H_
diff --git a/base_paths_win.h b/base_paths_win.h index 4ab6af1..9ac9e45 100644 --- a/base_paths_win.h +++ b/base_paths_win.h
@@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef BASE_BASE_PATHS_WIN_H__ -#define BASE_BASE_PATHS_WIN_H__ +#ifndef BASE_BASE_PATHS_WIN_H_ +#define BASE_BASE_PATHS_WIN_H_ // This file declares windows-specific path keys for the base module. // These can be used with the PathService to access various special @@ -51,4 +51,4 @@ } // namespace base -#endif // BASE_BASE_PATHS_WIN_H__ +#endif // BASE_BASE_PATHS_WIN_H_
diff --git a/bind_internal.h b/bind_internal.h index 5fc1459..e053218 100644 --- a/bind_internal.h +++ b/bind_internal.h
@@ -364,9 +364,10 @@ struct BindState; template <typename Runnable, - typename R, typename... Args, + typename R, + typename... Args, typename... BoundArgs> -struct BindState<Runnable, R(Args...), TypeList<BoundArgs...>> +struct BindState<Runnable, R(Args...), TypeList<BoundArgs...>> final : public BindStateBase { private: using StorageType = BindState<Runnable, R(Args...), TypeList<BoundArgs...>>; @@ -398,14 +399,21 @@ using UnboundRunType = MakeFunctionType<R, UnboundArgs>; BindState(const Runnable& runnable, const BoundArgs&... bound_args) - : runnable_(runnable), ref_(bound_args...), bound_args_(bound_args...) {} + : BindStateBase(&Destroy), + runnable_(runnable), + ref_(bound_args...), + bound_args_(bound_args...) {} RunnableType runnable_; MaybeScopedRefPtr<HasIsMethodTag<Runnable>::value, BoundArgs...> ref_; Tuple<BoundArgs...> bound_args_; private: - ~BindState() override {} + ~BindState() {} + + static void Destroy(BindStateBase* self) { + delete static_cast<BindState*>(self); + } }; } // namespace internal
diff --git a/callback_internal.cc b/callback_internal.cc index f360388..2553fe7 100644 --- a/callback_internal.cc +++ b/callback_internal.cc
@@ -9,6 +9,15 @@ namespace base { namespace internal { +void BindStateBase::AddRef() { + AtomicRefCountInc(&ref_count_); +} + +void BindStateBase::Release() { + if (!AtomicRefCountDec(&ref_count_)) + destructor_(this); +} + CallbackBase::CallbackBase(const CallbackBase& c) = default; CallbackBase& CallbackBase::operator=(const CallbackBase& c) = default; @@ -27,7 +36,7 @@ CallbackBase::CallbackBase(BindStateBase* bind_state) : bind_state_(bind_state), polymorphic_invoke_(NULL) { - DCHECK(!bind_state_.get() || bind_state_->HasOneRef()); + DCHECK(!bind_state_.get() || bind_state_->ref_count_ == 1); } CallbackBase::~CallbackBase() {
diff --git a/callback_internal.h b/callback_internal.h index 8a5c437..fefd7a2 100644 --- a/callback_internal.h +++ b/callback_internal.h
@@ -10,15 +10,19 @@ #include <stddef.h> +#include "base/atomic_ref_count.h" #include "base/base_export.h" +#include "base/macros.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" +#include "base/template_util.h" template <typename T> class ScopedVector; namespace base { namespace internal { +class CallbackBase; // BindStateBase is used to provide an opaque handle that the Callback // class can use to represent a function object with bound arguments. It @@ -26,10 +30,30 @@ // DoInvoke function to perform the function execution. This allows // us to shield the Callback class from the types of the bound argument via // "type erasure." -class BindStateBase : public RefCountedThreadSafe<BindStateBase> { +// At the base level, the only task is to add reference counting data. Don't use +// RefCountedThreadSafe since it requires the destructor to be a virtual method. +// Creating a vtable for every BindState template instantiation results in a lot +// of bloat. Its only task is to call the destructor which can be done with a +// function pointer. +class BindStateBase { protected: - friend class RefCountedThreadSafe<BindStateBase>; - virtual ~BindStateBase() {} + explicit BindStateBase(void (*destructor)(BindStateBase*)) + : ref_count_(0), destructor_(destructor) {} + ~BindStateBase() = default; + + private: + friend class scoped_refptr<BindStateBase>; + friend class CallbackBase; + + void AddRef(); + void Release(); + + AtomicRefCount ref_count_; + + // Pointer to a function that will properly destroy |this|. + void (*destructor_)(BindStateBase*); + + DISALLOW_COPY_AND_ASSIGN(BindStateBase); }; // Holds the Callback methods that don't require specialization to reduce
diff --git a/callback_unittest.cc b/callback_unittest.cc index 5644b85..2844aa9 100644 --- a/callback_unittest.cc +++ b/callback_unittest.cc
@@ -35,9 +35,13 @@ struct BindState<void(void), void(void), void(FakeInvoker)> : public BindStateBase { public: + BindState() : BindStateBase(&Destroy) {} typedef FakeInvoker InvokerType; private: - ~BindState() override {} + ~BindState() {} + static void Destroy(BindStateBase* self) { + delete static_cast<BindState*>(self); + } }; template <> @@ -45,9 +49,13 @@ void(FakeInvoker, FakeInvoker)> : public BindStateBase { public: + BindState() : BindStateBase(&Destroy) {} typedef FakeInvoker InvokerType; private: - ~BindState() override {} + ~BindState() {} + static void Destroy(BindStateBase* self) { + delete static_cast<BindState*>(self); + } }; } // namespace internal
diff --git a/debug/profiler.h b/debug/profiler.h index c50555e..2920d8a 100644 --- a/debug/profiler.h +++ b/debug/profiler.h
@@ -87,4 +87,4 @@ } // namespace debug } // namespace base -#endif // BASE_DEBUG_PROFILER_H__ +#endif // BASE_DEBUG_PROFILER_H_
diff --git a/file_version_info.h b/file_version_info.h index e18ba13..57b837c 100644 --- a/file_version_info.h +++ b/file_version_info.h
@@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef BASE_FILE_VERSION_INFO_H__ -#define BASE_FILE_VERSION_INFO_H__ +#ifndef BASE_FILE_VERSION_INFO_H_ +#define BASE_FILE_VERSION_INFO_H_ #include "build/build_config.h" @@ -32,22 +32,21 @@ // version returns values from the Info.plist as appropriate. TODO(avi): make // this a less-obvious Windows-ism. -class FileVersionInfo { +class BASE_EXPORT FileVersionInfo { public: virtual ~FileVersionInfo() {} #if defined(OS_WIN) || defined(OS_MACOSX) // Creates a FileVersionInfo for the specified path. Returns NULL if something // goes wrong (typically the file does not exit or cannot be opened). The // returned object should be deleted when you are done with it. - BASE_EXPORT static FileVersionInfo* CreateFileVersionInfo( + static FileVersionInfo* CreateFileVersionInfo( const base::FilePath& file_path); #endif // OS_WIN || OS_MACOSX #if defined(OS_WIN) // Creates a FileVersionInfo for the specified module. Returns NULL in case // of error. The returned object should be deleted when you are done with it. - BASE_EXPORT static FileVersionInfo* CreateFileVersionInfoForModule( - HMODULE module); + static FileVersionInfo* CreateFileVersionInfoForModule(HMODULE module); // Creates a FileVersionInfo for the current module. Returns NULL in case // of error. The returned object should be deleted when you are done with it. @@ -61,7 +60,7 @@ #else // Creates a FileVersionInfo for the current module. Returns NULL in case // of error. The returned object should be deleted when you are done with it. - BASE_EXPORT static FileVersionInfo* CreateFileVersionInfoForCurrentModule(); + static FileVersionInfo* CreateFileVersionInfoForCurrentModule(); #endif // OS_WIN // Accessors to the different version properties. @@ -84,4 +83,4 @@ virtual bool is_official_build() = 0; }; -#endif // BASE_FILE_VERSION_INFO_H__ +#endif // BASE_FILE_VERSION_INFO_H_
diff --git a/file_version_info_win.h b/file_version_info_win.h index 92d0844..09d8d37 100644 --- a/file_version_info_win.h +++ b/file_version_info_win.h
@@ -15,10 +15,10 @@ struct tagVS_FIXEDFILEINFO; typedef tagVS_FIXEDFILEINFO VS_FIXEDFILEINFO; -class FileVersionInfoWin : public FileVersionInfo { +class BASE_EXPORT FileVersionInfoWin : public FileVersionInfo { public: - BASE_EXPORT FileVersionInfoWin(void* data, WORD language, WORD code_page); - BASE_EXPORT ~FileVersionInfoWin() override; + FileVersionInfoWin(void* data, WORD language, WORD code_page); + ~FileVersionInfoWin() override; // Accessors to the different version properties. // Returns an empty string if the property is not found. @@ -40,11 +40,11 @@ bool is_official_build() override; // Lets you access other properties not covered above. - BASE_EXPORT bool GetValue(const wchar_t* name, std::wstring* value); + bool GetValue(const wchar_t* name, std::wstring* value); // Similar to GetValue but returns a wstring (empty string if the property // does not exist). - BASE_EXPORT std::wstring GetStringValue(const wchar_t* name); + std::wstring GetStringValue(const wchar_t* name); // Get the fixed file info if it exists. Otherwise NULL VS_FIXEDFILEINFO* fixed_file_info() { return fixed_file_info_; }
diff --git a/files/dir_reader_linux.h b/files/dir_reader_linux.h index cb0cbd3..abf2595 100644 --- a/files/dir_reader_linux.h +++ b/files/dir_reader_linux.h
@@ -95,4 +95,4 @@ } // namespace base -#endif // BASE_FILES_DIR_READER_LINUX_H_ +#endif // BASE_FILES_DIR_READER_LINUX_H_
diff --git a/files/dir_reader_posix.h b/files/dir_reader_posix.h index 6a20ced..6a32d9f 100644 --- a/files/dir_reader_posix.h +++ b/files/dir_reader_posix.h
@@ -33,4 +33,4 @@ } // namespace base -#endif // BASE_FILES_DIR_READER_POSIX_H_ +#endif // BASE_FILES_DIR_READER_POSIX_H_
diff --git a/files/file.cc b/files/file.cc index 8030bf1..58f80c5 100644 --- a/files/file.cc +++ b/files/file.cc
@@ -4,6 +4,7 @@ #include "base/files/file.h" #include "base/files/file_path.h" +#include "base/files/file_tracing.h" #include "base/metrics/histogram.h" #include "base/timer/elapsed_timer.h" @@ -25,11 +26,11 @@ } #if !defined(OS_NACL) -File::File(const FilePath& name, uint32 flags) +File::File(const FilePath& path, uint32 flags) : error_details_(FILE_OK), created_(false), async_(false) { - Initialize(name, flags); + Initialize(path, flags); } #endif @@ -51,6 +52,7 @@ File::File(RValue other) : file_(other.object->TakePlatformFile()), + path_(other.object->path_), error_details_(other.object->error_details()), created_(other.object->created()), async_(other.object->async_) { @@ -65,6 +67,7 @@ if (this != other.object) { Close(); SetPlatformFile(other.object->TakePlatformFile()); + path_ = other.object->path_; error_details_ = other.object->error_details(); created_ = other.object->created(); async_ = other.object->async_; @@ -73,12 +76,14 @@ } #if !defined(OS_NACL) -void File::Initialize(const FilePath& name, uint32 flags) { - if (name.ReferencesParent()) { +void File::Initialize(const FilePath& path, uint32 flags) { + if (path.ReferencesParent()) { error_details_ = FILE_ERROR_ACCESS_DENIED; return; } - DoInitialize(name, flags); + path_ = path; + SCOPED_FILE_TRACE("Initialize"); + DoInitialize(flags); } #endif @@ -128,6 +133,7 @@ bool File::Flush() { ElapsedTimer timer; + SCOPED_FILE_TRACE("Flush"); bool return_value = DoFlush(); UMA_HISTOGRAM_TIMES("PlatformFile.FlushTime", timer.Elapsed()); return return_value;
diff --git a/files/file.h b/files/file.h index 89077b4..b21b159 100644 --- a/files/file.h +++ b/files/file.h
@@ -18,6 +18,8 @@ #include "base/base_export.h" #include "base/basictypes.h" +#include "base/files/file_path.h" +#include "base/files/file_tracing.h" #include "base/files/scoped_file.h" #include "base/gtest_prod_util.h" #include "base/move.h" @@ -31,8 +33,6 @@ namespace base { -class FilePath; - #if defined(OS_WIN) typedef HANDLE PlatformFile; #elif defined(OS_POSIX) @@ -162,8 +162,8 @@ File(); // Creates or opens the given file. This will fail with 'access denied' if the - // |name| contains path traversal ('..') components. - File(const FilePath& name, uint32 flags); + // |path| contains path traversal ('..') components. + File(const FilePath& path, uint32 flags); // Takes ownership of |platform_file|. explicit File(PlatformFile platform_file); @@ -180,7 +180,7 @@ File& operator=(RValue other); // Creates or opens the given file. - void Initialize(const FilePath& name, uint32 flags); + void Initialize(const FilePath& path, uint32 flags); bool IsValid() const; @@ -191,7 +191,7 @@ // Returns the OS result of opening this file. Note that the way to verify // the success of the operation is to use IsValid(), not this method: - // File file(name, flags); + // File file(path, flags); // if (!file.IsValid()) // return; Error error_details() const { return error_details_; } @@ -305,6 +305,8 @@ private: FRIEND_TEST_ALL_PREFIXES(::FileTest, MemoryCorruption); + friend class FileTracing::ScopedTrace; + #if defined(OS_POSIX) // Encloses a single ScopedFD, saving a cheap tamper resistent memory checksum // alongside it. This checksum is validated at every access, allowing early @@ -350,9 +352,9 @@ }; #endif - // Creates or opens the given file. Only called if |name| has no traversal - // ('..') components. - void DoInitialize(const FilePath& name, uint32 flags); + // Creates or opens the given file. Only called if |path_| has no + // traversal ('..') components. + void DoInitialize(uint32 flags); // TODO(tnagel): Reintegrate into Flush() once histogram isn't needed anymore, // cf. issue 473337. @@ -366,6 +368,12 @@ MemoryCheckingScopedFD file_; #endif + // Path that |Initialize()| was called with. Only set if safe (i.e. no '..'). + FilePath path_; + + // Object tied to the lifetime of |this| that enables/disables tracing. + FileTracing::ScopedEnabler trace_enabler_; + Error error_details_; bool created_; bool async_;
diff --git a/files/file_posix.cc b/files/file_posix.cc index 4c79057..bb49d2d 100644 --- a/files/file_posix.cc +++ b/files/file_posix.cc
@@ -9,7 +9,6 @@ #include <sys/stat.h> #include <unistd.h> -#include "base/files/file_path.h" #include "base/logging.h" #include "base/metrics/sparse_histogram.h" #include "base/posix/eintr_wrapper.h" @@ -173,6 +172,7 @@ if (!IsValid()) return; + SCOPED_FILE_TRACE("Close"); ThreadRestrictions::AssertIOAllowed(); file_.reset(); } @@ -181,6 +181,8 @@ ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); + SCOPED_FILE_TRACE_WITH_SIZE("Seek", offset); + #if defined(OS_ANDROID) COMPILE_ASSERT(sizeof(int64) == sizeof(off64_t), off64_t_64_bit); return lseek64(file_.get(), static_cast<off64_t>(offset), @@ -198,6 +200,8 @@ if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("Read", size); + int bytes_read = 0; int rv; do { @@ -218,6 +222,8 @@ if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("ReadAtCurrentPos", size); + int bytes_read = 0; int rv; do { @@ -234,7 +240,7 @@ int File::ReadNoBestEffort(int64 offset, char* data, int size) { ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); - + SCOPED_FILE_TRACE_WITH_SIZE("ReadNoBestEffort", size); return HANDLE_EINTR(pread(file_.get(), data, size, offset)); } @@ -244,6 +250,7 @@ if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("ReadAtCurrentPosNoBestEffort", size); return HANDLE_EINTR(read(file_.get(), data, size)); } @@ -257,6 +264,8 @@ if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("Write", size); + int bytes_written = 0; int rv; do { @@ -277,6 +286,8 @@ if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("WriteAtCurrentPos", size); + int bytes_written = 0; int rv; do { @@ -297,12 +308,15 @@ if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("WriteAtCurrentPosNoBestEffort", size); return HANDLE_EINTR(write(file_.get(), data, size)); } int64 File::GetLength() { DCHECK(IsValid()); + SCOPED_FILE_TRACE("GetLength"); + stat_wrapper_t file_info; if (CallFstat(file_.get(), &file_info)) return false; @@ -313,6 +327,8 @@ bool File::SetLength(int64 length) { ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); + + SCOPED_FILE_TRACE_WITH_SIZE("SetLength", length); return !CallFtruncate(file_.get(), length); } @@ -320,6 +336,8 @@ ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); + SCOPED_FILE_TRACE("SetTimes"); + timeval times[2]; times[0] = last_access_time.ToTimeVal(); times[1] = last_modified_time.ToTimeVal(); @@ -330,6 +348,8 @@ bool File::GetInfo(Info* info) { DCHECK(IsValid()); + SCOPED_FILE_TRACE("GetInfo"); + stat_wrapper_t file_info; if (CallFstat(file_.get(), &file_info)) return false; @@ -339,10 +359,12 @@ } File::Error File::Lock() { + SCOPED_FILE_TRACE("Lock"); return CallFctnlFlock(file_.get(), true); } File::Error File::Unlock() { + SCOPED_FILE_TRACE("Unlock"); return CallFctnlFlock(file_.get(), false); } @@ -350,6 +372,8 @@ if (!IsValid()) return File(); + SCOPED_FILE_TRACE("Duplicate"); + PlatformFile other_fd = dup(GetPlatformFile()); if (other_fd == -1) return File(OSErrorToFileError(errno)); @@ -442,7 +466,7 @@ // NaCl doesn't implement system calls to open files directly. #if !defined(OS_NACL) // TODO(erikkay): does it make sense to support FLAG_EXCLUSIVE_* here? -void File::DoInitialize(const FilePath& name, uint32 flags) { +void File::DoInitialize(uint32 flags) { ThreadRestrictions::AssertIOAllowed(); DCHECK(!IsValid()); @@ -497,7 +521,7 @@ mode |= S_IRGRP | S_IROTH; #endif - int descriptor = HANDLE_EINTR(open(name.value().c_str(), open_flags, mode)); + int descriptor = HANDLE_EINTR(open(path_.value().c_str(), open_flags, mode)); if (flags & FLAG_OPEN_ALWAYS) { if (descriptor < 0) { @@ -505,7 +529,7 @@ if (flags & FLAG_EXCLUSIVE_READ || flags & FLAG_EXCLUSIVE_WRITE) open_flags |= O_EXCL; // together with O_CREAT implies O_NOFOLLOW - descriptor = HANDLE_EINTR(open(name.value().c_str(), open_flags, mode)); + descriptor = HANDLE_EINTR(open(path_.value().c_str(), open_flags, mode)); if (descriptor >= 0) created_ = true; } @@ -520,7 +544,7 @@ created_ = true; if (flags & FLAG_DELETE_ON_CLOSE) - unlink(name.value().c_str()); + unlink(path_.value().c_str()); async_ = ((flags & FLAG_ASYNC) == FLAG_ASYNC); error_details_ = FILE_OK; @@ -531,6 +555,7 @@ bool File::DoFlush() { ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); + #if defined(OS_NACL) NOTIMPLEMENTED(); // NaCl doesn't implement fsync. return true;
diff --git a/files/file_tracing.cc b/files/file_tracing.cc new file mode 100644 index 0000000..a1919c4 --- /dev/null +++ b/files/file_tracing.cc
@@ -0,0 +1,56 @@ +// Copyright 2015 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/files/file_tracing.h" + +#include "base/files/file.h" + +namespace base { + +namespace { +FileTracing::Provider* g_provider = nullptr; +} + +// static +void FileTracing::SetProvider(FileTracing::Provider* provider) { + g_provider = provider; +} + +FileTracing::ScopedEnabler::ScopedEnabler() { + if (g_provider) + g_provider->FileTracingEnable(this); +} + +FileTracing::ScopedEnabler::~ScopedEnabler() { + if (g_provider) + g_provider->FileTracingDisable(this); +} + +FileTracing::ScopedTrace::ScopedTrace() : initialized_(false) {} + +FileTracing::ScopedTrace::~ScopedTrace() { + if (initialized_ && g_provider) { + g_provider->FileTracingEventEnd( + name_, &file_->trace_enabler_, file_->path_, size_); + } +} + +bool FileTracing::ScopedTrace::ShouldInitialize() const { + return g_provider && g_provider->FileTracingCategoryIsEnabled(); +} + +void FileTracing::ScopedTrace::Initialize( + const char* name, File* file, int64 size) { + file_ = file; + name_ = name; + size_ = size; + initialized_ = true; + + if (g_provider) { + g_provider->FileTracingEventBegin( + name_, &file_->trace_enabler_, file_->path_, size_); + } +} + +} // namespace base
diff --git a/files/file_tracing.h b/files/file_tracing.h new file mode 100644 index 0000000..8452037 --- /dev/null +++ b/files/file_tracing.h
@@ -0,0 +1,95 @@ +// Copyright 2015 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_FILES_FILE_TRACING_H_ +#define BASE_FILES_FILE_TRACING_H_ + +#include "base/base_export.h" +#include "base/basictypes.h" +#include "base/macros.h" + +#define FILE_TRACING_PREFIX "File" + +#define SCOPED_FILE_TRACE_WITH_SIZE(name, size) \ + FileTracing::ScopedTrace scoped_file_trace; \ + if (scoped_file_trace.ShouldInitialize()) \ + scoped_file_trace.Initialize(FILE_TRACING_PREFIX "::" name, this, size) + +#define SCOPED_FILE_TRACE(name) SCOPED_FILE_TRACE_WITH_SIZE(name, 0) + +namespace base { + +class File; +class FilePath; + +class BASE_EXPORT FileTracing { + public: + class Provider { + public: + // Whether the file tracing category is currently enabled. + virtual bool FileTracingCategoryIsEnabled() const = 0; + + // Enables file tracing for |id|. Must be called before recording events. + virtual void FileTracingEnable(void* id) = 0; + + // Disables file tracing for |id|. + virtual void FileTracingDisable(void* id) = 0; + + // Begins an event for |id| with |name|. |path| tells where in the directory + // structure the event is happening (and may be blank). |size| is reported + // if not 0. + virtual void FileTracingEventBegin( + const char* name, void* id, const FilePath& path, int64 size) = 0; + + // Ends an event for |id| with |name|. |path| tells where in the directory + // structure the event is happening (and may be blank). |size| is reported + // if not 0. + virtual void FileTracingEventEnd( + const char* name, void* id, const FilePath& path, int64 size) = 0; + }; + + // Sets a global file tracing provider to query categories and record events. + static void SetProvider(Provider* provider); + + // Enables file tracing while in scope. + class ScopedEnabler { + public: + ScopedEnabler(); + ~ScopedEnabler(); + }; + + class ScopedTrace { + public: + ScopedTrace(); + ~ScopedTrace(); + + // Whether this trace should be initialized or not. + bool ShouldInitialize() const; + + // Called only if the tracing category is enabled. + void Initialize(const char* event, File* file, int64 size); + + private: + // True if |Initialize()| has been called. Don't touch |path_|, |event_|, + // or |bytes_| if |initialized_| is false. + bool initialized_; + + // The event name to trace (e.g. "Read", "Write"). Prefixed with "File". + const char* name_; + + // The file being traced. Must outlive this class. + File* file_; + + // The size (in bytes) of this trace. Not reported if 0. + int64 size_; + + DISALLOW_COPY_AND_ASSIGN(ScopedTrace); + }; + + DISALLOW_COPY_AND_ASSIGN(FileTracing); +}; + +} // namespace base + +#endif // BASE_FILES_FILE_TRACING_H_
diff --git a/files/file_win.cc b/files/file_win.cc index 200ea29..9792852 100644 --- a/files/file_win.cc +++ b/files/file_win.cc
@@ -6,7 +6,6 @@ #include <io.h> -#include "base/files/file_path.h" #include "base/logging.h" #include "base/metrics/sparse_histogram.h" #include "base/threading/thread_restrictions.h" @@ -31,16 +30,20 @@ } void File::Close() { - if (file_.IsValid()) { - ThreadRestrictions::AssertIOAllowed(); - file_.Close(); - } + if (!file_.IsValid()) + return; + + ThreadRestrictions::AssertIOAllowed(); + SCOPED_FILE_TRACE("Close"); + file_.Close(); } int64 File::Seek(Whence whence, int64 offset) { ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); + SCOPED_FILE_TRACE_WITH_SIZE("Seek", offset); + LARGE_INTEGER distance, res; distance.QuadPart = offset; DWORD move_method = static_cast<DWORD>(whence); @@ -56,6 +59,8 @@ if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("Read", size); + LARGE_INTEGER offset_li; offset_li.QuadPart = offset; @@ -79,6 +84,8 @@ if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("ReadAtCurrentPos", size); + DWORD bytes_read; if (::ReadFile(file_.Get(), data, size, &bytes_read, NULL)) return bytes_read; @@ -89,10 +96,12 @@ } int File::ReadNoBestEffort(int64 offset, char* data, int size) { + // TODO(dbeam): trace this separately? return Read(offset, data, size); } int File::ReadAtCurrentPosNoBestEffort(char* data, int size) { + // TODO(dbeam): trace this separately? return ReadAtCurrentPos(data, size); } @@ -101,6 +110,8 @@ DCHECK(IsValid()); DCHECK(!async_); + SCOPED_FILE_TRACE_WITH_SIZE("Write", size); + LARGE_INTEGER offset_li; offset_li.QuadPart = offset; @@ -122,6 +133,8 @@ if (size < 0) return -1; + SCOPED_FILE_TRACE_WITH_SIZE("WriteAtCurrentPos", size); + DWORD bytes_written; if (::WriteFile(file_.Get(), data, size, &bytes_written, NULL)) return bytes_written; @@ -136,6 +149,9 @@ int64 File::GetLength() { ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); + + SCOPED_FILE_TRACE("GetLength"); + LARGE_INTEGER size; if (!::GetFileSizeEx(file_.Get(), &size)) return -1; @@ -147,6 +163,8 @@ ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); + SCOPED_FILE_TRACE_WITH_SIZE("SetLength", length); + // Get the current file pointer. LARGE_INTEGER file_pointer; LARGE_INTEGER zero; @@ -176,6 +194,8 @@ ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); + SCOPED_FILE_TRACE("SetTimes"); + FILETIME last_access_filetime = last_access_time.ToFileTime(); FILETIME last_modified_filetime = last_modified_time.ToFileTime(); return (::SetFileTime(file_.Get(), NULL, &last_access_filetime, @@ -186,6 +206,8 @@ ThreadRestrictions::AssertIOAllowed(); DCHECK(IsValid()); + SCOPED_FILE_TRACE("GetInfo"); + BY_HANDLE_FILE_INFORMATION file_info; if (!GetFileInformationByHandle(file_.Get(), &file_info)) return false; @@ -205,6 +227,9 @@ File::Error File::Lock() { DCHECK(IsValid()); + + SCOPED_FILE_TRACE("Lock"); + BOOL result = LockFile(file_.Get(), 0, 0, MAXDWORD, MAXDWORD); if (!result) return OSErrorToFileError(GetLastError()); @@ -213,6 +238,9 @@ File::Error File::Unlock() { DCHECK(IsValid()); + + SCOPED_FILE_TRACE("Unlock"); + BOOL result = UnlockFile(file_.Get(), 0, 0, MAXDWORD, MAXDWORD); if (!result) return OSErrorToFileError(GetLastError()); @@ -223,6 +251,8 @@ if (!IsValid()) return File(); + SCOPED_FILE_TRACE("Duplicate"); + HANDLE other_handle = nullptr; if (!::DuplicateHandle(GetCurrentProcess(), // hSourceProcessHandle @@ -278,7 +308,7 @@ } } -void File::DoInitialize(const FilePath& name, uint32 flags) { +void File::DoInitialize(uint32 flags) { ThreadRestrictions::AssertIOAllowed(); DCHECK(!IsValid()); @@ -346,7 +376,7 @@ if (flags & FLAG_BACKUP_SEMANTICS) create_flags |= FILE_FLAG_BACKUP_SEMANTICS; - file_.Set(CreateFile(name.value().c_str(), access, sharing, NULL, + file_.Set(CreateFile(path_.value().c_str(), access, sharing, NULL, disposition, create_flags, NULL)); if (file_.IsValid()) {
diff --git a/format_macros.h b/format_macros.h index 4d90c59..d58658d 100644 --- a/format_macros.h +++ b/format_macros.h
@@ -23,19 +23,20 @@ #include "build/build_config.h" -#if defined(OS_POSIX) - -#if (defined(_INTTYPES_H) || defined(_INTTYPES_H_)) && !defined(PRId64) +#if defined(OS_POSIX) && (defined(_INTTYPES_H) || defined(_INTTYPES_H_)) && \ + !defined(PRId64) #error "inttypes.h has already been included before this header file, but " #error "without __STDC_FORMAT_MACROS defined." #endif -#if !defined(__STDC_FORMAT_MACROS) +#if defined(OS_POSIX) && !defined(__STDC_FORMAT_MACROS) #define __STDC_FORMAT_MACROS #endif #include <inttypes.h> +#if defined(OS_POSIX) + // GCC will concatenate wide and narrow strings correctly, so nothing needs to // be done here. #define WidePRId64 PRId64 @@ -76,16 +77,8 @@ #else // OS_WIN -#if !defined(PRId64) -#define PRId64 "I64d" -#endif - -#if !defined(PRIu64) -#define PRIu64 "I64u" -#endif - -#if !defined(PRIx64) -#define PRIx64 "I64x" +#if !defined(PRId64) || !defined(PRIu64) || !defined(PRIx64) +#error "inttypes.h provided by win toolchain should define these." #endif #define WidePRId64 L"I64d"
diff --git a/i18n/icu_string_conversions.cc b/i18n/icu_string_conversions.cc index 1530117..edb31c3 100644 --- a/i18n/icu_string_conversions.cc +++ b/i18n/icu_string_conversions.cc
@@ -134,14 +134,6 @@ } } -inline UConverterType utf32_platform_endian() { -#if U_IS_BIG_ENDIAN - return UCNV_UTF32_BigEndian; -#else - return UCNV_UTF32_LittleEndian; -#endif -} - } // namespace // Codepage <-> Wide/UTF-16 --------------------------------------------------- @@ -197,74 +189,6 @@ return true; } -bool WideToCodepage(const std::wstring& wide, - const char* codepage_name, - OnStringConversionError::Type on_error, - std::string* encoded) { -#if defined(WCHAR_T_IS_UTF16) - return UTF16ToCodepage(wide, codepage_name, on_error, encoded); -#elif defined(WCHAR_T_IS_UTF32) - encoded->clear(); - - UErrorCode status = U_ZERO_ERROR; - UConverter* converter = ucnv_open(codepage_name, &status); - if (!U_SUCCESS(status)) - return false; - - int utf16_len; - // When wchar_t is wider than UChar (16 bits), transform |wide| into a - // UChar* string. Size the UChar* buffer to be large enough to hold twice - // as many UTF-16 code units (UChar's) as there are Unicode code points, - // in case each code points translates to a UTF-16 surrogate pair, - // and leave room for a NUL terminator. - std::vector<UChar> utf16(wide.length() * 2 + 1); - u_strFromUTF32(&utf16[0], utf16.size(), &utf16_len, - reinterpret_cast<const UChar32*>(wide.c_str()), - wide.length(), &status); - DCHECK(U_SUCCESS(status)) << "failed to convert wstring to UChar*"; - - return ConvertFromUTF16(converter, &utf16[0], utf16_len, on_error, encoded); -#endif // defined(WCHAR_T_IS_UTF32) -} - -bool CodepageToWide(const std::string& encoded, - const char* codepage_name, - OnStringConversionError::Type on_error, - std::wstring* wide) { -#if defined(WCHAR_T_IS_UTF16) - return CodepageToUTF16(encoded, codepage_name, on_error, wide); -#elif defined(WCHAR_T_IS_UTF32) - wide->clear(); - - UErrorCode status = U_ZERO_ERROR; - UConverter* converter = ucnv_open(codepage_name, &status); - if (!U_SUCCESS(status)) - return false; - - // The maximum length in 4 byte unit of UTF-32 output would be - // at most the same as the number of bytes in input. In the worst - // case of GB18030 (excluding escaped-based encodings like ISO-2022-JP), - // this can be 4 times larger than actually needed. - size_t wchar_max_length = encoded.length() + 1; - - SetUpErrorHandlerForToUChars(on_error, converter, &status); - scoped_ptr<wchar_t[]> buffer(new wchar_t[wchar_max_length]); - int actual_size = ucnv_toAlgorithmic(utf32_platform_endian(), converter, - reinterpret_cast<char*>(buffer.get()), - static_cast<int>(wchar_max_length) * sizeof(wchar_t), encoded.data(), - static_cast<int>(encoded.length()), &status); - ucnv_close(converter); - if (!U_SUCCESS(status)) { - wide->clear(); // Make sure the output is empty on error. - return false; - } - - // actual_size is # of bytes. - wide->assign(buffer.get(), actual_size / sizeof(wchar_t)); - return true; -#endif // defined(WCHAR_T_IS_UTF32) -} - bool ConvertToUtf8AndNormalize(const std::string& text, const std::string& charset, std::string* result) {
diff --git a/i18n/icu_string_conversions.h b/i18n/icu_string_conversions.h index fd2ae46..9135da8 100644 --- a/i18n/icu_string_conversions.h +++ b/i18n/icu_string_conversions.h
@@ -13,8 +13,7 @@ namespace base { -// Defines the error handling modes of UTF16ToCodepage, CodepageToUTF16, -// WideToCodepage and CodepageToWide. +// Defines the error handling modes of UTF16ToCodepage and CodepageToUTF16. class OnStringConversionError { public: enum Type { @@ -47,18 +46,6 @@ OnStringConversionError::Type on_error, string16* utf16); -// Converts between wide strings and the encoding specified. If the -// encoding doesn't exist or the encoding fails (when on_error is FAIL), -// returns false. -BASE_I18N_EXPORT bool WideToCodepage(const std::wstring& wide, - const char* codepage_name, - OnStringConversionError::Type on_error, - std::string* encoded); -BASE_I18N_EXPORT bool CodepageToWide(const std::string& encoded, - const char* codepage_name, - OnStringConversionError::Type on_error, - std::wstring* wide); - // Converts from any codepage to UTF-8 and ensures the resulting UTF-8 is // normalized. BASE_I18N_EXPORT bool ConvertToUtf8AndNormalize(const std::string& text,
diff --git a/i18n/icu_string_conversions_unittest.cc b/i18n/icu_string_conversions_unittest.cc index d4d3251..821529d 100644 --- a/i18n/icu_string_conversions_unittest.cc +++ b/i18n/icu_string_conversions_unittest.cc
@@ -42,49 +42,8 @@ #endif } -const wchar_t* const kConvertRoundtripCases[] = { - L"Google Video", - // "网页 图片 资讯更多 »" - L"\x7f51\x9875\x0020\x56fe\x7247\x0020\x8d44\x8baf\x66f4\x591a\x0020\x00bb", - // "Παγκόσμιος Ιστός" - L"\x03a0\x03b1\x03b3\x03ba\x03cc\x03c3\x03bc\x03b9" - L"\x03bf\x03c2\x0020\x0399\x03c3\x03c4\x03cc\x03c2", - // "Поиск страниц на русском" - L"\x041f\x043e\x0438\x0441\x043a\x0020\x0441\x0442" - L"\x0440\x0430\x043d\x0438\x0446\x0020\x043d\x0430" - L"\x0020\x0440\x0443\x0441\x0441\x043a\x043e\x043c", - // "전체서비스" - L"\xc804\xccb4\xc11c\xbe44\xc2a4", - - // Test characters that take more than 16 bits. This will depend on whether - // wchar_t is 16 or 32 bits. -#if defined(WCHAR_T_IS_UTF16) - L"\xd800\xdf00", - // ????? (Mathematical Alphanumeric Symbols (U+011d40 - U+011d44 : A,B,C,D,E) - L"\xd807\xdd40\xd807\xdd41\xd807\xdd42\xd807\xdd43\xd807\xdd44", -#elif defined(WCHAR_T_IS_UTF32) - L"\x10300", - // ????? (Mathematical Alphanumeric Symbols (U+011d40 - U+011d44 : A,B,C,D,E) - L"\x11d40\x11d41\x11d42\x11d43\x11d44", -#endif -}; - } // namespace -TEST(ICUStringConversionsTest, ConvertCodepageUTF8) { - // Make sure WideToCodepage works like WideToUTF8. - for (size_t i = 0; i < arraysize(kConvertRoundtripCases); ++i) { - SCOPED_TRACE(base::StringPrintf("Test[%" PRIuS "]: %ls", - i, kConvertRoundtripCases[i])); - - std::string expected(WideToUTF8(kConvertRoundtripCases[i])); - std::string utf8; - EXPECT_TRUE(WideToCodepage(kConvertRoundtripCases[i], kCodepageUTF8, - OnStringConversionError::SKIP, &utf8)); - EXPECT_EQ(expected, utf8); - } -} - // kConverterCodepageCases is not comprehensive. There are a number of cases // to add if we really want to have a comprehensive coverage of various // codepages and their 'idiosyncrasies'. Currently, the only implementation @@ -233,73 +192,6 @@ NULL}, }; -TEST(ICUStringConversionsTest, ConvertBetweenCodepageAndWide) { - for (size_t i = 0; i < arraysize(kConvertCodepageCases); ++i) { - SCOPED_TRACE(base::StringPrintf( - "Test[%" PRIuS "]: <encoded: %s> <codepage: %s>", i, - kConvertCodepageCases[i].encoded, - kConvertCodepageCases[i].codepage_name)); - - std::wstring wide; - bool success = CodepageToWide(kConvertCodepageCases[i].encoded, - kConvertCodepageCases[i].codepage_name, - kConvertCodepageCases[i].on_error, - &wide); - EXPECT_EQ(kConvertCodepageCases[i].success, success); - EXPECT_EQ(kConvertCodepageCases[i].wide, wide); - - // When decoding was successful and nothing was skipped, we also check the - // reverse conversion. Not all conversions are round-trippable, but - // kConverterCodepageCases does not have any one-way conversion at the - // moment. - if (success && - kConvertCodepageCases[i].on_error == - OnStringConversionError::FAIL) { - std::string encoded; - success = WideToCodepage(wide, kConvertCodepageCases[i].codepage_name, - kConvertCodepageCases[i].on_error, &encoded); - EXPECT_EQ(kConvertCodepageCases[i].success, success); - EXPECT_EQ(kConvertCodepageCases[i].encoded, encoded); - } - } - - // The above cases handled codepage->wide errors, but not wide->codepage. - // Test that here. - std::string encoded("Temp data"); // Make sure the string gets cleared. - - // First test going to an encoding that can not represent that character. - EXPECT_FALSE(WideToCodepage(L"Chinese\xff27", "iso-8859-1", - OnStringConversionError::FAIL, &encoded)); - EXPECT_TRUE(encoded.empty()); - EXPECT_TRUE(WideToCodepage(L"Chinese\xff27", "iso-8859-1", - OnStringConversionError::SKIP, &encoded)); - EXPECT_STREQ("Chinese", encoded.c_str()); - // From Unicode, SUBSTITUTE is the same as SKIP for now. - EXPECT_TRUE(WideToCodepage(L"Chinese\xff27", "iso-8859-1", - OnStringConversionError::SUBSTITUTE, - &encoded)); - EXPECT_STREQ("Chinese", encoded.c_str()); - -#if defined(WCHAR_T_IS_UTF16) - // When we're in UTF-16 mode, test an invalid UTF-16 character in the input. - EXPECT_FALSE(WideToCodepage(L"a\xd800z", "iso-8859-1", - OnStringConversionError::FAIL, &encoded)); - EXPECT_TRUE(encoded.empty()); - EXPECT_TRUE(WideToCodepage(L"a\xd800z", "iso-8859-1", - OnStringConversionError::SKIP, &encoded)); - EXPECT_STREQ("az", encoded.c_str()); -#endif // WCHAR_T_IS_UTF16 - - // Invalid characters should fail. - EXPECT_TRUE(WideToCodepage(L"a\xffffz", "iso-8859-1", - OnStringConversionError::SKIP, &encoded)); - EXPECT_STREQ("az", encoded.c_str()); - - // Invalid codepages should fail. - EXPECT_FALSE(WideToCodepage(L"Hello, world", "awesome-8571-2", - OnStringConversionError::SKIP, &encoded)); -} - TEST(ICUStringConversionsTest, ConvertBetweenCodepageAndUTF16) { for (size_t i = 0; i < arraysize(kConvertCodepageCases); ++i) { SCOPED_TRACE(base::StringPrintf(
diff --git a/i18n/icu_util.cc b/i18n/icu_util.cc index ede6467..a9f0b12 100644 --- a/i18n/icu_util.cc +++ b/i18n/icu_util.cc
@@ -59,10 +59,10 @@ #endif } -#if defined(OS_ANDROID) +#if !defined(OS_NACL) bool InitializeICUWithFileDescriptor( - int data_fd, - base::MemoryMappedFile::Region data_region) { + PlatformFile data_fd, + MemoryMappedFile::Region data_region) { #if !defined(NDEBUG) DCHECK(!g_check_called_once || !g_called_once); g_called_once = true; @@ -72,9 +72,9 @@ // The ICU data is statically linked. return true; #elif (ICU_UTIL_DATA_IMPL == ICU_UTIL_DATA_FILE) - CR_DEFINE_STATIC_LOCAL(base::MemoryMappedFile, mapped_file, ()); + CR_DEFINE_STATIC_LOCAL(MemoryMappedFile, mapped_file, ()); if (!mapped_file.IsValid()) { - if (!mapped_file.Initialize(base::File(data_fd), data_region)) { + if (!mapped_file.Initialize(File(data_fd), data_region)) { LOG(ERROR) << "Couldn't mmap icu data file"; return false; } @@ -84,10 +84,8 @@ return err == U_ZERO_ERROR; #endif // ICU_UTIL_DATA_FILE } -#endif -#if !defined(OS_NACL) bool InitializeICU() { #if !defined(NDEBUG) DCHECK(!g_check_called_once || !g_called_once); @@ -98,7 +96,7 @@ #if (ICU_UTIL_DATA_IMPL == ICU_UTIL_DATA_SHARED) // We expect to find the ICU data module alongside the current module. FilePath data_path; - PathService::Get(base::DIR_MODULE, &data_path); + PathService::Get(DIR_MODULE, &data_path); data_path = data_path.AppendASCII(ICU_UTIL_DATA_SHARED_MODULE_NAME); HMODULE module = LoadLibrary(data_path.value().c_str()); @@ -128,24 +126,24 @@ // Chrome doesn't normally shut down ICU, so the mapped data shouldn't ever // be released. - CR_DEFINE_STATIC_LOCAL(base::MemoryMappedFile, mapped_file, ()); + CR_DEFINE_STATIC_LOCAL(MemoryMappedFile, mapped_file, ()); if (!mapped_file.IsValid()) { #if !defined(OS_MACOSX) FilePath data_path; #if defined(OS_WIN) // The data file will be in the same directory as the current module. - bool path_ok = PathService::Get(base::DIR_MODULE, &data_path); + bool path_ok = PathService::Get(DIR_MODULE, &data_path); wchar_t tmp_buffer[_MAX_PATH] = {0}; wcscpy_s(tmp_buffer, data_path.value().c_str()); - base::debug::Alias(tmp_buffer); + debug::Alias(tmp_buffer); CHECK(path_ok); // TODO(scottmg): http://crbug.com/445616 #elif defined(OS_ANDROID) - bool path_ok = PathService::Get(base::DIR_ANDROID_APP_DATA, &data_path); + bool path_ok = PathService::Get(DIR_ANDROID_APP_DATA, &data_path); #else // For now, expect the data file to be alongside the executable. // This is sufficient while we work on unit tests, but will eventually // likely live in a data directory. - bool path_ok = PathService::Get(base::DIR_EXE, &data_path); + bool path_ok = PathService::Get(DIR_EXE, &data_path); #endif DCHECK(path_ok); data_path = data_path.AppendASCII(kIcuDataFileName); @@ -154,15 +152,15 @@ // TODO(scottmg): http://crbug.com/445616 wchar_t tmp_buffer2[_MAX_PATH] = {0}; wcscpy_s(tmp_buffer2, data_path.value().c_str()); - base::debug::Alias(tmp_buffer2); + debug::Alias(tmp_buffer2); #endif #else // Assume it is in the framework bundle's Resources directory. - base::ScopedCFTypeRef<CFStringRef> data_file_name( + ScopedCFTypeRef<CFStringRef> data_file_name( SysUTF8ToCFStringRef(kIcuDataFileName)); FilePath data_path = - base::mac::PathForFrameworkBundleResource(data_file_name); + mac::PathForFrameworkBundleResource(data_file_name); if (data_path.empty()) { LOG(ERROR) << kIcuDataFileName << " not found in bundle"; return false;
diff --git a/i18n/icu_util.h b/i18n/icu_util.h index e9f7c3d..65de0ad 100644 --- a/i18n/icu_util.h +++ b/i18n/icu_util.h
@@ -18,14 +18,12 @@ // Call this function to load ICU's data tables for the current process. This // function should be called before ICU is used. BASE_I18N_EXPORT bool InitializeICU(); -#endif -#if defined(OS_ANDROID) -// Android uses a file descriptor passed by browser process to initialize ICU -// in render processes. +// Android and html_viewer use a file descriptor passed by browser process to +// initialize ICU in render processes. BASE_I18N_EXPORT bool InitializeICUWithFileDescriptor( - int data_fd, - base::MemoryMappedFile::Region data_region); + PlatformFile data_fd, + MemoryMappedFile::Region data_region); #endif // In a test binary, the call above might occur twice.
diff --git a/json/json_parser.cc b/json/json_parser.cc index e9a27bc..4d79be3 100644 --- a/json/json_parser.cc +++ b/json/json_parser.cc
@@ -932,7 +932,7 @@ return NULL; } NextNChars(kNullLen - 1); - return Value::CreateNullValue(); + return Value::CreateNullValue().release(); } default: ReportError(JSONReader::JSON_UNEXPECTED_TOKEN, 1);
diff --git a/json/json_writer_unittest.cc b/json/json_writer_unittest.cc index ae46800..5ac2590 100644 --- a/json/json_writer_unittest.cc +++ b/json/json_writer_unittest.cc
@@ -12,75 +12,63 @@ std::string output_js; // Test null. - Value* root = Value::CreateNullValue(); - EXPECT_TRUE(JSONWriter::Write(root, &output_js)); + EXPECT_TRUE(JSONWriter::Write(Value::CreateNullValue().get(), &output_js)); EXPECT_EQ("null", output_js); - delete root; // Test empty dict. - root = new DictionaryValue; - EXPECT_TRUE(JSONWriter::Write(root, &output_js)); + DictionaryValue dict; + EXPECT_TRUE(JSONWriter::Write(&dict, &output_js)); EXPECT_EQ("{}", output_js); - delete root; // Test empty list. - root = new ListValue; - EXPECT_TRUE(JSONWriter::Write(root, &output_js)); + ListValue list; + EXPECT_TRUE(JSONWriter::Write(&list, &output_js)); EXPECT_EQ("[]", output_js); - delete root; // Test integer values. - root = new FundamentalValue(42); - EXPECT_TRUE(JSONWriter::Write(root, &output_js)); + FundamentalValue int_value(42); + EXPECT_TRUE(JSONWriter::Write(&int_value, &output_js)); EXPECT_EQ("42", output_js); - delete root; // Test boolean values. - root = new FundamentalValue(true); - EXPECT_TRUE(JSONWriter::Write(root, &output_js)); + FundamentalValue bool_value(true); + EXPECT_TRUE(JSONWriter::Write(&bool_value, &output_js)); EXPECT_EQ("true", output_js); - delete root; // Test Real values should always have a decimal or an 'e'. - root = new FundamentalValue(1.0); - EXPECT_TRUE(JSONWriter::Write(root, &output_js)); + FundamentalValue double_value(1.0); + EXPECT_TRUE(JSONWriter::Write(&double_value, &output_js)); EXPECT_EQ("1.0", output_js); - delete root; // Test Real values in the the range (-1, 1) must have leading zeros - root = new FundamentalValue(0.2); - EXPECT_TRUE(JSONWriter::Write(root, &output_js)); + FundamentalValue double_value2(0.2); + EXPECT_TRUE(JSONWriter::Write(&double_value2, &output_js)); EXPECT_EQ("0.2", output_js); - delete root; // Test Real values in the the range (-1, 1) must have leading zeros - root = new FundamentalValue(-0.8); - EXPECT_TRUE(JSONWriter::Write(root, &output_js)); + FundamentalValue double_value3(-0.8); + EXPECT_TRUE(JSONWriter::Write(&double_value3, &output_js)); EXPECT_EQ("-0.8", output_js); - delete root; // Test String values. - root = new StringValue("foo"); - EXPECT_TRUE(JSONWriter::Write(root, &output_js)); + StringValue string_value("foo"); + EXPECT_TRUE(JSONWriter::Write(&string_value, &output_js)); EXPECT_EQ("\"foo\"", output_js); - delete root; } - TEST(JSONWriterTest, NestedTypes) { std::string output_js; // Writer unittests like empty list/dict nesting, // list list nesting, etc. DictionaryValue root_dict; - ListValue* list = new ListValue; - root_dict.Set("list", list); - DictionaryValue* inner_dict = new DictionaryValue; - list->Append(inner_dict); + scoped_ptr<ListValue> list(new ListValue()); + scoped_ptr<DictionaryValue> inner_dict(new DictionaryValue()); inner_dict->SetInteger("inner int", 10); - ListValue* inner_list = new ListValue; - list->Append(inner_list); - list->Append(new FundamentalValue(true)); + list->Append(inner_dict.Pass()); + list->Append(make_scoped_ptr(new ListValue())); + list->AppendBoolean(true); + root_dict.Set("list", list.Pass()); // Test the pretty-printer. EXPECT_TRUE(JSONWriter::Write(&root_dict, &output_js)); @@ -109,17 +97,17 @@ std::string output_js; DictionaryValue period_dict; - period_dict.SetWithoutPathExpansion("a.b", new FundamentalValue(3)); - period_dict.SetWithoutPathExpansion("c", new FundamentalValue(2)); - DictionaryValue* period_dict2 = new DictionaryValue; - period_dict2->SetWithoutPathExpansion("g.h.i.j", new FundamentalValue(1)); - period_dict.SetWithoutPathExpansion("d.e.f", period_dict2); + period_dict.SetIntegerWithoutPathExpansion("a.b", 3); + period_dict.SetIntegerWithoutPathExpansion("c", 2); + scoped_ptr<DictionaryValue> period_dict2(new DictionaryValue()); + period_dict2->SetIntegerWithoutPathExpansion("g.h.i.j", 1); + period_dict.SetWithoutPathExpansion("d.e.f", period_dict2.Pass()); EXPECT_TRUE(JSONWriter::Write(&period_dict, &output_js)); EXPECT_EQ("{\"a.b\":3,\"c\":2,\"d.e.f\":{\"g.h.i.j\":1}}", output_js); DictionaryValue period_dict3; - period_dict3.Set("a.b", new FundamentalValue(2)); - period_dict3.SetWithoutPathExpansion("a.b", new FundamentalValue(1)); + period_dict3.SetInteger("a.b", 2); + period_dict3.SetIntegerWithoutPathExpansion("a.b", 1); EXPECT_TRUE(JSONWriter::Write(&period_dict3, &output_js)); EXPECT_EQ("{\"a\":{\"b\":2},\"a.b\":1}", output_js); } @@ -129,18 +117,17 @@ // Binary values should return errors unless suppressed via the // OPTIONS_OMIT_BINARY_VALUES flag. - Value* root = BinaryValue::CreateWithCopiedBuffer("asdf", 4); - EXPECT_FALSE(JSONWriter::Write(root, &output_js)); + scoped_ptr<Value> root(BinaryValue::CreateWithCopiedBuffer("asdf", 4)); + EXPECT_FALSE(JSONWriter::Write(root.get(), &output_js)); EXPECT_TRUE(JSONWriter::WriteWithOptions( - root, JSONWriter::OPTIONS_OMIT_BINARY_VALUES, &output_js)); + root.get(), JSONWriter::OPTIONS_OMIT_BINARY_VALUES, &output_js)); EXPECT_TRUE(output_js.empty()); - delete root; ListValue binary_list; binary_list.Append(BinaryValue::CreateWithCopiedBuffer("asdf", 4)); - binary_list.Append(new FundamentalValue(5)); + binary_list.Append(make_scoped_ptr(new FundamentalValue(5))); binary_list.Append(BinaryValue::CreateWithCopiedBuffer("asdf", 4)); - binary_list.Append(new FundamentalValue(2)); + binary_list.Append(make_scoped_ptr(new FundamentalValue(2))); binary_list.Append(BinaryValue::CreateWithCopiedBuffer("asdf", 4)); EXPECT_FALSE(JSONWriter::Write(&binary_list, &output_js)); EXPECT_TRUE(JSONWriter::WriteWithOptions( @@ -148,11 +135,14 @@ EXPECT_EQ("[5,2]", output_js); DictionaryValue binary_dict; - binary_dict.Set("a", BinaryValue::CreateWithCopiedBuffer("asdf", 4)); - binary_dict.Set("b", new FundamentalValue(5)); - binary_dict.Set("c", BinaryValue::CreateWithCopiedBuffer("asdf", 4)); - binary_dict.Set("d", new FundamentalValue(2)); - binary_dict.Set("e", BinaryValue::CreateWithCopiedBuffer("asdf", 4)); + binary_dict.Set( + "a", make_scoped_ptr(BinaryValue::CreateWithCopiedBuffer("asdf", 4))); + binary_dict.SetInteger("b", 5); + binary_dict.Set( + "c", make_scoped_ptr(BinaryValue::CreateWithCopiedBuffer("asdf", 4))); + binary_dict.SetInteger("d", 2); + binary_dict.Set( + "e", make_scoped_ptr(BinaryValue::CreateWithCopiedBuffer("asdf", 4))); EXPECT_FALSE(JSONWriter::Write(&binary_dict, &output_js)); EXPECT_TRUE(JSONWriter::WriteWithOptions( &binary_dict, JSONWriter::OPTIONS_OMIT_BINARY_VALUES, &output_js));
diff --git a/json/string_escape_unittest.cc b/json/string_escape_unittest.cc index 3eb4e8e..100373f 100644 --- a/json/string_escape_unittest.cc +++ b/json/string_escape_unittest.cc
@@ -159,7 +159,7 @@ const char* escaped; } cases[] = { {"b\x0f\x7f\xf0\xff!", "b\\u000F\\u007F\\u00F0\\u00FF!"}, - {"\xe5\xc4\x4f\x05\xb6\xfd\0", "\\u00E5\\u00C4O\\u0005\\u00B6\\u00FD"}, + {"\xe5\xc4\x4f\x05\xb6\xfd", "\\u00E5\\u00C4O\\u0005\\u00B6\\u00FD"}, }; for (size_t i = 0; i < arraysize(cases); ++i) {
diff --git a/mac/dispatch_source_mach.cc b/mac/dispatch_source_mach.cc new file mode 100644 index 0000000..745c9de --- /dev/null +++ b/mac/dispatch_source_mach.cc
@@ -0,0 +1,62 @@ +// 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/mac/dispatch_source_mach.h" + +namespace base { + +DispatchSourceMach::DispatchSourceMach(const char* name, + mach_port_t port, + void (^event_handler)()) + // TODO(rsesek): Specify DISPATCH_QUEUE_SERIAL, in the 10.7 SDK. NULL + // means the same thing but is not symbolically clear. + : DispatchSourceMach(dispatch_queue_create(name, NULL), + port, + event_handler) { + // Since the queue was created above in the delegated constructor, and it was + // subsequently retained, release it here. + dispatch_release(queue_); +} + +DispatchSourceMach::DispatchSourceMach(dispatch_queue_t queue, + mach_port_t port, + void (^event_handler)()) + : queue_(queue), + source_(dispatch_source_create(DISPATCH_SOURCE_TYPE_MACH_RECV, + port, 0, queue_)), + source_canceled_(dispatch_semaphore_create(0)) { + dispatch_retain(queue); + + dispatch_source_set_event_handler(source_, event_handler); + dispatch_source_set_cancel_handler(source_, ^{ + dispatch_semaphore_signal(source_canceled_); + }); +} + +DispatchSourceMach::~DispatchSourceMach() { + Cancel(); +} + +void DispatchSourceMach::Resume() { + dispatch_resume(source_); +} + +void DispatchSourceMach::Cancel() { + if (source_) { + dispatch_source_cancel(source_); + dispatch_release(source_); + source_ = NULL; + + dispatch_semaphore_wait(source_canceled_, DISPATCH_TIME_FOREVER); + dispatch_release(source_canceled_); + source_canceled_ = NULL; + } + + if (queue_) { + dispatch_release(queue_); + queue_ = NULL; + } +} + +} // namespace base
diff --git a/mac/dispatch_source_mach.h b/mac/dispatch_source_mach.h new file mode 100644 index 0000000..e7d5cb2 --- /dev/null +++ b/mac/dispatch_source_mach.h
@@ -0,0 +1,61 @@ +// 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_MAC_DISPATCH_SOURCE_MACH_H_ +#define BASE_MAC_DISPATCH_SOURCE_MACH_H_ + +#include <dispatch/dispatch.h> + +#include "base/base_export.h" +#include "base/macros.h" + +namespace base { + +// This class encapsulates a MACH_RECV dispatch source. When this object is +// destroyed, the source will be cancelled and it will wait for the source +// to stop executing work. The source can run on either a user-supplied queue, +// or it can create its own for the source. +class BASE_EXPORT DispatchSourceMach { + public: + // Creates a new dispatch source for the |port| and schedules it on a new + // queue that will be created with |name|. When a Mach message is received, + // the |event_handler| will be called. + DispatchSourceMach(const char* name, + mach_port_t port, + void (^event_handler)()); + + // Creates a new dispatch source with the same semantics as above, but rather + // than creating a new queue, it schedules the source on |queue|. + DispatchSourceMach(dispatch_queue_t queue, + mach_port_t port, + void (^event_handler)()); + + // Cancels the source and waits for it to become fully cancelled before + // releasing the source. + ~DispatchSourceMach(); + + // Resumes the source. This must be called before any Mach messages will + // be received. + void Resume(); + + private: + // Cancels the source, after which this class will no longer receive Mach + // messages. Calling this more than once is a no-op. + void Cancel(); + + // The dispatch queue used to service the source_. + dispatch_queue_t queue_; + + // A MACH_RECV dispatch source. + dispatch_source_t source_; + + // Semaphore used to wait on the |source_|'s cancellation in the destructor. + dispatch_semaphore_t source_canceled_; + + DISALLOW_COPY_AND_ASSIGN(DispatchSourceMach); +}; + +} // namespace base + +#endif // BASE_MAC_DISPATCH_SOURCE_MACH_H_
diff --git a/mac/dispatch_source_mach_unittest.cc b/mac/dispatch_source_mach_unittest.cc new file mode 100644 index 0000000..82dc136 --- /dev/null +++ b/mac/dispatch_source_mach_unittest.cc
@@ -0,0 +1,125 @@ +// 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/mac/dispatch_source_mach.h" + +#include <mach/mach.h> + +#include "base/logging.h" +#include "base/mac/scoped_mach_port.h" +#include "base/memory/scoped_ptr.h" +#include "base/test/test_timeouts.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace base { + +class DispatchSourceMachTest : public testing::Test { + public: + void SetUp() override { + mach_port_t port = MACH_PORT_NULL; + ASSERT_EQ(KERN_SUCCESS, mach_port_allocate(mach_task_self(), + MACH_PORT_RIGHT_RECEIVE, &port)); + receive_right_.reset(port); + + ASSERT_EQ(KERN_SUCCESS, mach_port_insert_right(mach_task_self(), port, + port, MACH_MSG_TYPE_MAKE_SEND)); + send_right_.reset(port); + } + + mach_port_t GetPort() { return receive_right_.get(); } + + void WaitForSemaphore(dispatch_semaphore_t semaphore) { + dispatch_semaphore_wait(semaphore, dispatch_time( + DISPATCH_TIME_NOW, + TestTimeouts::action_timeout().InSeconds() * NSEC_PER_SEC)); + } + + private: + base::mac::ScopedMachReceiveRight receive_right_; + base::mac::ScopedMachSendRight send_right_; +}; + +TEST_F(DispatchSourceMachTest, ReceiveAfterResume) { + dispatch_semaphore_t signal = dispatch_semaphore_create(0); + mach_port_t port = GetPort(); + + bool __block did_receive = false; + DispatchSourceMach source("org.chromium.base.test.ReceiveAfterResume", + port, ^{ + mach_msg_empty_rcv_t msg = {{0}}; + msg.header.msgh_size = sizeof(msg); + msg.header.msgh_local_port = port; + mach_msg_receive(&msg.header); + did_receive = true; + + dispatch_semaphore_signal(signal); + }); + + mach_msg_empty_send_t msg = {{0}}; + msg.header.msgh_size = sizeof(msg); + msg.header.msgh_remote_port = port; + msg.header.msgh_bits = MACH_MSGH_BITS_REMOTE(MACH_MSG_TYPE_COPY_SEND); + ASSERT_EQ(KERN_SUCCESS, mach_msg_send(&msg.header)); + + EXPECT_FALSE(did_receive); + + source.Resume(); + + WaitForSemaphore(signal); + dispatch_release(signal); + + EXPECT_TRUE(did_receive); +} + +TEST_F(DispatchSourceMachTest, NoMessagesAfterDestruction) { + mach_port_t port = GetPort(); + + scoped_ptr<int> count(new int(0)); + int* __block count_ptr = count.get(); + + scoped_ptr<DispatchSourceMach> source(new DispatchSourceMach( + "org.chromium.base.test.NoMessagesAfterDestruction", + port, ^{ + mach_msg_empty_rcv_t msg = {{0}}; + msg.header.msgh_size = sizeof(msg); + msg.header.msgh_local_port = port; + mach_msg_receive(&msg.header); + LOG(INFO) << "Receieve " << *count_ptr; + ++(*count_ptr); + })); + source->Resume(); + + dispatch_queue_t queue = + dispatch_queue_create("org.chromium.base.test.MessageSend", NULL); + dispatch_semaphore_t signal = dispatch_semaphore_create(0); + for (int i = 0; i < 30; ++i) { + dispatch_async(queue, ^{ + mach_msg_empty_send_t msg = {{0}}; + msg.header.msgh_size = sizeof(msg); + msg.header.msgh_remote_port = port; + msg.header.msgh_bits = + MACH_MSGH_BITS_REMOTE(MACH_MSG_TYPE_COPY_SEND); + mach_msg_send(&msg.header); + }); + + // After sending five messages, shut down the source and taint the + // pointer the handler dereferences. The test will crash if |count_ptr| + // is being used after "free". + if (i == 5) { + scoped_ptr<DispatchSourceMach>* source_ptr = &source; + dispatch_async(queue, ^{ + source_ptr->reset(); + count_ptr = reinterpret_cast<int*>(0xdeaddead); + dispatch_semaphore_signal(signal); + }); + } + } + + WaitForSemaphore(signal); + dispatch_release(signal); + + dispatch_release(queue); +} + +} // namespace base
diff --git a/mac/memory_pressure_monitor_mac.h b/mac/memory_pressure_monitor_mac.h index aebdd65..8dc2622 100644 --- a/mac/memory_pressure_monitor_mac.h +++ b/mac/memory_pressure_monitor_mac.h
@@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef BASE_MEMORY_MEMORY_PRESSURE_MONITOR_MAC_H_ -#define BASE_MEMORY_MEMORY_PRESSURE_MONITOR_MAC_H_ +#ifndef BASE_MAC_MEMORY_PRESSURE_MONITOR_MAC_H_ +#define BASE_MAC_MEMORY_PRESSURE_MONITOR_MAC_H_ #include <dispatch/dispatch.h> @@ -59,4 +59,4 @@ } // namespace base -#endif // BASE_MEMORY_MEMORY_PRESSURE_MONITOR_MAC_H_ +#endif // BASE_MAC_MEMORY_PRESSURE_MONITOR_MAC_H_
diff --git a/mac/sdk_forward_declarations.h b/mac/sdk_forward_declarations.h index d7be320..d70a6aa 100644 --- a/mac/sdk_forward_declarations.h +++ b/mac/sdk_forward_declarations.h
@@ -244,6 +244,8 @@ BASE_EXPORT extern NSString* const NSWindowDidChangeBackingPropertiesNotification; BASE_EXPORT extern NSString* const CBAdvertisementDataServiceDataKey; +BASE_EXPORT extern NSString* const + NSPreferredScrollerStyleDidChangeNotification; #endif // MAC_OS_X_VERSION_10_7 #if !defined(MAC_OS_X_VERSION_10_9) || \
diff --git a/mac/sdk_forward_declarations.mm b/mac/sdk_forward_declarations.mm index 347e87c..2e4b2d9 100644 --- a/mac/sdk_forward_declarations.mm +++ b/mac/sdk_forward_declarations.mm
@@ -22,6 +22,9 @@ @"NSWindowDidChangeBackingPropertiesNotification"; NSString* const CBAdvertisementDataServiceDataKey = @"kCBAdvDataServiceData"; + +NSString* const NSPreferredScrollerStyleDidChangeNotification = + @"NSPreferredScrollerStyleDidChangeNotification"; #endif // MAC_OS_X_VERSION_10_7 #if !defined(MAC_OS_X_VERSION_10_9) || \
diff --git a/macros.h b/macros.h index 3ea576c..d904328 100644 --- a/macros.h +++ b/macros.h
@@ -52,18 +52,8 @@ // This template function declaration is used in defining arraysize. // Note that the function doesn't need an implementation, as we only // use its type. -template <typename T, size_t N> -char (&ArraySizeHelper(T (&array)[N]))[N]; - -// That gcc wants both of these prototypes seems mysterious. VC, for -// its part, can't decide which to use (another mystery). Matching of -// template overloads: the final frontier. -#ifndef _MSC_VER -template <typename T, size_t N> -char (&ArraySizeHelper(const T (&array)[N]))[N]; -#endif - -#define arraysize(array) (sizeof(::ArraySizeHelper(array))) +template <typename T, size_t N> char (&ArraySizeHelper(T (&array)[N]))[N]; +#define arraysize(array) (sizeof(ArraySizeHelper(array))) // Use implicit_cast as a safe version of static_cast or const_cast
diff --git a/md5_unittest.cc b/md5_unittest.cc index 8d817e9..08c99f1 100644 --- a/md5_unittest.cc +++ b/md5_unittest.cc
@@ -246,7 +246,7 @@ EXPECT_TRUE(!memcmp(&header_digest, &check_header_digest, sizeof(header_digest))); EXPECT_TRUE(!memcmp(&digest, &check_full_digest, sizeof(digest))); - EXPECT_FALSE(!memcmp(&digest, &header_digest, sizeof(digest))); + EXPECT_TRUE(memcmp(&digest, &header_digest, sizeof(digest))); } } // namespace base
diff --git a/memory/memory_pressure_monitor.h b/memory/memory_pressure_monitor.h index 9aeeaca..50260c4 100644 --- a/memory/memory_pressure_monitor.h +++ b/memory/memory_pressure_monitor.h
@@ -20,6 +20,8 @@ public: using MemoryPressureLevel = base::MemoryPressureListener::MemoryPressureLevel; + virtual ~MemoryPressureMonitor(); + // Return the singleton MemoryPressureMonitor. static MemoryPressureMonitor* Get(); @@ -28,7 +30,6 @@ protected: MemoryPressureMonitor(); - virtual ~MemoryPressureMonitor(); private: DISALLOW_COPY_AND_ASSIGN(MemoryPressureMonitor);
diff --git a/message_loop/incoming_task_queue.cc b/message_loop/incoming_task_queue.cc index 5e9a461..c1ce939 100644 --- a/message_loop/incoming_task_queue.cc +++ b/message_loop/incoming_task_queue.cc
@@ -44,8 +44,7 @@ message_loop_(message_loop), next_sequence_num_(0), message_loop_scheduled_(false), - always_schedule_work_(AlwaysNotifyPump(message_loop_->type())), - is_ready_for_scheduling_(false) { + always_schedule_work_(AlwaysNotifyPump(message_loop_->type())) { } bool IncomingTaskQueue::AddToIncomingQueue( @@ -110,15 +109,6 @@ message_loop_ = NULL; } -void IncomingTaskQueue::StartScheduling() { - AutoLock lock(incoming_queue_lock_); - DCHECK(!is_ready_for_scheduling_); - DCHECK(!message_loop_scheduled_); - is_ready_for_scheduling_ = true; - if (!incoming_queue_.empty()) - ScheduleWork(); -} - IncomingTaskQueue::~IncomingTaskQueue() { // Verify that WillDestroyCurrentMessageLoop() has been called. DCHECK(!message_loop_); @@ -158,25 +148,19 @@ incoming_queue_.push(*pending_task); pending_task->task.Reset(); - if (is_ready_for_scheduling_ && - (always_schedule_work_ || (!message_loop_scheduled_ && was_empty))) { - ScheduleWork(); + if (always_schedule_work_ || (!message_loop_scheduled_ && was_empty)) { + // Wake up the message loop. + message_loop_->ScheduleWork(); + // After we've scheduled the message loop, we do not need to do so again + // until we know it has processed all of the work in our queue and is + // waiting for more work again. The message loop will always attempt to + // reload from the incoming queue before waiting again so we clear this flag + // in ReloadWorkQueue(). + message_loop_scheduled_ = true; } return true; } -void IncomingTaskQueue::ScheduleWork() { - DCHECK(is_ready_for_scheduling_); - // Wake up the message loop. - message_loop_->ScheduleWork(); - // After we've scheduled the message loop, we do not need to do so again - // until we know it has processed all of the work in our queue and is - // waiting for more work again. The message loop will always attempt to - // reload from the incoming queue before waiting again so we clear this flag - // in ReloadWorkQueue(). - message_loop_scheduled_ = true; -} - } // namespace internal } // namespace base
diff --git a/message_loop/incoming_task_queue.h b/message_loop/incoming_task_queue.h index 7dd1e82..72e1f30 100644 --- a/message_loop/incoming_task_queue.h +++ b/message_loop/incoming_task_queue.h
@@ -53,10 +53,6 @@ // Disconnects |this| from the parent message loop. void WillDestroyCurrentMessageLoop(); - // This should be called when the message loop becomes ready for - // scheduling work. - void StartScheduling(); - private: friend class RefCountedThreadSafe<IncomingTaskQueue>; virtual ~IncomingTaskQueue(); @@ -70,9 +66,6 @@ // does not retain |pending_task->task| beyond this function call. bool PostPendingTask(PendingTask* pending_task); - // Wakes up the message loop and schedules work. - void ScheduleWork(); - // Number of tasks that require high resolution timing. This value is kept // so that ReloadWorkQueue() completes in constant time. int high_res_task_count_; @@ -99,9 +92,6 @@ // if the incoming queue was not empty. const bool always_schedule_work_; - // False until StartScheduling() is called. - bool is_ready_for_scheduling_; - DISALLOW_COPY_AND_ASSIGN(IncomingTaskQueue); };
diff --git a/message_loop/message_loop.cc b/message_loop/message_loop.cc index 4222c77..eb0f968 100644 --- a/message_loop/message_loop.cc +++ b/message_loop/message_loop.cc
@@ -100,10 +100,6 @@ } #endif // !defined(OS_NACL_SFI) -scoped_ptr<MessagePump> ReturnPump(scoped_ptr<MessagePump> pump) { - return pump; -} - } // namespace //------------------------------------------------------------------------------ @@ -120,19 +116,41 @@ //------------------------------------------------------------------------------ MessageLoop::MessageLoop(Type type) - : MessageLoop(type, MessagePumpFactoryCallback()) { - BindToCurrentThread(); + : type_(type), +#if defined(OS_WIN) + pending_high_res_tasks_(0), + in_high_res_mode_(false), +#endif + nestable_tasks_allowed_(true), +#if defined(OS_WIN) + os_modal_loop_(false), +#endif // OS_WIN + message_histogram_(NULL), + run_loop_(NULL) { + Init(); + + pump_ = CreateMessagePumpForType(type).Pass(); } MessageLoop::MessageLoop(scoped_ptr<MessagePump> pump) - : MessageLoop(TYPE_CUSTOM, Bind(&ReturnPump, Passed(&pump))) { - BindToCurrentThread(); + : pump_(pump.Pass()), + type_(TYPE_CUSTOM), +#if defined(OS_WIN) + pending_high_res_tasks_(0), + in_high_res_mode_(false), +#endif + nestable_tasks_allowed_(true), +#if defined(OS_WIN) + os_modal_loop_(false), +#endif // OS_WIN + message_histogram_(NULL), + run_loop_(NULL) { + DCHECK(pump_.get()); + Init(); } MessageLoop::~MessageLoop() { - // current() could be NULL if this message loop is destructed before it is - // bound to a thread. - DCHECK(current() == this || !current()); + DCHECK_EQ(this, current()); // iOS just attaches to the loop, it doesn't Run it. // TODO(stuartmorgan): Consider wiring up a Detach(). @@ -281,13 +299,11 @@ } void MessageLoop::Run() { - DCHECK(pump_); RunLoop run_loop; run_loop.Run(); } void MessageLoop::RunUntilIdle() { - DCHECK(pump_); RunLoop run_loop; run_loop.RunUntilIdle(); } @@ -367,43 +383,13 @@ //------------------------------------------------------------------------------ -scoped_ptr<MessageLoop> MessageLoop::CreateUnbound( - Type type, MessagePumpFactoryCallback pump_factory) { - return make_scoped_ptr(new MessageLoop(type, pump_factory)); -} - -MessageLoop::MessageLoop(Type type, MessagePumpFactoryCallback pump_factory) - : type_(type), -#if defined(OS_WIN) - pending_high_res_tasks_(0), - in_high_res_mode_(false), -#endif - nestable_tasks_allowed_(true), -#if defined(OS_WIN) - os_modal_loop_(false), -#endif // OS_WIN - pump_factory_(pump_factory), - message_histogram_(NULL), - run_loop_(NULL), - incoming_task_queue_(new internal::IncomingTaskQueue(this)), - message_loop_proxy_( - new internal::MessageLoopProxyImpl(incoming_task_queue_)) { - // If type is TYPE_CUSTOM non-null pump_factory must be given. - DCHECK_EQ(type_ == TYPE_CUSTOM, !pump_factory_.is_null()); -} - -void MessageLoop::BindToCurrentThread() { - DCHECK(!pump_); - if (!pump_factory_.is_null()) - pump_ = pump_factory_.Run(); - else - pump_ = CreateMessagePumpForType(type_); - +void MessageLoop::Init() { DCHECK(!current()) << "should only have one message loop per thread"; lazy_tls_ptr.Pointer()->Set(this); - incoming_task_queue_->StartScheduling(); - message_loop_proxy_->BindToCurrentThread(); + incoming_task_queue_ = new internal::IncomingTaskQueue(this); + message_loop_proxy_ = + new internal::MessageLoopProxyImpl(incoming_task_queue_); thread_task_runner_handle_.reset( new ThreadTaskRunnerHandle(message_loop_proxy_)); }
diff --git a/message_loop/message_loop.h b/message_loop/message_loop.h index f2f89d0..fd7596a 100644 --- a/message_loop/message_loop.h +++ b/message_loop/message_loop.h
@@ -114,8 +114,7 @@ explicit MessageLoop(Type type = TYPE_DEFAULT); // Creates a TYPE_CUSTOM MessageLoop with the supplied MessagePump, which must // be non-NULL. - explicit MessageLoop(scoped_ptr<MessagePump> pump); - + explicit MessageLoop(scoped_ptr<base::MessagePump> pump); ~MessageLoop() override; // Returns the MessageLoop object for the current thread, or null if none. @@ -395,6 +394,10 @@ // Returns true if the message loop is "idle". Provided for testing. bool IsIdleForTesting(); + // Wakes up the message pump. Can be called on any thread. The caller is + // responsible for synchronizing ScheduleWork() calls. + void ScheduleWork(); + // Returns the TaskAnnotator which is used to add debug information to posted // tasks. debug::TaskAnnotator* task_annotator() { return &task_annotator_; } @@ -408,33 +411,9 @@ private: friend class RunLoop; - friend class internal::IncomingTaskQueue; - friend class ScheduleWorkTest; - friend class Thread; - using MessagePumpFactoryCallback = Callback<scoped_ptr<MessagePump>()>; - - // Creates a MessageLoop without binding to a thread. - // If |type| is TYPE_CUSTOM non-null |pump_factory| must be also given - // to create a message pump for this message loop. Otherwise a default - // message pump for the |type| is created. - // - // It is valid to call this to create a new message loop on one thread, - // and then pass it to the thread where the message loop actually runs. - // The message loop's BindToCurrentThread() method must be called on the - // thread the message loop runs on, before calling Run(). - // Before BindToCurrentThread() is called only Post*Task() functions can - // be called on the message loop. - scoped_ptr<MessageLoop> CreateUnbound( - Type type, - MessagePumpFactoryCallback pump_factory); - - // Common private constructor. Other constructors delegate the initialization - // to this constructor. - MessageLoop(Type type, MessagePumpFactoryCallback pump_factory); - - // Configure various members and bind this message loop to the current thread. - void BindToCurrentThread(); + // Configures various members for the two constructors. + void Init(); // Invokes the actual run loop using the message pump. void RunHandler(); @@ -458,10 +437,6 @@ // empty. void ReloadWorkQueue(); - // Wakes up the message pump. Can be called on any thread. The caller is - // responsible for synchronizing ScheduleWork() calls. - void ScheduleWork(); - // Start recording histogram info about events and action IF it was enabled // and IF the statistics recorder can accept a registration of our histogram. void StartHistogrammer(); @@ -515,10 +490,6 @@ bool os_modal_loop_; #endif - // pump_factory_.Run() is called to create a message pump for this loop - // if type_ is TYPE_CUSTOM and pump_ is null. - MessagePumpFactoryCallback pump_factory_; - std::string thread_name_; // A profiling histogram showing the counts of various messages and events. HistogramBase* message_histogram_;
diff --git a/message_loop/message_loop_proxy_impl.cc b/message_loop/message_loop_proxy_impl.cc index 0da21a0..b7abca3 100644 --- a/message_loop/message_loop_proxy_impl.cc +++ b/message_loop/message_loop_proxy_impl.cc
@@ -15,12 +15,7 @@ MessageLoopProxyImpl::MessageLoopProxyImpl( scoped_refptr<IncomingTaskQueue> incoming_queue) : incoming_queue_(incoming_queue), - valid_thread_id_(kInvalidThreadId) { -} - -void MessageLoopProxyImpl::BindToCurrentThread() { - DCHECK_EQ(kInvalidThreadId, valid_thread_id_); - valid_thread_id_ = PlatformThread::CurrentId(); + valid_thread_id_(PlatformThread::CurrentId()) { } bool MessageLoopProxyImpl::PostDelayedTask(
diff --git a/message_loop/message_loop_proxy_impl.h b/message_loop/message_loop_proxy_impl.h index c477320..0fe629f 100644 --- a/message_loop/message_loop_proxy_impl.h +++ b/message_loop/message_loop_proxy_impl.h
@@ -24,9 +24,6 @@ explicit MessageLoopProxyImpl( scoped_refptr<IncomingTaskQueue> incoming_queue); - // Initialize this message loop proxy on the current thread. - void BindToCurrentThread(); - // MessageLoopProxy implementation bool PostDelayedTask(const tracked_objects::Location& from_here, const base::Closure& task,
diff --git a/message_loop/message_pump_perftest.cc b/message_loop/message_pump_perftest.cc index 0bf3d0c..b3e5604 100644 --- a/message_loop/message_pump_perftest.cc +++ b/message_loop/message_pump_perftest.cc
@@ -20,6 +20,7 @@ #endif namespace base { +namespace { class ScheduleWorkTest : public testing::Test { public: @@ -223,6 +224,9 @@ } #endif +static void DoNothing() { +} + class FakeMessagePump : public MessagePump { public: FakeMessagePump() {} @@ -285,4 +289,5 @@ Run(1000, 100); } +} // namespace } // namespace base
diff --git a/metrics/histogram_base.cc b/metrics/histogram_base.cc index f09c84e..de34c79 100644 --- a/metrics/histogram_base.cc +++ b/metrics/histogram_base.cc
@@ -111,8 +111,8 @@ root.SetInteger("count", count); root.SetDouble("sum", static_cast<double>(sum)); root.SetInteger("flags", flags()); - root.Set("params", parameters.release()); - root.Set("buckets", buckets.release()); + root.Set("params", parameters.Pass()); + root.Set("buckets", buckets.Pass()); root.SetInteger("pid", GetCurrentProcId()); serializer.Serialize(root); }
diff --git a/metrics/statistics_recorder.cc b/metrics/statistics_recorder.cc index 39ecc30..fb069a5 100644 --- a/metrics/statistics_recorder.cc +++ b/metrics/statistics_recorder.cc
@@ -286,8 +286,6 @@ // static void StatisticsRecorder::DumpHistogramsToVlog(void* instance) { - DCHECK(VLOG_IS_ON(1)); - string output; StatisticsRecorder::WriteGraph(std::string(), &output); VLOG(1) << output;
diff --git a/observer_list.h b/observer_list.h index 9ea344d..fb6f026 100644 --- a/observer_list.h +++ b/observer_list.h
@@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef BASE_OBSERVER_LIST_H__ -#define BASE_OBSERVER_LIST_H__ +#ifndef BASE_OBSERVER_LIST_H_ +#define BASE_OBSERVER_LIST_H_ #include <algorithm> #include <limits> @@ -240,4 +240,4 @@ } \ } while (0) -#endif // BASE_OBSERVER_LIST_H__ +#endif // BASE_OBSERVER_LIST_H_
diff --git a/pickle.h b/pickle.h index e6b9d81..9589e2a 100644 --- a/pickle.h +++ b/pickle.h
@@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef BASE_PICKLE_H__ -#define BASE_PICKLE_H__ +#ifndef BASE_PICKLE_H_ +#define BASE_PICKLE_H_ #include <string> @@ -304,4 +304,4 @@ FRIEND_TEST_ALL_PREFIXES(PickleTest, FindNextOverflow); }; -#endif // BASE_PICKLE_H__ +#endif // BASE_PICKLE_H_
diff --git a/prefs/json_pref_store.cc b/prefs/json_pref_store.cc index 416e43f..da545b8 100644 --- a/prefs/json_pref_store.cc +++ b/prefs/json_pref_store.cc
@@ -150,17 +150,10 @@ const base::FilePath& filename, const scoped_refptr<base::SequencedTaskRunner>& sequenced_task_runner, scoped_ptr<PrefFilter> pref_filter) - : path_(filename), - sequenced_task_runner_(sequenced_task_runner), - prefs_(new base::DictionaryValue()), - read_only_(false), - writer_(filename, sequenced_task_runner), - pref_filter_(pref_filter.Pass()), - initialized_(false), - filtering_in_progress_(false), - read_error_(PREF_READ_ERROR_NONE), - write_count_histogram_(writer_.commit_interval(), path_) { - DCHECK(!path_.empty()); + : JsonPrefStore(filename, + base::FilePath(), + sequenced_task_runner, + pref_filter.Pass()) { } JsonPrefStore::JsonPrefStore( @@ -177,6 +170,7 @@ pref_filter_(pref_filter.Pass()), initialized_(false), filtering_in_progress_(false), + pending_lossy_write_(false), read_error_(PREF_READ_ERROR_NONE), write_count_histogram_(writer_.commit_interval(), path_) { DCHECK(!path_.empty()); @@ -226,7 +220,9 @@ return prefs_->Get(key, result); } -void JsonPrefStore::SetValue(const std::string& key, base::Value* value) { +void JsonPrefStore::SetValue(const std::string& key, + base::Value* value, + uint32 flags) { DCHECK(CalledOnValidThread()); DCHECK(value); @@ -234,13 +230,14 @@ base::Value* old_value = NULL; prefs_->Get(key, &old_value); if (!old_value || !value->Equals(old_value)) { - prefs_->Set(key, new_value.release()); - ReportValueChanged(key); + prefs_->Set(key, new_value.Pass()); + ReportValueChanged(key, flags); } } void JsonPrefStore::SetValueSilently(const std::string& key, - base::Value* value) { + base::Value* value, + uint32 flags) { DCHECK(CalledOnValidThread()); DCHECK(value); @@ -248,25 +245,23 @@ base::Value* old_value = NULL; prefs_->Get(key, &old_value); if (!old_value || !value->Equals(old_value)) { - prefs_->Set(key, new_value.release()); - if (!read_only_) - writer_.ScheduleWrite(this); + prefs_->Set(key, new_value.Pass()); + ScheduleWrite(flags); } } -void JsonPrefStore::RemoveValue(const std::string& key) { +void JsonPrefStore::RemoveValue(const std::string& key, uint32 flags) { DCHECK(CalledOnValidThread()); if (prefs_->RemovePath(key, NULL)) - ReportValueChanged(key); + ReportValueChanged(key, flags); } -void JsonPrefStore::RemoveValueSilently(const std::string& key) { +void JsonPrefStore::RemoveValueSilently(const std::string& key, uint32 flags) { DCHECK(CalledOnValidThread()); prefs_->RemovePath(key, NULL); - if (!read_only_) - writer_.ScheduleWrite(this); + ScheduleWrite(flags); } bool JsonPrefStore::ReadOnly() const { @@ -306,11 +301,16 @@ void JsonPrefStore::CommitPendingWrite() { DCHECK(CalledOnValidThread()); + // Schedule a write for any lossy writes that are outstanding to ensure that + // they get flushed when this function is called. + if (pending_lossy_write_) + writer_.ScheduleWrite(this); + if (writer_.HasPendingWrite() && !read_only_) writer_.DoScheduledWrite(); } -void JsonPrefStore::ReportValueChanged(const std::string& key) { +void JsonPrefStore::ReportValueChanged(const std::string& key, uint32 flags) { DCHECK(CalledOnValidThread()); if (pref_filter_) @@ -318,8 +318,7 @@ FOR_EACH_OBSERVER(PrefStore::Observer, observers_, OnPrefValueChanged(key)); - if (!read_only_) - writer_.ScheduleWrite(this); + ScheduleWrite(flags); } void JsonPrefStore::RegisterOnNextSuccessfulWriteCallback( @@ -398,6 +397,8 @@ bool JsonPrefStore::SerializeData(std::string* output) { DCHECK(CalledOnValidThread()); + pending_lossy_write_ = false; + write_count_histogram_.RecordWriteOccured(); if (pref_filter_) @@ -429,8 +430,8 @@ initialized_ = true; - if (schedule_write && !read_only_) - writer_.ScheduleWrite(this); + if (schedule_write) + ScheduleWrite(DEFAULT_PREF_WRITE_FLAGS); if (error_delegate_ && read_error_ != PREF_READ_ERROR_NONE) error_delegate_->OnError(read_error_); @@ -442,6 +443,16 @@ return; } +void JsonPrefStore::ScheduleWrite(uint32 flags) { + if (read_only_) + return; + + if (flags & LOSSY_PREF_WRITE_FLAG) + pending_lossy_write_ = true; + else + writer_.ScheduleWrite(this); +} + // NOTE: This value should NOT be changed without renaming the histogram // otherwise it will create incompatible buckets. const int32_t
diff --git a/prefs/json_pref_store.h b/prefs/json_pref_store.h index 1badcf2..ef260eb 100644 --- a/prefs/json_pref_store.h +++ b/prefs/json_pref_store.h
@@ -29,6 +29,7 @@ class DictionaryValue; class FilePath; class HistogramBase; +class JsonPrefStoreLossyWriteTest; class SequencedTaskRunner; class SequencedWorkerPool; class Value; @@ -82,9 +83,13 @@ // PersistentPrefStore overrides: bool GetMutableValue(const std::string& key, base::Value** result) override; - void SetValue(const std::string& key, base::Value* value) override; - void SetValueSilently(const std::string& key, base::Value* value) override; - void RemoveValue(const std::string& key) override; + void SetValue(const std::string& key, + base::Value* value, + uint32 flags) override; + void SetValueSilently(const std::string& key, + base::Value* value, + uint32 flags) override; + void RemoveValue(const std::string& key, uint32 flags) override; bool ReadOnly() const override; PrefReadError GetReadError() const override; // Note this method may be asynchronous if this instance has a |pref_filter_| @@ -93,11 +98,11 @@ PrefReadError ReadPrefs() override; void ReadPrefsAsync(ReadErrorDelegate* error_delegate) override; void CommitPendingWrite() override; - void ReportValueChanged(const std::string& key) override; + void ReportValueChanged(const std::string& key, uint32 flags) override; // Just like RemoveValue(), but doesn't notify observers. Used when doing some // cleanup that shouldn't otherwise alert observers. - void RemoveValueSilently(const std::string& key); + void RemoveValueSilently(const std::string& key, uint32 flags); // Registers |on_next_successful_write| to be called once, on the next // successful write event of |writer_|. @@ -161,6 +166,7 @@ WriteCountHistogramTestMultiplePeriods); FRIEND_TEST_ALL_PREFIXES(base::JsonPrefStoreTest, WriteCountHistogramTestPeriodWithGaps); + friend class base::JsonPrefStoreLossyWriteTest; ~JsonPrefStore() override; @@ -186,6 +192,10 @@ scoped_ptr<base::DictionaryValue> prefs, bool schedule_write); + // Schedule a write with the file writer as long as |flags| doesn't contain + // WriteablePrefStore::LOSSY_PREF_WRITE_FLAG. + void ScheduleWrite(uint32 flags); + const base::FilePath path_; const base::FilePath alternate_path_; const scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner_; @@ -204,6 +214,7 @@ bool initialized_; bool filtering_in_progress_; + bool pending_lossy_write_; PrefReadError read_error_; std::set<std::string> keys_need_empty_value_;
diff --git a/prefs/json_pref_store_unittest.cc b/prefs/json_pref_store_unittest.cc index 728a57d..67a8adb 100644 --- a/prefs/json_pref_store_unittest.cc +++ b/prefs/json_pref_store_unittest.cc
@@ -108,9 +108,7 @@ void TearDown() override { // Make sure all pending tasks have been processed (e.g., deleting the // JsonPrefStore may post write tasks). - message_loop_.task_runner()->PostTask(FROM_HERE, - MessageLoop::QuitWhenIdleClosure()); - message_loop_.Run(); + RunLoop().RunUntilIdle(); } // The path to temporary directory used to contain the test operations. @@ -127,7 +125,7 @@ // Test fallback behavior for a nonexistent file. TEST_F(JsonPrefStoreTest, NonExistentFile) { - base::FilePath bogus_input_file = data_dir_.AppendASCII("read.txt"); + base::FilePath bogus_input_file = temp_dir_.path().AppendASCII("read.txt"); ASSERT_FALSE(PathExists(bogus_input_file)); scoped_refptr<JsonPrefStore> pref_store = new JsonPrefStore( bogus_input_file, message_loop_.task_runner(), scoped_ptr<PrefFilter>()); @@ -138,9 +136,9 @@ // Test fallback behavior for a nonexistent file and alternate file. TEST_F(JsonPrefStoreTest, NonExistentFileAndAlternateFile) { - base::FilePath bogus_input_file = data_dir_.AppendASCII("read.txt"); + base::FilePath bogus_input_file = temp_dir_.path().AppendASCII("read.txt"); base::FilePath bogus_alternate_input_file = - data_dir_.AppendASCII("read_alternate.txt"); + temp_dir_.path().AppendASCII("read_alternate.txt"); ASSERT_FALSE(PathExists(bogus_input_file)); ASSERT_FALSE(PathExists(bogus_alternate_input_file)); scoped_refptr<JsonPrefStore> pref_store = @@ -194,7 +192,8 @@ EXPECT_EQ(base::FilePath::StringType(FILE_PATH_LITERAL("/usr/local/")), path); base::FilePath some_path(FILE_PATH_LITERAL("/usr/sbin/")); - pref_store->SetValue(kSomeDirectory, new StringValue(some_path.value())); + pref_store->SetValue(kSomeDirectory, new StringValue(some_path.value()), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); EXPECT_TRUE(pref_store->GetValue(kSomeDirectory, &actual)); EXPECT_TRUE(actual->GetAsString(&path)); EXPECT_EQ(some_path.value(), path); @@ -205,7 +204,8 @@ EXPECT_TRUE(actual->GetAsBoolean(&boolean)); EXPECT_TRUE(boolean); - pref_store->SetValue(kNewWindowsInTabs, new FundamentalValue(false)); + pref_store->SetValue(kNewWindowsInTabs, new FundamentalValue(false), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); EXPECT_TRUE(pref_store->GetValue(kNewWindowsInTabs, &actual)); EXPECT_TRUE(actual->GetAsBoolean(&boolean)); EXPECT_FALSE(boolean); @@ -214,13 +214,15 @@ int integer = 0; EXPECT_TRUE(actual->GetAsInteger(&integer)); EXPECT_EQ(20, integer); - pref_store->SetValue(kMaxTabs, new FundamentalValue(10)); + pref_store->SetValue(kMaxTabs, new FundamentalValue(10), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); EXPECT_TRUE(pref_store->GetValue(kMaxTabs, &actual)); EXPECT_TRUE(actual->GetAsInteger(&integer)); EXPECT_EQ(10, integer); pref_store->SetValue(kLongIntPref, - new StringValue(base::Int64ToString(214748364842LL))); + new StringValue(base::Int64ToString(214748364842LL)), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); EXPECT_TRUE(pref_store->GetValue(kLongIntPref, &actual)); EXPECT_TRUE(actual->GetAsString(&string_value)); int64 value; @@ -310,12 +312,14 @@ pref_file, message_loop_.task_runner(), scoped_ptr<PrefFilter>()); // Set some keys with empty values. - pref_store->SetValue("list", new base::ListValue); - pref_store->SetValue("dict", new base::DictionaryValue); + pref_store->SetValue("list", new base::ListValue, + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); + pref_store->SetValue("dict", new base::DictionaryValue, + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); // Write to file. pref_store->CommitPendingWrite(); - MessageLoop::current()->RunUntilIdle(); + RunLoop().RunUntilIdle(); // Reload. pref_store = new JsonPrefStore(pref_file, message_loop_.task_runner(), @@ -341,9 +345,11 @@ base::DictionaryValue* dict = new base::DictionaryValue; dict->SetString("key", "value"); - pref_store->SetValue("dict", dict); + pref_store->SetValue("dict", dict, + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); - pref_store->RemoveValue("dict.key"); + pref_store->RemoveValue("dict.key", + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); const base::Value* retrieved_dict = NULL; bool has_dict = pref_store->GetValue("dict", &retrieved_dict); @@ -352,7 +358,7 @@ // Tests asynchronous reading of the file when there is no file. TEST_F(JsonPrefStoreTest, AsyncNonExistingFile) { - base::FilePath bogus_input_file = data_dir_.AppendASCII("read.txt"); + base::FilePath bogus_input_file = temp_dir_.path().AppendASCII("read.txt"); ASSERT_FALSE(PathExists(bogus_input_file)); scoped_refptr<JsonPrefStore> pref_store = new JsonPrefStore( bogus_input_file, message_loop_.task_runner(), scoped_ptr<PrefFilter>()); @@ -801,4 +807,125 @@ ASSERT_EQ(6, samples->TotalCount()); } +class JsonPrefStoreLossyWriteTest : public JsonPrefStoreTest { + protected: + void SetUp() override { + JsonPrefStoreTest::SetUp(); + test_file_ = temp_dir_.path().AppendASCII("test.json"); + } + + // Creates a JsonPrefStore with the given |file_writer|. + scoped_refptr<JsonPrefStore> CreatePrefStore() { + return new JsonPrefStore(test_file_, message_loop_.task_runner(), + scoped_ptr<PrefFilter>()); + } + + // Return the ImportantFileWriter for a given JsonPrefStore. + ImportantFileWriter* GetImportantFileWriter( + scoped_refptr<JsonPrefStore> pref_store) { + return &(pref_store->writer_); + } + + // Get the contents of kTestFile. Pumps the message loop before returning the + // result. + std::string GetTestFileContents() { + RunLoop().RunUntilIdle(); + std::string file_contents; + ReadFileToString(test_file_, &file_contents); + return file_contents; + } + + private: + base::FilePath test_file_; +}; + +TEST_F(JsonPrefStoreLossyWriteTest, LossyWriteBasic) { + scoped_refptr<JsonPrefStore> pref_store = CreatePrefStore(); + ImportantFileWriter* file_writer = GetImportantFileWriter(pref_store); + + // Set a normal pref and check that it gets scheduled to be written. + ASSERT_FALSE(file_writer->HasPendingWrite()); + pref_store->SetValue("normal", new base::StringValue("normal"), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); + ASSERT_TRUE(file_writer->HasPendingWrite()); + file_writer->DoScheduledWrite(); + ASSERT_EQ("{\"normal\":\"normal\"}", GetTestFileContents()); + ASSERT_FALSE(file_writer->HasPendingWrite()); + + // Set a lossy pref and check that it is not scheduled to be written. + // SetValue/RemoveValue. + pref_store->SetValue("lossy", new base::StringValue("lossy"), + WriteablePrefStore::LOSSY_PREF_WRITE_FLAG); + ASSERT_FALSE(file_writer->HasPendingWrite()); + pref_store->RemoveValue("lossy", WriteablePrefStore::LOSSY_PREF_WRITE_FLAG); + ASSERT_FALSE(file_writer->HasPendingWrite()); + + // SetValueSilently/RemoveValueSilently. + pref_store->SetValueSilently("lossy", new base::StringValue("lossy"), + WriteablePrefStore::LOSSY_PREF_WRITE_FLAG); + ASSERT_FALSE(file_writer->HasPendingWrite()); + pref_store->RemoveValueSilently("lossy", + WriteablePrefStore::LOSSY_PREF_WRITE_FLAG); + ASSERT_FALSE(file_writer->HasPendingWrite()); + + // ReportValueChanged. + pref_store->SetValue("lossy", new base::StringValue("lossy"), + WriteablePrefStore::LOSSY_PREF_WRITE_FLAG); + ASSERT_FALSE(file_writer->HasPendingWrite()); + pref_store->ReportValueChanged("lossy", + WriteablePrefStore::LOSSY_PREF_WRITE_FLAG); + ASSERT_FALSE(file_writer->HasPendingWrite()); + + // Call CommitPendingWrite and check that the lossy pref and the normal pref + // are there with the last values set above. + pref_store->CommitPendingWrite(); + ASSERT_FALSE(file_writer->HasPendingWrite()); + ASSERT_EQ("{\"lossy\":\"lossy\",\"normal\":\"normal\"}", + GetTestFileContents()); +} + +TEST_F(JsonPrefStoreLossyWriteTest, LossyWriteMixedLossyFirst) { + scoped_refptr<JsonPrefStore> pref_store = CreatePrefStore(); + ImportantFileWriter* file_writer = GetImportantFileWriter(pref_store); + + // Set a lossy pref and check that it is not scheduled to be written. + ASSERT_FALSE(file_writer->HasPendingWrite()); + pref_store->SetValue("lossy", new base::StringValue("lossy"), + WriteablePrefStore::LOSSY_PREF_WRITE_FLAG); + ASSERT_FALSE(file_writer->HasPendingWrite()); + + // Set a normal pref and check that it is scheduled to be written. + pref_store->SetValue("normal", new base::StringValue("normal"), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); + ASSERT_TRUE(file_writer->HasPendingWrite()); + + // Call DoScheduledWrite and check both prefs get written. + file_writer->DoScheduledWrite(); + ASSERT_EQ("{\"lossy\":\"lossy\",\"normal\":\"normal\"}", + GetTestFileContents()); + ASSERT_FALSE(file_writer->HasPendingWrite()); +} + +TEST_F(JsonPrefStoreLossyWriteTest, LossyWriteMixedLossySecond) { + scoped_refptr<JsonPrefStore> pref_store = CreatePrefStore(); + ImportantFileWriter* file_writer = GetImportantFileWriter(pref_store); + + // Set a normal pref and check that it is scheduled to be written. + ASSERT_FALSE(file_writer->HasPendingWrite()); + pref_store->SetValue("normal", new base::StringValue("normal"), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); + ASSERT_TRUE(file_writer->HasPendingWrite()); + + // Set a lossy pref and check that the write is still scheduled. + pref_store->SetValue("lossy", new base::StringValue("lossy"), + WriteablePrefStore::LOSSY_PREF_WRITE_FLAG); + ASSERT_TRUE(file_writer->HasPendingWrite()); + + // Call DoScheduledWrite and check both prefs get written. + file_writer->DoScheduledWrite(); + ASSERT_EQ("{\"lossy\":\"lossy\",\"normal\":\"normal\"}", + GetTestFileContents()); + ASSERT_FALSE(file_writer->HasPendingWrite()); +} + } // namespace base
diff --git a/prefs/overlay_user_pref_store.cc b/prefs/overlay_user_pref_store.cc index a708bb6..e93dffd 100644 --- a/prefs/overlay_user_pref_store.cc +++ b/prefs/overlay_user_pref_store.cc
@@ -63,34 +63,36 @@ } void OverlayUserPrefStore::SetValue(const std::string& key, - base::Value* value) { + base::Value* value, + uint32 flags) { if (!ShallBeStoredInOverlay(key)) { - underlay_->SetValue(GetUnderlayKey(key), value); + underlay_->SetValue(GetUnderlayKey(key), value, flags); return; } if (overlay_.SetValue(key, value)) - ReportValueChanged(key); + ReportValueChanged(key, flags); } void OverlayUserPrefStore::SetValueSilently(const std::string& key, - base::Value* value) { + base::Value* value, + uint32 flags) { if (!ShallBeStoredInOverlay(key)) { - underlay_->SetValueSilently(GetUnderlayKey(key), value); + underlay_->SetValueSilently(GetUnderlayKey(key), value, flags); return; } overlay_.SetValue(key, value); } -void OverlayUserPrefStore::RemoveValue(const std::string& key) { +void OverlayUserPrefStore::RemoveValue(const std::string& key, uint32 flags) { if (!ShallBeStoredInOverlay(key)) { - underlay_->RemoveValue(GetUnderlayKey(key)); + underlay_->RemoveValue(GetUnderlayKey(key), flags); return; } if (overlay_.RemoveValue(key)) - ReportValueChanged(key); + ReportValueChanged(key, flags); } bool OverlayUserPrefStore::ReadOnly() const { @@ -119,13 +121,14 @@ // We do not write our content intentionally. } -void OverlayUserPrefStore::ReportValueChanged(const std::string& key) { +void OverlayUserPrefStore::ReportValueChanged(const std::string& key, + uint32 flags) { FOR_EACH_OBSERVER(PrefStore::Observer, observers_, OnPrefValueChanged(key)); } void OverlayUserPrefStore::OnPrefValueChanged(const std::string& key) { if (!overlay_.GetValue(GetOverlayKey(key), NULL)) - ReportValueChanged(GetOverlayKey(key)); + ReportValueChanged(GetOverlayKey(key), DEFAULT_PREF_WRITE_FLAGS); } void OverlayUserPrefStore::OnInitializationCompleted(bool succeeded) {
diff --git a/prefs/overlay_user_pref_store.h b/prefs/overlay_user_pref_store.h index 5194a7b..04c309d 100644 --- a/prefs/overlay_user_pref_store.h +++ b/prefs/overlay_user_pref_store.h
@@ -39,15 +39,19 @@ // Methods of PersistentPrefStore. bool GetMutableValue(const std::string& key, base::Value** result) override; - void SetValue(const std::string& key, base::Value* value) override; - void SetValueSilently(const std::string& key, base::Value* value) override; - void RemoveValue(const std::string& key) override; + void SetValue(const std::string& key, + base::Value* value, + uint32 flags) override; + void SetValueSilently(const std::string& key, + base::Value* value, + uint32 flags) override; + void RemoveValue(const std::string& key, uint32 flags) override; bool ReadOnly() const override; PrefReadError GetReadError() const override; PrefReadError ReadPrefs() override; void ReadPrefsAsync(ReadErrorDelegate* delegate) override; void CommitPendingWrite() override; - void ReportValueChanged(const std::string& key) override; + void ReportValueChanged(const std::string& key, uint32 flags) override; // Methods of PrefStore::Observer. void OnPrefValueChanged(const std::string& key) override;
diff --git a/prefs/overlay_user_pref_store_unittest.cc b/prefs/overlay_user_pref_store_unittest.cc index 9836fbf..06b4ec9 100644 --- a/prefs/overlay_user_pref_store_unittest.cc +++ b/prefs/overlay_user_pref_store_unittest.cc
@@ -48,38 +48,47 @@ overlay_->AddObserver(&obs); // Check that underlay first value is reported. - underlay_->SetValue(overlay_key, new FundamentalValue(42)); + underlay_->SetValue(overlay_key, new FundamentalValue(42), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); obs.VerifyAndResetChangedKey(overlay_key); // Check that underlay overwriting is reported. - underlay_->SetValue(overlay_key, new FundamentalValue(43)); + underlay_->SetValue(overlay_key, new FundamentalValue(43), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); obs.VerifyAndResetChangedKey(overlay_key); // Check that overwriting change in overlay is reported. - overlay_->SetValue(overlay_key, new FundamentalValue(44)); + overlay_->SetValue(overlay_key, new FundamentalValue(44), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); obs.VerifyAndResetChangedKey(overlay_key); // Check that hidden underlay change is not reported. - underlay_->SetValue(overlay_key, new FundamentalValue(45)); + underlay_->SetValue(overlay_key, new FundamentalValue(45), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); EXPECT_TRUE(obs.changed_keys.empty()); // Check that overlay remove is reported. - overlay_->RemoveValue(overlay_key); + overlay_->RemoveValue(overlay_key, + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); obs.VerifyAndResetChangedKey(overlay_key); // Check that underlay remove is reported. - underlay_->RemoveValue(overlay_key); + underlay_->RemoveValue(overlay_key, + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); obs.VerifyAndResetChangedKey(overlay_key); // Check respecting of silence. - overlay_->SetValueSilently(overlay_key, new FundamentalValue(46)); + overlay_->SetValueSilently(overlay_key, new FundamentalValue(46), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); EXPECT_TRUE(obs.changed_keys.empty()); overlay_->RemoveObserver(&obs); // Check successful unsubscription. - underlay_->SetValue(overlay_key, new FundamentalValue(47)); - overlay_->SetValue(overlay_key, new FundamentalValue(48)); + underlay_->SetValue(overlay_key, new FundamentalValue(47), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); + overlay_->SetValue(overlay_key, new FundamentalValue(48), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); EXPECT_TRUE(obs.changed_keys.empty()); } @@ -88,7 +97,8 @@ EXPECT_FALSE(overlay_->GetValue(overlay_key, &value)); EXPECT_FALSE(underlay_->GetValue(overlay_key, &value)); - underlay_->SetValue(overlay_key, new FundamentalValue(42)); + underlay_->SetValue(overlay_key, new FundamentalValue(42), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); // Value shines through: EXPECT_TRUE(overlay_->GetValue(overlay_key, &value)); @@ -97,7 +107,8 @@ EXPECT_TRUE(underlay_->GetValue(overlay_key, &value)); EXPECT_TRUE(base::FundamentalValue(42).Equals(value)); - overlay_->SetValue(overlay_key, new FundamentalValue(43)); + overlay_->SetValue(overlay_key, new FundamentalValue(43), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); EXPECT_TRUE(overlay_->GetValue(overlay_key, &value)); EXPECT_TRUE(base::FundamentalValue(43).Equals(value)); @@ -105,7 +116,8 @@ EXPECT_TRUE(underlay_->GetValue(overlay_key, &value)); EXPECT_TRUE(base::FundamentalValue(42).Equals(value)); - overlay_->RemoveValue(overlay_key); + overlay_->RemoveValue(overlay_key, + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); // Value shines through: EXPECT_TRUE(overlay_->GetValue(overlay_key, &value)); @@ -117,7 +129,8 @@ // Check that GetMutableValue does not return the dictionary of the underlay. TEST_F(OverlayUserPrefStoreTest, ModifyDictionaries) { - underlay_->SetValue(overlay_key, new DictionaryValue); + underlay_->SetValue(overlay_key, new DictionaryValue, + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); Value* modify = NULL; EXPECT_TRUE(overlay_->GetMutableValue(overlay_key, &modify)); @@ -146,11 +159,13 @@ const Value* value = NULL; // Check that underlay first value is reported. - underlay_->SetValue(regular_key, new FundamentalValue(42)); + underlay_->SetValue(regular_key, new FundamentalValue(42), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); obs.VerifyAndResetChangedKey(regular_key); // Check that underlay overwriting is reported. - underlay_->SetValue(regular_key, new FundamentalValue(43)); + underlay_->SetValue(regular_key, new FundamentalValue(43), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); obs.VerifyAndResetChangedKey(regular_key); // Check that we get this value from the overlay @@ -158,7 +173,8 @@ EXPECT_TRUE(base::FundamentalValue(43).Equals(value)); // Check that overwriting change in overlay is reported. - overlay_->SetValue(regular_key, new FundamentalValue(44)); + overlay_->SetValue(regular_key, new FundamentalValue(44), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); obs.VerifyAndResetChangedKey(regular_key); // Check that we get this value from the overlay and the underlay. @@ -168,7 +184,8 @@ EXPECT_TRUE(base::FundamentalValue(44).Equals(value)); // Check that overlay remove is reported. - overlay_->RemoveValue(regular_key); + overlay_->RemoveValue(regular_key, + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); obs.VerifyAndResetChangedKey(regular_key); // Check that value was removed from overlay and underlay @@ -176,14 +193,17 @@ EXPECT_FALSE(underlay_->GetValue(regular_key, &value)); // Check respecting of silence. - overlay_->SetValueSilently(regular_key, new FundamentalValue(46)); + overlay_->SetValueSilently(regular_key, new FundamentalValue(46), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); EXPECT_TRUE(obs.changed_keys.empty()); overlay_->RemoveObserver(&obs); // Check successful unsubscription. - underlay_->SetValue(regular_key, new FundamentalValue(47)); - overlay_->SetValue(regular_key, new FundamentalValue(48)); + underlay_->SetValue(regular_key, new FundamentalValue(47), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); + overlay_->SetValue(regular_key, new FundamentalValue(48), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); EXPECT_TRUE(obs.changed_keys.empty()); } @@ -196,11 +216,13 @@ // Check that if there is no override in the overlay, changing underlay value // is reported as changing an overlay value. - underlay_->SetValue(mapped_underlay_key, new FundamentalValue(42)); + underlay_->SetValue(mapped_underlay_key, new FundamentalValue(42), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); obs.VerifyAndResetChangedKey(mapped_overlay_key); // Check that underlay overwriting is reported. - underlay_->SetValue(mapped_underlay_key, new FundamentalValue(43)); + underlay_->SetValue(mapped_underlay_key, new FundamentalValue(43), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); obs.VerifyAndResetChangedKey(mapped_overlay_key); // Check that we get this value from the overlay with both keys @@ -211,7 +233,8 @@ EXPECT_TRUE(base::FundamentalValue(43).Equals(value)); // Check that overwriting change in overlay is reported. - overlay_->SetValue(mapped_overlay_key, new FundamentalValue(44)); + overlay_->SetValue(mapped_overlay_key, new FundamentalValue(44), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); obs.VerifyAndResetChangedKey(mapped_overlay_key); // Check that we get an overriden value from overlay, while reading the @@ -224,15 +247,18 @@ EXPECT_TRUE(base::FundamentalValue(43).Equals(value)); // Check that hidden underlay change is not reported. - underlay_->SetValue(mapped_underlay_key, new FundamentalValue(45)); + underlay_->SetValue(mapped_underlay_key, new FundamentalValue(45), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); EXPECT_TRUE(obs.changed_keys.empty()); // Check that overlay remove is reported. - overlay_->RemoveValue(mapped_overlay_key); + overlay_->RemoveValue(mapped_overlay_key, + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); obs.VerifyAndResetChangedKey(mapped_overlay_key); // Check that underlay remove is reported. - underlay_->RemoveValue(mapped_underlay_key); + underlay_->RemoveValue(mapped_underlay_key, + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); obs.VerifyAndResetChangedKey(mapped_overlay_key); // Check that value was removed. @@ -240,14 +266,17 @@ EXPECT_FALSE(overlay_->GetValue(mapped_underlay_key, &value)); // Check respecting of silence. - overlay_->SetValueSilently(mapped_overlay_key, new FundamentalValue(46)); + overlay_->SetValueSilently(mapped_overlay_key, new FundamentalValue(46), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); EXPECT_TRUE(obs.changed_keys.empty()); overlay_->RemoveObserver(&obs); // Check successful unsubscription. - underlay_->SetValue(mapped_underlay_key, new FundamentalValue(47)); - overlay_->SetValue(mapped_overlay_key, new FundamentalValue(48)); + underlay_->SetValue(mapped_underlay_key, new FundamentalValue(47), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); + overlay_->SetValue(mapped_overlay_key, new FundamentalValue(48), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); EXPECT_TRUE(obs.changed_keys.empty()); }
diff --git a/prefs/pref_registry.h b/prefs/pref_registry.h index cc5804e..caf2a1a 100644 --- a/prefs/pref_registry.h +++ b/prefs/pref_registry.h
@@ -32,11 +32,15 @@ // behave or be stored. This will be passed in a bitmask when the pref is // registered. Subclasses of PrefRegistry can specify their own flags. Care // must be taken to ensure none of these overlap with the flags below. - enum PrefRegistrationFlags { + enum PrefRegistrationFlags : uint32 { // No flags are specified. NO_REGISTRATION_FLAGS = 0, // The first 8 bits are reserved for subclasses of PrefRegistry to use. + + // This marks the pref as "lossy". There is no strict time guarantee on when + // a lossy pref will be persisted to permanent storage when it is modified. + LOSSY_PREF = 1 << 8, }; typedef PrefValueMap::const_iterator const_iterator;
diff --git a/prefs/pref_service.cc b/prefs/pref_service.cc index 3ccdae7..6e1d58c 100644 --- a/prefs/pref_service.cc +++ b/prefs/pref_service.cc
@@ -38,6 +38,19 @@ base::Callback<void(PersistentPrefStore::PrefReadError)> callback_; }; +// Returns the WriteablePrefStore::PrefWriteFlags for the pref with the given +// |path|. +uint32 GetWriteFlags(const PrefService::Preference* pref) { + uint32 write_flags = WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS; + + if (!pref) + return write_flags; + + if (pref->registration_flags() & PrefRegistry::LOSSY_PREF) + write_flags |= WriteablePrefStore::LOSSY_PREF_WRITE_FLAG; + return write_flags; +} + } // namespace PrefService::PrefService( @@ -177,8 +190,7 @@ DCHECK(CalledOnValidThread()); scoped_ptr<base::DictionaryValue> out(new base::DictionaryValue); for (const auto& it : *pref_registry_) { - const base::Value* value = GetPreferenceValue(it.first); - out->Set(it.first, value->DeepCopy()); + out->Set(it.first, GetPreferenceValue(it.first)->CreateDeepCopy()); } return out.Pass(); } @@ -191,7 +203,7 @@ const Preference* pref = FindPreference(it.first); if (pref->IsDefaultValue()) continue; - out->Set(it.first, pref->GetValue()->DeepCopy()); + out->Set(it.first, pref->GetValue()->CreateDeepCopy()); } return out.Pass(); } @@ -203,7 +215,7 @@ for (const auto& it : *pref_registry_) { const base::Value* value = GetPreferenceValue(it.first); DCHECK(value); - out->SetWithoutPathExpansion(it.first, value->DeepCopy()); + out->SetWithoutPathExpansion(it.first, value->CreateDeepCopy()); } return out.Pass(); } @@ -357,7 +369,7 @@ NOTREACHED() << "Trying to clear an unregistered pref: " << path; return; } - user_pref_store_->RemoveValue(path); + user_pref_store_->RemoveValue(path, GetWriteFlags(pref)); } void PrefService::Set(const std::string& path, const base::Value& value) { @@ -454,14 +466,14 @@ } else { NOTREACHED(); } - user_pref_store_->SetValueSilently(path, value); + user_pref_store_->SetValueSilently(path, value, GetWriteFlags(pref)); } return value; } void PrefService::ReportUserPrefChanged(const std::string& key) { DCHECK(CalledOnValidThread()); - user_pref_store_->ReportValueChanged(key); + user_pref_store_->ReportValueChanged(key, GetWriteFlags(FindPreference(key))); } void PrefService::SetUserPrefValue(const std::string& path, @@ -481,7 +493,7 @@ return; } - user_pref_store_->SetValue(path, owned_value.release()); + user_pref_store_->SetValue(path, owned_value.release(), GetWriteFlags(pref)); } void PrefService::UpdateCommandLinePrefStore(PrefStore* command_line_store) { @@ -496,6 +508,9 @@ base::Value::Type type) : name_(name), type_(type), pref_service_(service) { DCHECK(service); + // Cache the registration flags at creation time to avoid multiple map lookups + // later. + registration_flags_ = service->pref_registry_->GetRegistrationFlags(name_); } const std::string PrefService::Preference::name() const {
diff --git a/prefs/pref_service.h b/prefs/pref_service.h index d9b03c9..1fc6c12 100644 --- a/prefs/pref_service.h +++ b/prefs/pref_service.h
@@ -128,6 +128,10 @@ // the Preference. bool IsExtensionModifiable() const; + // Return the registration flags for this pref as a bitmask of + // PrefRegistry::PrefRegistrationFlags. + uint32 registration_flags() const { return registration_flags_; } + private: friend class PrefService; @@ -139,6 +143,8 @@ const base::Value::Type type_; + uint32 registration_flags_; + // Reference to the PrefService in which this pref was created. const PrefService* pref_service_; }; @@ -307,6 +313,7 @@ // Give access to ReportUserPrefChanged() and GetMutableUserPref(). friend class subtle::ScopedUserPrefUpdateBase; + friend class PrefServiceTest_WriteablePrefStoreFlags_Test; // Registration of pref change observers must be done using the // PrefChangeRegistrar, which is declared as a friend here to grant it
diff --git a/prefs/pref_service_unittest.cc b/prefs/pref_service_unittest.cc index 36ad887..262d7e9 100644 --- a/prefs/pref_service_unittest.cc +++ b/prefs/pref_service_unittest.cc
@@ -8,6 +8,7 @@ #include "base/prefs/mock_pref_change_callback.h" #include "base/prefs/pref_change_registrar.h" #include "base/prefs/pref_registry_simple.h" +#include "base/prefs/pref_service_factory.h" #include "base/prefs/pref_value_store.h" #include "base/prefs/testing_pref_service.h" #include "base/prefs/testing_pref_store.h" @@ -227,6 +228,115 @@ EXPECT_EQ(kRecommendedValue, actual_int_value); } +// A PrefStore which just stores the last write flags that were used to write +// values to it. +class WriteFlagChecker : public TestingPrefStore { + public: + WriteFlagChecker() {} + + void ReportValueChanged(const std::string& key, uint32 flags) override { + SetLastWriteFlags(flags); + } + + void SetValue(const std::string& key, + base::Value* value, + uint32 flags) override { + SetLastWriteFlags(flags); + delete value; + } + + void SetValueSilently(const std::string& key, + base::Value* value, + uint32 flags) override { + SetLastWriteFlags(flags); + delete value; + } + + void RemoveValue(const std::string& key, uint32 flags) override { + SetLastWriteFlags(flags); + } + + uint32 GetLastFlagsAndClear() { + CHECK(last_write_flags_set_); + uint32 result = last_write_flags_; + last_write_flags_set_ = false; + last_write_flags_ = WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS; + return result; + } + + bool last_write_flags_set() { return last_write_flags_set_; } + + private: + ~WriteFlagChecker() override {} + + void SetLastWriteFlags(uint32 flags) { + CHECK(!last_write_flags_set_); + last_write_flags_set_ = true; + last_write_flags_ = flags; + } + + bool last_write_flags_set_ = false; + uint32 last_write_flags_ = WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS; +}; + +TEST(PrefServiceTest, WriteablePrefStoreFlags) { + scoped_refptr<WriteFlagChecker> flag_checker(new WriteFlagChecker); + scoped_refptr<PrefRegistrySimple> registry(new PrefRegistrySimple); + base::PrefServiceFactory factory; + factory.set_user_prefs(flag_checker); + scoped_ptr<PrefService> prefs(factory.Create(registry.get())); + + // The first 8 bits of write flags are reserved for subclasses. Create a + // custom flag in this range + uint32 kCustomRegistrationFlag = 1 << 2; + + // A map of the registration flags that will be tested and the write flags + // they are expected to convert to. + struct RegistrationToWriteFlags { + const char* pref_name; + uint32 registration_flags; + uint32 write_flags; + }; + const RegistrationToWriteFlags kRegistrationToWriteFlags[] = { + {"none", + PrefRegistry::NO_REGISTRATION_FLAGS, + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS}, + {"lossy", + PrefRegistry::LOSSY_PREF, + WriteablePrefStore::LOSSY_PREF_WRITE_FLAG}, + {"custom", + kCustomRegistrationFlag, + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS}, + {"lossyandcustom", + PrefRegistry::LOSSY_PREF | kCustomRegistrationFlag, + WriteablePrefStore::LOSSY_PREF_WRITE_FLAG}}; + + for (size_t i = 0; i < arraysize(kRegistrationToWriteFlags); ++i) { + RegistrationToWriteFlags entry = kRegistrationToWriteFlags[i]; + registry->RegisterDictionaryPref( + entry.pref_name, new base::DictionaryValue(), entry.registration_flags); + + SCOPED_TRACE("Currently testing pref with name: " + + std::string(entry.pref_name)); + + prefs->GetMutableUserPref(entry.pref_name, base::Value::TYPE_DICTIONARY); + EXPECT_TRUE(flag_checker->last_write_flags_set()); + EXPECT_EQ(entry.write_flags, flag_checker->GetLastFlagsAndClear()); + + prefs->ReportUserPrefChanged(entry.pref_name); + EXPECT_TRUE(flag_checker->last_write_flags_set()); + EXPECT_EQ(entry.write_flags, flag_checker->GetLastFlagsAndClear()); + + prefs->ClearPref(entry.pref_name); + EXPECT_TRUE(flag_checker->last_write_flags_set()); + EXPECT_EQ(entry.write_flags, flag_checker->GetLastFlagsAndClear()); + + prefs->SetUserPrefValue(entry.pref_name, new base::DictionaryValue()); + EXPECT_TRUE(flag_checker->last_write_flags_set()); + EXPECT_EQ(entry.write_flags, flag_checker->GetLastFlagsAndClear()); + } +} + class PrefServiceSetValueTest : public testing::Test { protected: static const char kName[];
diff --git a/prefs/testing_pref_service.h b/prefs/testing_pref_service.h index 40fd66a..7587383 100644 --- a/prefs/testing_pref_service.h +++ b/prefs/testing_pref_service.h
@@ -182,13 +182,14 @@ SetPref(TestingPrefStore* pref_store, const std::string& path, base::Value* value) { - pref_store->SetValue(path, value); + pref_store->SetValue(path, value, + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); } template <class SuperPrefService, class ConstructionPrefRegistry> void TestingPrefServiceBase<SuperPrefService, ConstructionPrefRegistry>:: RemovePref(TestingPrefStore* pref_store, const std::string& path) { - pref_store->RemoveValue(path); + pref_store->RemoveValue(path, WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); } #endif // BASE_PREFS_TESTING_PREF_SERVICE_H_
diff --git a/prefs/testing_pref_store.cc b/prefs/testing_pref_store.cc index f20824e..35c9763 100644 --- a/prefs/testing_pref_store.cc +++ b/prefs/testing_pref_store.cc
@@ -42,7 +42,9 @@ return init_complete_; } -void TestingPrefStore::SetValue(const std::string& key, base::Value* value) { +void TestingPrefStore::SetValue(const std::string& key, + base::Value* value, + uint32 flags) { if (prefs_.SetValue(key, value)) { committed_ = false; NotifyPrefValueChanged(key); @@ -50,12 +52,13 @@ } void TestingPrefStore::SetValueSilently(const std::string& key, - base::Value* value) { + base::Value* value, + uint32 flags) { if (prefs_.SetValue(key, value)) committed_ = false; } -void TestingPrefStore::RemoveValue(const std::string& key) { +void TestingPrefStore::RemoveValue(const std::string& key, uint32 flags) { if (prefs_.RemoveValue(key)) { committed_ = false; NotifyPrefValueChanged(key); @@ -103,21 +106,22 @@ Observer, observers_, OnInitializationCompleted(read_success_)); } -void TestingPrefStore::ReportValueChanged(const std::string& key) { +void TestingPrefStore::ReportValueChanged(const std::string& key, + uint32 flags) { FOR_EACH_OBSERVER(Observer, observers_, OnPrefValueChanged(key)); } void TestingPrefStore::SetString(const std::string& key, const std::string& value) { - SetValue(key, new base::StringValue(value)); + SetValue(key, new base::StringValue(value), DEFAULT_PREF_WRITE_FLAGS); } void TestingPrefStore::SetInteger(const std::string& key, int value) { - SetValue(key, new base::FundamentalValue(value)); + SetValue(key, new base::FundamentalValue(value), DEFAULT_PREF_WRITE_FLAGS); } void TestingPrefStore::SetBoolean(const std::string& key, bool value) { - SetValue(key, new base::FundamentalValue(value)); + SetValue(key, new base::FundamentalValue(value), DEFAULT_PREF_WRITE_FLAGS); } bool TestingPrefStore::GetString(const std::string& key,
diff --git a/prefs/testing_pref_store.h b/prefs/testing_pref_store.h index 866b4ae..3de5cac 100644 --- a/prefs/testing_pref_store.h +++ b/prefs/testing_pref_store.h
@@ -30,10 +30,14 @@ // PersistentPrefStore overrides: bool GetMutableValue(const std::string& key, base::Value** result) override; - void ReportValueChanged(const std::string& key) override; - void SetValue(const std::string& key, base::Value* value) override; - void SetValueSilently(const std::string& key, base::Value* value) override; - void RemoveValue(const std::string& key) override; + void ReportValueChanged(const std::string& key, uint32 flags) override; + void SetValue(const std::string& key, + base::Value* value, + uint32 flags) override; + void SetValueSilently(const std::string& key, + base::Value* value, + uint32 flags) override; + void RemoveValue(const std::string& key, uint32 flags) override; bool ReadOnly() const override; PrefReadError GetReadError() const override; PersistentPrefStore::PrefReadError ReadPrefs() override;
diff --git a/prefs/value_map_pref_store.cc b/prefs/value_map_pref_store.cc index 8af1282..d850150 100644 --- a/prefs/value_map_pref_store.cc +++ b/prefs/value_map_pref_store.cc
@@ -28,12 +28,14 @@ return observers_.might_have_observers(); } -void ValueMapPrefStore::SetValue(const std::string& key, base::Value* value) { +void ValueMapPrefStore::SetValue(const std::string& key, + base::Value* value, + uint32 flags) { if (prefs_.SetValue(key, value)) FOR_EACH_OBSERVER(Observer, observers_, OnPrefValueChanged(key)); } -void ValueMapPrefStore::RemoveValue(const std::string& key) { +void ValueMapPrefStore::RemoveValue(const std::string& key, uint32 flags) { if (prefs_.RemoveValue(key)) FOR_EACH_OBSERVER(Observer, observers_, OnPrefValueChanged(key)); } @@ -43,12 +45,14 @@ return prefs_.GetValue(key, value); } -void ValueMapPrefStore::ReportValueChanged(const std::string& key) { +void ValueMapPrefStore::ReportValueChanged(const std::string& key, + uint32 flags) { FOR_EACH_OBSERVER(Observer, observers_, OnPrefValueChanged(key)); } void ValueMapPrefStore::SetValueSilently(const std::string& key, - base::Value* value) { + base::Value* value, + uint32 flags) { prefs_.SetValue(key, value); }
diff --git a/prefs/value_map_pref_store.h b/prefs/value_map_pref_store.h index 8c515ed..86c94bb 100644 --- a/prefs/value_map_pref_store.h +++ b/prefs/value_map_pref_store.h
@@ -28,11 +28,15 @@ bool HasObservers() const override; // WriteablePrefStore overrides: - void SetValue(const std::string& key, base::Value* value) override; - void RemoveValue(const std::string& key) override; + void SetValue(const std::string& key, + base::Value* value, + uint32 flags) override; + void RemoveValue(const std::string& key, uint32 flags) override; bool GetMutableValue(const std::string& key, base::Value** value) override; - void ReportValueChanged(const std::string& key) override; - void SetValueSilently(const std::string& key, base::Value* value) override; + void ReportValueChanged(const std::string& key, uint32 flags) override; + void SetValueSilently(const std::string& key, + base::Value* value, + uint32 flags) override; protected: ~ValueMapPrefStore() override;
diff --git a/prefs/writeable_pref_store.h b/prefs/writeable_pref_store.h index 5ebab64..d85b4c8 100644 --- a/prefs/writeable_pref_store.h +++ b/prefs/writeable_pref_store.h
@@ -17,14 +17,27 @@ // A pref store that can be written to as well as read from. class BASE_PREFS_EXPORT WriteablePrefStore : public PrefStore { public: + // PrefWriteFlags can be used to change the way a pref will be written to + // storage. + enum PrefWriteFlags : uint32 { + // No flags are specified. + DEFAULT_PREF_WRITE_FLAGS = 0, + + // This marks the pref as "lossy". There is no strict time guarantee on when + // a lossy pref will be persisted to permanent storage when it is modified. + LOSSY_PREF_WRITE_FLAG = 1 << 1 + }; + WriteablePrefStore() {} // Sets a |value| for |key| in the store. Assumes ownership of |value|, which - // must be non-NULL. - virtual void SetValue(const std::string& key, base::Value* value) = 0; + // must be non-NULL. |flags| is a bitmask of PrefWriteFlags. + virtual void SetValue(const std::string& key, + base::Value* value, + uint32 flags) = 0; // Removes the value for |key|. - virtual void RemoveValue(const std::string& key) = 0; + virtual void RemoveValue(const std::string& key, uint32 flags) = 0; // Equivalent to PrefStore::GetValue but returns a mutable value. virtual bool GetMutableValue(const std::string& key, @@ -34,13 +47,17 @@ // if one retrieves a list or dictionary with GetMutableValue and change its // value. SetValue takes care of notifications itself. Note that // ReportValueChanged will trigger notifications even if nothing has changed. - virtual void ReportValueChanged(const std::string& key) = 0; + // |flags| is a bitmask of PrefWriteFlags. + virtual void ReportValueChanged(const std::string& key, uint32 flags) = 0; // Same as SetValue, but doesn't generate notifications. This is used by // PrefService::GetMutableUserPref() in order to put empty entries // into the user pref store. Using SetValue is not an option since existing - // tests rely on the number of notifications generated. - virtual void SetValueSilently(const std::string& key, base::Value* value) = 0; + // tests rely on the number of notifications generated. |flags| is a bitmask + // of PrefWriteFlags. + virtual void SetValueSilently(const std::string& key, + base::Value* value, + uint32 flags) = 0; protected: ~WriteablePrefStore() override {}
diff --git a/process/process_metrics.cc b/process/process_metrics.cc index 2edd9c7..e486339 100644 --- a/process/process_metrics.cc +++ b/process/process_metrics.cc
@@ -33,11 +33,11 @@ res->SetInteger("committed_memory", static_cast<int>(committed_memory_)); #if defined(OS_LINUX) || defined(OS_ANDROID) - res->Set("meminfo", memory_info_.ToValue().release()); - res->Set("diskinfo", disk_info_.ToValue().release()); + res->Set("meminfo", memory_info_.ToValue()); + res->Set("diskinfo", disk_info_.ToValue()); #endif #if defined(OS_CHROMEOS) - res->Set("swapinfo", swap_info_.ToValue().release()); + res->Set("swapinfo", swap_info_.ToValue()); #endif return res.Pass();
diff --git a/profiler/scoped_profile.h b/profiler/scoped_profile.h index 6b0c800..2c4105d 100644 --- a/profiler/scoped_profile.h +++ b/profiler/scoped_profile.h
@@ -62,4 +62,4 @@ } // namespace tracked_objects -#endif // BASE_PROFILER_SCOPED_PROFILE_H_ +#endif // BASE_PROFILER_SCOPED_PROFILE_H_
diff --git a/profiler/tracked_time_unittest.cc b/profiler/tracked_time_unittest.cc index 4806a90..c105688 100644 --- a/profiler/tracked_time_unittest.cc +++ b/profiler/tracked_time_unittest.cc
@@ -70,16 +70,14 @@ TEST(TrackedTimeTest, TrackedTimerDisabled) { // Check to be sure disabling the collection of data induces a null time // (which we know will return much faster). - if (!ThreadData::InitializeAndSetTrackingStatus(ThreadData::DEACTIVATED)) - return; + ThreadData::InitializeAndSetTrackingStatus(ThreadData::DEACTIVATED); // Since we disabled tracking, we should get a null response. TrackedTime track_now = ThreadData::Now(); EXPECT_TRUE(track_now.is_null()); } TEST(TrackedTimeTest, TrackedTimerEnabled) { - if (!ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE)) - return; + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); // Make sure that when we enable tracking, we get a real timer result. // First get a 64 bit timer (which should not be null).
diff --git a/sys_info.cc b/sys_info.cc index 72a9944..8640dc1 100644 --- a/sys_info.cc +++ b/sys_info.cc
@@ -7,7 +7,9 @@ #include "base/base_switches.h" #include "base/command_line.h" #include "base/lazy_instance.h" +#include "base/metrics/field_trial.h" #include "base/strings/string_number_conversions.h" +#include "base/strings/string_util.h" #include "base/sys_info_internal.h" #include "base/time/time.h" @@ -34,6 +36,14 @@ // static bool SysInfo::IsLowEndDevice() { + const std::string group_name = + base::FieldTrialList::FindFullName("MemoryReduction"); + + // Low End Device Mode will be enabled if this client is assigned to + // one of those EnabledXXX groups. + if (StartsWithASCII(group_name, "Enabled", true)) + return true; + return g_lazy_low_end_device.Get().value(); } #endif
diff --git a/test/launcher/test_results_tracker.cc b/test/launcher/test_results_tracker.cc index b553fd6..c4cb233 100644 --- a/test/launcher/test_results_tracker.cc +++ b/test/launcher/test_results_tracker.cc
@@ -239,52 +239,39 @@ bool TestResultsTracker::SaveSummaryAsJSON(const FilePath& path) const { scoped_ptr<DictionaryValue> summary_root(new DictionaryValue); - ListValue* global_tags = new ListValue; - summary_root->Set("global_tags", global_tags); - - for (std::set<std::string>::const_iterator i = global_tags_.begin(); - i != global_tags_.end(); - ++i) { - global_tags->AppendString(*i); + scoped_ptr<ListValue> global_tags(new ListValue); + for (const auto& global_tag : global_tags_) { + global_tags->AppendString(global_tag); } + summary_root->Set("global_tags", global_tags.Pass()); - ListValue* all_tests = new ListValue; - summary_root->Set("all_tests", all_tests); - - for (std::set<std::string>::const_iterator i = all_tests_.begin(); - i != all_tests_.end(); - ++i) { - all_tests->AppendString(*i); + scoped_ptr<ListValue> all_tests(new ListValue); + for (const auto& test : all_tests_) { + all_tests->AppendString(test); } + summary_root->Set("all_tests", all_tests.Pass()); - ListValue* disabled_tests = new ListValue; - summary_root->Set("disabled_tests", disabled_tests); - - for (std::set<std::string>::const_iterator i = disabled_tests_.begin(); - i != disabled_tests_.end(); - ++i) { - disabled_tests->AppendString(*i); + scoped_ptr<ListValue> disabled_tests(new ListValue); + for (const auto& disabled_test : disabled_tests_) { + disabled_tests->AppendString(disabled_test); } + summary_root->Set("disabled_tests", disabled_tests.Pass()); - ListValue* per_iteration_data = new ListValue; - summary_root->Set("per_iteration_data", per_iteration_data); + scoped_ptr<ListValue> per_iteration_data(new ListValue); for (int i = 0; i <= iteration_; i++) { - DictionaryValue* current_iteration_data = new DictionaryValue; - per_iteration_data->Append(current_iteration_data); + scoped_ptr<DictionaryValue> current_iteration_data(new DictionaryValue); for (PerIterationData::ResultsMap::const_iterator j = per_iteration_data_[i].results.begin(); j != per_iteration_data_[i].results.end(); ++j) { - ListValue* test_results = new ListValue; - current_iteration_data->SetWithoutPathExpansion(j->first, test_results); + scoped_ptr<ListValue> test_results(new ListValue); for (size_t k = 0; k < j->second.test_results.size(); k++) { const TestResult& test_result = j->second.test_results[k]; - DictionaryValue* test_result_value = new DictionaryValue; - test_results->Append(test_result_value); + scoped_ptr<DictionaryValue> test_result_value(new DictionaryValue); test_result_value->SetString("status", test_result.StatusAsString()); test_result_value->SetInteger( @@ -310,8 +297,14 @@ Base64Encode(test_result.output_snippet, &base64_output_snippet); test_result_value->SetString("output_snippet_base64", base64_output_snippet); + test_results->Append(test_result_value.Pass()); } + + current_iteration_data->SetWithoutPathExpansion(j->first, + test_results.Pass()); } + per_iteration_data->Append(current_iteration_data.Pass()); + summary_root->Set("per_iteration_data", per_iteration_data.Pass()); } JSONFileValueSerializer serializer(path);
diff --git a/test/trace_event_analyzer_unittest.cc b/test/trace_event_analyzer_unittest.cc index 4bdb941..cf85fd0 100644 --- a/test/trace_event_analyzer_unittest.cc +++ b/test/trace_event_analyzer_unittest.cc
@@ -225,7 +225,7 @@ scoped_ptr<TraceAnalyzer> analyzer(TraceAnalyzer::Create(output_.json_output)); - ASSERT_TRUE(!!analyzer.get()); + ASSERT_TRUE(analyzer); analyzer->SetIgnoreMetadataEvents(true); TraceEventVector found;
diff --git a/test/values_test_util.cc b/test/values_test_util.cc index c5dfd79..3e762b9 100644 --- a/test/values_test_util.cc +++ b/test/values_test_util.cc
@@ -69,7 +69,7 @@ NULL, &error_msg)); if (!result) { ADD_FAILURE() << "Failed to parse \"" << json << "\": " << error_msg; - result.reset(Value::CreateNullValue()); + result = Value::CreateNullValue(); } return result.Pass(); }
diff --git a/threading/platform_thread.h b/threading/platform_thread.h index 69a2b0d..d8f06e5 100644 --- a/threading/platform_thread.h +++ b/threading/platform_thread.h
@@ -89,10 +89,6 @@ id_(id) { } - PlatformThreadId id() const { - return id_; - } - bool is_equal(const PlatformThreadHandle& other) const { return handle_ == other.handle_; }
diff --git a/threading/platform_thread_win.cc b/threading/platform_thread_win.cc index aeaa7c7..4eb2cb2 100644 --- a/threading/platform_thread_win.cc +++ b/threading/platform_thread_win.cc
@@ -108,16 +108,15 @@ // have to work running on CreateThread() threads anyway, since we run code // on the Windows thread pool, etc. For some background on the difference: // http://www.microsoft.com/msj/1099/win32/win321099.aspx - PlatformThreadId thread_id; void* thread_handle = CreateThread( - NULL, stack_size, ThreadFunc, params, flags, &thread_id); + NULL, stack_size, ThreadFunc, params, flags, NULL); if (!thread_handle) { delete params; return false; } if (out_thread_handle) - *out_thread_handle = PlatformThreadHandle(thread_handle, thread_id); + *out_thread_handle = PlatformThreadHandle(thread_handle); else CloseHandle(thread_handle); return true;
diff --git a/threading/thread.cc b/threading/thread.cc index bd565c9..0e4aab2 100644 --- a/threading/thread.cc +++ b/threading/thread.cc
@@ -37,6 +37,20 @@ Thread::SetThreadWasQuitProperly(true); } +// Used to pass data to ThreadMain. This structure is allocated on the stack +// from within StartWithOptions. +struct Thread::StartupData { + // We get away with a const reference here because of how we are allocated. + const Thread::Options& options; + + // Used to synchronize thread startup. + WaitableEvent event; + + explicit StartupData(const Options& opt) + : options(opt), + event(false, false) {} +}; + Thread::Options::Options() : message_loop_type(MessageLoop::TYPE_DEFAULT), timer_slack(TIMER_SLACK_NONE), @@ -58,11 +72,13 @@ #if defined(OS_WIN) com_status_(NONE), #endif + started_(false), stopping_(false), running_(false), + startup_data_(NULL), thread_(0), - message_loop_(nullptr), - message_loop_timer_slack_(TIMER_SLACK_NONE), + message_loop_(NULL), + thread_id_(kInvalidThreadId), name_(name) { } @@ -88,45 +104,34 @@ SetThreadWasQuitProperly(false); - MessageLoop::Type type = options.message_loop_type; - if (!options.message_pump_factory.is_null()) - type = MessageLoop::TYPE_CUSTOM; - - message_loop_timer_slack_ = options.timer_slack; - message_loop_ = new MessageLoop(type, options.message_pump_factory); - - start_event_.reset(new WaitableEvent(false, false)); + StartupData startup_data(options); + startup_data_ = &startup_data; if (!PlatformThread::Create(options.stack_size, this, &thread_)) { DLOG(ERROR) << "failed to create thread"; - delete message_loop_; - message_loop_ = nullptr; - start_event_.reset(); + startup_data_ = NULL; return false; } + // TODO(kinuko): Remove once crbug.com/465458 is solved. + tracked_objects::ScopedTracker tracking_profile_wait( + FROM_HERE_WITH_EXPLICIT_FUNCTION( + "465458 base::Thread::StartWithOptions (Wait)")); + + // Wait for the thread to start and initialize message_loop_ + base::ThreadRestrictions::ScopedAllowWait allow_wait; + startup_data.event.Wait(); + + // set it to NULL so we don't keep a pointer to some object on the stack. + startup_data_ = NULL; + started_ = true; + DCHECK(message_loop_); return true; } -bool Thread::StartAndWaitForTesting() { - bool result = Start(); - if (!result) - return false; - WaitUntilThreadStarted(); - return result; -} - -bool Thread::WaitUntilThreadStarted() { - if (!start_event_) - return false; - base::ThreadRestrictions::ScopedAllowWait allow_wait; - start_event_->Wait(); - return true; -} - void Thread::Stop() { - if (!start_event_) + if (!started_) return; StopSoon(); @@ -142,7 +147,7 @@ DCHECK(!message_loop_); // The thread no longer needs to be joined. - start_event_.reset(); + started_ = false; stopping_ = false; } @@ -150,7 +155,9 @@ void Thread::StopSoon() { // We should only be called on the same thread that started us. - DCHECK_NE(thread_id(), PlatformThread::CurrentId()); + // Reading thread_id_ without a lock can lead to a benign data race + // with ThreadMain, so we annotate it to stay silent under ThreadSanitizer. + DCHECK_NE(ANNOTATE_UNPROTECTED_READ(thread_id_), PlatformThread::CurrentId()); if (stopping_ || !message_loop_) return; @@ -160,22 +167,13 @@ } bool Thread::IsRunning() const { - // If the thread's already started (i.e. message_loop_ is non-null) and - // not yet requested to stop (i.e. stopping_ is false) we can just return - // true. (Note that stopping_ is touched only on the same thread that - // starts / started the new thread so we need no locking here.) - if (message_loop_ && !stopping_) - return true; - // Otherwise check the running_ flag, which is set to true by the new thread - // only while it is inside Run(). - AutoLock lock(lock_); return running_; } void Thread::SetPriority(ThreadPriority priority) { // The thread must be started (and id known) for this to be // compatible with all platforms. - DCHECK(message_loop_ != nullptr); + DCHECK_NE(thread_id_, kInvalidThreadId); PlatformThread::SetThreadPriority(thread_, priority); } @@ -196,57 +194,60 @@ } void Thread::ThreadMain() { - // Complete the initialization of our Thread object. - DCHECK_EQ(thread_id(), PlatformThread::CurrentId()); - PlatformThread::SetName(name_.c_str()); - ANNOTATE_THREAD_NAME(name_.c_str()); // Tell the name to race detector. + { + // The message loop for this thread. + // Allocated on the heap to centralize any leak reports at this line. + scoped_ptr<MessageLoop> message_loop; + if (!startup_data_->options.message_pump_factory.is_null()) { + message_loop.reset( + new MessageLoop(startup_data_->options.message_pump_factory.Run())); + } else { + message_loop.reset( + new MessageLoop(startup_data_->options.message_loop_type)); + } - // Lazily initialize the message_loop so that it can run on this thread. - DCHECK(message_loop_); - scoped_ptr<MessageLoop> message_loop(message_loop_); - message_loop_->BindToCurrentThread(); - message_loop_->set_thread_name(name_); - message_loop_->SetTimerSlack(message_loop_timer_slack_); + // Complete the initialization of our Thread object. + thread_id_ = PlatformThread::CurrentId(); + PlatformThread::SetName(name_); + ANNOTATE_THREAD_NAME(name_.c_str()); // Tell the name to race detector. + message_loop->set_thread_name(name_); + message_loop->SetTimerSlack(startup_data_->options.timer_slack); + message_loop_ = message_loop.get(); #if defined(OS_WIN) - scoped_ptr<win::ScopedCOMInitializer> com_initializer; - if (com_status_ != NONE) { - com_initializer.reset((com_status_ == STA) ? - new win::ScopedCOMInitializer() : - new win::ScopedCOMInitializer(win::ScopedCOMInitializer::kMTA)); - } + scoped_ptr<win::ScopedCOMInitializer> com_initializer; + if (com_status_ != NONE) { + com_initializer.reset((com_status_ == STA) ? + new win::ScopedCOMInitializer() : + new win::ScopedCOMInitializer(win::ScopedCOMInitializer::kMTA)); + } #endif - // Let the thread do extra initialization. - Init(); + // Let the thread do extra initialization. + // Let's do this before signaling we are started. + Init(); - { - AutoLock lock(lock_); running_ = true; - } + startup_data_->event.Signal(); + // startup_data_ can't be touched anymore since the starting thread is now + // unlocked. - start_event_->Signal(); - - Run(message_loop_); - - { - AutoLock lock(lock_); + Run(message_loop_); running_ = false; - } - // Let the thread do extra cleanup. - CleanUp(); + // Let the thread do extra cleanup. + CleanUp(); #if defined(OS_WIN) - com_initializer.reset(); + com_initializer.reset(); #endif - // Assert that MessageLoop::Quit was called by ThreadQuitHelper. - DCHECK(GetThreadWasQuitProperly()); + // Assert that MessageLoop::Quit was called by ThreadQuitHelper. + DCHECK(GetThreadWasQuitProperly()); - // We can't receive messages anymore. - // (The message loop is destructed at the end of this block) - message_loop_ = NULL; + // We can't receive messages anymore. + message_loop_ = NULL; + } } } // namespace base
diff --git a/threading/thread.h b/threading/thread.h index 2e19b0a..4915606 100644 --- a/threading/thread.h +++ b/threading/thread.h
@@ -13,13 +13,11 @@ #include "base/message_loop/message_loop.h" #include "base/message_loop/timer_slack.h" #include "base/single_thread_task_runner.h" -#include "base/synchronization/lock.h" #include "base/threading/platform_thread.h" namespace base { class MessagePump; -class WaitableEvent; // A simple thread abstraction that establishes a MessageLoop on a new thread. // The consumer uses the MessageLoop of the thread to cause code to execute on @@ -47,7 +45,7 @@ // This is ignored if message_pump_factory.is_null() is false. MessageLoop::Type message_loop_type; - // Specifies timer slack for thread message loop. + // Specify timer slack for thread message loop. TimerSlack timer_slack; // Used to create the MessagePump for the MessageLoop. The callback is Run() @@ -83,7 +81,7 @@ // init_com_with_mta(false) and then StartWithOptions() with any message loop // type other than TYPE_UI. void init_com_with_mta(bool use_mta) { - DCHECK(!start_event_); + DCHECK(!started_); com_status_ = use_mta ? MTA : STA; } #endif @@ -105,18 +103,6 @@ // callback. bool StartWithOptions(const Options& options); - // Starts the thread and wait for the thread to start and run initialization - // before returning. It's same as calling Start() and then - // WaitUntilThreadStarted(). - // Note that using this (instead of Start() or StartWithOptions() causes - // jank on the calling thread, should be used only in testing code. - bool StartAndWaitForTesting(); - - // Blocks until the thread starts running. Called within StartAndWait(). - // Note that calling this causes jank on the calling thread, must be used - // carefully for production code. - bool WaitUntilThreadStarted(); - // Signals the thread to exit and returns once the thread has exited. After // this method returns, the Thread object is completely reset and may be used // as if it were newly constructed (i.e., Start may be called again). @@ -180,7 +166,7 @@ PlatformThreadHandle thread_handle() { return thread_; } // The thread ID. - PlatformThreadId thread_id() const { return thread_.id(); } + PlatformThreadId thread_id() const { return thread_id_; } // Returns true if the thread has been started, and not yet stopped. bool IsRunning() const; @@ -222,13 +208,19 @@ ComStatus com_status_; #endif + // Whether we successfully started the thread. + bool started_; + // If true, we're in the middle of stopping, and shouldn't access // |message_loop_|. It may non-NULL and invalid. bool stopping_; // True while inside of Run(). bool running_; - mutable base::Lock lock_; // Protects running_. + + // Used to pass data to ThreadMain. + struct StartupData; + StartupData* startup_data_; // The thread's handle. PlatformThreadHandle thread_; @@ -237,16 +229,12 @@ // by the created thread. MessageLoop* message_loop_; - // Stores Options::timer_slack_ until the message loop has been bound to - // a thread. - TimerSlack message_loop_timer_slack_; + // Our thread's ID. + PlatformThreadId thread_id_; // The name of the thread. Used for debugging purposes. std::string name_; - // Non-null if the thread has successfully started. - scoped_ptr<WaitableEvent> start_event_; - friend void ThreadQuitHelper(); DISALLOW_COPY_AND_ASSIGN(Thread);
diff --git a/threading/thread_id_name_manager_unittest.cc b/threading/thread_id_name_manager_unittest.cc index b17c681..b5953d5 100644 --- a/threading/thread_id_name_manager_unittest.cc +++ b/threading/thread_id_name_manager_unittest.cc
@@ -21,8 +21,8 @@ base::Thread thread_a(kAThread); base::Thread thread_b(kBThread); - thread_a.StartAndWaitForTesting(); - thread_b.StartAndWaitForTesting(); + thread_a.Start(); + thread_b.Start(); EXPECT_STREQ(kAThread, manager->GetName(thread_a.thread_id())); EXPECT_STREQ(kBThread, manager->GetName(thread_b.thread_id())); @@ -35,10 +35,10 @@ base::ThreadIdNameManager* manager = base::ThreadIdNameManager::GetInstance(); base::Thread thread_a(kAThread); - thread_a.StartAndWaitForTesting(); + thread_a.Start(); { base::Thread thread_b(kBThread); - thread_b.StartAndWaitForTesting(); + thread_b.Start(); thread_b.Stop(); } EXPECT_STREQ(kAThread, manager->GetName(thread_a.thread_id())); @@ -51,12 +51,12 @@ base::ThreadIdNameManager* manager = base::ThreadIdNameManager::GetInstance(); base::Thread thread_a(kAThread); - thread_a.StartAndWaitForTesting(); + thread_a.Start(); base::PlatformThreadId a_id = thread_a.thread_id(); EXPECT_STREQ(kAThread, manager->GetName(a_id)); thread_a.Stop(); - thread_a.StartAndWaitForTesting(); + thread_a.Start(); EXPECT_STREQ("", manager->GetName(a_id)); EXPECT_STREQ(kAThread, manager->GetName(thread_a.thread_id())); thread_a.Stop();
diff --git a/threading/thread_local_android.cc b/threading/thread_local_android.cc index c890237..813dd78 100644 --- a/threading/thread_local_android.cc +++ b/threading/thread_local_android.cc
@@ -4,15 +4,12 @@ #include "base/threading/thread_local.h" -#include "base/logging.h" - namespace base { namespace internal { // static void ThreadLocalPlatform::AllocateSlot(SlotType* slot) { - bool succeed = slot->Initialize(NULL); - CHECK(succeed); + slot->Initialize(nullptr); } // static
diff --git a/threading/thread_local_storage.cc b/threading/thread_local_storage.cc index 54928ed..0bb396c 100644 --- a/threading/thread_local_storage.cc +++ b/threading/thread_local_storage.cc
@@ -197,7 +197,7 @@ Initialize(destructor); } -bool ThreadLocalStorage::StaticSlot::Initialize(TLSDestructorFunc destructor) { +void ThreadLocalStorage::StaticSlot::Initialize(TLSDestructorFunc destructor) { PlatformThreadLocalStorage::TLSKey key = base::subtle::NoBarrier_Load(&g_native_tls_key); if (key == PlatformThreadLocalStorage::TLS_KEY_OUT_OF_INDEXES || @@ -212,7 +212,6 @@ // Setup our destructor. g_tls_destructors[slot_] = destructor; initialized_ = true; - return true; } void ThreadLocalStorage::StaticSlot::Free() {
diff --git a/threading/thread_local_storage.h b/threading/thread_local_storage.h index ea41b34..50f8868 100644 --- a/threading/thread_local_storage.h +++ b/threading/thread_local_storage.h
@@ -98,8 +98,7 @@ // Set up the TLS slot. Called by the constructor. // 'destructor' is a pointer to a function to perform per-thread cleanup of // this object. If set to NULL, no cleanup is done for this TLS slot. - // Returns false on error. - bool Initialize(TLSDestructorFunc destructor); + void Initialize(TLSDestructorFunc destructor); // Free a previously allocated TLS 'slot'. // If a destructor was set for this slot, removes
diff --git a/threading/thread_unittest.cc b/threading/thread_unittest.cc index e86c758..a89768e 100644 --- a/threading/thread_unittest.cc +++ b/threading/thread_unittest.cc
@@ -194,12 +194,11 @@ EXPECT_EQ("ThreadName", a.thread_name()); } -// Make sure Init() is called after Start() and before -// WaitUntilThreadInitialized() returns. +// Make sure we can't use a thread between Start() and Init(). TEST_F(ThreadTest, SleepInsideInit) { SleepInsideInitThread t; EXPECT_FALSE(t.InitCalled()); - t.StartAndWaitForTesting(); + t.Start(); EXPECT_TRUE(t.InitCalled()); }
diff --git a/time/time.cc b/time/time.cc index bf6c998..9834188 100644 --- a/time/time.cc +++ b/time/time.cc
@@ -97,20 +97,21 @@ return delta_; } -int64 TimeDelta::SaturatedAdd(int64 value) const { - CheckedNumeric<int64> rv(delta_); +namespace time_internal { + +int64 SaturatedAdd(TimeDelta delta, int64 value) { + CheckedNumeric<int64> rv(delta.delta_); rv += value; return FromCheckedNumeric(rv); } -int64 TimeDelta::SaturatedSub(int64 value) const { - CheckedNumeric<int64> rv(delta_); +int64 SaturatedSub(TimeDelta delta, int64 value) { + CheckedNumeric<int64> rv(delta.delta_); rv -= value; return FromCheckedNumeric(rv); } -// static -int64 TimeDelta::FromCheckedNumeric(const CheckedNumeric<int64> value) { +int64 FromCheckedNumeric(const CheckedNumeric<int64> value) { if (value.IsValid()) return value.ValueUnsafe(); @@ -124,6 +125,8 @@ return value.ValueOrDefault(limit); } +} // namespace time_internal + std::ostream& operator<<(std::ostream& os, TimeDelta time_delta) { return os << time_delta.InSecondsF() << "s"; } @@ -305,15 +308,12 @@ TimeDelta tick_interval) const { // |interval_offset| is the offset from |this| to the next multiple of // |tick_interval| after |tick_phase|, possibly negative if in the past. - TimeDelta interval_offset = TimeDelta::FromInternalValue( - (tick_phase - *this).ToInternalValue() % tick_interval.ToInternalValue()); + TimeDelta interval_offset = (tick_phase - *this) % tick_interval; // If |this| is exactly on the interval (i.e. offset==0), don't adjust. // Otherwise, if |tick_phase| was in the past, adjust forward to the next // tick after |this|. - if (interval_offset.ToInternalValue() != 0 && tick_phase < *this) { + if (!interval_offset.is_zero() && tick_phase < *this) interval_offset += tick_interval; - } - return *this + interval_offset; }
diff --git a/time/time.h b/time/time.h index 5c8b89c..0a26778 100644 --- a/time/time.h +++ b/time/time.h
@@ -57,8 +57,23 @@ namespace base { -class Time; -class TimeTicks; +class TimeDelta; + +// The functions in the time_internal namespace are meant to be used only by the +// time classes and functions. Please use the math operators defined in the +// time classes instead. +namespace time_internal { + +// Add or subtract |value| from a TimeDelta. The int64 argument and return value +// are in terms of a microsecond timebase. +BASE_EXPORT int64 SaturatedAdd(TimeDelta delta, int64 value); +BASE_EXPORT int64 SaturatedSub(TimeDelta delta, int64 value); + +// Clamp |value| on overflow and underflow conditions. The int64 argument and +// return value are in terms of a microsecond timebase. +BASE_EXPORT int64 FromCheckedNumeric(const CheckedNumeric<int64> value); + +} // namespace time_internal // TimeDelta ------------------------------------------------------------------ @@ -110,6 +125,11 @@ return TimeDelta((delta_ + mask) ^ mask); } + // Returns true if the time delta is zero. + bool is_zero() const { + return delta_ == 0; + } + // Returns true if the time delta is the maximum time delta. bool is_max() const { return delta_ == std::numeric_limits<int64>::max(); @@ -141,19 +161,17 @@ // Computations with other deltas. TimeDelta operator+(TimeDelta other) const { - return TimeDelta(SaturatedAdd(other.delta_)); + return TimeDelta(time_internal::SaturatedAdd(*this, other.delta_)); } TimeDelta operator-(TimeDelta other) const { - return TimeDelta(SaturatedSub(other.delta_)); + return TimeDelta(time_internal::SaturatedSub(*this, other.delta_)); } TimeDelta& operator+=(TimeDelta other) { - delta_ = SaturatedAdd(other.delta_); - return *this; + return *this = (*this + other); } TimeDelta& operator-=(TimeDelta other) { - delta_ = SaturatedSub(other.delta_); - return *this; + return *this = (*this - other); } TimeDelta operator-() const { return TimeDelta(-delta_); @@ -164,36 +182,29 @@ TimeDelta operator*(T a) const { CheckedNumeric<int64> rv(delta_); rv *= a; - return TimeDelta(FromCheckedNumeric(rv)); + return TimeDelta(time_internal::FromCheckedNumeric(rv)); } template<typename T> TimeDelta operator/(T a) const { CheckedNumeric<int64> rv(delta_); rv /= a; - return TimeDelta(FromCheckedNumeric(rv)); + return TimeDelta(time_internal::FromCheckedNumeric(rv)); } template<typename T> TimeDelta& operator*=(T a) { - CheckedNumeric<int64> rv(delta_); - rv *= a; - delta_ = FromCheckedNumeric(rv); - return *this; + return *this = (*this * a); } template<typename T> TimeDelta& operator/=(T a) { - CheckedNumeric<int64> rv(delta_); - rv /= a; - delta_ = FromCheckedNumeric(rv); - return *this; + return *this = (*this / a); } int64 operator/(TimeDelta a) const { return delta_ / a.delta_; } - - // Defined below because it depends on the definition of the other classes. - Time operator+(Time t) const; - TimeTicks operator+(TimeTicks t) const; + TimeDelta operator%(TimeDelta a) const { + return TimeDelta(delta_ % a.delta_); + } // Comparison operators. bool operator==(TimeDelta other) const { @@ -216,8 +227,8 @@ } private: - friend class Time; - friend class TimeTicks; + friend int64 time_internal::SaturatedAdd(TimeDelta delta, int64 value); + friend int64 time_internal::SaturatedSub(TimeDelta delta, int64 value); // Constructs a delta given the duration in microseconds. This is private // to avoid confusion by callers with an integer constructor. Use @@ -225,13 +236,6 @@ explicit TimeDelta(int64 delta_us) : delta_(delta_us) { } - // Add or subtract |value| from this delta. - int64 SaturatedAdd(int64 value) const; - int64 SaturatedSub(int64 value) const; - - // Clamp |value| on overflow and underflow conditions. - static int64 FromCheckedNumeric(const CheckedNumeric<int64> value); - // Delta in microseconds. int64 delta_; }; @@ -244,10 +248,19 @@ // For logging use only. BASE_EXPORT std::ostream& operator<<(std::ostream& os, TimeDelta time_delta); -// Time ----------------------------------------------------------------------- +// Do not reference the time_internal::TimeBase template class directly. Please +// use one of the time subclasses instead, and only reference the public +// TimeBase members via those classes. +namespace time_internal { -// Represents a wall clock time in UTC. -class BASE_EXPORT Time { +// TimeBase-------------------------------------------------------------------- + +// Provides value storage and comparison/math operations common to all time +// classes. Each subclass provides for strong type-checking to ensure +// semantically meaningful comparison/math of time values from the same clock +// source or timeline. +template<class TimeClass> +class TimeBase { public: static const int64 kHoursPerDay = 24; static const int64 kMillisecondsPerSecond = 1000; @@ -264,6 +277,102 @@ static const int64 kNanosecondsPerSecond = kNanosecondsPerMicrosecond * kMicrosecondsPerSecond; + // Returns true if this object has not been initialized. + // + // Warning: Be careful when writing code that performs math on time values, + // since it's possible to produce a valid "zero" result that should not be + // interpreted as a "null" value. + bool is_null() const { + return us_ == 0; + } + + // Returns true if this object represents the maximum time. + bool is_max() const { + return us_ == std::numeric_limits<int64>::max(); + } + + // For serializing only. Use FromInternalValue() to reconstitute. Please don't + // use this and do arithmetic on it, as it is more error prone than using the + // provided operators. + int64 ToInternalValue() const { + return us_; + } + + TimeClass& operator=(TimeClass other) { + us_ = other.us_; + return *(static_cast<TimeClass*>(this)); + } + + // Compute the difference between two times. + TimeDelta operator-(TimeClass other) const { + return TimeDelta::FromMicroseconds(us_ - other.us_); + } + + // Return a new time modified by some delta. + TimeClass operator+(TimeDelta delta) const { + return TimeClass(time_internal::SaturatedAdd(delta, us_)); + } + TimeClass operator-(TimeDelta delta) const { + return TimeClass(-time_internal::SaturatedSub(delta, us_)); + } + + // Modify by some time delta. + TimeClass& operator+=(TimeDelta delta) { + return static_cast<TimeClass&>(*this = (*this + delta)); + } + TimeClass& operator-=(TimeDelta delta) { + return static_cast<TimeClass&>(*this = (*this - delta)); + } + + // Comparison operators + bool operator==(TimeClass other) const { + return us_ == other.us_; + } + bool operator!=(TimeClass other) const { + return us_ != other.us_; + } + bool operator<(TimeClass other) const { + return us_ < other.us_; + } + bool operator<=(TimeClass other) const { + return us_ <= other.us_; + } + bool operator>(TimeClass other) const { + return us_ > other.us_; + } + bool operator>=(TimeClass other) const { + return us_ >= other.us_; + } + + // Converts an integer value representing TimeClass to a class. This is used + // when deserializing a |TimeClass| structure, using a value known to be + // compatible. It is not provided as a constructor because the integer type + // may be unclear from the perspective of a caller. + static TimeClass FromInternalValue(int64 us) { + return TimeClass(us); + } + + protected: + explicit TimeBase(int64 us) : us_(us) { + } + + // Time value in a microsecond timebase. + int64 us_; +}; + +} // namespace time_internal + +template<class TimeClass> +inline TimeClass operator+(TimeDelta delta, TimeClass t) { + return t + delta; +} + +// Time ----------------------------------------------------------------------- + +// Represents a wall clock time in UTC. Values are not guaranteed to be +// monotonically non-decreasing and are subject to large amounts of skew. +class BASE_EXPORT Time : public time_internal::TimeBase<Time> { + public: // The representation of Jan 1, 1970 UTC in microseconds since the // platform-dependent epoch. static const int64 kTimeTToMicrosecondsOffset; @@ -303,17 +412,7 @@ }; // Contains the NULL time. Use Time::Now() to get the current time. - Time() : us_(0) { - } - - // Returns true if the time object has not been initialized. - bool is_null() const { - return us_ == 0; - } - - // Returns true if the time object is the maximum time. - bool is_max() const { - return us_ == std::numeric_limits<int64>::max(); + Time() : TimeBase(0) { } // Returns the time for epoch in Unix-like system (Jan 1, 1970). @@ -412,14 +511,6 @@ return FromExploded(true, exploded); } - // Converts an integer value representing Time to a class. This is used - // when deserializing a |Time| structure, using a value known to be - // compatible. It is not provided as a constructor because the integer type - // may be unclear from the perspective of a caller. - static Time FromInternalValue(int64 us) { - return Time(us); - } - // Converts a string representation of time to a Time object. // An example of a time string which is converted is as below:- // "Tue, 15 Nov 1994 12:45:26 GMT". If the timezone is not specified @@ -435,13 +526,6 @@ return FromStringInternal(time_string, false, parsed_time); } - // For serializing, use FromInternalValue to reconstitute. Please don't use - // this and do arithmetic on it, as it is more error prone than using the - // provided operators. - int64 ToInternalValue() const { - return us_; - } - // Fills the given exploded structure with either the local time or UTC from // this time structure (containing UTC). void UTCExplode(Exploded* exploded) const { @@ -455,58 +539,10 @@ // midnight on that day. Time LocalMidnight() const; - Time& operator=(Time other) { - us_ = other.us_; - return *this; - } - - // Compute the difference between two times. - TimeDelta operator-(Time other) const { - return TimeDelta(us_ - other.us_); - } - - // Modify by some time delta. - Time& operator+=(TimeDelta delta) { - us_ = delta.SaturatedAdd(us_); - return *this; - } - Time& operator-=(TimeDelta delta) { - us_ = -delta.SaturatedSub(us_); - return *this; - } - - // Return a new time modified by some delta. - Time operator+(TimeDelta delta) const { - return Time(delta.SaturatedAdd(us_)); - } - Time operator-(TimeDelta delta) const { - return Time(-delta.SaturatedSub(us_)); - } - - // Comparison operators - bool operator==(Time other) const { - return us_ == other.us_; - } - bool operator!=(Time other) const { - return us_ != other.us_; - } - bool operator<(Time other) const { - return us_ < other.us_; - } - bool operator<=(Time other) const { - return us_ <= other.us_; - } - bool operator>(Time other) const { - return us_ > other.us_; - } - bool operator>=(Time other) const { - return us_ >= other.us_; - } - private: - friend class TimeDelta; + friend class time_internal::TimeBase<Time>; - explicit Time(int64 us) : us_(us) { + explicit Time(int64 us) : TimeBase(us) { } // Explodes the given time to either local time |is_local = true| or UTC @@ -527,9 +563,6 @@ static bool FromStringInternal(const char* time_string, bool is_local, Time* parsed_time); - - // Time in microseconds in UTC. - int64 us_; }; // Inline the TimeDelta factory methods, for fast TimeDelta construction. @@ -598,16 +631,13 @@ return TimeDelta(us); } -inline Time TimeDelta::operator+(Time t) const { - return Time(SaturatedAdd(t.us_)); -} - // For logging use only. BASE_EXPORT std::ostream& operator<<(std::ostream& os, Time time); // TimeTicks ------------------------------------------------------------------ -class BASE_EXPORT TimeTicks { +// Represents monotonically non-decreasing clock time. +class BASE_EXPORT TimeTicks : public time_internal::TimeBase<TimeTicks> { public: // We define this even without OS_CHROMEOS for seccomp sandbox testing. #if defined(OS_LINUX) @@ -617,7 +647,7 @@ static const clockid_t kClockSystemTrace = 11; #endif - TimeTicks() : ticks_(0) { + TimeTicks() : TimeBase(0) { } // Platform-dependent tick count representing "right now." When @@ -676,24 +706,6 @@ static TimeTicks FromQPCValue(LONGLONG qpc_value); #endif - // Returns true if this object has not been initialized. - bool is_null() const { - return ticks_ == 0; - } - - // Returns true if the time delta is the maximum delta. - bool is_max() const { - return ticks_ == std::numeric_limits<int64>::max(); - } - - // Converts an integer value representing TimeTicks to a class. This is used - // when deserializing a |TimeTicks| structure, using a value known to be - // compatible. It is not provided as a constructor because the integer type - // may be unclear from the perspective of a caller. - static TimeTicks FromInternalValue(int64 ticks) { - return TimeTicks(ticks); - } - // Get the TimeTick value at the time of the UnixEpoch. This is useful when // you need to relate the value of TimeTicks to a real time and date. // Note: Upon first invocation, this function takes a snapshot of the realtime @@ -702,86 +714,26 @@ // application runs. static TimeTicks UnixEpoch(); - // Returns the internal numeric value of the TimeTicks object. - // For serializing, use FromInternalValue to reconstitute. - int64 ToInternalValue() const { - return ticks_; - } - // Returns |this| snapped to the next tick, given a |tick_phase| and // repeating |tick_interval| in both directions. |this| may be before, // after, or equal to the |tick_phase|. TimeTicks SnappedToNextTick(TimeTicks tick_phase, TimeDelta tick_interval) const; - TimeTicks& operator=(TimeTicks other) { - ticks_ = other.ticks_; - return *this; - } - - // Compute the difference between two times. - TimeDelta operator-(TimeTicks other) const { - return TimeDelta(ticks_ - other.ticks_); - } - - // Modify by some time delta. - TimeTicks& operator+=(TimeDelta delta) { - ticks_ = delta.SaturatedAdd(ticks_); - return *this; - } - TimeTicks& operator-=(TimeDelta delta) { - ticks_ = -delta.SaturatedSub(ticks_); - return *this; - } - - // Return a new TimeTicks modified by some delta. - TimeTicks operator+(TimeDelta delta) const { - return TimeTicks(delta.SaturatedAdd(ticks_)); - } - TimeTicks operator-(TimeDelta delta) const { - return TimeTicks(-delta.SaturatedSub(ticks_)); - } - - // Comparison operators - bool operator==(TimeTicks other) const { - return ticks_ == other.ticks_; - } - bool operator!=(TimeTicks other) const { - return ticks_ != other.ticks_; - } - bool operator<(TimeTicks other) const { - return ticks_ < other.ticks_; - } - bool operator<=(TimeTicks other) const { - return ticks_ <= other.ticks_; - } - bool operator>(TimeTicks other) const { - return ticks_ > other.ticks_; - } - bool operator>=(TimeTicks other) const { - return ticks_ >= other.ticks_; - } - - protected: - friend class TimeDelta; - - // Please use Now() to create a new object. This is for internal use - // and testing. Ticks is in microseconds. - explicit TimeTicks(int64 ticks) : ticks_(ticks) { - } - - // Tick count in microseconds. - int64 ticks_; - #if defined(OS_WIN) + protected: typedef DWORD (*TickFunctionType)(void); static TickFunctionType SetMockTickFunction(TickFunctionType ticker); #endif -}; -inline TimeTicks TimeDelta::operator+(TimeTicks t) const { - return TimeTicks(SaturatedAdd(t.ticks_)); -} + private: + friend class time_internal::TimeBase<TimeTicks>; + + // Please use Now() to create a new object. This is for internal use + // and testing. + explicit TimeTicks(int64 us) : TimeBase(us) { + } +}; // For logging use only. BASE_EXPORT std::ostream& operator<<(std::ostream& os, TimeTicks time_ticks);
diff --git a/time/time_unittest.cc b/time/time_unittest.cc index 6c12423..456782c 100644 --- a/time/time_unittest.cc +++ b/time/time_unittest.cc
@@ -717,7 +717,7 @@ TEST(TimeTicks, SnappedToNextTickBasic) { base::TimeTicks phase = base::TimeTicks::FromInternalValue(4000); - base::TimeDelta interval = base::TimeDelta::FromInternalValue(1000); + base::TimeDelta interval = base::TimeDelta::FromMicroseconds(1000); base::TimeTicks timestamp; // Timestamp in previous interval. @@ -760,7 +760,7 @@ // int(big_timestamp / interval) < 0, so this causes a crash if the number of // intervals elapsed is attempted to be stored in an int. base::TimeTicks phase = base::TimeTicks::FromInternalValue(0); - base::TimeDelta interval = base::TimeDelta::FromInternalValue(4000); + base::TimeDelta interval = base::TimeDelta::FromMicroseconds(4000); base::TimeTicks big_timestamp = base::TimeTicks::FromInternalValue(8635916564000);
diff --git a/trace_event/memory_allocator_dump.cc b/trace_event/memory_allocator_dump.cc index 77c32ec..edec31b 100644 --- a/trace_event/memory_allocator_dump.cc +++ b/trace_event/memory_allocator_dump.cc
@@ -109,7 +109,7 @@ value->BeginDictionary("attrs"); for (DictionaryValue::Iterator it(attributes_); !it.IsAtEnd(); it.Advance()) - value->SetValue(it.key().c_str(), it.value().DeepCopy()); + value->SetValue(it.key().c_str(), it.value().CreateDeepCopy()); value->EndDictionary(); // "attrs": { ... } value->EndDictionary(); // "allocator_name/heap_subheap": { ... }
diff --git a/trace_event/memory_dump_request_args.h b/trace_event/memory_dump_request_args.h index 4fb0335..4d3763a 100644 --- a/trace_event/memory_dump_request_args.h +++ b/trace_event/memory_dump_request_args.h
@@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef BASE_TRACE_EVENT_MEMORY_DUMP_REQUEST_H_ -#define BASE_TRACE_EVENT_MEMORY_DUMP_REQUEST_H_ +#ifndef BASE_TRACE_EVENT_MEMORY_DUMP_REQUEST_ARGS_H_ +#define BASE_TRACE_EVENT_MEMORY_DUMP_REQUEST_ARGS_H_ // This file defines the types and structs used to issue memory dump requests. // These are also used in the IPCs for coordinating inter-process memory dumps. @@ -38,4 +38,4 @@ } // namespace trace_event } // namespace base -#endif // BASE_TRACE_EVENT_MEMORY_DUMP_REQUEST_H_ +#endif // BASE_TRACE_EVENT_MEMORY_DUMP_REQUEST_ARGS_H_
diff --git a/trace_event/trace_event.h b/trace_event/trace_event.h index 86f959f..2c30b33 100644 --- a/trace_event/trace_event.h +++ b/trace_event/trace_event.h
@@ -400,6 +400,13 @@ INTERNAL_TRACE_EVENT_ADD_WITH_ID_TID_AND_TIMESTAMP( \ TRACE_EVENT_PHASE_ASYNC_BEGIN, category_group, name, id, thread_id, \ timestamp, TRACE_EVENT_FLAG_COPY, arg1_name, arg1_val) +#define TRACE_EVENT_COPY_BEGIN_WITH_ID_TID_AND_TIMESTAMP2( \ + category_group, name, id, thread_id, timestamp, arg1_name, arg1_val, \ + arg2_name, arg2_val) \ + INTERNAL_TRACE_EVENT_ADD_WITH_ID_TID_AND_TIMESTAMP( \ + TRACE_EVENT_PHASE_ASYNC_BEGIN, category_group, name, id, thread_id, \ + timestamp, TRACE_EVENT_FLAG_COPY, arg1_name, arg1_val, arg2_name, \ + arg2_val) // Records a single END event for "name" immediately. If the category // is not enabled, then this does nothing. @@ -449,6 +456,13 @@ INTERNAL_TRACE_EVENT_ADD_WITH_ID_TID_AND_TIMESTAMP( \ TRACE_EVENT_PHASE_ASYNC_END, category_group, name, id, thread_id, \ timestamp, TRACE_EVENT_FLAG_COPY, arg1_name, arg1_val) +#define TRACE_EVENT_COPY_END_WITH_ID_TID_AND_TIMESTAMP2( \ + category_group, name, id, thread_id, timestamp, arg1_name, arg1_val, \ + arg2_name, arg2_val) \ + INTERNAL_TRACE_EVENT_ADD_WITH_ID_TID_AND_TIMESTAMP( \ + TRACE_EVENT_PHASE_ASYNC_END, category_group, name, id, thread_id, \ + timestamp, TRACE_EVENT_FLAG_COPY, arg1_name, arg1_val, arg2_name, \ + arg2_val) // Records the value of a counter called "name" immediately. Value // must be representable as a 32 bit integer. @@ -702,27 +716,43 @@ // NESTABLE_ASYNC event of that id. Corresponding warning messages for // unmatched events will be shown in the analysis view. -// Records a single NESTABLE_ASYNC_BEGIN event called "name" immediately, with 2 -// associated arguments. If the category is not enabled, then this does nothing. +// Records a single NESTABLE_ASYNC_BEGIN event called "name" immediately, with +// 0, 1 or 2 associated arguments. If the category is not enabled, then this +// does nothing. +#define TRACE_EVENT_NESTABLE_ASYNC_BEGIN0(category_group, name, id) \ + INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_NESTABLE_ASYNC_BEGIN, \ + category_group, name, id, TRACE_EVENT_FLAG_NONE) +#define TRACE_EVENT_NESTABLE_ASYNC_BEGIN1(category_group, name, id, arg1_name, \ + arg1_val) \ + INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_NESTABLE_ASYNC_BEGIN, \ + category_group, name, id, TRACE_EVENT_FLAG_NONE, arg1_name, arg1_val) #define TRACE_EVENT_NESTABLE_ASYNC_BEGIN2(category_group, name, id, arg1_name, \ arg1_val, arg2_name, arg2_val) \ INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_NESTABLE_ASYNC_BEGIN, \ category_group, name, id, TRACE_EVENT_FLAG_NONE, arg1_name, arg1_val, \ arg2_name, arg2_val) +// Records a single NESTABLE_ASYNC_END event called "name" immediately, with 0, +// 1, or 2 associated arguments. If the category is not enabled, then this does +// nothing. +#define TRACE_EVENT_NESTABLE_ASYNC_END0(category_group, name, id) \ + INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_NESTABLE_ASYNC_END, \ + category_group, name, id, TRACE_EVENT_FLAG_NONE) +#define TRACE_EVENT_NESTABLE_ASYNC_END1(category_group, name, id, arg1_name, \ + arg1_val) \ + INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_NESTABLE_ASYNC_END, \ + category_group, name, id, TRACE_EVENT_FLAG_NONE, arg1_name, arg1_val) +#define TRACE_EVENT_NESTABLE_ASYNC_END2(category_group, name, id, arg1_name, \ + arg1_val, arg2_name, arg2_val) \ + INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_NESTABLE_ASYNC_END, \ + category_group, name, id, TRACE_EVENT_FLAG_NONE, arg1_name, arg1_val, \ + arg2_name, arg2_val) + #define TRACE_EVENT_COPY_NESTABLE_ASYNC_BEGIN_WITH_TTS2(category_group, name, \ id, arg1_name, arg1_val, arg2_name, arg2_val) \ INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_NESTABLE_ASYNC_BEGIN, \ category_group, name, id, \ TRACE_EVENT_FLAG_ASYNC_TTS | TRACE_EVENT_FLAG_COPY, \ arg1_name, arg1_val, arg2_name, arg2_val) - -// Records a single NESTABLE_ASYNC_END event called "name" immediately, with 2 -// associated arguments. If the category is not enabled, then this does nothing. -#define TRACE_EVENT_NESTABLE_ASYNC_END2(category_group, name, id, arg1_name, \ - arg1_val, arg2_name, arg2_val) \ - INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_NESTABLE_ASYNC_END, \ - category_group, name, id, TRACE_EVENT_FLAG_NONE, arg1_name, arg1_val, \ - arg2_name, arg2_val) #define TRACE_EVENT_COPY_NESTABLE_ASYNC_END_WITH_TTS2(category_group, name, \ id, arg1_name, arg1_val, arg2_name, arg2_val) \ INTERNAL_TRACE_EVENT_ADD_WITH_ID(TRACE_EVENT_PHASE_NESTABLE_ASYNC_END, \ @@ -1445,8 +1475,8 @@ const char* name, unsigned long long id, unsigned char flags) { - int thread_id = static_cast<int>(base::PlatformThread::CurrentId()); - base::TimeTicks now = base::TimeTicks::NowFromSystemTraceTime(); + const int thread_id = static_cast<int>(base::PlatformThread::CurrentId()); + const base::TimeTicks now = base::TimeTicks::NowFromSystemTraceTime(); return AddTraceEventWithThreadIdAndTimestamp(phase, category_group_enabled, name, id, thread_id, now, flags); }
diff --git a/trace_event/trace_event_argument.cc b/trace_event/trace_event_argument.cc index 91ed9e8..88b1879 100644 --- a/trace_event/trace_event_argument.cc +++ b/trace_event/trace_event_argument.cc
@@ -34,19 +34,19 @@ GetCurrentDictionary()->SetString(name, value); } -void TracedValue::SetValue(const char* name, Value* value) { - GetCurrentDictionary()->Set(name, value); +void TracedValue::SetValue(const char* name, scoped_ptr<Value> value) { + GetCurrentDictionary()->Set(name, value.Pass()); } void TracedValue::BeginDictionary(const char* name) { DictionaryValue* dictionary = new DictionaryValue(); - GetCurrentDictionary()->Set(name, dictionary); + GetCurrentDictionary()->Set(name, make_scoped_ptr(dictionary)); stack_.push_back(dictionary); } void TracedValue::BeginArray(const char* name) { ListValue* array = new ListValue(); - GetCurrentDictionary()->Set(name, array); + GetCurrentDictionary()->Set(name, make_scoped_ptr(array)); stack_.push_back(array); }
diff --git a/trace_event/trace_event_argument.h b/trace_event/trace_event_argument.h index 78d37d4..d86cfd1 100644 --- a/trace_event/trace_event_argument.h +++ b/trace_event/trace_event_argument.h
@@ -29,7 +29,7 @@ void SetDouble(const char* name, double); void SetBoolean(const char* name, bool value); void SetString(const char* name, const std::string& value); - void SetValue(const char* name, Value* value); + void SetValue(const char* name, scoped_ptr<Value> value); void BeginDictionary(const char* name); void BeginArray(const char* name); @@ -49,7 +49,7 @@ ListValue* GetCurrentArray(); scoped_ptr<base::Value> root_; - std::vector<Value*> stack_; + std::vector<Value*> stack_; // Weak references. DISALLOW_COPY_AND_ASSIGN(TracedValue); };
diff --git a/trace_event/trace_event_etw_export_win.cc b/trace_event/trace_event_etw_export_win.cc index f7f9ecc..1cb3b8c 100644 --- a/trace_event/trace_event_etw_export_win.cc +++ b/trace_event/trace_event_etw_export_win.cc
@@ -106,12 +106,14 @@ // static void TraceEventETWExport::EnableETWExport() { - GetInstance()->ETWExportEnabled_ = true; + if (GetInstance()) + GetInstance()->ETWExportEnabled_ = true; } // static void TraceEventETWExport::DisableETWExport() { - GetInstance()->ETWExportEnabled_ = false; + if (GetInstance()) + GetInstance()->ETWExportEnabled_ = false; } // static @@ -126,7 +128,8 @@ const unsigned long long* arg_values, const scoped_refptr<ConvertableToTraceFormat>* convertable_values) { // We bail early in case exporting is disabled or no consumer is listening. - if (!GetInstance()->ETWExportEnabled_ || !EventEnabledChromeEvent()) + if (!GetInstance() || !GetInstance()->ETWExportEnabled_ || + !EventEnabledChromeEvent()) return; std::string phase_string; @@ -224,7 +227,8 @@ const char* arg_value_2, const char* arg_name_3, const char* arg_value_3) { - if (!GetInstance()->ETWExportEnabled_ || !EventEnabledChromeEvent()) + if (!GetInstance() || !GetInstance()->ETWExportEnabled_ || + !EventEnabledChromeEvent()) return; EventWriteChromeEvent(name, phase, arg_name_1, arg_value_1, arg_name_2,
diff --git a/trace_event/trace_event_etw_export_win.h b/trace_event/trace_event_etw_export_win.h index 0a551c3..eefe820 100644 --- a/trace_event/trace_event_etw_export_win.h +++ b/trace_event/trace_event_etw_export_win.h
@@ -3,8 +3,8 @@ // found in the LICENSE file. // This file contains the Windows-specific exporting to ETW. -#ifndef BASE_TRACE_EVENT_TRACE_ETW_EXPORT_H_ -#define BASE_TRACE_EVENT_TRACE_ETW_EXPORT_H_ +#ifndef BASE_TRACE_EVENT_TRACE_EVENT_ETW_EXPORT_WIN_H_ +#define BASE_TRACE_EVENT_TRACE_EVENT_ETW_EXPORT_WIN_H_ #include "base/base_export.h" #include "base/trace_event/trace_event_impl.h" @@ -29,7 +29,9 @@ static void EnableETWExport(); static void DisableETWExport(); - static bool isETWExportEnabled() { return GetInstance()->ETWExportEnabled_; } + static bool isETWExportEnabled() { + return (GetInstance() && GetInstance()->ETWExportEnabled_); + } // Exports an event to ETW. This is mainly used in // TraceLog::AddTraceEventWithThreadIdAndTimestamp to export internal events. @@ -70,4 +72,4 @@ } // namespace trace_event } // namespace base -#endif // BASE_TRACE_EVENT_TRACE_ETW_EXPORT_H_ +#endif // BASE_TRACE_EVENT_TRACE_EVENT_ETW_EXPORT_WIN_H_
diff --git a/trace_event/trace_event_memory.cc b/trace_event/trace_event_memory.cc index ab8ba0d..8959589 100644 --- a/trace_event/trace_event_memory.cc +++ b/trace_event/trace_event_memory.cc
@@ -71,13 +71,12 @@ delete stack; } -// Initializes the thread-local TraceMemoryStack pointer. Returns true on -// success or if it is already initialized. -bool InitThreadLocalStorage() { +// Initializes the thread-local TraceMemoryStack pointer. +void InitThreadLocalStorage() { if (tls_trace_memory_stack.initialized()) - return true; - // Initialize the thread-local storage key, returning true on success. - return tls_trace_memory_stack.Initialize(&DeleteStackOnThreadCleanup); + return; + // Initialize the thread-local storage key. + tls_trace_memory_stack.Initialize(&DeleteStackOnThreadCleanup); } // Clean up thread-local-storage in the main thread. @@ -195,8 +194,7 @@ if (dump_timer_.IsRunning()) return; DVLOG(1) << "Starting trace memory"; - if (!InitThreadLocalStorage()) - return; + InitThreadLocalStorage(); ScopedTraceMemory::set_enabled(true); // Call ::HeapProfilerWithPseudoStackStart(). heap_profiler_start_function_(&GetPseudoStack);
diff --git a/trace_event/trace_event_unittest.cc b/trace_event/trace_event_unittest.cc index 17953e7..d032243 100644 --- a/trace_event/trace_event_unittest.cc +++ b/trace_event/trace_event_unittest.cc
@@ -1527,7 +1527,7 @@ // See if this thread name is one of the threads we just created for (int j = 0; j < kNumThreads; j++) { - if(static_cast<int>(thread_ids[j]) != tmp_int) + if (static_cast<int>(thread_ids[j]) != tmp_int) continue; std::string expected_name = StringPrintf("Thread %d", j); @@ -2214,7 +2214,7 @@ } void TearDown() override { TraceLog::GetInstance()->SetDisabled(); - ASSERT_TRUE(!!s_instance); + ASSERT_TRUE(s_instance); s_instance = NULL; TraceEventTestFixture::TearDown(); }
diff --git a/tracked_objects.cc b/tracked_objects.cc index 9029f4f..9db05c0 100644 --- a/tracked_objects.cc +++ b/tracked_objects.cc
@@ -168,32 +168,6 @@ } } -int DeathData::count() const { return count_; } - -int32 DeathData::run_duration_sum() const { return run_duration_sum_; } - -int32 DeathData::run_duration_max() const { return run_duration_max_; } - -int32 DeathData::run_duration_sample() const { - return run_duration_sample_; -} - -int32 DeathData::queue_duration_sum() const { - return queue_duration_sum_; -} - -int32 DeathData::queue_duration_max() const { - return queue_duration_max_; -} - -int32 DeathData::queue_duration_sample() const { - return queue_duration_sample_; -} - -const DeathDataPhaseSnapshot* DeathData::last_phase_snapshot() const { - return last_phase_snapshot_; -} - void DeathData::OnProfilingPhaseCompleted(int profiling_phase) { // Snapshotting and storing current state. last_phase_snapshot_ = new DeathDataPhaseSnapshot( @@ -388,8 +362,7 @@ // static void ThreadData::InitializeThreadContext(const std::string& suggested_name) { - if (!Initialize()) // Always initialize if needed. - return; + Initialize(); ThreadData* current_thread_data = reinterpret_cast<ThreadData*>(tls_index_.Get()); if (current_thread_data) @@ -718,9 +691,9 @@ ThreadData::SetAlternateTimeSource(alternate_time_source); } -bool ThreadData::Initialize() { +void ThreadData::Initialize() { if (status_ >= DEACTIVATED) - return true; // Someone else did the initialization. + return; // Someone else did the initialization. // Due to racy lazy initialization in tests, we'll need to recheck status_ // after we acquire the lock. @@ -729,7 +702,7 @@ // initialization. base::AutoLock lock(*list_lock_.Pointer()); if (status_ >= DEACTIVATED) - return true; // Someone raced in here and beat us. + return; // Someone raced in here and beat us. // Put an alternate timer in place if the environment calls for it, such as // for tracking TCMalloc allocations. This insertion is idempotent, so we @@ -743,8 +716,7 @@ if (!tls_index_.initialized()) { // Testing may have initialized this. DCHECK_EQ(status_, UNINITIALIZED); tls_index_.Initialize(&ThreadData::OnThreadTermination); - if (!tls_index_.initialized()) - return false; + DCHECK(tls_index_.initialized()); } else { // TLS was initialzed for us earlier. DCHECK_EQ(status_, DORMANT_DURING_TESTS); @@ -759,21 +731,18 @@ // we get the lock earlier in this method. status_ = kInitialStartupState; DCHECK(status_ != UNINITIALIZED); - return true; } // static -bool ThreadData::InitializeAndSetTrackingStatus(Status status) { +void ThreadData::InitializeAndSetTrackingStatus(Status status) { DCHECK_GE(status, DEACTIVATED); DCHECK_LE(status, PROFILING_ACTIVE); - if (!Initialize()) // No-op if already initialized. - return false; // Not compiled in. + Initialize(); // No-op if already initialized. if (status > DEACTIVATED) status = PROFILING_ACTIVE; status_ = status; - return true; } // static @@ -827,8 +796,8 @@ // This is only called from test code, where we need to cleanup so that // additional tests can be run. // We must be single threaded... but be careful anyway. - if (!InitializeAndSetTrackingStatus(DEACTIVATED)) - return; + InitializeAndSetTrackingStatus(DEACTIVATED); + ThreadData* thread_data_list; { base::AutoLock lock(*list_lock_.Pointer());
diff --git a/tracked_objects.h b/tracked_objects.h index cd69fb7..8f83794 100644 --- a/tracked_objects.h +++ b/tracked_objects.h
@@ -330,14 +330,16 @@ // Metrics and past snapshots accessors, used only for serialization and in // tests. - int count() const; - int32 run_duration_sum() const; - int32 run_duration_max() const; - int32 run_duration_sample() const; - int32 queue_duration_sum() const; - int32 queue_duration_max() const; - int32 queue_duration_sample() const; - const DeathDataPhaseSnapshot* last_phase_snapshot() const; + int count() const { return count_; } + int32 run_duration_sum() const { return run_duration_sum_; } + int32 run_duration_max() const { return run_duration_max_; } + int32 run_duration_sample() const { return run_duration_sample_; } + int32 queue_duration_sum() const { return queue_duration_sum_; } + int32 queue_duration_max() const { return queue_duration_max_; } + int32 queue_duration_sample() const { return queue_duration_sample_; } + const DeathDataPhaseSnapshot* last_phase_snapshot() const { + return last_phase_snapshot_; + } // Called when the current profiling phase, identified by |profiling_phase|, // ends. @@ -497,14 +499,13 @@ const std::string& thread_name() const { return thread_name_; } // Initializes all statics if needed (this initialization call should be made - // while we are single threaded). Returns false if unable to initialize. - static bool Initialize(); + // while we are single threaded). + static void Initialize(); // Sets internal status_. // If |status| is false, then status_ is set to DEACTIVATED. // If |status| is true, then status_ is set to PROFILING_ACTIVE. - // If it fails to initialize the TLS slot, this function will return false. - static bool InitializeAndSetTrackingStatus(Status status); + static void InitializeAndSetTrackingStatus(Status status); static Status status();
diff --git a/tracked_objects_unittest.cc b/tracked_objects_unittest.cc index 3537e54..cdbf9ac 100644 --- a/tracked_objects_unittest.cc +++ b/tracked_objects_unittest.cc
@@ -119,11 +119,7 @@ unsigned int TrackedObjectsTest::test_time_; TEST_F(TrackedObjectsTest, TaskStopwatchNoStartStop) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); // Check that creating and destroying a stopwatch without starting it doesn't // crash. @@ -132,11 +128,7 @@ TEST_F(TrackedObjectsTest, MinimalStartupShutdown) { // Minimal test doesn't even create any tasks. - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); EXPECT_FALSE(ThreadData::first()); // No activity even on this thread. ThreadData* data = ThreadData::Get(); @@ -154,8 +146,7 @@ Reset(); // Do it again, just to be sure we reset state completely. - EXPECT_TRUE( - ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE)); + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); EXPECT_FALSE(ThreadData::first()); // No activity even on this thread. data = ThreadData::Get(); EXPECT_TRUE(ThreadData::first()); // Now class was constructed. @@ -170,11 +161,7 @@ } TEST_F(TrackedObjectsTest, TinyStartupShutdown) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); // Instigate tracking on a single tracked object, on our thread. const char kFunction[] = "TinyStartupShutdown"; @@ -250,11 +237,7 @@ } TEST_F(TrackedObjectsTest, DeathDataTestRecordDeath) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); scoped_ptr<DeathData> data(new DeathData()); ASSERT_NE(data, reinterpret_cast<DeathData*>(NULL)); @@ -293,11 +276,7 @@ } TEST_F(TrackedObjectsTest, DeathDataTest2Phases) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); scoped_ptr<DeathData> data(new DeathData()); ASSERT_NE(data, reinterpret_cast<DeathData*>(NULL)); @@ -362,11 +341,7 @@ } TEST_F(TrackedObjectsTest, Delta) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); DeathDataSnapshot snapshot; snapshot.count = 10; @@ -398,10 +373,7 @@ TEST_F(TrackedObjectsTest, DeactivatedBirthOnlyToSnapshotWorkerThread) { // Start in the deactivated state. - if (!ThreadData::InitializeAndSetTrackingStatus(ThreadData::DEACTIVATED)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::DEACTIVATED); const char kFunction[] = "DeactivatedBirthOnlyToSnapshotWorkerThread"; Location location(kFunction, kFile, kLineNumber, NULL); @@ -423,10 +395,7 @@ TEST_F(TrackedObjectsTest, DeactivatedBirthOnlyToSnapshotMainThread) { // Start in the deactivated state. - if (!ThreadData::InitializeAndSetTrackingStatus(ThreadData::DEACTIVATED)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::DEACTIVATED); const char kFunction[] = "DeactivatedBirthOnlyToSnapshotMainThread"; Location location(kFunction, kFile, kLineNumber, NULL); @@ -447,11 +416,7 @@ } TEST_F(TrackedObjectsTest, BirthOnlyToSnapshotWorkerThread) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); const char kFunction[] = "BirthOnlyToSnapshotWorkerThread"; Location location(kFunction, kFile, kLineNumber, NULL); @@ -464,11 +429,7 @@ } TEST_F(TrackedObjectsTest, BirthOnlyToSnapshotMainThread) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); const char kFunction[] = "BirthOnlyToSnapshotMainThread"; Location location(kFunction, kFile, kLineNumber, NULL); @@ -481,11 +442,7 @@ } TEST_F(TrackedObjectsTest, LifeCycleToSnapshotMainThread) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); const char kFunction[] = "LifeCycleToSnapshotMainThread"; Location location(kFunction, kFile, kLineNumber, NULL); @@ -514,11 +471,7 @@ } TEST_F(TrackedObjectsTest, TwoPhases) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); const char kFunction[] = "TwoPhases"; Location location(kFunction, kFile, kLineNumber, NULL); @@ -617,11 +570,7 @@ } TEST_F(TrackedObjectsTest, ThreePhases) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); const char kFunction[] = "ThreePhases"; Location location(kFunction, kFile, kLineNumber, NULL); @@ -762,11 +711,7 @@ } TEST_F(TrackedObjectsTest, TwoPhasesSecondEmpty) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); const char kFunction[] = "TwoPhasesSecondEmpty"; Location location(kFunction, kFile, kLineNumber, NULL); @@ -829,11 +774,7 @@ } TEST_F(TrackedObjectsTest, TwoPhasesFirstEmpty) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); ThreadData::OnProfilingPhaseCompleted(0); @@ -894,11 +835,7 @@ // our tallied births are matched by tallied deaths (except for when the // task is still running, or is queued). TEST_F(TrackedObjectsTest, LifeCycleMidDeactivatedToSnapshotMainThread) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); const char kFunction[] = "LifeCycleMidDeactivatedToSnapshotMainThread"; Location location(kFunction, kFile, kLineNumber, NULL); @@ -911,8 +848,7 @@ pending_task.time_posted = kTimePosted; // Overwrite implied Now(). // Turn off tracking now that we have births. - EXPECT_TRUE( - ThreadData::InitializeAndSetTrackingStatus(ThreadData::DEACTIVATED)); + ThreadData::InitializeAndSetTrackingStatus(ThreadData::DEACTIVATED); const unsigned int kStartOfRun = 5; const unsigned int kEndOfRun = 7; @@ -934,10 +870,7 @@ // the birth nor the death will be recorded. TEST_F(TrackedObjectsTest, LifeCyclePreDeactivatedToSnapshotMainThread) { // Start in the deactivated state. - if (!ThreadData::InitializeAndSetTrackingStatus(ThreadData::DEACTIVATED)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::DEACTIVATED); const char kFunction[] = "LifeCyclePreDeactivatedToSnapshotMainThread"; Location location(kFunction, kFile, kLineNumber, NULL); @@ -974,11 +907,7 @@ } TEST_F(TrackedObjectsTest, TwoLives) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); const char kFunction[] = "TwoLives"; Location location(kFunction, kFile, kLineNumber, NULL); @@ -1018,11 +947,7 @@ } TEST_F(TrackedObjectsTest, DifferentLives) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); // Use a well named thread. ThreadData::InitializeThreadContext(kMainThreadName); @@ -1094,11 +1019,7 @@ } TEST_F(TrackedObjectsTest, TaskWithNestedExclusion) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); const char kFunction[] = "TaskWithNestedExclusion"; Location location(kFunction, kFile, kLineNumber, NULL); @@ -1132,11 +1053,7 @@ } TEST_F(TrackedObjectsTest, TaskWith2NestedExclusions) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); const char kFunction[] = "TaskWith2NestedExclusions"; Location location(kFunction, kFile, kLineNumber, NULL); @@ -1176,11 +1093,7 @@ } TEST_F(TrackedObjectsTest, TaskWithNestedExclusionWithNestedTask) { - if (!ThreadData::InitializeAndSetTrackingStatus( - ThreadData::PROFILING_ACTIVE)) { - // Don't run the test if task tracking is not compiled in. - return; - } + ThreadData::InitializeAndSetTrackingStatus(ThreadData::PROFILING_ACTIVE); const char kFunction[] = "TaskWithNestedExclusionWithNestedTask"; Location location(kFunction, kFile, kLineNumber, NULL);
diff --git a/tuple.h b/tuple.h index 88c3075..4628aa9 100644 --- a/tuple.h +++ b/tuple.h
@@ -25,8 +25,8 @@ // DispatchToMethod(&foo, &Foo::SomeMeth, MakeTuple(1, 2, 3)); // // foo->SomeMeth(1, 2, 3); -#ifndef BASE_TUPLE_H__ -#define BASE_TUPLE_H__ +#ifndef BASE_TUPLE_H_ +#define BASE_TUPLE_H_ #include "base/bind_helpers.h" @@ -329,4 +329,4 @@ MakeIndexSequence<sizeof...(OutTs)>()); } -#endif // BASE_TUPLE_H__ +#endif // BASE_TUPLE_H_
diff --git a/values.cc b/values.cc index 0e1e2b1..4093eba 100644 --- a/values.cc +++ b/values.cc
@@ -85,8 +85,8 @@ } // static -Value* Value::CreateNullValue() { - return new Value(TYPE_NULL); +scoped_ptr<Value> Value::CreateNullValue() { + return make_scoped_ptr(new Value(TYPE_NULL)); } bool Value::GetAsBinary(const BinaryValue** out_value) const { @@ -137,7 +137,11 @@ // This method should only be getting called for null Values--all subclasses // need to provide their own implementation;. DCHECK(IsType(TYPE_NULL)); - return CreateNullValue(); + return CreateNullValue().release(); +} + +scoped_ptr<Value> Value::CreateDeepCopy() const { + return make_scoped_ptr(DeepCopy()); } bool Value::Equals(const Value* other) const { @@ -829,6 +833,10 @@ return result; } +scoped_ptr<DictionaryValue> DictionaryValue::CreateDeepCopy() const { + return make_scoped_ptr(DeepCopy()); +} + bool DictionaryValue::Equals(const Value* other) const { if (other->GetType() != GetType()) return false; @@ -883,6 +891,10 @@ return true; } +bool ListValue::Set(size_t index, scoped_ptr<Value> in_value) { + return Set(index, in_value.release()); +} + bool ListValue::Get(size_t index, const Value** out_value) const { if (index >= list_.size()) return false; @@ -1032,6 +1044,10 @@ return list_.erase(iter); } +void ListValue::Append(scoped_ptr<Value> in_value) { + Append(in_value.release()); +} + void ListValue::Append(Value* in_value) { DCHECK(in_value); list_.push_back(in_value); @@ -1121,6 +1137,10 @@ return result; } +scoped_ptr<ListValue> ListValue::CreateDeepCopy() const { + return make_scoped_ptr(DeepCopy()); +} + bool ListValue::Equals(const Value* other) const { if (other->GetType() != GetType()) return false;
diff --git a/values.h b/values.h index 1e1cae3..e32edec 100644 --- a/values.h +++ b/values.h
@@ -64,7 +64,7 @@ virtual ~Value(); - static Value* CreateNullValue(); + static scoped_ptr<Value> CreateNullValue(); // Returns the type of the value stored by the current Value object. // Each type will be implemented by only one subclass of Value, so it's @@ -99,6 +99,8 @@ // Subclasses return their own type directly in their overrides; // this works because C++ supports covariant return types. virtual Value* DeepCopy() const; + // Preferred version of DeepCopy. TODO(estade): remove the above. + scoped_ptr<Value> CreateDeepCopy() const; // Compares if two Value objects have equal contents. virtual bool Equals(const Value* other) const; @@ -368,6 +370,8 @@ // Overridden from Value: DictionaryValue* DeepCopy() const override; + // Preferred version of DeepCopy. TODO(estade): remove the above. + scoped_ptr<DictionaryValue> CreateDeepCopy() const; bool Equals(const Value* other) const override; private: @@ -400,6 +404,8 @@ // Returns true if successful, or false if the index was negative or // the value is a null pointer. bool Set(size_t index, Value* in_value); + // Preferred version of the above. TODO(estade): remove the above. + bool Set(size_t index, scoped_ptr<Value> in_value); // Gets the Value at the given index. Modifies |out_value| (and returns true) // only if the index falls within the current list range. @@ -445,6 +451,8 @@ iterator Erase(iterator iter, scoped_ptr<Value>* out_value); // Appends a Value to the end of the list. + void Append(scoped_ptr<Value> in_value); + // Deprecated version of the above. TODO(estade): remove. void Append(Value* in_value); // Convenience forms of Append. @@ -486,6 +494,9 @@ ListValue* DeepCopy() const override; bool Equals(const Value* other) const override; + // Preferred version of DeepCopy. TODO(estade): remove DeepCopy. + scoped_ptr<ListValue> CreateDeepCopy() const; + private: ValueVector list_;
diff --git a/values_unittest.cc b/values_unittest.cc index b66730b..6466a96 100644 --- a/values_unittest.cc +++ b/values_unittest.cc
@@ -20,7 +20,7 @@ ASSERT_EQ(std::string("http://google.com"), homepage); ASSERT_FALSE(settings.Get("global", NULL)); - settings.Set("global", new FundamentalValue(true)); + settings.SetBoolean("global", true); ASSERT_TRUE(settings.Get("global", NULL)); settings.SetString("global.homepage", "http://scurvy.com"); ASSERT_TRUE(settings.Get("global", NULL)); @@ -33,14 +33,14 @@ ASSERT_FALSE( settings.GetList("global.toolbar.bookmarks", &toolbar_bookmarks)); - toolbar_bookmarks = new ListValue; - settings.Set("global.toolbar.bookmarks", toolbar_bookmarks); + scoped_ptr<ListValue> new_toolbar_bookmarks(new ListValue); + settings.Set("global.toolbar.bookmarks", new_toolbar_bookmarks.Pass()); ASSERT_TRUE(settings.GetList("global.toolbar.bookmarks", &toolbar_bookmarks)); - DictionaryValue* new_bookmark = new DictionaryValue; + scoped_ptr<DictionaryValue> new_bookmark(new DictionaryValue); new_bookmark->SetString("name", "Froogle"); new_bookmark->SetString("url", "http://froogle.com"); - toolbar_bookmarks->Append(new_bookmark); + toolbar_bookmarks->Append(new_bookmark.Pass()); ListValue* bookmark_list; ASSERT_TRUE(settings.GetList("global.toolbar.bookmarks", &bookmark_list)); @@ -57,10 +57,10 @@ TEST(ValuesTest, List) { scoped_ptr<ListValue> mixed_list(new ListValue()); - mixed_list->Set(0, new FundamentalValue(true)); - mixed_list->Set(1, new FundamentalValue(42)); - mixed_list->Set(2, new FundamentalValue(88.8)); - mixed_list->Set(3, new StringValue("foo")); + mixed_list->Set(0, make_scoped_ptr(new FundamentalValue(true))); + mixed_list->Set(1, make_scoped_ptr(new FundamentalValue(42))); + mixed_list->Set(2, make_scoped_ptr(new FundamentalValue(88.8))); + mixed_list->Set(3, make_scoped_ptr(new StringValue("foo"))); ASSERT_EQ(4u, mixed_list->GetSize()); Value *value = NULL; @@ -112,11 +112,12 @@ ASSERT_EQ(0U, binary->GetSize()); // Test the common case of a non-empty buffer - char* buffer = new char[15]; - binary.reset(new BinaryValue(scoped_ptr<char[]>(buffer), 15)); + scoped_ptr<char[]> buffer(new char[15]); + char* original_buffer = buffer.get(); + binary.reset(new BinaryValue(buffer.Pass(), 15)); ASSERT_TRUE(binary.get()); ASSERT_TRUE(binary->GetBuffer()); - ASSERT_EQ(buffer, binary->GetBuffer()); + ASSERT_EQ(original_buffer, binary->GetBuffer()); ASSERT_EQ(15U, binary->GetSize()); char stack_buffer[42]; @@ -194,14 +195,14 @@ { ListValue list; - list.Append(new DeletionTestValue(&deletion_flag)); + list.Append(make_scoped_ptr(new DeletionTestValue(&deletion_flag))); EXPECT_FALSE(deletion_flag); } EXPECT_TRUE(deletion_flag); { ListValue list; - list.Append(new DeletionTestValue(&deletion_flag)); + list.Append(make_scoped_ptr(new DeletionTestValue(&deletion_flag))); EXPECT_FALSE(deletion_flag); list.Clear(); EXPECT_TRUE(deletion_flag); @@ -209,7 +210,7 @@ { ListValue list; - list.Append(new DeletionTestValue(&deletion_flag)); + list.Append(make_scoped_ptr(new DeletionTestValue(&deletion_flag))); EXPECT_FALSE(deletion_flag); EXPECT_TRUE(list.Set(0, Value::CreateNullValue())); EXPECT_TRUE(deletion_flag); @@ -222,7 +223,7 @@ { ListValue list; - list.Append(new DeletionTestValue(&deletion_flag)); + list.Append(make_scoped_ptr(new DeletionTestValue(&deletion_flag))); EXPECT_FALSE(deletion_flag); EXPECT_EQ(1U, list.GetSize()); EXPECT_FALSE(list.Remove(std::numeric_limits<size_t>::max(), @@ -238,7 +239,7 @@ { ListValue list; - list.Append(new DeletionTestValue(&deletion_flag)); + list.Append(make_scoped_ptr(new DeletionTestValue(&deletion_flag))); EXPECT_FALSE(deletion_flag); EXPECT_TRUE(list.Remove(0, NULL)); EXPECT_TRUE(deletion_flag); @@ -247,11 +248,12 @@ { ListValue list; - DeletionTestValue* value = new DeletionTestValue(&deletion_flag); - list.Append(value); + scoped_ptr<DeletionTestValue> value(new DeletionTestValue(&deletion_flag)); + DeletionTestValue* original_value = value.get(); + list.Append(value.Pass()); EXPECT_FALSE(deletion_flag); size_t index = 0; - list.Remove(*value, &index); + list.Remove(*original_value, &index); EXPECT_EQ(0U, index); EXPECT_TRUE(deletion_flag); EXPECT_EQ(0U, list.GetSize()); @@ -264,14 +266,14 @@ { DictionaryValue dict; - dict.Set(key, new DeletionTestValue(&deletion_flag)); + dict.Set(key, make_scoped_ptr(new DeletionTestValue(&deletion_flag))); EXPECT_FALSE(deletion_flag); } EXPECT_TRUE(deletion_flag); { DictionaryValue dict; - dict.Set(key, new DeletionTestValue(&deletion_flag)); + dict.Set(key, make_scoped_ptr(new DeletionTestValue(&deletion_flag))); EXPECT_FALSE(deletion_flag); dict.Clear(); EXPECT_TRUE(deletion_flag); @@ -279,7 +281,7 @@ { DictionaryValue dict; - dict.Set(key, new DeletionTestValue(&deletion_flag)); + dict.Set(key, make_scoped_ptr(new DeletionTestValue(&deletion_flag))); EXPECT_FALSE(deletion_flag); dict.Set(key, Value::CreateNullValue()); EXPECT_TRUE(deletion_flag); @@ -293,7 +295,7 @@ { DictionaryValue dict; - dict.Set(key, new DeletionTestValue(&deletion_flag)); + dict.Set(key, make_scoped_ptr(new DeletionTestValue(&deletion_flag))); EXPECT_FALSE(deletion_flag); EXPECT_TRUE(dict.HasKey(key)); EXPECT_FALSE(dict.Remove("absent key", &removed_item)); @@ -307,7 +309,7 @@ { DictionaryValue dict; - dict.Set(key, new DeletionTestValue(&deletion_flag)); + dict.Set(key, make_scoped_ptr(new DeletionTestValue(&deletion_flag))); EXPECT_FALSE(deletion_flag); EXPECT_TRUE(dict.HasKey(key)); EXPECT_TRUE(dict.Remove(key, NULL)); @@ -318,9 +320,8 @@ TEST(ValuesTest, DictionaryWithoutPathExpansion) { DictionaryValue dict; - dict.Set("this.is.expanded", make_scoped_ptr(Value::CreateNullValue())); - dict.SetWithoutPathExpansion("this.isnt.expanded", - make_scoped_ptr(Value::CreateNullValue())); + dict.Set("this.is.expanded", Value::CreateNullValue()); + dict.SetWithoutPathExpansion("this.isnt.expanded", Value::CreateNullValue()); EXPECT_FALSE(dict.HasKey("this.is.expanded")); EXPECT_TRUE(dict.HasKey("this")); @@ -390,36 +391,49 @@ TEST(ValuesTest, DeepCopy) { DictionaryValue original_dict; - Value* original_null = Value::CreateNullValue(); - original_dict.Set("null", make_scoped_ptr(original_null)); - FundamentalValue* original_bool = new FundamentalValue(true); - original_dict.Set("bool", make_scoped_ptr(original_bool)); - FundamentalValue* original_int = new FundamentalValue(42); - original_dict.Set("int", make_scoped_ptr(original_int)); - FundamentalValue* original_double = new FundamentalValue(3.14); - original_dict.Set("double", make_scoped_ptr(original_double)); - StringValue* original_string = new StringValue("hello"); - original_dict.Set("string", make_scoped_ptr(original_string)); - StringValue* original_string16 = new StringValue(ASCIIToUTF16("hello16")); - original_dict.Set("string16", make_scoped_ptr(original_string16)); + scoped_ptr<Value> scoped_null = Value::CreateNullValue(); + Value* original_null = scoped_null.get(); + original_dict.Set("null", scoped_null.Pass()); + scoped_ptr<FundamentalValue> scoped_bool(new FundamentalValue(true)); + FundamentalValue* original_bool = scoped_bool.get(); + original_dict.Set("bool", scoped_bool.Pass()); + scoped_ptr<FundamentalValue> scoped_int(new FundamentalValue(42)); + FundamentalValue* original_int = scoped_int.get(); + original_dict.Set("int", scoped_int.Pass()); + scoped_ptr<FundamentalValue> scoped_double(new FundamentalValue(3.14)); + FundamentalValue* original_double = scoped_double.get(); + original_dict.Set("double", scoped_double.Pass()); + scoped_ptr<StringValue> scoped_string(new StringValue("hello")); + StringValue* original_string = scoped_string.get(); + original_dict.Set("string", scoped_string.Pass()); + scoped_ptr<StringValue> scoped_string16( + new StringValue(ASCIIToUTF16("hello16"))); + StringValue* original_string16 = scoped_string16.get(); + original_dict.Set("string16", scoped_string16.Pass()); scoped_ptr<char[]> original_buffer(new char[42]); memset(original_buffer.get(), '!', 42); - BinaryValue* original_binary = new BinaryValue(original_buffer.Pass(), 42); - original_dict.Set("binary", original_binary); + scoped_ptr<BinaryValue> scoped_binary( + new BinaryValue(original_buffer.Pass(), 42)); + BinaryValue* original_binary = scoped_binary.get(); + original_dict.Set("binary", scoped_binary.Pass()); - ListValue* original_list = new ListValue(); - FundamentalValue* original_list_element_0 = new FundamentalValue(0); - original_list->Append(original_list_element_0); - FundamentalValue* original_list_element_1 = new FundamentalValue(1); - original_list->Append(original_list_element_1); - original_dict.Set("list", make_scoped_ptr(original_list)); + scoped_ptr<ListValue> scoped_list(new ListValue()); + Value* original_list = scoped_list.get(); + scoped_ptr<FundamentalValue> scoped_list_element_0(new FundamentalValue(0)); + Value* original_list_element_0 = scoped_list_element_0.get(); + scoped_list->Append(scoped_list_element_0.Pass()); + scoped_ptr<FundamentalValue> scoped_list_element_1(new FundamentalValue(1)); + Value* original_list_element_1 = scoped_list_element_1.get(); + scoped_list->Append(scoped_list_element_1.Pass()); + original_dict.Set("list", scoped_list.Pass()); - DictionaryValue* original_nested_dictionary = new DictionaryValue(); - original_nested_dictionary->SetString("key", "value"); - original_dict.Set("dictionary", make_scoped_ptr(original_nested_dictionary)); + scoped_ptr<DictionaryValue> scoped_nested_dictionary(new DictionaryValue()); + Value* original_nested_dictionary = scoped_nested_dictionary.get(); + scoped_nested_dictionary->SetString("key", "value"); + original_dict.Set("dictionary", scoped_nested_dictionary.Pass()); - scoped_ptr<DictionaryValue> copy_dict(original_dict.DeepCopy()); + scoped_ptr<DictionaryValue> copy_dict = original_dict.CreateDeepCopy(); ASSERT_TRUE(copy_dict.get()); ASSERT_NE(copy_dict.get(), &original_dict); @@ -529,16 +543,13 @@ } TEST(ValuesTest, Equals) { - Value* null1 = Value::CreateNullValue(); - Value* null2 = Value::CreateNullValue(); - EXPECT_NE(null1, null2); - EXPECT_TRUE(null1->Equals(null2)); + scoped_ptr<Value> null1(Value::CreateNullValue()); + scoped_ptr<Value> null2(Value::CreateNullValue()); + EXPECT_NE(null1.get(), null2.get()); + EXPECT_TRUE(null1->Equals(null2.get())); - Value* boolean = new FundamentalValue(false); - EXPECT_FALSE(null1->Equals(boolean)); - delete null1; - delete null2; - delete boolean; + FundamentalValue boolean(false); + EXPECT_FALSE(null1->Equals(&boolean)); DictionaryValue dv; dv.SetBoolean("a", false); @@ -546,26 +557,27 @@ dv.SetDouble("c", 2.5); dv.SetString("d1", "string"); dv.SetString("d2", ASCIIToUTF16("http://google.com")); - dv.Set("e", make_scoped_ptr(Value::CreateNullValue())); + dv.Set("e", Value::CreateNullValue()); - scoped_ptr<DictionaryValue> copy; - copy.reset(dv.DeepCopy()); + scoped_ptr<DictionaryValue> copy = dv.CreateDeepCopy(); EXPECT_TRUE(dv.Equals(copy.get())); - ListValue* list = new ListValue; + scoped_ptr<ListValue> list(new ListValue); + ListValue* original_list = list.get(); list->Append(Value::CreateNullValue()); - list->Append(new DictionaryValue); - dv.Set("f", make_scoped_ptr(list)); + list->Append(make_scoped_ptr(new DictionaryValue)); + scoped_ptr<Value> list_copy(list->CreateDeepCopy()); + dv.Set("f", list.Pass()); EXPECT_FALSE(dv.Equals(copy.get())); - copy->Set("f", list->DeepCopy()); + copy->Set("f", list_copy.Pass()); EXPECT_TRUE(dv.Equals(copy.get())); - list->Append(new FundamentalValue(true)); + original_list->Append(make_scoped_ptr(new FundamentalValue(true))); EXPECT_FALSE(dv.Equals(copy.get())); // Check if Equals detects differences in only the keys. - copy.reset(dv.DeepCopy()); + copy = dv.CreateDeepCopy(); EXPECT_TRUE(dv.Equals(copy.get())); copy->Remove("a", NULL); copy->SetBoolean("aa", false); @@ -597,57 +609,60 @@ TEST(ValuesTest, DeepCopyCovariantReturnTypes) { DictionaryValue original_dict; - Value* original_null = Value::CreateNullValue(); - original_dict.Set("null", original_null); - FundamentalValue* original_bool = new FundamentalValue(true); - original_dict.Set("bool", original_bool); - FundamentalValue* original_int = new FundamentalValue(42); - original_dict.Set("int", original_int); - FundamentalValue* original_double = new FundamentalValue(3.14); - original_dict.Set("double", original_double); - StringValue* original_string = new StringValue("hello"); - original_dict.Set("string", original_string); - StringValue* original_string16 = new StringValue(ASCIIToUTF16("hello16")); - original_dict.Set("string16", original_string16); + scoped_ptr<Value> scoped_null(Value::CreateNullValue()); + Value* original_null = scoped_null.get(); + original_dict.Set("null", scoped_null.Pass()); + scoped_ptr<FundamentalValue> scoped_bool(new FundamentalValue(true)); + Value* original_bool = scoped_bool.get(); + original_dict.Set("bool", scoped_bool.Pass()); + scoped_ptr<FundamentalValue> scoped_int(new FundamentalValue(42)); + Value* original_int = scoped_int.get(); + original_dict.Set("int", scoped_int.Pass()); + scoped_ptr<FundamentalValue> scoped_double(new FundamentalValue(3.14)); + Value* original_double = scoped_double.get(); + original_dict.Set("double", scoped_double.Pass()); + scoped_ptr<StringValue> scoped_string(new StringValue("hello")); + Value* original_string = scoped_string.get(); + original_dict.Set("string", scoped_string.Pass()); + scoped_ptr<StringValue> scoped_string16( + new StringValue(ASCIIToUTF16("hello16"))); + Value* original_string16 = scoped_string16.get(); + original_dict.Set("string16", scoped_string16.Pass()); scoped_ptr<char[]> original_buffer(new char[42]); memset(original_buffer.get(), '!', 42); - BinaryValue* original_binary = new BinaryValue(original_buffer.Pass(), 42); - original_dict.Set("binary", original_binary); + scoped_ptr<BinaryValue> scoped_binary( + new BinaryValue(original_buffer.Pass(), 42)); + Value* original_binary = scoped_binary.get(); + original_dict.Set("binary", scoped_binary.Pass()); - ListValue* original_list = new ListValue(); - FundamentalValue* original_list_element_0 = new FundamentalValue(0); - original_list->Append(original_list_element_0); - FundamentalValue* original_list_element_1 = new FundamentalValue(1); - original_list->Append(original_list_element_1); - original_dict.Set("list", original_list); + scoped_ptr<ListValue> scoped_list(new ListValue()); + Value* original_list = scoped_list.get(); + scoped_ptr<FundamentalValue> scoped_list_element_0(new FundamentalValue(0)); + scoped_list->Append(scoped_list_element_0.Pass()); + scoped_ptr<FundamentalValue> scoped_list_element_1(new FundamentalValue(1)); + scoped_list->Append(scoped_list_element_1.Pass()); + original_dict.Set("list", scoped_list.Pass()); - Value* original_dict_value = &original_dict; - Value* original_bool_value = original_bool; - Value* original_int_value = original_int; - Value* original_double_value = original_double; - Value* original_string_value = original_string; - Value* original_string16_value = original_string16; - Value* original_binary_value = original_binary; - Value* original_list_value = original_list; + scoped_ptr<Value> copy_dict = original_dict.CreateDeepCopy(); + scoped_ptr<Value> copy_null = original_null->CreateDeepCopy(); + scoped_ptr<Value> copy_bool = original_bool->CreateDeepCopy(); + scoped_ptr<Value> copy_int = original_int->CreateDeepCopy(); + scoped_ptr<Value> copy_double = original_double->CreateDeepCopy(); + scoped_ptr<Value> copy_string = original_string->CreateDeepCopy(); + scoped_ptr<Value> copy_string16 = original_string16->CreateDeepCopy(); + scoped_ptr<Value> copy_binary = original_binary->CreateDeepCopy(); + scoped_ptr<Value> copy_list = original_list->CreateDeepCopy(); - scoped_ptr<Value> copy_dict_value(original_dict_value->DeepCopy()); - scoped_ptr<Value> copy_bool_value(original_bool_value->DeepCopy()); - scoped_ptr<Value> copy_int_value(original_int_value->DeepCopy()); - scoped_ptr<Value> copy_double_value(original_double_value->DeepCopy()); - scoped_ptr<Value> copy_string_value(original_string_value->DeepCopy()); - scoped_ptr<Value> copy_string16_value(original_string16_value->DeepCopy()); - scoped_ptr<Value> copy_binary_value(original_binary_value->DeepCopy()); - scoped_ptr<Value> copy_list_value(original_list_value->DeepCopy()); - - EXPECT_TRUE(original_dict_value->Equals(copy_dict_value.get())); - EXPECT_TRUE(original_bool_value->Equals(copy_bool_value.get())); - EXPECT_TRUE(original_int_value->Equals(copy_int_value.get())); - EXPECT_TRUE(original_double_value->Equals(copy_double_value.get())); - EXPECT_TRUE(original_string_value->Equals(copy_string_value.get())); - EXPECT_TRUE(original_string16_value->Equals(copy_string16_value.get())); - EXPECT_TRUE(original_binary_value->Equals(copy_binary_value.get())); - EXPECT_TRUE(original_list_value->Equals(copy_list_value.get())); + EXPECT_TRUE(original_dict.Equals(copy_dict.get())); + EXPECT_TRUE(original_null->Equals(copy_null.get())); + EXPECT_TRUE(original_bool->Equals(copy_bool.get())); + EXPECT_TRUE(original_int->Equals(copy_int.get())); + EXPECT_TRUE(original_double->Equals(copy_double.get())); + EXPECT_TRUE(original_string->Equals(copy_string.get())); + EXPECT_TRUE(original_string16->Equals(copy_string16.get())); + EXPECT_TRUE(original_binary->Equals(copy_binary.get())); + EXPECT_TRUE(original_list->Equals(copy_list.get())); } TEST(ValuesTest, RemoveEmptyChildren) { @@ -662,7 +677,7 @@ // Make sure we don't prune too much. root->SetBoolean("bool", true); - root->Set("empty_dict", new DictionaryValue); + root->Set("empty_dict", make_scoped_ptr(new DictionaryValue)); root->SetString("empty_string", std::string()); root.reset(root->DeepCopyWithoutEmptyChildren()); EXPECT_EQ(2U, root->size()); @@ -674,55 +689,57 @@ // Nested test cases. These should all reduce back to the bool and string // set above. { - root->Set("a.b.c.d.e", new DictionaryValue); + root->Set("a.b.c.d.e", make_scoped_ptr(new DictionaryValue)); root.reset(root->DeepCopyWithoutEmptyChildren()); EXPECT_EQ(2U, root->size()); } { - DictionaryValue* inner = new DictionaryValue; - root->Set("dict_with_emtpy_children", inner); - inner->Set("empty_dict", new DictionaryValue); - inner->Set("empty_list", new ListValue); + scoped_ptr<DictionaryValue> inner(new DictionaryValue); + inner->Set("empty_dict", make_scoped_ptr(new DictionaryValue)); + inner->Set("empty_list", make_scoped_ptr(new ListValue)); + root->Set("dict_with_empty_children", inner.Pass()); root.reset(root->DeepCopyWithoutEmptyChildren()); EXPECT_EQ(2U, root->size()); } { - ListValue* inner = new ListValue; - root->Set("list_with_empty_children", inner); - inner->Append(new DictionaryValue); - inner->Append(new ListValue); + scoped_ptr<ListValue> inner(new ListValue); + inner->Append(make_scoped_ptr(new DictionaryValue)); + inner->Append(make_scoped_ptr(new ListValue)); + root->Set("list_with_empty_children", inner.Pass()); root.reset(root->DeepCopyWithoutEmptyChildren()); EXPECT_EQ(2U, root->size()); } // Nested with siblings. { - ListValue* inner = new ListValue; - root->Set("list_with_empty_children", inner); - inner->Append(new DictionaryValue); - inner->Append(new ListValue); - DictionaryValue* inner2 = new DictionaryValue; - root->Set("dict_with_empty_children", inner2); - inner2->Set("empty_dict", new DictionaryValue); - inner2->Set("empty_list", new ListValue); + scoped_ptr<ListValue> inner(new ListValue()); + inner->Append(make_scoped_ptr(new DictionaryValue)); + inner->Append(make_scoped_ptr(new ListValue)); + root->Set("list_with_empty_children", inner.Pass()); + scoped_ptr<DictionaryValue> inner2(new DictionaryValue); + inner2->Set("empty_dict", make_scoped_ptr(new DictionaryValue)); + inner2->Set("empty_list", make_scoped_ptr(new ListValue)); + root->Set("dict_with_empty_children", inner2.Pass()); root.reset(root->DeepCopyWithoutEmptyChildren()); EXPECT_EQ(2U, root->size()); } // Make sure nested values don't get pruned. { - ListValue* inner = new ListValue; - root->Set("list_with_empty_children", inner); - ListValue* inner2 = new ListValue; - inner->Append(new DictionaryValue); - inner->Append(inner2); - inner2->Append(new StringValue("hello")); + scoped_ptr<ListValue> inner(new ListValue); + scoped_ptr<ListValue> inner2(new ListValue); + inner2->Append(make_scoped_ptr(new StringValue("hello"))); + inner->Append(make_scoped_ptr(new DictionaryValue)); + inner->Append(inner2.Pass()); + root->Set("list_with_empty_children", inner.Pass()); root.reset(root->DeepCopyWithoutEmptyChildren()); EXPECT_EQ(3U, root->size()); - EXPECT_TRUE(root->GetList("list_with_empty_children", &inner)); - EXPECT_EQ(1U, inner->GetSize()); // Dictionary was pruned. - EXPECT_TRUE(inner->GetList(0, &inner2)); - EXPECT_EQ(1U, inner2->GetSize()); + + ListValue* inner_value, *inner_value2; + EXPECT_TRUE(root->GetList("list_with_empty_children", &inner_value)); + EXPECT_EQ(1U, inner_value->GetSize()); // Dictionary was pruned. + EXPECT_TRUE(inner_value->GetList(0, &inner_value2)); + EXPECT_EQ(1U, inner_value2->GetSize()); } } @@ -730,18 +747,18 @@ scoped_ptr<DictionaryValue> base(new DictionaryValue); base->SetString("base_key", "base_key_value_base"); base->SetString("collide_key", "collide_key_value_base"); - DictionaryValue* base_sub_dict = new DictionaryValue; + scoped_ptr<DictionaryValue> base_sub_dict(new DictionaryValue); base_sub_dict->SetString("sub_base_key", "sub_base_key_value_base"); base_sub_dict->SetString("sub_collide_key", "sub_collide_key_value_base"); - base->Set("sub_dict_key", base_sub_dict); + base->Set("sub_dict_key", base_sub_dict.Pass()); scoped_ptr<DictionaryValue> merge(new DictionaryValue); merge->SetString("merge_key", "merge_key_value_merge"); merge->SetString("collide_key", "collide_key_value_merge"); - DictionaryValue* merge_sub_dict = new DictionaryValue; + scoped_ptr<DictionaryValue> merge_sub_dict(new DictionaryValue); merge_sub_dict->SetString("sub_merge_key", "sub_merge_key_value_merge"); merge_sub_dict->SetString("sub_collide_key", "sub_collide_key_value_merge"); - merge->Set("sub_dict_key", merge_sub_dict); + merge->Set("sub_dict_key", merge_sub_dict.Pass()); base->MergeDictionary(merge.get()); @@ -772,7 +789,8 @@ } TEST(ValuesTest, MergeDictionaryDeepCopy) { - DictionaryValue* child = new DictionaryValue; + scoped_ptr<DictionaryValue> child(new DictionaryValue); + DictionaryValue* original_child = child.get(); child->SetString("test", "value"); EXPECT_EQ(1U, child->size()); @@ -781,22 +799,22 @@ EXPECT_EQ("value", value); scoped_ptr<DictionaryValue> base(new DictionaryValue); - base->Set("dict", child); + base->Set("dict", child.Pass()); EXPECT_EQ(1U, base->size()); DictionaryValue* ptr; EXPECT_TRUE(base->GetDictionary("dict", &ptr)); - EXPECT_EQ(child, ptr); + EXPECT_EQ(original_child, ptr); scoped_ptr<DictionaryValue> merged(new DictionaryValue); merged->MergeDictionary(base.get()); EXPECT_EQ(1U, merged->size()); EXPECT_TRUE(merged->GetDictionary("dict", &ptr)); - EXPECT_NE(child, ptr); + EXPECT_NE(original_child, ptr); EXPECT_TRUE(ptr->GetString("test", &value)); EXPECT_EQ("value", value); - child->SetString("test", "overwrite"); + original_child->SetString("test", "overwrite"); base.reset(); EXPECT_TRUE(ptr->GetString("test", &value)); EXPECT_EQ("value", value); @@ -809,7 +827,7 @@ } StringValue value1("value1"); - dict.Set("key1", value1.DeepCopy()); + dict.Set("key1", value1.CreateDeepCopy()); bool seen1 = false; for (DictionaryValue::Iterator it(dict); !it.IsAtEnd(); it.Advance()) { EXPECT_FALSE(seen1); @@ -820,7 +838,7 @@ EXPECT_TRUE(seen1); StringValue value2("value2"); - dict.Set("key2", value2.DeepCopy()); + dict.Set("key2", value2.CreateDeepCopy()); bool seen2 = seen1 = false; for (DictionaryValue::Iterator it(dict); !it.IsAtEnd(); it.Advance()) { if (it.key() == "key1") { @@ -853,21 +871,21 @@ DictionaryValue dict_value; ListValue list_value; - main_dict.Set("bool", bool_value.DeepCopy()); - main_dict.Set("int", int_value.DeepCopy()); - main_dict.Set("double", double_value.DeepCopy()); - main_dict.Set("string", string_value.DeepCopy()); - main_dict.Set("binary", binary_value.DeepCopy()); - main_dict.Set("dict", dict_value.DeepCopy()); - main_dict.Set("list", list_value.DeepCopy()); + main_dict.Set("bool", bool_value.CreateDeepCopy()); + main_dict.Set("int", int_value.CreateDeepCopy()); + main_dict.Set("double", double_value.CreateDeepCopy()); + main_dict.Set("string", string_value.CreateDeepCopy()); + main_dict.Set("binary", binary_value.CreateDeepCopy()); + main_dict.Set("dict", dict_value.CreateDeepCopy()); + main_dict.Set("list", list_value.CreateDeepCopy()); - main_list.Append(bool_value.DeepCopy()); - main_list.Append(int_value.DeepCopy()); - main_list.Append(double_value.DeepCopy()); - main_list.Append(string_value.DeepCopy()); - main_list.Append(binary_value.DeepCopy()); - main_list.Append(dict_value.DeepCopy()); - main_list.Append(list_value.DeepCopy()); + main_list.Append(bool_value.CreateDeepCopy()); + main_list.Append(int_value.CreateDeepCopy()); + main_list.Append(double_value.CreateDeepCopy()); + main_list.Append(string_value.CreateDeepCopy()); + main_list.Append(binary_value.CreateDeepCopy()); + main_list.Append(dict_value.CreateDeepCopy()); + main_list.Append(list_value.CreateDeepCopy()); EXPECT_TRUE(main_dict.Get("bool", NULL)); EXPECT_TRUE(main_dict.Get("int", NULL));
diff --git a/win/memory_pressure_monitor.cc b/win/memory_pressure_monitor.cc new file mode 100644 index 0000000..ed49a40 --- /dev/null +++ b/win/memory_pressure_monitor.cc
@@ -0,0 +1,254 @@ +// Copyright 2015 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/win/memory_pressure_monitor.h" + +#include <windows.h> + +#include "base/metrics/histogram_macros.h" +#include "base/single_thread_task_runner.h" +#include "base/thread_task_runner_handle.h" +#include "base/time/time.h" + +namespace base { +namespace win { + +namespace { + +static const DWORDLONG kMBBytes = 1024 * 1024; + +// Enumeration of UMA memory pressure levels. This needs to be kept in sync with +// histograms.xml and the memory pressure levels defined in +// MemoryPressureListener. +enum MemoryPressureLevelUMA { + UMA_MEMORY_PRESSURE_LEVEL_NONE = 0, + UMA_MEMORY_PRESSURE_LEVEL_MODERATE = 1, + UMA_MEMORY_PRESSURE_LEVEL_CRITICAL = 2, + // This must be the last value in the enum. + UMA_MEMORY_PRESSURE_LEVEL_COUNT, +}; + +// Converts a memory pressure level to an UMA enumeration value. +MemoryPressureLevelUMA MemoryPressureLevelToUmaEnumValue( + MemoryPressureListener::MemoryPressureLevel level) { + switch (level) { + case MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE: + return UMA_MEMORY_PRESSURE_LEVEL_NONE; + case MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE: + return UMA_MEMORY_PRESSURE_LEVEL_MODERATE; + case MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL: + return UMA_MEMORY_PRESSURE_LEVEL_CRITICAL; + } + NOTREACHED(); + return UMA_MEMORY_PRESSURE_LEVEL_NONE; +} + +} // namespace + +// The following constants have been lifted from similar values in the ChromeOS +// memory pressure monitor. The values were determined experimentally to ensure +// sufficient responsiveness of the memory pressure subsystem, and minimal +// overhead. +const int MemoryPressureMonitor::kPollingIntervalMs = 5000; +const int MemoryPressureMonitor::kModeratePressureCooldownMs = 10000; +const int MemoryPressureMonitor::kModeratePressureCooldownCycles = + kModeratePressureCooldownMs / kPollingIntervalMs; + +// TODO(chrisha): Explore the following constants further with an experiment. + +// A system is considered 'high memory' if it has more than 1.5GB of system +// memory available for use by the memory manager (not reserved for hardware +// and drivers). This is a fuzzy version of the ~2GB discussed below. +const int MemoryPressureMonitor::kLargeMemoryThresholdMb = 1536; + +// These are the default thresholds used for systems with < ~2GB of physical +// memory. Such systems have been observed to always maintain ~100MB of +// available memory, paging until that is the case. To try to avoid paging a +// threshold slightly above this is chosen. The moderate threshold is slightly +// less grounded in reality and chosen as 2.5x critical. +const int MemoryPressureMonitor::kSmallMemoryDefaultModerateThresholdMb = 500; +const int MemoryPressureMonitor::kSmallMemoryDefaultCriticalThresholdMb = 200; + +// These are the default thresholds used for systems with >= ~2GB of physical +// memory. Such systems have been observed to always maintain ~300MB of +// available memory, paging until that is the case. +const int MemoryPressureMonitor::kLargeMemoryDefaultModerateThresholdMb = 1000; +const int MemoryPressureMonitor::kLargeMemoryDefaultCriticalThresholdMb = 400; + +MemoryPressureMonitor::MemoryPressureMonitor() + : moderate_threshold_mb_(0), + critical_threshold_mb_(0), + current_memory_pressure_level_( + MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE), + moderate_pressure_repeat_count_(0), + weak_ptr_factory_(this) { + InferThresholds(); + StartObserving(); +} + +MemoryPressureMonitor::MemoryPressureMonitor(int moderate_threshold_mb, + int critical_threshold_mb) + : moderate_threshold_mb_(moderate_threshold_mb), + critical_threshold_mb_(critical_threshold_mb), + current_memory_pressure_level_( + MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE), + moderate_pressure_repeat_count_(0), + weak_ptr_factory_(this) { + DCHECK_GE(moderate_threshold_mb_, critical_threshold_mb_); + DCHECK_LE(0, critical_threshold_mb_); + StartObserving(); +} + +MemoryPressureMonitor::~MemoryPressureMonitor() { + StopObserving(); +} + +void MemoryPressureMonitor::CheckMemoryPressureSoon() { + DCHECK(thread_checker_.CalledOnValidThread()); + + ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, Bind(&MemoryPressureMonitor::CheckMemoryPressure, + weak_ptr_factory_.GetWeakPtr())); +} + +MemoryPressureListener::MemoryPressureLevel +MemoryPressureMonitor::GetCurrentPressureLevel() const { + return current_memory_pressure_level_; +} + +void MemoryPressureMonitor::InferThresholds() { + // Default to a 'high' memory situation, which uses more conservative + // thresholds. + bool high_memory = true; + MEMORYSTATUSEX mem_status = {}; + if (GetSystemMemoryStatus(&mem_status)) { + static const DWORDLONG kLargeMemoryThresholdBytes = + static_cast<DWORDLONG>(kLargeMemoryThresholdMb) * kMBBytes; + high_memory = mem_status.ullTotalPhys >= kLargeMemoryThresholdBytes; + } + + if (high_memory) { + moderate_threshold_mb_ = kLargeMemoryDefaultModerateThresholdMb; + critical_threshold_mb_ = kLargeMemoryDefaultCriticalThresholdMb; + } else { + moderate_threshold_mb_ = kSmallMemoryDefaultModerateThresholdMb; + critical_threshold_mb_ = kSmallMemoryDefaultCriticalThresholdMb; + } +} + +void MemoryPressureMonitor::StartObserving() { + DCHECK(thread_checker_.CalledOnValidThread()); + + timer_.Start(FROM_HERE, + TimeDelta::FromMilliseconds(kPollingIntervalMs), + Bind(&MemoryPressureMonitor:: + CheckMemoryPressureAndRecordStatistics, + weak_ptr_factory_.GetWeakPtr())); +} + +void MemoryPressureMonitor::StopObserving() { + DCHECK(thread_checker_.CalledOnValidThread()); + + // If StartObserving failed, StopObserving will still get called. + timer_.Stop(); + weak_ptr_factory_.InvalidateWeakPtrs(); +} + +void MemoryPressureMonitor::CheckMemoryPressure() { + DCHECK(thread_checker_.CalledOnValidThread()); + + // Get the previous pressure level and update the current one. + MemoryPressureLevel old_pressure = current_memory_pressure_level_; + current_memory_pressure_level_ = CalculateCurrentPressureLevel(); + + // |notify| will be set to true if MemoryPressureListeners need to be + // notified of a memory pressure level state change. + bool notify = false; + switch (current_memory_pressure_level_) { + case MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE: + break; + + case MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE: + if (old_pressure != current_memory_pressure_level_) { + // This is a new transition to moderate pressure so notify. + moderate_pressure_repeat_count_ = 0; + notify = true; + } else { + // Already in moderate pressure, only notify if sustained over the + // cooldown period. + if (++moderate_pressure_repeat_count_ == + kModeratePressureCooldownCycles) { + moderate_pressure_repeat_count_ = 0; + notify = true; + } + } + break; + + case MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL: + // Always notify of critical pressure levels. + notify = true; + break; + } + + if (!notify) + return; + + // Emit a notification of the current memory pressure level. This can only + // happen for moderate and critical pressure levels. + DCHECK_NE(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE, + current_memory_pressure_level_); + MemoryPressureListener::NotifyMemoryPressure(current_memory_pressure_level_); +} + +void MemoryPressureMonitor::CheckMemoryPressureAndRecordStatistics() { + DCHECK(thread_checker_.CalledOnValidThread()); + + CheckMemoryPressure(); + + UMA_HISTOGRAM_ENUMERATION( + "Memory.PressureLevel", + MemoryPressureLevelToUmaEnumValue(current_memory_pressure_level_), + UMA_MEMORY_PRESSURE_LEVEL_COUNT); +} + +MemoryPressureListener::MemoryPressureLevel +MemoryPressureMonitor::CalculateCurrentPressureLevel() { + MEMORYSTATUSEX mem_status = {}; + if (!GetSystemMemoryStatus(&mem_status)) + return MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE; + + // How much system memory is actively available for use right now, in MBs. + int phys_free = static_cast<int>(mem_status.ullAvailPhys / kMBBytes); + + // TODO(chrisha): This should eventually care about address space pressure, + // but the browser process (where this is running) effectively never runs out + // of address space. Renderers occasionally do, but it does them no good to + // have the browser process monitor address space pressure. Long term, + // renderers should run their own address space pressure monitors and act + // accordingly, with the browser making cross-process decisions based on + // system memory pressure. + + // Determine if the physical memory is under critical memory pressure. + if (phys_free <= critical_threshold_mb_) + return MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL; + + // Determine if the physical memory is under moderate memory pressure. + if (phys_free <= moderate_threshold_mb_) + return MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE; + + // No memory pressure was detected. + return MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE; +} + +bool MemoryPressureMonitor::GetSystemMemoryStatus( + MEMORYSTATUSEX* mem_status) { + DCHECK(mem_status != nullptr); + mem_status->dwLength = sizeof(*mem_status); + if (!::GlobalMemoryStatusEx(mem_status)) + return false; + return true; +} + +} // namespace win +} // namespace base
diff --git a/win/memory_pressure_monitor.h b/win/memory_pressure_monitor.h new file mode 100644 index 0000000..933d912 --- /dev/null +++ b/win/memory_pressure_monitor.h
@@ -0,0 +1,144 @@ +// Copyright 2015 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_WIN_MEMORY_PRESSURE_MONITOR_H_ +#define BASE_WIN_MEMORY_PRESSURE_MONITOR_H_ + +#include "base/base_export.h" +#include "base/memory/memory_pressure_listener.h" +#include "base/memory/memory_pressure_monitor.h" +#include "base/memory/weak_ptr.h" +#include "base/threading/thread_checker.h" +#include "base/timer/timer.h" + +// To not pull in windows.h. +typedef struct _MEMORYSTATUSEX MEMORYSTATUSEX; + +namespace base { +namespace win { + +// Windows memory pressure monitor. Because there is no OS provided signal this +// polls at a low frequency (once per second), and applies internal hysteresis. +class BASE_EXPORT MemoryPressureMonitor : public base::MemoryPressureMonitor { + public: + // Constants governing the polling and hysteresis behaviour of the observer. + + // The polling interval, in milliseconds. While under critical pressure, this + // is also the timer to repeat cleanup attempts. + static const int kPollingIntervalMs; + // The time which should pass between 2 successive moderate memory pressure + // signals, in milliseconds. + static const int kModeratePressureCooldownMs; + // The number of cycles that should pass between 2 successive moderate memory + // pressure signals. + static const int kModeratePressureCooldownCycles; + + // Constants governing the memory pressure level detection. + + // The amount of total system memory beyond which a system is considered to be + // a large-memory system. + static const int kLargeMemoryThresholdMb; + // Default minimum free memory thresholds for small-memory systems, in MB. + static const int kSmallMemoryDefaultModerateThresholdMb; + static const int kSmallMemoryDefaultCriticalThresholdMb; + // Default minimum free memory thresholds for large-memory systems, in MB. + static const int kLargeMemoryDefaultModerateThresholdMb; + static const int kLargeMemoryDefaultCriticalThresholdMb; + + // Default constructor. Will choose thresholds automatically basd on the + // actual amount of system memory. + MemoryPressureMonitor(); + + // Constructor with explicit memory thresholds. These represent the amount of + // free memory below which the applicable memory pressure state engages. + MemoryPressureMonitor(int moderate_threshold_mb, int critical_threshold_mb); + + ~MemoryPressureMonitor() override; + + // Schedules a memory pressure check to run soon. This must be called on the + // same thread where the monitor was instantiated. + void CheckMemoryPressureSoon(); + + // Get the current memory pressure level. This can be called from any thread. + MemoryPressureLevel GetCurrentPressureLevel() const override; + + // Returns the moderate pressure level free memory threshold, in MB. + int moderate_threshold_mb() const { return moderate_threshold_mb_; } + + // Returns the critical pressure level free memory threshold, in MB. + int critical_threshold_mb() const { return critical_threshold_mb_; } + + protected: + // Internals are exposed for unittests. + + // Automatically infers threshold values based on system memory. This invokes + // GetMemoryStatus so it can be mocked in unittests. + void InferThresholds(); + + // Starts observing the memory fill level. Calls to StartObserving should + // always be matched with calls to StopObserving. + void StartObserving(); + + // Stop observing the memory fill level. May be safely called if + // StartObserving has not been called. Must be called from the same thread on + // which the monitor was instantiated. + void StopObserving(); + + // Checks memory pressure, storing the current level, applying any hysteresis + // and emitting memory pressure level change signals as necessary. This + // function is called periodically while the monitor is observing memory + // pressure. This is split out from CheckMemoryPressureAndRecordStatistics so + // that it may be called by CheckMemoryPressureSoon and not invoke UMA + // logging. Must be called from the same thread on which the monitor was + // instantiated. + void CheckMemoryPressure(); + + // Wrapper to CheckMemoryPressure that also records the observed memory + // pressure level via an UMA enumeration. This is the function that is called + // periodically by the timer. Must be called from the same thread on which the + // monitor was instantiated. + void CheckMemoryPressureAndRecordStatistics(); + + // Calculates the current instantaneous memory pressure level. This does not + // use any hysteresis and simply returns the result at the current moment. Can + // be called on any thread. + MemoryPressureLevel CalculateCurrentPressureLevel(); + + // Gets system memory status. This is virtual as a unittesting hook. Returns + // true if the system call succeeds, false otherwise. Can be called on any + // thread. + virtual bool GetSystemMemoryStatus(MEMORYSTATUSEX* mem_status); + + private: + // Threshold amounts of available memory that trigger pressure levels. See + // memory_pressure_monitor.cc for a discussion of reasonable values for these. + int moderate_threshold_mb_; + int critical_threshold_mb_; + + // A periodic timer to check for memory pressure changes. + base::RepeatingTimer<MemoryPressureMonitor> timer_; + + // The current memory pressure. + MemoryPressureLevel current_memory_pressure_level_; + + // To slow down the amount of moderate pressure event calls, this gets used to + // count the number of events since the last event occured. This is used by + // |CheckMemoryPressure| to apply hysteresis on the raw results of + // |CalculateCurrentPressureLevel|. + int moderate_pressure_repeat_count_; + + // Ensures that this object is used from a single thread. + base::ThreadChecker thread_checker_; + + // Weak pointer factory to ourself used for scheduling calls to + // CheckMemoryPressure/CheckMemoryPressureAndRecordStatistics via |timer_|. + base::WeakPtrFactory<MemoryPressureMonitor> weak_ptr_factory_; + + DISALLOW_COPY_AND_ASSIGN(MemoryPressureMonitor); +}; + +} // namespace win +} // namespace base + +#endif // BASE_WIN_MEMORY_PRESSURE_MONITOR_H_
diff --git a/win/memory_pressure_monitor_unittest.cc b/win/memory_pressure_monitor_unittest.cc new file mode 100644 index 0000000..40a25a7 --- /dev/null +++ b/win/memory_pressure_monitor_unittest.cc
@@ -0,0 +1,298 @@ +// Copyright 2015 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/win/memory_pressure_monitor.h" + +#include "base/basictypes.h" +#include "base/memory/memory_pressure_listener.h" +#include "base/message_loop/message_loop.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace base { +namespace win { + +namespace { + +struct PressureSettings { + int phys_left_mb; + MemoryPressureListener::MemoryPressureLevel level; +}; + +} // namespace + +// This is outside of the anonymous namespace so that it can be seen as a friend +// to the monitor class. +class TestMemoryPressureMonitor : public MemoryPressureMonitor { + public: + using MemoryPressureMonitor::CalculateCurrentPressureLevel; + using MemoryPressureMonitor::CheckMemoryPressure; + + static const DWORDLONG kMBBytes = 1024 * 1024; + + explicit TestMemoryPressureMonitor(bool large_memory) + : mem_status_() { + // Generate a plausible amount of memory. + mem_status_.ullTotalPhys = + static_cast<DWORDLONG>(GenerateTotalMemoryMb(large_memory)) * kMBBytes; + + // Rerun InferThresholds using the test fixture's GetSystemMemoryStatus. + InferThresholds(); + // Stop the timer. + StopObserving(); + } + + TestMemoryPressureMonitor(int system_memory_mb, + int moderate_threshold_mb, + int critical_threshold_mb) + : MemoryPressureMonitor(moderate_threshold_mb, critical_threshold_mb), + mem_status_() { + // Set the amount of system memory. + mem_status_.ullTotalPhys = static_cast<DWORDLONG>( + system_memory_mb * kMBBytes); + + // Stop the timer. + StopObserving(); + } + + virtual ~TestMemoryPressureMonitor() {} + + MOCK_METHOD1(OnMemoryPressure, + void(MemoryPressureListener::MemoryPressureLevel level)); + + // Generates an amount of total memory that is consistent with the requested + // memory model. + int GenerateTotalMemoryMb(bool large_memory) { + int total_mb = 64; + while (total_mb < MemoryPressureMonitor::kLargeMemoryThresholdMb) + total_mb *= 2; + if (large_memory) + return total_mb * 2; + return total_mb / 2; + } + + // Sets up the memory status to reflect the provided absolute memory left. + void SetMemoryFree(int phys_left_mb) { + // ullTotalPhys is set in the constructor and not modified. + + // Set the amount of available memory. + mem_status_.ullAvailPhys = + static_cast<DWORDLONG>(phys_left_mb) * kMBBytes; + DCHECK_LT(mem_status_.ullAvailPhys, mem_status_.ullTotalPhys); + + // These fields are unused. + mem_status_.dwMemoryLoad = 0; + mem_status_.ullTotalPageFile = 0; + mem_status_.ullAvailPageFile = 0; + mem_status_.ullTotalVirtual = 0; + mem_status_.ullAvailVirtual = 0; + } + + void SetNone() { + SetMemoryFree(moderate_threshold_mb() + 1); + } + + void SetModerate() { + SetMemoryFree(moderate_threshold_mb() - 1); + } + + void SetCritical() { + SetMemoryFree(critical_threshold_mb() - 1); + } + + private: + bool GetSystemMemoryStatus(MEMORYSTATUSEX* mem_status) override { + // Simply copy the memory status set by the test fixture. + *mem_status = mem_status_; + return true; + } + + MEMORYSTATUSEX mem_status_; + + DISALLOW_COPY_AND_ASSIGN(TestMemoryPressureMonitor); +}; + +class WinMemoryPressureMonitorTest : public testing::Test { + protected: + void CalculateCurrentMemoryPressureLevelTest( + TestMemoryPressureMonitor* monitor) { + + int mod = monitor->moderate_threshold_mb(); + monitor->SetMemoryFree(mod + 1); + EXPECT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE, + monitor->CalculateCurrentPressureLevel()); + + monitor->SetMemoryFree(mod); + EXPECT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE, + monitor->CalculateCurrentPressureLevel()); + + monitor->SetMemoryFree(mod - 1); + EXPECT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE, + monitor->CalculateCurrentPressureLevel()); + + int crit = monitor->critical_threshold_mb(); + monitor->SetMemoryFree(crit + 1); + EXPECT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE, + monitor->CalculateCurrentPressureLevel()); + + monitor->SetMemoryFree(crit); + EXPECT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL, + monitor->CalculateCurrentPressureLevel()); + + monitor->SetMemoryFree(crit - 1); + EXPECT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL, + monitor->CalculateCurrentPressureLevel()); + } + + base::MessageLoopForUI message_loop_; +}; + +// Tests the fundamental direct calculation of memory pressure with automatic +// small-memory thresholds. +TEST_F(WinMemoryPressureMonitorTest, CalculateCurrentMemoryPressureLevelSmall) { + static const int kModerateMb = + MemoryPressureMonitor::kSmallMemoryDefaultModerateThresholdMb; + static const int kCriticalMb = + MemoryPressureMonitor::kSmallMemoryDefaultCriticalThresholdMb; + + TestMemoryPressureMonitor monitor(false); // Small-memory model. + + EXPECT_EQ(kModerateMb, monitor.moderate_threshold_mb()); + EXPECT_EQ(kCriticalMb, monitor.critical_threshold_mb()); + + ASSERT_NO_FATAL_FAILURE(CalculateCurrentMemoryPressureLevelTest(&monitor)); +} + +// Tests the fundamental direct calculation of memory pressure with automatic +// large-memory thresholds. +TEST_F(WinMemoryPressureMonitorTest, CalculateCurrentMemoryPressureLevelLarge) { + static const int kModerateMb = + MemoryPressureMonitor::kLargeMemoryDefaultModerateThresholdMb; + static const int kCriticalMb = + MemoryPressureMonitor::kLargeMemoryDefaultCriticalThresholdMb; + + TestMemoryPressureMonitor monitor(true); // Large-memory model. + + EXPECT_EQ(kModerateMb, monitor.moderate_threshold_mb()); + EXPECT_EQ(kCriticalMb, monitor.critical_threshold_mb()); + + ASSERT_NO_FATAL_FAILURE(CalculateCurrentMemoryPressureLevelTest(&monitor)); +} + +// Tests the fundamental direct calculation of memory pressure with manually +// specified threshold levels. +TEST_F(WinMemoryPressureMonitorTest, + CalculateCurrentMemoryPressureLevelCustom) { + static const int kSystemMb = 512; + static const int kModerateMb = 256; + static const int kCriticalMb = 128; + + TestMemoryPressureMonitor monitor(kSystemMb, kModerateMb, kCriticalMb); + + EXPECT_EQ(kModerateMb, monitor.moderate_threshold_mb()); + EXPECT_EQ(kCriticalMb, monitor.critical_threshold_mb()); + + ASSERT_NO_FATAL_FAILURE(CalculateCurrentMemoryPressureLevelTest(&monitor)); +} + +// This test tests the various transition states from memory pressure, looking +// for the correct behavior on event reposting as well as state updates. +TEST_F(WinMemoryPressureMonitorTest, CheckMemoryPressure) { + // Large-memory. + testing::StrictMock<TestMemoryPressureMonitor> monitor(true); + MemoryPressureListener listener( + base::Bind(&TestMemoryPressureMonitor::OnMemoryPressure, + base::Unretained(&monitor))); + + // Checking the memory pressure at 0% load should not produce any + // events. + monitor.SetNone(); + monitor.CheckMemoryPressure(); + message_loop_.RunUntilIdle(); + EXPECT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE, + monitor.GetCurrentPressureLevel()); + + // Setting the memory level to 80% should produce a moderate pressure level. + EXPECT_CALL(monitor, + OnMemoryPressure(MemoryPressureListener:: + MEMORY_PRESSURE_LEVEL_MODERATE)); + monitor.SetModerate(); + monitor.CheckMemoryPressure(); + message_loop_.RunUntilIdle(); + EXPECT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE, + monitor.GetCurrentPressureLevel()); + testing::Mock::VerifyAndClearExpectations(&monitor); + + // Check that the event gets reposted after a while. + for (int i = 0; i < monitor.kModeratePressureCooldownCycles; ++i) { + if (i + 1 == monitor.kModeratePressureCooldownCycles) { + EXPECT_CALL(monitor, + OnMemoryPressure(MemoryPressureListener:: + MEMORY_PRESSURE_LEVEL_MODERATE)); + } + monitor.CheckMemoryPressure(); + message_loop_.RunUntilIdle(); + EXPECT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE, + monitor.GetCurrentPressureLevel()); + testing::Mock::VerifyAndClearExpectations(&monitor); + } + + // Setting the memory usage to 99% should produce critical levels. + EXPECT_CALL(monitor, + OnMemoryPressure(MemoryPressureListener:: + MEMORY_PRESSURE_LEVEL_CRITICAL)); + monitor.SetCritical(); + monitor.CheckMemoryPressure(); + message_loop_.RunUntilIdle(); + EXPECT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL, + monitor.GetCurrentPressureLevel()); + testing::Mock::VerifyAndClearExpectations(&monitor); + + // Calling it again should immediately produce a second call. + EXPECT_CALL(monitor, + OnMemoryPressure(MemoryPressureListener:: + MEMORY_PRESSURE_LEVEL_CRITICAL)); + monitor.CheckMemoryPressure(); + message_loop_.RunUntilIdle(); + EXPECT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL, + monitor.GetCurrentPressureLevel()); + testing::Mock::VerifyAndClearExpectations(&monitor); + + // When lowering the pressure again there should be a notification and the + // pressure should go back to moderate. + EXPECT_CALL(monitor, + OnMemoryPressure(MemoryPressureListener:: + MEMORY_PRESSURE_LEVEL_MODERATE)); + monitor.SetModerate(); + monitor.CheckMemoryPressure(); + message_loop_.RunUntilIdle(); + EXPECT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE, + monitor.GetCurrentPressureLevel()); + testing::Mock::VerifyAndClearExpectations(&monitor); + + // Check that the event gets reposted after a while. + for (int i = 0; i < monitor.kModeratePressureCooldownCycles; ++i) { + if (i + 1 == monitor.kModeratePressureCooldownCycles) { + EXPECT_CALL(monitor, + OnMemoryPressure(MemoryPressureListener:: + MEMORY_PRESSURE_LEVEL_MODERATE)); + } + monitor.CheckMemoryPressure(); + message_loop_.RunUntilIdle(); + EXPECT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE, + monitor.GetCurrentPressureLevel()); + testing::Mock::VerifyAndClearExpectations(&monitor); + } + + // Going down to no pressure should not produce an notification. + monitor.SetNone(); + monitor.CheckMemoryPressure(); + message_loop_.RunUntilIdle(); + EXPECT_EQ(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE, + monitor.GetCurrentPressureLevel()); + testing::Mock::VerifyAndClearExpectations(&monitor); +} + +} // namespace win +} // namespace base
diff --git a/win/resource_util.h b/win/resource_util.h index f3444ae..8a8baa0 100644 --- a/win/resource_util.h +++ b/win/resource_util.h
@@ -5,8 +5,8 @@ // This file contains utility functions for accessing resources in external // files (DLLs) or embedded in the executable itself. -#ifndef BASE_WIN_RESOURCE_UTIL_H__ -#define BASE_WIN_RESOURCE_UTIL_H__ +#ifndef BASE_WIN_RESOURCE_UTIL_H_ +#define BASE_WIN_RESOURCE_UTIL_H_ #include <windows.h> @@ -36,4 +36,4 @@ } // namespace win } // namespace base -#endif // BASE_WIN_RESOURCE_UTIL_H__ +#endif // BASE_WIN_RESOURCE_UTIL_H_
diff --git a/win/scoped_handle.cc b/win/scoped_handle.cc index 33a8aa5..ce944e4 100644 --- a/win/scoped_handle.cc +++ b/win/scoped_handle.cc
@@ -132,10 +132,6 @@ // This lock only protects against races in this module, which is fine. AutoNativeLock lock(g_lock.Get()); g_active_verifier = verifier ? verifier : new ActiveVerifier(true); - - // TODO(shrikant): Enable handle verifier after figuring out - // AppContainer/DuplicateHandle error. - g_active_verifier->Disable(); #endif } @@ -156,6 +152,12 @@ if (!enabled_) return; + // Idea here is to make our handles non-closable until we close it ourselves. + // Handles provided could be totally fabricated especially through our + // unittest, we are ignoring that for now by not checking return value. + ::SetHandleInformation(handle, HANDLE_FLAG_PROTECT_FROM_CLOSE, + HANDLE_FLAG_PROTECT_FROM_CLOSE); + // Grab the thread id before the lock. DWORD thread_id = GetCurrentThreadId(); @@ -176,6 +178,15 @@ if (!enabled_) return; + // We expect handle to be protected till this point. + DWORD flags = 0; + if (::GetHandleInformation(handle, &flags)) { + CHECK_NE(0U, (flags & HANDLE_FLAG_PROTECT_FROM_CLOSE)); + + // Unprotect handle so that it could be closed. + ::SetHandleInformation(handle, HANDLE_FLAG_PROTECT_FROM_CLOSE, 0); + } + AutoNativeLock lock(*lock_); HandleMap::iterator i = map_.find(handle); if (i == map_.end())
diff --git a/win/startup_information.h b/win/startup_information.h index 73d9f3e..3f18ee5 100644 --- a/win/startup_information.h +++ b/win/startup_information.h
@@ -47,4 +47,4 @@ } // namespace win } // namespace base -#endif // BASE_WIN_STARTUP_INFORMATION_H__ +#endif // BASE_WIN_STARTUP_INFORMATION_H_