diff --git a/WATCHLISTS b/WATCHLISTS index b431f06..671e845 100644 --- a/WATCHLISTS +++ b/WATCHLISTS
@@ -621,7 +621,7 @@ 'audio_message_filter|video_layer|media_internals', }, 'media_capture_from_element': { - 'filepath': 'content/renderer/media_capture_from_element/' \' + 'filepath': 'content/renderer/media_capture_from_element/' \ '|third_party/WebKit/Source/modules/mediacapture/' \ '|third_party/WebKit/Source/platform/exported/WebCanvasCapture' \ '|third_party/WebKit/LayoutTests/fast/mediacapturefromelement/' \ @@ -640,7 +640,7 @@ 'filepath': 'media/mojo/*' }, 'media_recorder': { - 'filepath': 'content/renderer/media_recorder/' \' + 'filepath': 'content/renderer/media_recorder/' \ '|third_party/WebKit/Source/modules/mediarecorder/' \ '|third_party/WebKit/Source/platform/exported/WebMediaRecorder' \ '|third_party/WebKit/LayoutTests/fast/mediarecorder/' \
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadInfo.java b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadInfo.java index 7588b4d..0e2be2d 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadInfo.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadInfo.java
@@ -32,6 +32,7 @@ private final boolean mIsOffTheRecord; private final boolean mIsOfflinePage; private final int mState; + private final long mLastAccessTime; private DownloadInfo(Builder builder) { mUrl = builder.mUrl; @@ -55,6 +56,7 @@ mIsOffTheRecord = builder.mIsOffTheRecord; mIsOfflinePage = builder.mIsOfflinePage; mState = builder.mState; + mLastAccessTime = builder.mLastAccessTime; } public String getUrl() { @@ -147,6 +149,10 @@ return mState; } + public long getLastAccessTime() { + return mLastAccessTime; + } + /** * Helper class for building the DownloadInfo object. */ @@ -172,6 +178,7 @@ private boolean mIsOffTheRecord; private boolean mIsOfflinePage; private int mState = DownloadState.IN_PROGRESS; + private long mLastAccessTime; public Builder setUrl(String url) { mUrl = url; @@ -279,6 +286,11 @@ return this; } + public Builder setLastAccessTime(long lastAccessTime) { + mLastAccessTime = lastAccessTime; + return this; + } + public DownloadInfo build() { return new DownloadInfo(this); } @@ -310,17 +322,18 @@ .setIsPaused(downloadInfo.isPaused()) .setIsOffTheRecord(downloadInfo.isOffTheRecord()) .setIsOfflinePage(downloadInfo.isOfflinePage()) - .setState(downloadInfo.state()); + .setState(downloadInfo.state()) + .setLastAccessTime(downloadInfo.getLastAccessTime()); return builder; } } @CalledByNative - private static DownloadInfo createDownloadInfo( - String downloadGuid, String fileName, String filePath, String url, String mimeType, - long bytesReceived, boolean isIncognito, int state, int percentCompleted, - boolean isPaused, boolean hasUserGesture, boolean isResumable, - String originalUrl, String referrerUrl, long timeRemainingInMs) { + private static DownloadInfo createDownloadInfo(String downloadGuid, String fileName, + String filePath, String url, String mimeType, long bytesReceived, boolean isIncognito, + int state, int percentCompleted, boolean isPaused, boolean hasUserGesture, + boolean isResumable, String originalUrl, String referrerUrl, long timeRemainingInMs, + long lastAccessTime) { String remappedMimeType = ChromeDownloadDelegate.remapGenericMimeType( mimeType, url, fileName); return new DownloadInfo.Builder() @@ -339,6 +352,8 @@ .setReferrer(referrerUrl) .setState(state) .setTimeRemainingInMillis(timeRemainingInMs) - .setUrl(url).build(); + .setLastAccessTime(lastAccessTime) + .setUrl(url) + .build(); } }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java index 3eacfd0..e339403 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java
@@ -1067,6 +1067,9 @@ /** See {@link #openDownloadedContent(Context, String, boolean, long)}. */ protected void openDownloadedContent(final DownloadInfo downloadInfo, final long downloadId) { + // TODO(shaktisahu): Move this to the broader openDownloadedContent() or a better place if + // possible. + updateLastAccessTime(downloadInfo.getDownloadGuid(), downloadInfo.isOffTheRecord()); openDownloadedContent(mContext, downloadInfo.getFilePath(), isSupportedMimeType(downloadInfo.getMimeType()), downloadId); } @@ -1777,6 +1780,16 @@ return mAutoResumptionLimit; } + /** + * Updates the last access time of a download. + * @param downloadGuid Download GUID. + * @param isOffTheRecord Whether the download is off the record. + */ + @Override + public void updateLastAccessTime(String downloadGuid, boolean isOffTheRecord) { + nativeUpdateLastAccessTime(getNativeDownloadManagerService(), downloadGuid, isOffTheRecord); + } + @Override public void onMaxBandwidthChanged(double maxBandwidthMbps) {} @@ -1809,4 +1822,6 @@ long nativeDownloadManagerService, boolean isOffTheRecord); private native void nativeCheckForExternallyRemovedDownloads( long nativeDownloadManagerService, boolean isOffTheRecord); + private native void nativeUpdateLastAccessTime( + long nativeDownloadManagerService, String downloadGuid, boolean isOffTheRecord); }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/ui/BackendProvider.java b/chrome/android/java/src/org/chromium/chrome/browser/download/ui/BackendProvider.java index 63512d01..ee4f317 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/download/ui/BackendProvider.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/download/ui/BackendProvider.java
@@ -42,6 +42,9 @@ /** See {@link DownloadManagerService#isDownloadOpenableInBrowser}. */ boolean isDownloadOpenableInBrowser(boolean isOffTheRecord, String mimeType); + + /** See {@link DownloadManagerService#updateLastAccessTime}. */ + void updateLastAccessTime(String downloadGuid, boolean isOffTheRecord); } /** Interacts with the Offline Pages backend. */
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryItemWrapper.java b/chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryItemWrapper.java index 02af5542..e36debb 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryItemWrapper.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryItemWrapper.java
@@ -330,6 +330,8 @@ } if (DownloadUtils.openFile(getFile(), getMimeType(), isOffTheRecord())) { + mBackendProvider.getDownloadDelegate().updateLastAccessTime( + mItem.getId(), isOffTheRecord()); recordOpenSuccess(); } else { recordOpenFailure();
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/download/ui/StubbedProvider.java b/chrome/android/javatests/src/org/chromium/chrome/browser/download/ui/StubbedProvider.java index 764ffb88..17f8744 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/download/ui/StubbedProvider.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/download/ui/StubbedProvider.java
@@ -88,6 +88,9 @@ public boolean isDownloadOpenableInBrowser(boolean isOffTheRecord, String mimeType) { return false; } + + @Override + public void updateLastAccessTime(String downloadGuid, boolean isOffTheRecord) {} } /** Stubs out the OfflinePageDownloadBridge. */
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn index 71c331f..b6cbea8 100644 --- a/chrome/browser/BUILD.gn +++ b/chrome/browser/BUILD.gn
@@ -1716,6 +1716,8 @@ "safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h", "safe_browsing/settings_reset_prompt/settings_reset_prompt_model.cc", "safe_browsing/settings_reset_prompt/settings_reset_prompt_model.h", + "safe_browsing/settings_reset_prompt/settings_reset_prompt_prefs_manager.cc", + "safe_browsing/settings_reset_prompt/settings_reset_prompt_prefs_manager.h", "search/hotword_audio_history_handler.cc", "search/hotword_audio_history_handler.h", "search/hotword_client.h",
diff --git a/chrome/browser/android/download/download_manager_service.cc b/chrome/browser/android/download/download_manager_service.cc index 65ebf67..5c825b5 100644 --- a/chrome/browser/android/download/download_manager_service.cc +++ b/chrome/browser/android/download/download_manager_service.cc
@@ -121,7 +121,8 @@ ConvertUTF8ToJavaString(env, original_url), ConvertUTF8ToJavaString(env, item->GetReferrerUrl().spec()), time_remaining_known ? time_delta.InMilliseconds() - : kUnknownRemainingTime); + : kUnknownRemainingTime, + item->GetLastAccessTime().ToJavaTime()); } static jlong Init(JNIEnv* env, const JavaParamRef<jobject>& jobj) { @@ -239,6 +240,21 @@ manager->CheckForHistoryFilesRemoval(); } +void DownloadManagerService::UpdateLastAccessTime( + JNIEnv* env, + const JavaParamRef<jobject>& obj, + const JavaParamRef<jstring>& jdownload_guid, + bool is_off_the_record) { + std::string download_guid = ConvertJavaStringToUTF8(env, jdownload_guid); + content::DownloadManager* manager = GetDownloadManager(is_off_the_record); + if (!manager) + return; + + content::DownloadItem* item = manager->GetDownloadByGuid(download_guid); + if (item) + item->SetLastAccessTime(base::Time::Now()); +} + void DownloadManagerService::CancelDownload( JNIEnv* env, jobject obj,
diff --git a/chrome/browser/android/download/download_manager_service.h b/chrome/browser/android/download/download_manager_service.h index fbc7f8a..2104f1f8 100644 --- a/chrome/browser/android/download/download_manager_service.h +++ b/chrome/browser/android/download/download_manager_service.h
@@ -94,6 +94,12 @@ const JavaParamRef<jobject>& obj, bool is_off_the_record); + // Called to update the last access time associated with a download. + void UpdateLastAccessTime(JNIEnv* env, + const JavaParamRef<jobject>& obj, + const JavaParamRef<jstring>& jdownload_guid, + bool is_off_the_record); + // DownloadHistory::Observer methods. void OnHistoryQueryComplete() override;
diff --git a/chrome/browser/apps/guest_view/web_view_browsertest.cc b/chrome/browser/apps/guest_view/web_view_browsertest.cc index b57c6b77..2f25f42 100644 --- a/chrome/browser/apps/guest_view/web_view_browsertest.cc +++ b/chrome/browser/apps/guest_view/web_view_browsertest.cc
@@ -2788,7 +2788,8 @@ download->GetLastModifiedTime(), download->GetReceivedBytes(), download->GetTotalBytes(), download->GetHash(), download->GetState(), download->GetDangerType(), download->GetLastReason(), - download->GetOpened(), download->GetReceivedSlices())); + download->GetOpened(), download->GetLastAccessTime(), + download->GetReceivedSlices())); } std::unique_ptr<content::DownloadTestObserver> completion_observer(
diff --git a/chrome/browser/browsing_data/downloads_counter_browsertest.cc b/chrome/browser/browsing_data/downloads_counter_browsertest.cc index 7bcab2b..ec0da22d 100644 --- a/chrome/browser/browsing_data/downloads_counter_browsertest.cc +++ b/chrome/browser/browsing_data/downloads_counter_browsertest.cc
@@ -129,28 +129,12 @@ content::DownloadManager* manager = incognito ? otr_manager_ : manager_; manager->CreateDownloadItem( - guid, - content::DownloadItem::kInvalidId + (++items_count_), + guid, content::DownloadItem::kInvalidId + (++items_count_), base::FilePath(FILE_PATH_LITERAL("current/path")), - base::FilePath(FILE_PATH_LITERAL("target/path")), - url_chain, - GURL(), - GURL(), - GURL(), - GURL(), - mime_type, - std::string(), - time_, - time_, - std::string(), - std::string(), - 1, - 1, - std::string(), - state, - danger, - reason, - false, + base::FilePath(FILE_PATH_LITERAL("target/path")), url_chain, GURL(), + GURL(), GURL(), GURL(), mime_type, std::string(), time_, time_, + std::string(), std::string(), 1, 1, std::string(), state, danger, + reason, false, time_, std::vector<content::DownloadItem::ReceivedSlice>()); return guid;
diff --git a/chrome/browser/chrome_browser_main_win.cc b/chrome/browser/chrome_browser_main_win.cc index bf52143..ff6c5f1 100644 --- a/chrome/browser/chrome_browser_main_win.cc +++ b/chrome/browser/chrome_browser_main_win.cc
@@ -37,6 +37,8 @@ #include "chrome/browser/first_run/first_run.h" #include "chrome/browser/install_verification/win/install_verification.h" #include "chrome/browser/profiles/profile_shortcut_manager.h" +#include "chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.h" +#include "chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h" #include "chrome/browser/shell_integration.h" #include "chrome/browser/ui/simple_message_box.h" #include "chrome/browser/ui/uninstall_browser_prompt.h" @@ -369,6 +371,14 @@ InitializeChromeElf(); + if (base::FeatureList::IsEnabled(safe_browsing::kSettingsResetPrompt)) { + content::BrowserThread::PostAfterStartupTask( + FROM_HERE, + content::BrowserThread::GetTaskRunnerForThread( + content::BrowserThread::UI), + base::Bind(safe_browsing::MaybeShowSettingsResetPromptWithDelay)); + } + // Record UMA data about whether the fault-tolerant heap is enabled. // Use a delayed task to minimize the impact on startup time. content::BrowserThread::PostDelayedTask(
diff --git a/chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc b/chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc index cadeb53..8e8c696 100644 --- a/chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc +++ b/chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc
@@ -3,30 +3,22 @@ // found in the LICENSE file. #include <memory> -#include <string> -#include <utility> -#include "base/bind.h" #include "base/command_line.h" #include "base/files/dir_reader_posix.h" #include "base/files/file_path.h" -#include "base/logging.h" #include "base/memory/ptr_util.h" #include "base/memory/ref_counted.h" #include "base/path_service.h" #include "base/run_loop.h" #include "base/test/null_task_runner.h" -#include "base/values.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process_platform_part.h" #include "chrome/browser/chromeos/extensions/signin_screen_policy_provider.h" #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h" #include "chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.h" -#include "chrome/browser/chromeos/policy/device_policy_builder.h" #include "chrome/browser/chromeos/policy/device_policy_cros_browser_test.h" -#include "chrome/browser/chromeos/policy/proto/chrome_device_policy.pb.h" #include "chrome/browser/chromeos/profiles/profile_helper.h" -#include "chrome/browser/chromeos/settings/device_settings_service.h" #include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/unpacked_installer.h" #include "chrome/browser/policy/test/local_policy_test_server.h" @@ -40,16 +32,12 @@ #include "chromeos/chromeos_paths.h" #include "chromeos/chromeos_switches.h" #include "chromeos/dbus/fake_session_manager_client.h" -#include "components/ownership/owner_key_util.h" -#include "components/policy/core/browser/browser_policy_connector.h" #include "components/policy/core/common/cloud/cloud_policy_client.h" #include "components/policy/core/common/cloud/device_management_service.h" #include "components/policy/core/common/cloud/mock_cloud_policy_client.h" #include "components/policy/core/common/cloud/policy_builder.h" #include "components/policy/core/common/cloud/resource_cache.h" -#include "components/policy/core/common/policy_service.h" #include "components/policy/core/common/policy_switches.h" -#include "components/policy/policy_constants.h" #include "components/policy/proto/chrome_extension_policy.pb.h" #include "components/policy/proto/device_management_backend.pb.h" #include "crypto/rsa_private_key.h" @@ -68,14 +56,11 @@ namespace policy { class DeviceCloudPolicyBrowserTest : public InProcessBrowserTest { - protected: - DeviceCloudPolicyBrowserTest() - : mock_client_(base::MakeUnique<MockCloudPolicyClient>()) {} + public: + DeviceCloudPolicyBrowserTest() : mock_client_(new MockCloudPolicyClient) { + } - std::unique_ptr<MockCloudPolicyClient> mock_client_; - - private: - DISALLOW_COPY_AND_ASSIGN(DeviceCloudPolicyBrowserTest); + MockCloudPolicyClient* mock_client_; }; IN_PROC_BROWSER_TEST_F(DeviceCloudPolicyBrowserTest, Initializer) { @@ -86,7 +71,7 @@ // Initializer is deleted when the manager connects. connector->GetDeviceCloudPolicyManager()->StartConnection( - std::move(mock_client_), connector->GetInstallAttributes()); + base::WrapUnique(mock_client_), connector->GetInstallAttributes()); EXPECT_FALSE(connector->GetDeviceCloudPolicyInitializer()); // Initializer is restarted when the manager disconnects. @@ -94,165 +79,6 @@ EXPECT_TRUE(connector->GetDeviceCloudPolicyInitializer()); } -// Tests for the rotation of the signing keys used for the device policy. -// -// The test is performed against a test policy server, which is set up for -// rotating the policy key automatically with each policy fetch. -class KeyRotationDeviceCloudPolicyTest : public DevicePolicyCrosBrowserTest { - protected: - const int kTestPolicyValue = 123; - const int kTestPolicyOtherValue = 456; - const char* const kTestPolicyKey = key::kDevicePolicyRefreshRate; - - KeyRotationDeviceCloudPolicyTest() {} - - void SetUp() override { - UpdateBuiltTestPolicyValue(kTestPolicyValue); - StartPolicyTestServer(); - DevicePolicyCrosBrowserTest::SetUp(); - } - - void SetUpCommandLine(base::CommandLine* command_line) override { - DevicePolicyCrosBrowserTest::SetUpCommandLine(command_line); - command_line->AppendSwitchASCII(policy::switches::kDeviceManagementUrl, - policy_test_server_.GetServiceURL().spec()); - } - - void SetUpInProcessBrowserTestFixture() override { - DevicePolicyCrosBrowserTest::SetUpInProcessBrowserTestFixture(); - InstallOwnerKey(); - MarkAsEnterpriseOwned(); - SetFakeDevicePolicy(); - } - - void SetUpOnMainThread() override { - DevicePolicyCrosBrowserTest::SetUpOnMainThread(); - g_browser_process->platform_part() - ->browser_policy_connector_chromeos() - ->device_management_service() - ->ScheduleInitialization(0); - StartObservingTestPolicy(); - } - - void TearDownOnMainThread() override { - policy_change_registrar_.reset(); - DevicePolicyCrosBrowserTest::TearDownOnMainThread(); - } - - void UpdateBuiltTestPolicyValue(int test_policy_value) { - device_policy() - ->payload() - .mutable_device_policy_refresh_rate() - ->set_device_policy_refresh_rate(test_policy_value); - device_policy()->Build(); - } - - void UpdateServedTestPolicy() { - EXPECT_TRUE(policy_test_server_.UpdatePolicy( - dm_protocol::kChromeDevicePolicyType, std::string() /* entity_id */, - device_policy()->payload().SerializeAsString())); - } - - int GetTestPolicyValue() { - PolicyService* const policy_service = - g_browser_process->platform_part() - ->browser_policy_connector_chromeos() - ->GetPolicyService(); - const base::Value* policy_value = - policy_service - ->GetPolicies(PolicyNamespace(POLICY_DOMAIN_CHROME, - std::string() /* component_id */)) - .GetValue(kTestPolicyKey); - EXPECT_TRUE(policy_value); - int refresh_rate = -1; - EXPECT_TRUE(policy_value->GetAsInteger(&refresh_rate)); - return refresh_rate; - } - - void WaitForTestPolicyValue(int expected_policy_value) { - if (GetTestPolicyValue() == expected_policy_value) - return; - awaited_policy_value_ = expected_policy_value; - // The run loop will be terminated by TestPolicyChangedCallback() once the - // policy value becomes equal to the awaited value. - DCHECK(!policy_change_waiting_run_loop_); - policy_change_waiting_run_loop_ = base::MakeUnique<base::RunLoop>(); - policy_change_waiting_run_loop_->Run(); - } - - private: - void StartPolicyTestServer() { - policy_test_server_.RegisterClient(PolicyBuilder::kFakeToken, - PolicyBuilder::kFakeDeviceId); - UpdateServedTestPolicy(); - policy_test_server_.EnableAutomaticRotationOfSigningKeys(); - EXPECT_TRUE(policy_test_server_.Start()); - } - - void SetFakeDevicePolicy() { - device_policy() - ->payload() - .mutable_device_policy_refresh_rate() - ->set_device_policy_refresh_rate(kTestPolicyValue); - device_policy()->Build(); - session_manager_client()->set_device_policy(device_policy()->GetBlob()); - } - - void StartObservingTestPolicy() { - policy_change_registrar_ = base::MakeUnique<PolicyChangeRegistrar>( - g_browser_process->platform_part() - ->browser_policy_connector_chromeos() - ->GetPolicyService(), - PolicyNamespace(POLICY_DOMAIN_CHROME, - std::string() /* component_id */)); - policy_change_registrar_->Observe( - kTestPolicyKey, - base::BindRepeating( - &KeyRotationDeviceCloudPolicyTest::TestPolicyChangedCallback, - base::Unretained(this))); - } - - void TestPolicyChangedCallback(const base::Value* old_value, - const base::Value* new_value) { - if (policy_change_waiting_run_loop_ && - GetTestPolicyValue() == awaited_policy_value_) { - policy_change_waiting_run_loop_->Quit(); - } - } - - LocalPolicyTestServer policy_test_server_; - std::unique_ptr<PolicyChangeRegistrar> policy_change_registrar_; - int awaited_policy_value_ = -1; - std::unique_ptr<base::RunLoop> policy_change_waiting_run_loop_; - - DISALLOW_COPY_AND_ASSIGN(KeyRotationDeviceCloudPolicyTest); -}; - -IN_PROC_BROWSER_TEST_F(KeyRotationDeviceCloudPolicyTest, Basic) { - // Initially, the policy has the first value. - EXPECT_EQ(kTestPolicyValue, GetTestPolicyValue()); - - const std::string original_owner_public_key = - chromeos::DeviceSettingsService::Get()->GetPublicKey()->as_string(); - - // The server is updated to serve the new policy value, and the client fetches - // it. - UpdateBuiltTestPolicyValue(kTestPolicyOtherValue); - UpdateServedTestPolicy(); - g_browser_process->platform_part() - ->browser_policy_connector_chromeos() - ->GetDeviceCloudPolicyManager() - ->RefreshPolicies(); - WaitForTestPolicyValue(kTestPolicyOtherValue); - EXPECT_EQ(kTestPolicyOtherValue, GetTestPolicyValue()); - - // The owner key has changed due to the key rotation performed by the policy - // test server. - EXPECT_NE( - original_owner_public_key, - chromeos::DeviceSettingsService::Get()->GetPublicKey()->as_string()); -} - // This class is the base class for the tests of the behavior regarding // extensions installed on the signin screen (which is generally possible only // through an admin policy, but the tests may install the extensions directly
diff --git a/chrome/browser/chromeos/settings/session_manager_operation.cc b/chrome/browser/chromeos/settings/session_manager_operation.cc index ed847589..fd9b3ba 100644 --- a/chrome/browser/chromeos/settings/session_manager_operation.cc +++ b/chrome/browser/chromeos/settings/session_manager_operation.cc
@@ -47,7 +47,7 @@ void SessionManagerOperation::RestartLoad(bool key_changed) { if (key_changed) - public_key_ = nullptr; + public_key_ = NULL; if (!is_loading_) return; @@ -84,17 +84,20 @@ } void SessionManagerOperation::EnsurePublicKey(const base::Closure& callback) { - if (force_key_load_ || !public_key_ || !public_key_->is_loaded()) { + if (force_key_load_ || !public_key_.get() || !public_key_->is_loaded()) { scoped_refptr<base::TaskRunner> task_runner = content::BrowserThread::GetBlockingPool() ->GetTaskRunnerWithShutdownBehavior( base::SequencedWorkerPool::SKIP_ON_SHUTDOWN); base::PostTaskAndReplyWithResult( - task_runner.get(), FROM_HERE, - base::Bind(&SessionManagerOperation::LoadPublicKey, owner_key_util_, - force_key_load_ ? nullptr : public_key_), + task_runner.get(), + FROM_HERE, + base::Bind(&SessionManagerOperation::LoadPublicKey, + owner_key_util_, + public_key_), base::Bind(&SessionManagerOperation::StorePublicKey, - weak_factory_.GetWeakPtr(), callback)); + weak_factory_.GetWeakPtr(), + callback)); } else { callback.Run(); } @@ -107,7 +110,7 @@ scoped_refptr<PublicKey> public_key(new PublicKey()); // Keep already-existing public key. - if (current_key && current_key->is_loaded()) { + if (current_key.get() && current_key->is_loaded()) { public_key->data() = current_key->data(); } if (!public_key->is_loaded() && util->IsPublicKeyPresent()) { @@ -123,7 +126,7 @@ force_key_load_ = false; public_key_ = new_key; - if (!public_key_ || !public_key_->is_loaded()) { + if (!public_key_.get() || !public_key_->is_loaded()) { ReportResult(DeviceSettingsService::STORE_KEY_UNAVAILABLE); return; } @@ -258,10 +261,7 @@ std::unique_ptr<em::PolicyFetchResponse> policy) : SessionManagerOperation(callback), policy_(std::move(policy)), - weak_factory_(this) { - if (policy_->has_new_public_key()) - force_key_load_ = true; -} + weak_factory_(this) {} StoreSettingsOperation::~StoreSettingsOperation() {}
diff --git a/chrome/browser/download/download_history.cc b/chrome/browser/download/download_history.cc index c1f0721..e55b5d2 100644 --- a/chrome/browser/download/download_history.cc +++ b/chrome/browser/download/download_history.cc
@@ -141,7 +141,7 @@ history::ToHistoryDownloadInterruptReason(item->GetLastReason()), std::string(), // Hash value (not available yet) history::ToHistoryDownloadId(item->GetId()), item->GetGuid(), - item->GetOpened(), by_ext_id, by_ext_name, + item->GetOpened(), item->GetLastAccessTime(), by_ext_id, by_ext_name, history::GetHistoryDownloadSliceInfos(*item)); } @@ -178,6 +178,7 @@ (previous->interrupt_reason != current.interrupt_reason) || (previous->hash != current.hash) || (previous->opened != current.opened) || + (previous->last_access_time != current.last_access_time) || (previous->by_ext_id != current.by_ext_id) || (previous->by_ext_name != current.by_ext_name) || (previous->download_slice_info != current.download_slice_info)) { @@ -298,7 +299,8 @@ history::ToContentDownloadState(it->state), history::ToContentDownloadDangerType(it->danger_type), history::ToContentDownloadInterruptReason(it->interrupt_reason), - it->opened, history::ToContentReceivedSlices(it->download_slice_info)); + it->opened, it->last_access_time, + history::ToContentReceivedSlices(it->download_slice_info)); #if BUILDFLAG(ENABLE_EXTENSIONS) if (!it->by_ext_id.empty() && !it->by_ext_name.empty()) { new extensions::DownloadedByExtension(
diff --git a/chrome/browser/download/download_history_unittest.cc b/chrome/browser/download/download_history_unittest.cc index 66276599..d9df5259 100644 --- a/chrome/browser/download/download_history_unittest.cc +++ b/chrome/browser/download/download_history_unittest.cc
@@ -235,7 +235,7 @@ history::ToContentDownloadState(row.state), history::ToContentDownloadDangerType(row.danger_type), history::ToContentDownloadInterruptReason(row.interrupt_reason), - row.opened, + row.opened, row.last_access_time, history::ToContentReceivedSlices(row.download_slice_info)); EXPECT_CALL(manager(), MockCreateDownloadItem(adapter)) .WillOnce(DoAll( @@ -347,37 +347,38 @@ (base::Time::Now() - base::TimeDelta::FromMinutes(1)), "Etag", "abc", 100, 100, state, content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - content::DOWNLOAD_INTERRUPT_REASON_NONE, false, std::string(), - std::string(), std::vector<content::DownloadItem::ReceivedSlice>(), - info); + content::DOWNLOAD_INTERRUPT_REASON_NONE, false, base::Time::Now(), + std::string(), std::string(), + std::vector<content::DownloadItem::ReceivedSlice>(), info); } - void InitItem(const std::string& guid, - uint32_t id, - const base::FilePath& current_path, - const base::FilePath& target_path, - const std::vector<GURL>& url_chain, - const GURL& referrer, - const GURL& site_url, - const GURL& tab_url, - const GURL& tab_referrer_url, - const std::string& mime_type, - const std::string& original_mime_type, - const base::Time& start_time, - const base::Time& end_time, - const std::string& etag, - const std::string& last_modified, - int64_t received_bytes, - int64_t total_bytes, - content::DownloadItem::DownloadState state, - content::DownloadDangerType danger_type, - content::DownloadInterruptReason interrupt_reason, - bool opened, - const std::string& by_extension_id, - const std::string& by_extension_name, - const std::vector<content::DownloadItem::ReceivedSlice>& - received_slices, - history::DownloadRow* info) { + void InitItem( + const std::string& guid, + uint32_t id, + const base::FilePath& current_path, + const base::FilePath& target_path, + const std::vector<GURL>& url_chain, + const GURL& referrer, + const GURL& site_url, + const GURL& tab_url, + const GURL& tab_referrer_url, + const std::string& mime_type, + const std::string& original_mime_type, + base::Time start_time, + base::Time end_time, + const std::string& etag, + const std::string& last_modified, + int64_t received_bytes, + int64_t total_bytes, + content::DownloadItem::DownloadState state, + content::DownloadDangerType danger_type, + content::DownloadInterruptReason interrupt_reason, + bool opened, + base::Time last_access_time, + const std::string& by_extension_id, + const std::string& by_extension_name, + const std::vector<content::DownloadItem::ReceivedSlice>& received_slices, + history::DownloadRow* info) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); size_t index = items_.size(); @@ -405,6 +406,7 @@ info->id = history::ToHistoryDownloadId(id); info->guid = guid; info->opened = opened; + info->last_access_time = last_access_time; info->by_ext_id = by_extension_id; info->by_ext_name = by_extension_name; @@ -447,6 +449,8 @@ EXPECT_CALL(item(index), GetLastReason()) .WillRepeatedly(Return(interrupt_reason)); EXPECT_CALL(item(index), GetOpened()).WillRepeatedly(Return(opened)); + EXPECT_CALL(item(index), GetLastAccessTime()) + .WillRepeatedly(Return(last_access_time)); EXPECT_CALL(item(index), GetTargetDisposition()) .WillRepeatedly( Return(content::DownloadItem::TARGET_DISPOSITION_OVERWRITE));
diff --git a/chrome/browser/download/download_ui_controller_unittest.cc b/chrome/browser/download/download_ui_controller_unittest.cc index b77cf64..7c812c4 100644 --- a/chrome/browser/download/download_ui_controller_unittest.cc +++ b/chrome/browser/download/download_ui_controller_unittest.cc
@@ -240,6 +240,7 @@ EXPECT_CALL(*item, GetTargetDisposition()).WillRepeatedly( Return(content::DownloadItem::TARGET_DISPOSITION_OVERWRITE)); EXPECT_CALL(*item, GetOpened()).WillRepeatedly(Return(false)); + EXPECT_CALL(*item, GetLastAccessTime()).WillRepeatedly(Return(base::Time())); EXPECT_CALL(*item, GetMimeType()).WillRepeatedly(Return(std::string())); EXPECT_CALL(*item, GetURL()).WillRepeatedly(ReturnRefOfCopy(GURL())); EXPECT_CALL(*item, GetWebContents()).WillRepeatedly(Return(nullptr));
diff --git a/chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc b/chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc index ec5b4b1..025967e 100644 --- a/chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc +++ b/chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc
@@ -432,7 +432,8 @@ (history_info[i].state != content::DownloadItem::CANCELLED ? content::DOWNLOAD_INTERRUPT_REASON_NONE : content::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED), - false, // opened + false, // opened + current, // last_access_time std::vector<DownloadItem::ReceivedSlice>()); items->push_back(item); }
diff --git a/chrome/browser/first_run/first_run_internal.h b/chrome/browser/first_run/first_run_internal.h index 82f91cc4..6af463d 100644 --- a/chrome/browser/first_run/first_run_internal.h +++ b/chrome/browser/first_run/first_run_internal.h
@@ -43,7 +43,6 @@ void DoPostImportPlatformSpecificTasks(Profile* profile); // Returns true if the sentinel file exists (or the path cannot be obtained). -// Migrates Windows legacy sentinel files to the corrent location, if needed. bool IsFirstRunSentinelPresent(); // This function has a common implementationin for all non-linux platforms, and
diff --git a/chrome/browser/first_run/first_run_internal_win.cc b/chrome/browser/first_run/first_run_internal_win.cc index d707c2ed..5b949c71 100644 --- a/chrome/browser/first_run/first_run_internal_win.cc +++ b/chrome/browser/first_run/first_run_internal_win.cc
@@ -122,26 +122,7 @@ bool IsFirstRunSentinelPresent() { base::FilePath sentinel; - if (!GetFirstRunSentinelFilePath(&sentinel) || base::PathExists(sentinel)) - return true; - - // Copy any legacy first run sentinel file for Windows user-level installs - // from the application directory to the user data directory. - base::FilePath exe_path; - if (PathService::Get(base::DIR_EXE, &exe_path) && - InstallUtil::IsPerUserInstall()) { - base::FilePath legacy_sentinel = exe_path.Append(chrome::kFirstRunSentinel); - if (base::PathExists(legacy_sentinel)) { - // Copy the file instead of moving it to avoid breaking developer builds - // where the sentinel is dropped beside chrome.exe by a build action. - bool migrated = base::CopyFile(legacy_sentinel, sentinel); - DPCHECK(migrated); - // The sentinel is present regardless of whether or not it was migrated. - return true; - } - } - - return false; + return !GetFirstRunSentinelFilePath(&sentinel) || base::PathExists(sentinel); } bool ShowPostInstallEULAIfNeeded(installer::MasterPreferences* install_prefs) {
diff --git a/chrome/browser/ntp_snippets/fake_download_item.cc b/chrome/browser/ntp_snippets/fake_download_item.cc index 2f2db42..05d8a52 100644 --- a/chrome/browser/ntp_snippets/fake_download_item.cc +++ b/chrome/browser/ntp_snippets/fake_download_item.cc
@@ -80,7 +80,7 @@ return is_file_externally_removed_; } -void FakeDownloadItem::SetStartTime(const base::Time& start_time) { +void FakeDownloadItem::SetStartTime(base::Time start_time) { start_time_ = start_time; } @@ -88,7 +88,7 @@ return start_time_; } -void FakeDownloadItem::SetEndTime(const base::Time& end_time) { +void FakeDownloadItem::SetEndTime(base::Time end_time) { end_time_ = end_time; } @@ -363,6 +363,11 @@ return false; } +base::Time FakeDownloadItem::GetLastAccessTime() const { + NOTREACHED(); + return base::Time(); +} + content::BrowserContext* FakeDownloadItem::GetBrowserContext() const { NOTREACHED(); return nullptr; @@ -386,6 +391,10 @@ NOTREACHED(); } +void FakeDownloadItem::SetLastAccessTime(base::Time time) { + NOTREACHED(); +} + void FakeDownloadItem::SetDisplayName(const base::FilePath& name) { NOTREACHED(); }
diff --git a/chrome/browser/ntp_snippets/fake_download_item.h b/chrome/browser/ntp_snippets/fake_download_item.h index 09f54e5..c91dfd9 100644 --- a/chrome/browser/ntp_snippets/fake_download_item.h +++ b/chrome/browser/ntp_snippets/fake_download_item.h
@@ -48,10 +48,10 @@ void SetFileExternallyRemoved(bool is_file_externally_removed); bool GetFileExternallyRemoved() const override; - void SetStartTime(const base::Time& start_time); + void SetStartTime(base::Time start_time); base::Time GetStartTime() const override; - void SetEndTime(const base::Time& end_time); + void SetEndTime(base::Time end_time); base::Time GetEndTime() const override; void SetState(const DownloadState& state); @@ -115,12 +115,14 @@ bool GetOpenWhenComplete() const override; bool GetAutoOpened() override; bool GetOpened() const override; + base::Time GetLastAccessTime() const override; content::BrowserContext* GetBrowserContext() const override; content::WebContents* GetWebContents() const override; void OnContentCheckCompleted( content::DownloadDangerType danger_type) override; void SetOpenWhenComplete(bool open) override; void SetOpened(bool opened) override; + void SetLastAccessTime(base::Time time) override; void SetDisplayName(const base::FilePath& name) override; std::string DebugString(bool verbose) const override;
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc b/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc index 78d8183..f7dd24d1f 100644 --- a/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc +++ b/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
@@ -45,7 +45,8 @@ // A bitvector to express which timing fields to match on. enum ExpectedTimingFields { FIRST_PAINT = 1 << 0, - FIRST_CONTENTFUL_PAINT = 1 << 1 + FIRST_CONTENTFUL_PAINT = 1 << 1, + STYLE_UPDATE_BEFORE_FCP = 1 << 2 }; explicit TimingUpdatedObserver(content::RenderWidgetHost* render_widget_host) @@ -112,7 +113,9 @@ if ((!(matching_fields_ & FIRST_PAINT) || timing.first_paint) && (!(matching_fields_ & FIRST_CONTENTFUL_PAINT) || - timing.first_contentful_paint)) { + timing.first_contentful_paint) && + (!(matching_fields_ & STYLE_UPDATE_BEFORE_FCP) || + timing.style_sheet_timing.update_style_duration_before_fcp)) { // Ensure that any other handlers of this message, for example the real // PageLoadMetric observers, get a chance to handle this message before // this waiter unblocks. @@ -720,6 +723,11 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest, CSSTiming) { ASSERT_TRUE(embedded_test_server()->Start()); + scoped_refptr<TimingUpdatedObserver> fcp_observer = + CreateTimingUpdatedObserver(); + fcp_observer->AddMatchingFields( + TimingUpdatedObserver::STYLE_UPDATE_BEFORE_FCP); + // Careful: Blink code clamps timestamps to 5us, so any CSS parsing we do here // must take >> 5us, otherwise we'll log 0 for the value and it will remain // unset here. @@ -727,6 +735,7 @@ browser(), embedded_test_server()->GetURL("/page_load_metrics/page_with_css.html")); NavigateToUntrackedUrl(); + fcp_observer->WaitForMatchingIPC(); histogram_tester_.ExpectTotalCount(internal::kHistogramFirstContentfulPaint, 1);
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc index c71e894..318b76c 100644 --- a/chrome/browser/prefs/browser_prefs.cc +++ b/chrome/browser/prefs/browser_prefs.cc
@@ -241,6 +241,7 @@ #if defined(OS_WIN) #include "chrome/browser/apps/app_launch_for_metro_restart_win.h" #include "chrome/browser/component_updater/sw_reporter_installer_win.h" +#include "chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_prefs_manager.h" #include "chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_util.h" #endif @@ -618,6 +619,8 @@ component_updater::RegisterProfilePrefsForSwReporter(registry); desktop_ios_promotion::RegisterProfilePrefs(registry); NetworkProfileBubble::RegisterProfilePrefs(registry); + safe_browsing::SettingsResetPromptPrefsManager::RegisterProfilePrefs( + registry); #endif #if defined(TOOLKIT_VIEWS)
diff --git a/chrome/browser/prefs/chrome_pref_service_factory.cc b/chrome/browser/prefs/chrome_pref_service_factory.cc index 4e7a23e..4e7c09fe 100644 --- a/chrome/browser/prefs/chrome_pref_service_factory.cc +++ b/chrome/browser/prefs/chrome_pref_service_factory.cc
@@ -221,6 +221,32 @@ PrefHashFilter::TRACKING_STRATEGY_ATOMIC, PrefHashFilter::VALUE_PERSONAL }, +#if defined(OS_WIN) + { + 25, prefs::kSettingsResetPromptPromptWave, + PrefHashFilter::ENFORCE_ON_LOAD, + PrefHashFilter::TRACKING_STRATEGY_ATOMIC, + PrefHashFilter::VALUE_IMPERSONAL + }, + { + 26, prefs::kSettingsResetPromptLastTriggeredForDefaultSearch, + PrefHashFilter::ENFORCE_ON_LOAD, + PrefHashFilter::TRACKING_STRATEGY_ATOMIC, + PrefHashFilter::VALUE_IMPERSONAL + }, + { + 27, prefs::kSettingsResetPromptLastTriggeredForStartupUrls, + PrefHashFilter::ENFORCE_ON_LOAD, + PrefHashFilter::TRACKING_STRATEGY_ATOMIC, + PrefHashFilter::VALUE_IMPERSONAL + }, + { + 28, prefs::kSettingsResetPromptLastTriggeredForHomepage, + PrefHashFilter::ENFORCE_ON_LOAD, + PrefHashFilter::TRACKING_STRATEGY_ATOMIC, + PrefHashFilter::VALUE_IMPERSONAL + }, +#endif // defined(OS_WIN) // See note at top, new items added here also need to be added to // histograms.xml's TrackedPreference enum. };
diff --git a/chrome/browser/safe_browsing/incident_reporting/last_download_finder_unittest.cc b/chrome/browser/safe_browsing/incident_reporting/last_download_finder_unittest.cc index 8804366f..3fb14fa 100644 --- a/chrome/browser/safe_browsing/incident_reporting/last_download_finder_unittest.cc +++ b/chrome/browser/safe_browsing/incident_reporting/last_download_finder_unittest.cc
@@ -299,6 +299,7 @@ download_id_++, // id base::GenerateGUID(), // GUID false, // download_opened + now - base::TimeDelta::FromMinutes(5), // last_access_time std::string(), // ext_id std::string(), // ext_name std::vector<history::DownloadSliceInfo>()); // download_slice_info
diff --git a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.cc b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.cc index 994a4c7..af62eaf 100644 --- a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.cc +++ b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.cc
@@ -101,6 +101,14 @@ return delay_before_prompt_; } +int SettingsResetPromptConfig::prompt_wave() const { + return prompt_wave_; +} + +base::TimeDelta SettingsResetPromptConfig::time_between_prompts() const { + return time_between_prompts_; +} + // Implements the hash function for SHA256Hash objects. Simply uses the // first bytes of the SHA256 hash as its own hash. size_t SettingsResetPromptConfig::SHA256HashHasher::operator()( @@ -123,6 +131,8 @@ CONFIG_ERROR_BAD_DOMAIN_ID = 5, CONFIG_ERROR_DUPLICATE_DOMAIN_HASH = 6, CONFIG_ERROR_BAD_DELAY_BEFORE_PROMPT_SECONDS_PARAM = 7, + CONFIG_ERROR_BAD_PROMPT_WAVE_PARAM = 8, + CONFIG_ERROR_BAD_TIME_BETWEEN_PROMPTS_SECONDS_PARAM = 9, CONFIG_ERROR_MAX }; @@ -152,6 +162,28 @@ delay_before_prompt_ = base::TimeDelta::FromSeconds(delay_before_prompt_seconds); + // Get the prompt_wave feature paramter. + prompt_wave_ = base::GetFieldTrialParamByFeatureAsInt(kSettingsResetPrompt, + "prompt_wave", 0); + if (prompt_wave_ <= 0) { + UMA_HISTOGRAM_ENUMERATION("SettingsResetPrompt.ConfigError", + CONFIG_ERROR_BAD_PROMPT_WAVE_PARAM, + CONFIG_ERROR_MAX); + return false; + } + + // Get the time_between_prompts_seconds feature parameter. + int time_between_prompts_seconds = base::GetFieldTrialParamByFeatureAsInt( + kSettingsResetPrompt, "time_between_prompts_seconds", -1); + if (time_between_prompts_seconds < 0) { + UMA_HISTOGRAM_ENUMERATION( + "SettingsResetPrompt.ConfigError", + CONFIG_ERROR_BAD_TIME_BETWEEN_PROMPTS_SECONDS_PARAM, CONFIG_ERROR_MAX); + return false; + } + time_between_prompts_ = + base::TimeDelta::FromSeconds(time_between_prompts_seconds); + UMA_HISTOGRAM_ENUMERATION("SettingsResetPrompt.ConfigError", CONFIG_ERROR_OK, CONFIG_ERROR_MAX); return true;
diff --git a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.h b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.h index e2a09265..7162a01e 100644 --- a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.h +++ b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.h
@@ -40,6 +40,13 @@ // The delay before showing the reset prompt after Chrome startup. base::TimeDelta delay_before_prompt() const; + // Integer that identifies the current prompt wave. This number will increase + // with each new prompt wave. + int prompt_wave() const; + // The minimum time that must pass since the last time the prompt was shown + // before a new prompt can be shown. Applies only to prompts shown during the + // same prompt wave. + base::TimeDelta time_between_prompts() const; protected: SettingsResetPromptConfig(); @@ -59,6 +66,8 @@ // Other feature parameters. base::TimeDelta delay_before_prompt_; + int prompt_wave_ = 0; + base::TimeDelta time_between_prompts_; DISALLOW_COPY_AND_ASSIGN(SettingsResetPromptConfig); };
diff --git a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config_unittest.cc b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config_unittest.cc index 15a57249..6e25b28 100644 --- a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config_unittest.cc +++ b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config_unittest.cc
@@ -45,7 +45,9 @@ Parameters GetDefaultFeatureParams() { return { {"domain_hashes", base::StringPrintf("{\"%s\": \"1\"}", kDomainHash)}, - {"delay_before_prompt_seconds", "42"}}; + {"delay_before_prompt_seconds", "42"}, + {"prompt_wave", "20170101"}, + {"time_between_prompts_seconds", "3600"}}; } variations::testing::VariationParamsManager params_manager_; @@ -205,7 +207,7 @@ } TEST_F(SettingsResetPromptConfigTest, DelayBeforePromptSecondsParam) { - constexpr char kDelayParam[] = "delay_before_prompt_seconds"; + constexpr const char kDelayParam[] = "delay_before_prompt_seconds"; Parameters params = GetDefaultFeatureParams(); @@ -248,4 +250,75 @@ } } +TEST_F(SettingsResetPromptConfigTest, PromptWaveParam) { + constexpr const char kPromptWaveParam[] = "prompt_wave"; + + Parameters params = GetDefaultFeatureParams(); + + // Missing parameter. + ASSERT_EQ(params.erase(kPromptWaveParam), 1U); + SetFeatureParams(params); + EXPECT_FALSE(SettingsResetPromptConfig::Create()); + + // Empty parameter. + params[kPromptWaveParam] = ""; + SetFeatureParams(params); + EXPECT_FALSE(SettingsResetPromptConfig::Create()); + + // Bad parameter value. + params[kPromptWaveParam] = "not-a-number"; + SetFeatureParams(params); + EXPECT_FALSE(SettingsResetPromptConfig::Create()); + + // Negative parameter value. + params[kPromptWaveParam] = "-3"; + SetFeatureParams(params); + EXPECT_FALSE(SettingsResetPromptConfig::Create()); + + // Correct parameter value. + params[kPromptWaveParam] = "20170202"; + SetFeatureParams(params); + { + auto config = SettingsResetPromptConfig::Create(); + ASSERT_TRUE(config); + EXPECT_EQ(config->prompt_wave(), 20170202); + } +} + +TEST_F(SettingsResetPromptConfigTest, TimeBetweenPromptsParam) { + constexpr const char kParamName[] = "time_between_prompts_seconds"; + + Parameters params = GetDefaultFeatureParams(); + + // Missing parameter. + ASSERT_EQ(params.erase(kParamName), 1U); + SetFeatureParams(params); + EXPECT_FALSE(SettingsResetPromptConfig::Create()); + + // Empty parameter. + params[kParamName] = ""; + SetFeatureParams(params); + EXPECT_FALSE(SettingsResetPromptConfig::Create()); + + // Bad parameter value. + params[kParamName] = "not-a-number"; + SetFeatureParams(params); + EXPECT_FALSE(SettingsResetPromptConfig::Create()); + + // Negative parameter value. + params[kParamName] = "-3"; + SetFeatureParams(params); + EXPECT_FALSE(SettingsResetPromptConfig::Create()); + + // Correct parameter value. + params[kParamName] = "3600"; + SetFeatureParams(params); + { + auto config = SettingsResetPromptConfig::Create(); + ASSERT_TRUE(config); + EXPECT_EQ(config->time_between_prompts(), + base::TimeDelta::FromSeconds(3600)); + } +} + } // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.cc b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.cc index 5c8b4e4..1689fd8b 100644 --- a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.cc +++ b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.cc
@@ -48,6 +48,44 @@ SettingsResetPromptModel::RESET_REQUIRED; } +// Function that is passed the newly created |SettingsResetPromptModel| object +// as a result of a call to |MaybeShowSettingsResetPrompt()| below. Will display +// the settings reset prompt if required by the model and there is at least one +// non-incognito browser available for the corresponding profile. +void OnModelCreated(std::unique_ptr<SettingsResetPromptModel> model) { + if (!model || !model->ShouldPromptForReset()) + return; + + Profile* profile = model->profile(); + + // Ensure that there is at least one non-incognito browser open for the + // profile before attempting to show the dialog. + if (!chrome::FindTabbedBrowser(profile, /*match_original_profiles=*/false)) + return; + + chrome::ScopedTabbedBrowserDisplayer displayer(profile); + // The |SettingsResetPromptController| object will delete itself after the + // reset prompt dialog has been closed. + SettingsResetPromptController::ShowSettingsResetPrompt( + displayer.browser(), new SettingsResetPromptController(std::move(model))); +} + +void MaybeShowSettingsResetPrompt( + std::unique_ptr<SettingsResetPromptConfig> config) { + DCHECK(config); + + Browser* browser = chrome::FindLastActive(); + if (!browser) + return; + + // Get the original profile in case the last active browser was incognito. We + // ensure that there is at least one non-incognito browser open before + // displaying the dialog. + Profile* profile = browser->profile()->GetOriginalProfile(); + SettingsResetPromptModel::Create(profile, std::move(config), + base::Bind(OnModelCreated)); +} + } // namespace. SettingsResetPromptController::SettingsResetPromptController( @@ -97,6 +135,10 @@ return main_text_url_range_; } +void SettingsResetPromptController::DialogShown() { + model_->DialogShown(); +} + void SettingsResetPromptController::Accept() { model_->PerformReset( base::Bind(&SettingsResetPromptController::OnInteractionDone, @@ -165,4 +207,17 @@ // TODO(alito): Add metrics reporting here. delete this; } + +void MaybeShowSettingsResetPromptWithDelay() { + std::unique_ptr<SettingsResetPromptConfig> config = + SettingsResetPromptConfig::Create(); + if (!config) + return; + + base::TimeDelta delay = config->delay_before_prompt(); + content::BrowserThread::PostDelayedTask( + content::BrowserThread::UI, FROM_HERE, + base::Bind(MaybeShowSettingsResetPrompt, base::Passed(&config)), delay); +} + } // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h index 748ae4eb..31a7afe 100644 --- a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h +++ b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h
@@ -40,6 +40,10 @@ // Returns the offset into the main text string where a URL was inserted. To // be used by the dialog to apply an appropriate style to the URL text. gfx::Range GetMainTextUrlRange() const; + // |DialogShown()| will be called by the dialog when the dialog's |Show()| + // method is called. This allows the controller to notify the model that the + // dialog has been shown so that preferences can be updated. + void DialogShown(); // |Accept()| will be called by the dialog when the user clicks the main // button, after which the dialog will be closed. void Accept(); @@ -62,6 +66,12 @@ DISALLOW_COPY_AND_ASSIGN(SettingsResetPromptController); }; +// Function to be called after startup in order to display the settings reset +// prompt. The function will figure out if a prompt is needed, and if so, show +// the dialog after a delay as determined by the |kSettingsResetPrompt| +// feature parameters. +void MaybeShowSettingsResetPromptWithDelay(); + } // namespace safe_browsing #endif // CHROME_BROWSER_SAFE_BROWSING_SETTINGS_RESET_PROMPT_SETTINGS_RESET_PROMPT_CONTROLLER_H_
diff --git a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model.cc b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model.cc index 23cbdab9..807a38e 100644 --- a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model.cc +++ b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model.cc
@@ -218,6 +218,19 @@ done_callback); } +void SettingsResetPromptModel::DialogShown() { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + DCHECK(SomeSettingRequiresReset()); + + base::Time now = base::Time::Now(); + if (default_search_reset_state() == RESET_REQUIRED) + prefs_manager_.RecordPromptShownForDefaultSearch(now); + if (startup_urls_reset_state() == RESET_REQUIRED) + prefs_manager_.RecordPromptShownForStartupUrls(now); + if (homepage_reset_state() == RESET_REQUIRED) + prefs_manager_.RecordPromptShownForHomepage(now); +} + GURL SettingsResetPromptModel::homepage() const { return homepage_url_; } @@ -283,10 +296,13 @@ std::unique_ptr<BrandcodedDefaultSettings> default_settings, std::unique_ptr<ProfileResetter> profile_resetter) : profile_(profile), + prefs_manager_(profile, prompt_config->prompt_wave()), prompt_config_(std::move(prompt_config)), settings_snapshot_(std::move(settings_snapshot)), default_settings_(std::move(default_settings)), profile_resetter_(std::move(profile_resetter)), + time_since_last_prompt_(base::Time::Now() - + prefs_manager_.LastTriggeredPrompt()), settings_types_initialized_(0), homepage_reset_domain_id_(-1), homepage_reset_state_(NO_RESET_REQUIRED_DUE_TO_DOMAIN_NOT_MATCHED), @@ -322,7 +338,8 @@ if (default_search_reset_domain_id_ < 0) return; - default_search_reset_state_ = RESET_REQUIRED; + default_search_reset_state_ = GetResetStateForSetting( + prefs_manager_.LastTriggeredPromptForDefaultSearch()); } void SettingsResetPromptModel::InitStartupUrlsData() { @@ -341,14 +358,16 @@ startup_urls_.push_back(fixed_url); int reset_domain_id = prompt_config_->UrlToResetDomainId(fixed_url); if (reset_domain_id >= 0) { - startup_urls_reset_state_ = - SomeSettingRequiresReset() - ? NO_RESET_REQUIRED_DUE_TO_OTHER_SETTING_REQUIRING_RESET - : RESET_REQUIRED; startup_urls_to_reset_.push_back(fixed_url); domain_ids_for_startup_urls_to_reset_.insert(reset_domain_id); } } + + if (startup_urls_to_reset_.empty()) + return; + + startup_urls_reset_state_ = GetResetStateForSetting( + prefs_manager_.LastTriggeredPromptForStartupUrls()); } void SettingsResetPromptModel::InitHomepageData() { @@ -376,9 +395,7 @@ return; homepage_reset_state_ = - SomeSettingRequiresReset() - ? NO_RESET_REQUIRED_DUE_TO_OTHER_SETTING_REQUIRING_RESET - : RESET_REQUIRED; + GetResetStateForSetting(prefs_manager_.LastTriggeredPromptForHomepage()); } // Populate |extensions_to_disable_| with all enabled extensions that override @@ -420,6 +437,21 @@ } } +SettingsResetPromptModel::ResetState +SettingsResetPromptModel::GetResetStateForSetting( + const base::Time& last_triggered_for_setting) const { + if (!last_triggered_for_setting.is_null()) + return NO_RESET_REQUIRED_DUE_TO_ALREADY_PROMPTED_FOR_SETTING; + + if (time_since_last_prompt_ < prompt_config_->time_between_prompts()) + return NO_RESET_REQUIRED_DUE_TO_RECENTLY_PROMPTED; + + if (SomeSettingRequiresReset()) + return NO_RESET_REQUIRED_DUE_TO_OTHER_SETTING_REQUIRING_RESET; + + return RESET_REQUIRED; +} + bool SettingsResetPromptModel::SomeSettingRequiresReset() const { return default_search_reset_state_ == RESET_REQUIRED || startup_urls_reset_state_ == RESET_REQUIRED ||
diff --git a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model.h b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model.h index 63bdfab1..207447e5 100644 --- a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model.h +++ b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model.h
@@ -15,7 +15,9 @@ #include "base/callback_forward.h" #include "base/macros.h" +#include "base/time/time.h" #include "chrome/browser/safe_browsing/settings_reset_prompt/extension_info.h" +#include "chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_prefs_manager.h" #include "extensions/common/extension_id.h" #include "url/gurl.h" @@ -42,7 +44,9 @@ enum ResetState { RESET_REQUIRED = 1, NO_RESET_REQUIRED_DUE_TO_DOMAIN_NOT_MATCHED = 2, - NO_RESET_REQUIRED_DUE_TO_OTHER_SETTING_REQUIRING_RESET = 3, + NO_RESET_REQUIRED_DUE_TO_ALREADY_PROMPTED_FOR_SETTING = 3, + NO_RESET_REQUIRED_DUE_TO_RECENTLY_PROMPTED = 4, + NO_RESET_REQUIRED_DUE_TO_OTHER_SETTING_REQUIRING_RESET = 5, }; using ExtensionMap = @@ -76,6 +80,9 @@ // // NOTE: Can only be called once during the lifetime of this object. virtual void PerformReset(const base::Closure& done_callback); + // To be called when the reset prompt dialog has been shown so that + // preferences can be updated. + virtual void DialogShown(); virtual GURL homepage() const; virtual ResetState homepage_reset_state() const; @@ -119,10 +126,18 @@ void InitStartupUrlsData(); void InitHomepageData(); void InitExtensionData(); + + // Helper function for the Init* functions above to determine the reset state + // of settings that have a match in the config. + ResetState GetResetStateForSetting( + const base::Time& last_triggered_for_setting) const; + // Return true if any setting's reset state is set to |RESET_REQUIRED|. bool SomeSettingRequiresReset() const; Profile* const profile_; + + SettingsResetPromptPrefsManager prefs_manager_; std::unique_ptr<SettingsResetPromptConfig> prompt_config_; std::unique_ptr<ResettableSettingsSnapshot> settings_snapshot_; // |default_settings_| should only be accessed on the UI thread after @@ -130,6 +145,13 @@ std::unique_ptr<BrandcodedDefaultSettings> default_settings_; std::unique_ptr<ProfileResetter> profile_resetter_; + // A single timestamp to be used by all initialization functions to determine + // if enough time has passed between the last time the prompt was shown and + // "now" for a new prompt to be shown. + base::Time now_; + // The time since the last prompt was shown for any setting. + base::TimeDelta time_since_last_prompt_; + // Bits to keep track of which settings types have been initialized. uint32_t settings_types_initialized_;
diff --git a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model_browsertest.cc b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model_browsertest_win.cc similarity index 95% rename from chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model_browsertest.cc rename to chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model_browsertest_win.cc index c76d9f72..aab9ca1 100644 --- a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model_browsertest.cc +++ b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model_browsertest_win.cc
@@ -51,20 +51,12 @@ const char kDefaultStartupUrl1[] = "http://mystart1.com"; const char kDefaultStartupUrl2[] = "http://mystart2.com"; -// Some tests and parts of tests should only be run on platforms where settings -// override for extensions is available. The settings reset prompt is currently -// designed for desktop, and Windows in particular. Getting the tests to run on -// other platforms that do not allow extensions to override settings (e.g., -// Linux) would require more #ifdefs and platform specific considerations than -// it is worth. -#if defined(OS_WIN) || defined(OS_MACOSX) const char kHomepage1[] = "http://homepage.com/"; const char kHomepage2[] = "http://otherhomepage.com/"; const char kSearchUrl1[] = "http://mysearch.com/s?q={searchTerms}"; const char kSearchUrl2[] = "http://othersearch.com/s?q={searchTerms}"; const char kStartupUrl1[] = "http://super-startup.com"; const char kStartupUrl2[] = "http://awesome-start-page.com"; -#endif // defined(OS_WIN) || defined(OS_MACOSX) // Extension manifests to override settings. const char kManifestNoOverride[] = @@ -256,9 +248,6 @@ EXPECT_THAT(model->extensions_to_disable(), IsEmpty()); } -// Some tests should run only on platforms where settings override for -// extensions is available. See comment at the top for more details. -#if defined(OS_WIN) || defined(OS_MACOSX) // Load extension that overrides homepage. Homepage no longer needs to be // reset. const Extension* homepage_extension1 = nullptr; @@ -315,7 +304,6 @@ UnorderedElementsAre(Pair(homepage_extension1->id(), _), Pair(homepage_extension2->id(), _))); } -#endif // defined(OS_WIN) || defined(OS_MACOSX) } IN_PROC_BROWSER_TEST_F(SettingsResetPromptModelBrowserTest, @@ -349,9 +337,6 @@ EXPECT_THAT(model->extensions_to_disable(), IsEmpty()); } -// Some tests should run only on platforms where settings override for -// extensions is available. See comment at the top for more details. -#if defined(OS_WIN) || defined(OS_MACOSX) // Load extension that overrides search. Search no longer needs to be reset. const Extension* search_extension1 = nullptr; LoadSearchExtension(kSearchUrl1, &search_extension1); @@ -407,7 +392,6 @@ UnorderedElementsAre(Pair(search_extension1->id(), _), Pair(search_extension2->id(), _))); } -#endif // defined(OS_WIN) || defined(OS_MACOSX) } IN_PROC_BROWSER_TEST_F(SettingsResetPromptModelBrowserTest, @@ -454,9 +438,6 @@ EXPECT_THAT(model->extensions_to_disable(), IsEmpty()); } -// Some tests should run only on platforms where settings override for -// extensions is available. See comment at the top for more details. -#if defined(OS_WIN) || defined(OS_MACOSX) // Load two other extensions that each override startup urls. const Extension* startup_url_extension1 = nullptr; LoadStartupUrlExtension(kStartupUrl1, &startup_url_extension1); @@ -515,12 +496,8 @@ UnorderedElementsAre(Pair(startup_url_extension1->id(), _), Pair(startup_url_extension2->id(), _))); } -#endif // defined(OS_WIN) || defined(OS_MACOSX) } -// Some tests should run only on platforms where settings override for -// extensions is available. See comment at the top for more details. -#if defined(OS_WIN) || defined(OS_MACOSX) IN_PROC_BROWSER_TEST_F(SettingsResetPromptModelBrowserTest, PerformReset_DefaultSearch) { // Load an extension that does not override settings and two extensions that @@ -789,7 +766,6 @@ INSTANTIATE_TEST_CASE_P(SettingsResetPromptModel, ExtensionSettingsOverrideTest, Combine(Bool(), Bool(), Bool())); -#endif // defined(OS_WIN) || defined(OS_MACOSX) } // namespace } // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model_unittest.cc b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model_unittest.cc index d8c2006..05f1b5c 100644 --- a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model_unittest.cc +++ b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model_unittest.cc
@@ -30,6 +30,7 @@ #include "components/search_engines/template_url_data.h" #include "components/search_engines/template_url_service.h" #include "components/search_engines/template_url_service_client.h" +#include "components/sync_preferences/testing_pref_service_syncable.h" #include "content/public/browser/browser_context.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -96,7 +97,22 @@ void SetUp() override { extensions::ExtensionServiceTestBase::SetUp(); - InitializeEmptyExtensionService(); + + // By not specifying a pref_file filepath, we get a + // sync_preferences::TestingPrefServiceSyncable, which in turn provides us + // with a convient way of registring preferences. + ExtensionServiceInitParams init_params = CreateDefaultInitParams(); + init_params.pref_file.clear(); + InitializeExtensionService(init_params); + +#if !defined(OS_WIN) + // In production code, the settings reset prompt profile preferences are + // registered on Windows only. We explicitly register the prefs on + // non-Windows systems so that we can continue testing the model on more + // than just Windows. + SettingsResetPromptPrefsManager::RegisterProfilePrefs( + testing_pref_service()->registry()); +#endif // !defined(OS_WIN) profile_->CreateWebDataService(); TemplateURLServiceFactory::GetInstance()->SetTestingFactory(
diff --git a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_prefs_manager.cc b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_prefs_manager.cc new file mode 100644 index 0000000..5598a1b --- /dev/null +++ b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_prefs_manager.cc
@@ -0,0 +1,92 @@ +// Copyright 2017 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 "chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_prefs_manager.h" + +#include <algorithm> +#include <initializer_list> + +#include "chrome/browser/profiles/profile.h" +#include "chrome/common/pref_names.h" +#include "components/pref_registry/pref_registry_syncable.h" +#include "components/prefs/pref_service.h" + +namespace safe_browsing { + +SettingsResetPromptPrefsManager::SettingsResetPromptPrefsManager( + Profile* profile, + int prompt_wave) + : profile_(profile), prefs_(profile->GetPrefs()) { + DCHECK(profile_); + DCHECK(prefs_); + + // If we are in a new prompt_wave, clear previous prefs. + int prefs_prompt_wave = + prefs_->GetInteger(prefs::kSettingsResetPromptPromptWave); + if (prompt_wave != prefs_prompt_wave) { + prefs_->SetInteger(prefs::kSettingsResetPromptPromptWave, prompt_wave); + prefs_->ClearPref(prefs::kSettingsResetPromptLastTriggeredForDefaultSearch); + prefs_->ClearPref(prefs::kSettingsResetPromptLastTriggeredForStartupUrls); + prefs_->ClearPref(prefs::kSettingsResetPromptLastTriggeredForHomepage); + } +} + +SettingsResetPromptPrefsManager::~SettingsResetPromptPrefsManager() {} + +// static +void SettingsResetPromptPrefsManager::RegisterProfilePrefs( + user_prefs::PrefRegistrySyncable* registry) { + DCHECK(registry); + registry->RegisterIntegerPref(prefs::kSettingsResetPromptPromptWave, 0); + registry->RegisterInt64Pref( + prefs::kSettingsResetPromptLastTriggeredForDefaultSearch, 0); + registry->RegisterInt64Pref( + prefs::kSettingsResetPromptLastTriggeredForStartupUrls, 0); + registry->RegisterInt64Pref( + prefs::kSettingsResetPromptLastTriggeredForHomepage, 0); +} + +base::Time SettingsResetPromptPrefsManager::LastTriggeredPrompt() const { + return std::max({LastTriggeredPromptForDefaultSearch(), + LastTriggeredPromptForStartupUrls(), + LastTriggeredPromptForHomepage()}); +} + +base::Time +SettingsResetPromptPrefsManager::LastTriggeredPromptForDefaultSearch() const { + return base::Time::FromInternalValue(prefs_->GetInt64( + prefs::kSettingsResetPromptLastTriggeredForDefaultSearch)); +} + +base::Time SettingsResetPromptPrefsManager::LastTriggeredPromptForStartupUrls() + const { + return base::Time::FromInternalValue( + prefs_->GetInt64(prefs::kSettingsResetPromptLastTriggeredForStartupUrls)); +} + +base::Time SettingsResetPromptPrefsManager::LastTriggeredPromptForHomepage() + const { + return base::Time::FromInternalValue( + prefs_->GetInt64(prefs::kSettingsResetPromptLastTriggeredForHomepage)); +} + +void SettingsResetPromptPrefsManager::RecordPromptShownForDefaultSearch( + const base::Time& prompt_time) { + prefs_->SetInt64(prefs::kSettingsResetPromptLastTriggeredForDefaultSearch, + prompt_time.ToInternalValue()); +} + +void SettingsResetPromptPrefsManager::RecordPromptShownForStartupUrls( + const base::Time& prompt_time) { + prefs_->SetInt64(prefs::kSettingsResetPromptLastTriggeredForStartupUrls, + prompt_time.ToInternalValue()); +} + +void SettingsResetPromptPrefsManager::RecordPromptShownForHomepage( + const base::Time& prompt_time) { + prefs_->SetInt64(prefs::kSettingsResetPromptLastTriggeredForHomepage, + prompt_time.ToInternalValue()); +} + +} // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_prefs_manager.h b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_prefs_manager.h new file mode 100644 index 0000000..6c47fb4 --- /dev/null +++ b/chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_prefs_manager.h
@@ -0,0 +1,49 @@ +// Copyright 2017 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 CHROME_BROWSER_SAFE_BROWSING_SETTINGS_RESET_PROMPT_SETTINGS_RESET_PROMPT_PREFS_MANAGER_H_ +#define CHROME_BROWSER_SAFE_BROWSING_SETTINGS_RESET_PROMPT_SETTINGS_RESET_PROMPT_PREFS_MANAGER_H_ + +#include "base/time/time.h" + +class Profile; +class PrefService; + +namespace user_prefs { +class PrefRegistrySyncable; +} // namespace user_prefs + +namespace safe_browsing { + +// Class responsible for reading and updating the preferences related to the +// settings reset prompt. +class SettingsResetPromptPrefsManager { + public: + // |prompt_wave| should be set to the prompt wave parameter obtained from the + // |SettingsResetPromptConfig| class. If a new prompt wave has been started + // (i.e., if the |prompt_wave| passed in to the constructor is greater than + // the one stored in preferences), other related settings in preferences will + // be reset. + SettingsResetPromptPrefsManager(Profile* profile, int prompt_wave); + ~SettingsResetPromptPrefsManager(); + + static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry); + + base::Time LastTriggeredPrompt() const; + base::Time LastTriggeredPromptForDefaultSearch() const; + base::Time LastTriggeredPromptForStartupUrls() const; + base::Time LastTriggeredPromptForHomepage() const; + + void RecordPromptShownForDefaultSearch(const base::Time& prompt_time); + void RecordPromptShownForStartupUrls(const base::Time& prompt_time); + void RecordPromptShownForHomepage(const base::Time& prompt_time); + + private: + Profile* const profile_; + PrefService* const prefs_; +}; + +} // namespace safe_browsing + +#endif // CHROME_BROWSER_SAFE_BROWSING_SETTINGS_RESET_PROMPT_SETTINGS_RESET_PROMPT_PREFS_MANAGER_H_
diff --git a/chrome/browser/ui/views/settings_reset_prompt_dialog.cc b/chrome/browser/ui/views/settings_reset_prompt_dialog.cc index 8677246..efbd888 100644 --- a/chrome/browser/ui/views/settings_reset_prompt_dialog.cc +++ b/chrome/browser/ui/views/settings_reset_prompt_dialog.cc
@@ -64,6 +64,7 @@ constrained_window::CreateBrowserModalDialogViews( this, browser_view->GetNativeWindow()) ->Show(); + controller_->DialogShown(); } // WidgetDelegate overrides.
diff --git a/chrome/browser/ui/views/settings_reset_prompt_dialog_browsertest.cc b/chrome/browser/ui/views/settings_reset_prompt_dialog_browsertest.cc index ffee55c..efa57bd 100644 --- a/chrome/browser/ui/views/settings_reset_prompt_dialog_browsertest.cc +++ b/chrome/browser/ui/views/settings_reset_prompt_dialog_browsertest.cc
@@ -90,9 +90,9 @@ id, "Extension " + base::IntToString(i)))); } - ON_CALL(*this, PerformReset(_)).WillByDefault(Return()); - ON_CALL(*this, ShouldPromptForReset()).WillByDefault(Return(true)); + ON_CALL(*this, PerformReset(_)).WillByDefault(Return()); + ON_CALL(*this, DialogShown()).WillByDefault(Return()); ON_CALL(*this, homepage()).WillByDefault(Return(GURL(kHomepageUrl))); ON_CALL(*this, homepage_reset_state()) @@ -124,6 +124,7 @@ MOCK_METHOD1(PerformReset, void(const base::Closure&)); MOCK_CONST_METHOD0(ShouldPromptForReset, bool()); + MOCK_METHOD0(DialogShown, void()); MOCK_CONST_METHOD0(homepage, GURL()); MOCK_CONST_METHOD0(homepage_reset_state, ResetState()); MOCK_CONST_METHOD0(default_search, GURL());
diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc index 12c6e00..f4a36c0 100644 --- a/chrome/common/pref_names.cc +++ b/chrome/common/pref_names.cc
@@ -2487,4 +2487,29 @@ const char kHistoryPageIOSPromoDismissed[] = "history_page_ios_promo_dismissed"; #endif +// An integer that keeps track of prompt waves for the settings reset +// prompt. Users will be prompted to reset settings at most once per prompt wave +// for each setting that the prompt targets (default search, startup URLs and +// homepage). The value is obtained via a feature parameter. When the stored +// value is different from the feature parameter, a new prompt wave begins. +const char kSettingsResetPromptPromptWave[] = + "settings_reset_prompt.prompt_wave"; + +// Timestamp of the last time the settings reset prompt was shown during the +// current prompt wave asking the user if they want to restore their search +// engine. +const char kSettingsResetPromptLastTriggeredForDefaultSearch[] = + "settings_reset_prompt.last_triggered_for_default_search"; + +// Timestamp of the last time the settings reset prompt was shown during the +// current prompt wave asking the user if they want to restore their startup +// settings. +const char kSettingsResetPromptLastTriggeredForStartupUrls[] = + "settings_reset_prompt.last_triggered_for_startup_urls"; + +// Timestamp of the last time the settings reset prompt was shown during the +// current prompt wave asking the user if they want to restore their homepage. +const char kSettingsResetPromptLastTriggeredForHomepage[] = + "settings_reset_prompt.last_triggered_for_homepage"; + } // namespace prefs
diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h index 32c8dbc..af128bc 100644 --- a/chrome/common/pref_names.h +++ b/chrome/common/pref_names.h
@@ -904,6 +904,10 @@ extern const char kHistoryPageIOSPromoDismissed[]; #endif +extern const char kSettingsResetPromptPromptWave[]; +extern const char kSettingsResetPromptLastTriggeredForDefaultSearch[]; +extern const char kSettingsResetPromptLastTriggeredForStartupUrls[]; +extern const char kSettingsResetPromptLastTriggeredForHomepage[]; } // namespace prefs #endif // CHROME_COMMON_PREF_NAMES_H_
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index 2689855..2fd34e49 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn
@@ -2089,7 +2089,7 @@ "../browser/extensions/extension_function_test_utils.h", "../browser/extensions/updater/extension_cache_fake.cc", "../browser/extensions/updater/extension_cache_fake.h", - "../browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model_browsertest.cc", + "../browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_model_browsertest_win.cc", "../browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_test_utils.cc", "../browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_test_utils.h", ]
diff --git a/chromeos/BUILD.gn b/chromeos/BUILD.gn index 0fc589e9..9d4883c 100644 --- a/chromeos/BUILD.gn +++ b/chromeos/BUILD.gn
@@ -137,7 +137,6 @@ ":chromeos", ":cryptohome_proto", ":power_manager_proto", - "//components/policy/proto", "//crypto", ] sources = [
diff --git a/chromeos/dbus/fake_session_manager_client.cc b/chromeos/dbus/fake_session_manager_client.cc index ad189c9..d0355fc 100644 --- a/chromeos/dbus/fake_session_manager_client.cc +++ b/chromeos/dbus/fake_session_manager_client.cc
@@ -5,44 +5,14 @@ #include "chromeos/dbus/fake_session_manager_client.h" #include "base/bind.h" -#include "base/files/file_path.h" -#include "base/files/file_util.h" #include "base/location.h" -#include "base/numerics/safe_conversions.h" -#include "base/path_service.h" #include "base/single_thread_task_runner.h" #include "base/strings/string_util.h" #include "base/threading/thread_task_runner_handle.h" -#include "chromeos/chromeos_paths.h" #include "chromeos/dbus/cryptohome_client.h" -#include "components/policy/proto/device_management_backend.pb.h" namespace chromeos { -namespace { - -// Store the owner key in a file on the disk, so that it can be loaded by -// DeviceSettingsService and used e.g. for validating policy signatures in the -// integration tests. This is done on behalf of the real session manager, that -// would be managing the owner key file on Chrome OS. -bool StoreOwnerKey(const std::string& public_key) { - base::FilePath owner_key_path; - DCHECK(base::PathService::Get(FILE_OWNER_KEY, &owner_key_path)); - if (!base::CreateDirectory(owner_key_path.DirName())) { - LOG(ERROR) << "Failed to create the directory for the owner key file"; - return false; - } - if (base::WriteFile(owner_key_path, public_key.c_str(), - public_key.length()) != - base::checked_cast<int>(public_key.length())) { - LOG(ERROR) << "Failed to store the owner key file"; - return false; - } - return true; -} - -} // namespace - FakeSessionManagerClient::FakeSessionManagerClient() : start_device_wipe_call_count_(0), request_lock_screen_call_count_(0), @@ -160,27 +130,11 @@ void FakeSessionManagerClient::StoreDevicePolicy( const std::string& policy_blob, const StorePolicyCallback& callback) { - enterprise_management::PolicyFetchResponse policy; - if (!policy.ParseFromString(policy_blob)) { - LOG(ERROR) << "Unable to parse policy protobuf"; - base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::Bind(callback, false /* success */)); - return; - } - - bool owner_key_store_success = false; - if (policy.has_new_public_key()) - owner_key_store_success = StoreOwnerKey(policy.new_public_key()); device_policy_ = policy_blob; - - base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::Bind(callback, true /* success */)); - if (policy.has_new_public_key()) { - for (auto& observer : observers_) - observer.OwnerKeySet(owner_key_store_success); - } + base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, + base::Bind(callback, true)); for (auto& observer : observers_) - observer.PropertyChangeComplete(true /* success */); + observer.PropertyChangeComplete(true); } void FakeSessionManagerClient::StorePolicyForUser(
diff --git a/components/history/core/browser/download_database.cc b/components/history/core/browser/download_database.cc index e82f394..9b7d4f7 100644 --- a/components/history/core/browser/download_database.cc +++ b/components/history/core/browser/download_database.cc
@@ -273,6 +273,10 @@ return EnsureColumnExists("site_url", "VARCHAR NOT NULL DEFAULT ''"); } +bool DownloadDatabase::MigrateDownloadLastAccessTime() { + return EnsureColumnExists("last_access_time", "INTEGER NOT NULL DEFAULT 0"); +} + bool DownloadDatabase::InitDownloadTable() { const char kSchema[] = "CREATE TABLE downloads (" @@ -290,6 +294,7 @@ "end_time INTEGER NOT NULL," // When the download completed. "opened INTEGER NOT NULL," // 1 if it has ever been opened // else 0 + "last_access_time INTEGER NOT NULL," // The last time it was accessed. "referrer VARCHAR NOT NULL," // HTTP Referrer "site_url VARCHAR NOT NULL," // Site URL for initiating site // instance. @@ -380,9 +385,10 @@ SQL_FROM_HERE, "SELECT id, guid, current_path, target_path, mime_type, " "original_mime_type, start_time, received_bytes, total_bytes, state, " - "danger_type, interrupt_reason, hash, end_time, opened, referrer, " - "site_url, tab_url, tab_referrer_url, http_method, by_ext_id, " - "by_ext_name, etag, last_modified FROM downloads ORDER BY start_time")); + "danger_type, interrupt_reason, hash, end_time, opened, " + "last_access_time, referrer, site_url, tab_url, tab_referrer_url, " + "http_method, by_ext_id, by_ext_name, etag, last_modified " + "FROM downloads ORDER BY start_time")); while (statement_main.Step()) { std::unique_ptr<DownloadRow> info(new DownloadRow()); @@ -414,6 +420,8 @@ info->end_time = base::Time::FromInternalValue(statement_main.ColumnInt64(column++)); info->opened = statement_main.ColumnInt(column++) != 0; + info->last_access_time = + base::Time::FromInternalValue(statement_main.ColumnInt64(column++)); info->referrer_url = GURL(statement_main.ColumnString(column++)); info->site_url = GURL(statement_main.ColumnString(column++)); info->tab_url = GURL(statement_main.ColumnString(column++)); @@ -531,8 +539,8 @@ "mime_type=?, original_mime_type=?, " "received_bytes=?, state=?, " "danger_type=?, interrupt_reason=?, hash=?, end_time=?, total_bytes=?, " - "opened=?, by_ext_id=?, by_ext_name=?, etag=?, last_modified=? " - "WHERE id=?")); + "opened=?, last_access_time=?, by_ext_id=?, by_ext_name=?, etag=?, " + "last_modified=? WHERE id=?")); int column = 0; BindFilePath(statement, data.current_path, column++); BindFilePath(statement, data.target_path, column++); @@ -547,6 +555,7 @@ statement.BindInt64(column++, data.end_time.ToInternalValue()); statement.BindInt64(column++, data.total_bytes); statement.BindInt(column++, (data.opened ? 1 : 0)); + statement.BindInt64(column++, data.last_access_time.ToInternalValue()); statement.BindString(column++, data.by_ext_id); statement.BindString(column++, data.by_ext_name); statement.BindString(column++, data.etag); @@ -604,12 +613,12 @@ "INSERT INTO downloads " "(id, guid, current_path, target_path, mime_type, original_mime_type, " " start_time, received_bytes, total_bytes, state, danger_type, " - " interrupt_reason, hash, end_time, opened, referrer, " - " site_url, tab_url, tab_referrer_url, http_method, " + " interrupt_reason, hash, end_time, opened, last_access_time, " + " referrer, site_url, tab_url, tab_referrer_url, http_method, " " by_ext_id, by_ext_name, etag, last_modified) " "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, " " ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, " - " ?, ?, ?, ?)")); + " ?, ?, ?, ?, ?)")); int column = 0; statement_insert.BindInt(column++, DownloadIdToInt(info.id)); @@ -629,6 +638,8 @@ statement_insert.BindBlob(column++, info.hash.data(), info.hash.size()); statement_insert.BindInt64(column++, info.end_time.ToInternalValue()); statement_insert.BindInt(column++, info.opened ? 1 : 0); + statement_insert.BindInt64(column++, + info.last_access_time.ToInternalValue()); statement_insert.BindString(column++, info.referrer_url.spec()); statement_insert.BindString(column++, info.site_url.spec()); statement_insert.BindString(column++, info.tab_url.spec());
diff --git a/components/history/core/browser/download_database.h b/components/history/core/browser/download_database.h index 931ad5c..421218d9 100644 --- a/components/history/core/browser/download_database.h +++ b/components/history/core/browser/download_database.h
@@ -93,6 +93,9 @@ // table. bool MigrateDownloadSiteInstanceUrl(); + // Returns true if able to add last_access_time column to the download table. + bool MigrateDownloadLastAccessTime(); + // Creates the downloads table if needed. bool InitDownloadTable();
diff --git a/components/history/core/browser/download_row.cc b/components/history/core/browser/download_row.cc index a377766a..fac2b5ca 100644 --- a/components/history/core/browser/download_row.cc +++ b/components/history/core/browser/download_row.cc
@@ -31,8 +31,8 @@ const std::string& http_method, const std::string& mime_type, const std::string& original_mime_type, - const base::Time& start, - const base::Time& end, + base::Time start, + base::Time end, const std::string& etag, const std::string& last_modified, int64_t received, @@ -44,6 +44,7 @@ DownloadId id, const std::string& guid, bool download_opened, + base::Time last_access, const std::string& ext_id, const std::string& ext_name, const std::vector<DownloadSliceInfo>& download_slice_info) @@ -70,6 +71,7 @@ id(id), guid(guid), opened(download_opened), + last_access_time(last_access), by_ext_id(ext_id), by_ext_name(ext_name), download_slice_info(download_slice_info) {} @@ -92,6 +94,7 @@ danger_type == rhs.danger_type && interrupt_reason == rhs.interrupt_reason && hash == rhs.hash && id == rhs.id && guid == rhs.guid && opened == rhs.opened && + last_access_time == rhs.last_access_time && by_ext_id == rhs.by_ext_id && by_ext_name == rhs.by_ext_name && download_slice_info == rhs.download_slice_info; }
diff --git a/components/history/core/browser/download_row.h b/components/history/core/browser/download_row.h index 1c47635..cb0f0cc8 100644 --- a/components/history/core/browser/download_row.h +++ b/components/history/core/browser/download_row.h
@@ -33,8 +33,8 @@ const std::string& http_method, const std::string& mime_type, const std::string& original_mime_type, - const base::Time& start, - const base::Time& end, + base::Time start, + base::Time end, const std::string& etag, const std::string& last_modified, int64_t received, @@ -46,6 +46,7 @@ DownloadId id, const std::string& guid, bool download_opened, + base::Time last_access, const std::string& ext_id, const std::string& ext_name, const std::vector<DownloadSliceInfo>& download_slice_info); @@ -134,6 +135,9 @@ // Whether this download has ever been opened from the browser. bool opened; + // The time when the download was last accessed. + base::Time last_access_time; + // The id and name of the extension that created this download. std::string by_ext_id; std::string by_ext_name;
diff --git a/components/history/core/browser/history_backend_db_unittest.cc b/components/history/core/browser/history_backend_db_unittest.cc index 2fbd5d02..1b210b2b 100644 --- a/components/history/core/browser/history_backend_db_unittest.cc +++ b/components/history/core/browser/history_backend_db_unittest.cc
@@ -777,6 +777,7 @@ base::Time start_time(base::Time::Now()); base::Time end_time(start_time + base::TimeDelta::FromHours(1)); + base::Time last_access_time; DownloadRow download_A( base::FilePath(FILE_PATH_LITERAL("/path/1")), @@ -787,14 +788,15 @@ "original/mime-type", start_time, end_time, "etag1", "last_modified_1", 100, 1000, DownloadState::INTERRUPTED, DownloadDangerType::NOT_DANGEROUS, kTestDownloadInterruptReasonCrash, "hash-value1", 1, - "FE672168-26EF-4275-A149-FEC25F6A75F9", false, "extension-id", - "extension-name", std::vector<DownloadSliceInfo>()); + "FE672168-26EF-4275-A149-FEC25F6A75F9", false, last_access_time, + "extension-id", "extension-name", std::vector<DownloadSliceInfo>()); ASSERT_TRUE(db_->CreateDownload(download_A)); url_chain.push_back(GURL("http://example.com/d")); base::Time start_time2(start_time + base::TimeDelta::FromHours(10)); base::Time end_time2(end_time + base::TimeDelta::FromHours(10)); + base::Time last_access_time2(start_time2 + base::TimeDelta::FromHours(5)); DownloadRow download_B( base::FilePath(FILE_PATH_LITERAL("/path/3")), @@ -805,8 +807,8 @@ "original/mime-type2", start_time2, end_time2, "etag2", "last_modified_2", 1001, 1001, DownloadState::COMPLETE, DownloadDangerType::DANGEROUS_FILE, kTestDownloadInterruptReasonNone, std::string(), 2, - "b70f3869-7d75-4878-acb4-4caf7026d12b", false, "extension-id", - "extension-name", std::vector<DownloadSliceInfo>()); + "b70f3869-7d75-4878-acb4-4caf7026d12b", false, last_access_time2, + "extension-id", "extension-name", std::vector<DownloadSliceInfo>()); ASSERT_TRUE(db_->CreateDownload(download_B)); EXPECT_EQ(2u, db_->CountDownloads()); @@ -835,6 +837,7 @@ base::Time start_time(base::Time::Now()); base::Time end_time(start_time + base::TimeDelta::FromHours(1)); + base::Time last_access_time(start_time + base::TimeDelta::FromHours(5)); DownloadRow download( base::FilePath(FILE_PATH_LITERAL("/path/1")), @@ -845,7 +848,8 @@ "original/mime-type", start_time, end_time, "etag1", "last_modified_1", 100, 1000, DownloadState::INTERRUPTED, DownloadDangerType::NOT_DANGEROUS, 3, "some-hash-value", 1, "FE672168-26EF-4275-A149-FEC25F6A75F9", false, - "extension-id", "extension-name", std::vector<DownloadSliceInfo>()); + last_access_time, "extension-id", "extension-name", + std::vector<DownloadSliceInfo>()); db_->CreateDownload(download); download.current_path = @@ -960,8 +964,8 @@ "application/octet-stream", now, now, std::string(), std::string(), 0, 512, DownloadState::COMPLETE, DownloadDangerType::NOT_DANGEROUS, kTestDownloadInterruptReasonNone, std::string(), 1, - "05AF6C8E-E4E0-45D7-B5CE-BC99F7019918", 0, "by_ext_id", "by_ext_name", - std::vector<DownloadSliceInfo>()); + "05AF6C8E-E4E0-45D7-B5CE-BC99F7019918", 0, now, "by_ext_id", + "by_ext_name", std::vector<DownloadSliceInfo>()); // Creating records without any urls should fail. EXPECT_FALSE(db_->CreateDownload(download)); @@ -1079,6 +1083,7 @@ slice_info.push_back(DownloadSliceInfo(id, 500, received)); base::Time start_time(base::Time::Now()); base::Time end_time(start_time + base::TimeDelta::FromHours(1)); + base::Time last_access_time(start_time + base::TimeDelta::FromHours(5)); DownloadRow download( base::FilePath(FILE_PATH_LITERAL("/path/1")), @@ -1089,8 +1094,8 @@ "original/mime-type", start_time, end_time, "etag1", "last_modified_1", received, 1500, DownloadState::INTERRUPTED, DownloadDangerType::NOT_DANGEROUS, kTestDownloadInterruptReasonCrash, - "hash-value1", id, "FE672168-26EF-4275-A149-FEC25F6A75F9", - false, "extension-id", "extension-name", slice_info); + "hash-value1", id, "FE672168-26EF-4275-A149-FEC25F6A75F9", false, + last_access_time, "extension-id", "extension-name", slice_info); ASSERT_TRUE(db_->CreateDownload(download)); std::vector<DownloadRow> results; db_->QueryDownloads(&results); @@ -1115,17 +1120,18 @@ DownloadId id = 1; base::Time start_time(base::Time::Now()); base::Time end_time(start_time + base::TimeDelta::FromHours(1)); + base::Time last_access_time(start_time + base::TimeDelta::FromHours(5)); DownloadRow download( base::FilePath(FILE_PATH_LITERAL("/path/1")), base::FilePath(FILE_PATH_LITERAL("/path/2")), url_chain, GURL("http://example.com/referrer"), GURL("http://example.com"), GURL("http://example.com/tab-url"), GURL("http://example.com/tab-referrer"), "GET", "mime/type", - "original/mime-type", start_time, end_time, "etag1", "last_modified_1", - 0, 1500, DownloadState::INTERRUPTED, DownloadDangerType::NOT_DANGEROUS, + "original/mime-type", start_time, end_time, "etag1", "last_modified_1", 0, + 1500, DownloadState::INTERRUPTED, DownloadDangerType::NOT_DANGEROUS, kTestDownloadInterruptReasonCrash, "hash-value1", id, - "FE672168-26EF-4275-A149-FEC25F6A75F9", false, "extension-id", - "extension-name", std::vector<DownloadSliceInfo>()); + "FE672168-26EF-4275-A149-FEC25F6A75F9", false, last_access_time, + "extension-id", "extension-name", std::vector<DownloadSliceInfo>()); ASSERT_TRUE(db_->CreateDownload(download)); // Add a new slice and call UpdateDownload(). @@ -1153,6 +1159,7 @@ slice_info.push_back(DownloadSliceInfo(id, 1500, 0)); base::Time start_time(base::Time::Now()); base::Time end_time(start_time + base::TimeDelta::FromHours(1)); + base::Time last_access_time(start_time + base::TimeDelta::FromHours(5)); DownloadRow download( base::FilePath(FILE_PATH_LITERAL("/path/1")), @@ -1163,8 +1170,8 @@ "original/mime-type", start_time, end_time, "etag1", "last_modified_1", received, 1500, DownloadState::INTERRUPTED, DownloadDangerType::NOT_DANGEROUS, kTestDownloadInterruptReasonCrash, - "hash-value1", id, "FE672168-26EF-4275-A149-FEC25F6A75F9", - false, "extension-id", "extension-name", slice_info); + "hash-value1", id, "FE672168-26EF-4275-A149-FEC25F6A75F9", false, + last_access_time, "extension-id", "extension-name", slice_info); ASSERT_TRUE(db_->CreateDownload(download)); std::vector<DownloadRow> results; db_->QueryDownloads(&results);
diff --git a/components/history/core/browser/history_database.cc b/components/history/core/browser/history_database.cc index 2768a8b..9884339 100644 --- a/components/history/core/browser/history_database.cc +++ b/components/history/core/browser/history_database.cc
@@ -36,7 +36,7 @@ // Current version number. We write databases at the "current" version number, // but any previous version that can read the "compatible" one can make do with // our database without *too* many bad effects. -const int kCurrentVersionNumber = 33; +const int kCurrentVersionNumber = 34; const int kCompatibleVersionNumber = 16; const char kEarlyExpirationThresholdKey[] = "early_expiration_threshold"; const int kMaxHostsInMemory = 10000; @@ -542,6 +542,15 @@ meta_table_.SetVersionNumber(cur_version); } + if (cur_version == 33) { + if (!MigrateDownloadLastAccessTime()) { + LOG(WARNING) << "Unable to migrate to version 34"; + return sql::INIT_FAILURE; + } + cur_version++; + meta_table_.SetVersionNumber(cur_version); + } + // When the version is too old, we just try to continue anyway, there should // not be a released product that makes a database too old for us to handle. LOG_IF(WARNING, cur_version < GetCurrentVersion()) <<
diff --git a/components/history/core/test/history_backend_db_base_test.cc b/components/history/core/test/history_backend_db_base_test.cc index 63e12c1..8feac3a 100644 --- a/components/history/core/test/history_backend_db_base_test.cc +++ b/components/history/core/test/history_backend_db_base_test.cc
@@ -135,7 +135,7 @@ "application/vnd.oasis.opendocument.text", "application/octet-stream", time, time, std::string(), std::string(), 0, 512, state, DownloadDangerType::NOT_DANGEROUS, kTestDownloadInterruptReasonNone, - std::string(), id, guid, false, "by_ext_id", "by_ext_name", + std::string(), id, guid, false, time, "by_ext_id", "by_ext_name", std::vector<DownloadSliceInfo>()); return db_->CreateDownload(download); }
diff --git a/components/omnibox/browser/zero_suggest_provider.cc b/components/omnibox/browser/zero_suggest_provider.cc index 9c33a77d..1a5d022 100644 --- a/components/omnibox/browser/zero_suggest_provider.cc +++ b/components/omnibox/browser/zero_suggest_provider.cc
@@ -19,7 +19,6 @@ #include "components/data_use_measurement/core/data_use_user_data.h" #include "components/history/core/browser/history_types.h" #include "components/history/core/browser/top_sites.h" -#include "components/metrics/proto/omnibox_event.pb.h" #include "components/metrics/proto/omnibox_input_type.pb.h" #include "components/omnibox/browser/autocomplete_classifier.h" #include "components/omnibox/browser/autocomplete_input.h" @@ -44,30 +43,19 @@ namespace { -// Represents whether ZeroSuggestProvider is allowed to display contextual -// suggestions on focus, and if not, why not. -// These values are written to logs. New enum values can be added, but existing -// enums must never be renumbered or deleted and reused. -enum class ZeroSuggestEligibility { - ELIGIBLE = 0, - // URL_INELIGIBLE would be ELIGIBLE except some property of the current URL - // itself prevents ZeroSuggest from triggering. - URL_INELIGIBLE = 1, - GENERALLY_INELIGIBLE = 2, - ELIGIBLE_MAX_VALUE -}; - // TODO(hfung): The histogram code was copied and modified from // search_provider.cc. Refactor and consolidate the code. // We keep track in a histogram how many suggest requests we send, how // many suggest requests we invalidate (e.g., due to a user typing // another character), and how many replies we receive. -// These values are written to logs. New enum values can be added, but existing -// enums must never be renumbered or deleted and reused. +// *** ADD NEW ENUMS AFTER ALL PREVIOUSLY DEFINED ONES! *** +// (excluding the end-of-list enum value) +// We do not want values of existing enums to change or else it screws +// up the statistics. enum ZeroSuggestRequestsHistogramValue { ZERO_SUGGEST_REQUEST_SENT = 1, - ZERO_SUGGEST_REQUEST_INVALIDATED = 2, - ZERO_SUGGEST_REPLY_RECEIVED = 3, + ZERO_SUGGEST_REQUEST_INVALIDATED, + ZERO_SUGGEST_REPLY_RECEIVED, ZERO_SUGGEST_MAX_REQUEST_HISTOGRAM_VALUE }; @@ -80,9 +68,6 @@ // Relevance value to use if it was not set explicitly by the server. const int kDefaultZeroSuggestRelevance = 100; -// Used for testing whether zero suggest is ever available. -std::string kArbitraryInsecureUrlString = "http://www.google.com/"; - } // namespace // static @@ -116,37 +101,32 @@ current_query_ = input.current_url().spec(); current_page_classification_ = input.current_page_classification(); current_url_match_ = MatchForCurrentURL(); + TemplateURLService* template_url_service = client()->GetTemplateURLService(); - std::string url_string = GetContextualSuggestionsUrl(); + const TemplateURL* default_provider = + template_url_service->GetDefaultSearchProvider(); + if (default_provider == NULL) + return; + + base::string16 prefix; + TemplateURLRef::SearchTermsArgs search_term_args(prefix); + std::string url_string; + if (OmniboxFieldTrial::InZeroSuggestRedirectToChromeFieldTrial()) { + url_string = OmniboxFieldTrial::ZeroSuggestRedirectToChromeServerAddress(); + } else { + url_string = default_provider->suggestions_url_ref().ReplaceSearchTerms( + search_term_args, template_url_service->search_terms_data()); + } GURL suggest_url(url_string); + if (!suggest_url.is_valid()) return; // No need to send the current page URL in personalized suggest or // most visited field trials. - const TemplateURLService* template_url_service = - client()->GetTemplateURLService(); - const TemplateURL* default_provider = - template_url_service->GetDefaultSearchProvider(); - const bool can_send_current_url = - CanSendURL(input.current_url(), suggest_url, default_provider, + if (CanSendURL(input.current_url(), suggest_url, default_provider, current_page_classification_, - template_url_service->search_terms_data(), client()); - GURL arbitrary_insecure_url(kArbitraryInsecureUrlString); - ZeroSuggestEligibility eligibility = ZeroSuggestEligibility::ELIGIBLE; - if (!can_send_current_url) { - const bool can_send_ordinary_url = - CanSendURL(arbitrary_insecure_url, suggest_url, default_provider, - current_page_classification_, - template_url_service->search_terms_data(), client()); - eligibility = can_send_ordinary_url - ? ZeroSuggestEligibility::URL_INELIGIBLE - : ZeroSuggestEligibility::GENERALLY_INELIGIBLE; - } - UMA_HISTOGRAM_ENUMERATION( - "Omnibox.ZeroSuggest.Eligible.OnFocus", static_cast<int>(eligibility), - static_cast<int>(ZeroSuggestEligibility::ELIGIBLE_MAX_VALUE)); - if (can_send_current_url && + template_url_service->search_terms_data(), client()) && !OmniboxFieldTrial::InZeroSuggestPersonalizedFieldTrial() && !OmniboxFieldTrial::InZeroSuggestMostVisitedFieldTrial()) { // Update suggest_url to include the current_page_url. @@ -156,8 +136,6 @@ OmniboxFieldTrial::ZeroSuggestRedirectToChromeAdditionalFields(); suggest_url = GURL(url_string); } else { - base::string16 prefix; - TemplateURLRef::SearchTermsArgs search_term_args(prefix); search_term_args.current_page_url = current_query_; suggest_url = GURL(default_provider->suggestions_url_ref().ReplaceSearchTerms( @@ -231,23 +209,6 @@ results_from_cache_(false), waiting_for_most_visited_urls_request_(false), weak_ptr_factory_(this) { - // Record whether contextual zero suggest is possible for this user / profile. - const TemplateURLService* template_url_service = - client->GetTemplateURLService(); - // Template URL service can be null in tests. - if (template_url_service != nullptr) { - GURL suggest_url(GetContextualSuggestionsUrl()); - // To check whether this is allowed, use an arbitrary insecure (http) URL - // as the URL we'd want suggestions for. The value of OTHER as the current - // page classification is to correspond with that URL. - UMA_HISTOGRAM_BOOLEAN( - "Omnibox.ZeroSuggest.Eligible.OnProfileOpen", - suggest_url.is_valid() && - CanSendURL(GURL(kArbitraryInsecureUrlString), suggest_url, - template_url_service->GetDefaultSearchProvider(), - metrics::OmniboxEventProto::OTHER, - template_url_service->search_terms_data(), client)); - } } ZeroSuggestProvider::~ZeroSuggestProvider() { @@ -518,24 +479,6 @@ return true; } -std::string ZeroSuggestProvider::GetContextualSuggestionsUrl() const { - // Without a default search provider, refuse to do anything (even if the user - // is in the redirect-to-chrome field trial). - const TemplateURLService* template_url_service = - client()->GetTemplateURLService(); - const TemplateURL* default_provider = - template_url_service->GetDefaultSearchProvider(); - if (default_provider == nullptr) - return std::string(); - - if (OmniboxFieldTrial::InZeroSuggestRedirectToChromeFieldTrial()) - return OmniboxFieldTrial::ZeroSuggestRedirectToChromeServerAddress(); - base::string16 prefix; - TemplateURLRef::SearchTermsArgs search_term_args(prefix); - return default_provider->suggestions_url_ref().ReplaceSearchTerms( - search_term_args, template_url_service->search_terms_data()); -} - void ZeroSuggestProvider::MaybeUseCachedSuggestions() { if (!OmniboxFieldTrial::InZeroSuggestPersonalizedFieldTrial()) return;
diff --git a/components/omnibox/browser/zero_suggest_provider.h b/components/omnibox/browser/zero_suggest_provider.h index b682e7b7..d4cf416 100644 --- a/components/omnibox/browser/zero_suggest_provider.h +++ b/components/omnibox/browser/zero_suggest_provider.h
@@ -122,12 +122,6 @@ bool ShouldShowNonContextualZeroSuggest(const GURL& suggest_url, const GURL& current_page_url) const; - // Returns a URL string that should be used to to request contextual - // suggestions from the default provider. Does not take into account whether - // sending this request is prohibited (e.g., in an incognito window). Returns - // an empty string in case of an error. - std::string GetContextualSuggestionsUrl() const; - // Checks whether we have a set of zero suggest results cached, and if so // populates |matches_| with cached results. void MaybeUseCachedSuggestions();
diff --git a/content/browser/download/download_browsertest.cc b/content/browser/download/download_browsertest.cc index d842e1e..5612c82 100644 --- a/content/browser/download/download_browsertest.cc +++ b/content/browser/download/download_browsertest.cc
@@ -1847,7 +1847,7 @@ base::Time(), parameters.etag, std::string(), kIntermediateSize, parameters.size, std::string(), DownloadItem::INTERRUPTED, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, false, + DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, false, base::Time(), std::vector<DownloadItem::ReceivedSlice>()); download->Resume(); @@ -1910,7 +1910,7 @@ base::Time(), parameters.etag, std::string(), kIntermediateSize, parameters.size, std::string(), DownloadItem::INTERRUPTED, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, false, + DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, false, base::Time(), std::vector<DownloadItem::ReceivedSlice>()); download->Resume(); @@ -1960,7 +1960,7 @@ base::Time(), "fake-etag", std::string(), kIntermediateSize, parameters.size, std::string(), DownloadItem::INTERRUPTED, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, false, + DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, false, base::Time(), std::vector<DownloadItem::ReceivedSlice>()); download->Resume(); @@ -2016,7 +2016,7 @@ parameters.size, std::string(std::begin(kPartialHash), std::end(kPartialHash)), DownloadItem::INTERRUPTED, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, false, + DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, false, base::Time(), std::vector<DownloadItem::ReceivedSlice>()); download->Resume(); @@ -2078,7 +2078,7 @@ parameters.size, std::string(std::begin(kPartialHash), std::end(kPartialHash)), DownloadItem::INTERRUPTED, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, false, + DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, false, base::Time(), std::vector<DownloadItem::ReceivedSlice>()); download->Resume(); @@ -2149,7 +2149,7 @@ base::Time(), parameters.etag, std::string(), kIntermediateSize, parameters.size, std::string(), DownloadItem::INTERRUPTED, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, false, + DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, false, base::Time(), std::vector<DownloadItem::ReceivedSlice>()); download->Resume(); @@ -2218,7 +2218,7 @@ base::Time(), parameters.etag, std::string(), kIntermediateSize, parameters.size, std::string(), DownloadItem::INTERRUPTED, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, false, + DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, false, base::Time(), std::vector<DownloadItem::ReceivedSlice>()); download->Resume();
diff --git a/content/browser/download/download_item_factory.h b/content/browser/download/download_item_factory.h index dc6e9a9..9242c238 100644 --- a/content/browser/download/download_item_factory.h +++ b/content/browser/download/download_item_factory.h
@@ -52,8 +52,8 @@ const GURL& tab_refererr_url, const std::string& mime_type, const std::string& original_mime_type, - const base::Time& start_time, - const base::Time& end_time, + base::Time start_time, + base::Time end_time, const std::string& etag, const std::string& last_modified, int64_t received_bytes, @@ -63,6 +63,7 @@ DownloadDangerType danger_type, DownloadInterruptReason interrupt_reason, bool opened, + base::Time last_access_time, const std::vector<DownloadItem::ReceivedSlice>& received_slices, const net::NetLogWithSource& net_log) = 0;
diff --git a/content/browser/download/download_item_impl.cc b/content/browser/download/download_item_impl.cc index 95e5aac..e7c9873 100644 --- a/content/browser/download/download_item_impl.cc +++ b/content/browser/download/download_item_impl.cc
@@ -137,8 +137,8 @@ const GURL& tab_refererr_url, const std::string& mime_type, const std::string& original_mime_type, - const base::Time& start_time, - const base::Time& end_time, + base::Time start_time, + base::Time end_time, const std::string& etag, const std::string& last_modified, int64_t received_bytes, @@ -148,6 +148,7 @@ DownloadDangerType danger_type, DownloadInterruptReason interrupt_reason, bool opened, + base::Time last_access_time, const std::vector<DownloadItem::ReceivedSlice>& received_slices, const net::NetLogWithSource& net_log) : guid_(base::ToUpperASCII(guid)), @@ -169,6 +170,7 @@ end_time_(end_time), delegate_(delegate), opened_(opened), + last_access_time_(last_access_time), current_path_(current_path), received_bytes_(received_bytes), all_data_saved_(state == COMPLETE), @@ -461,6 +463,7 @@ delegate_->CheckForFileRemoval(this); RecordOpen(GetEndTime(), !GetOpened()); opened_ = true; + last_access_time_ = base::Time::Now(); for (auto& observer : observers_) observer.OnDownloadOpened(this); delegate_->OpenDownload(this); @@ -774,6 +777,10 @@ return opened_; } +base::Time DownloadItemImpl::GetLastAccessTime() const { + return last_access_time_; +} + BrowserContext* DownloadItemImpl::GetBrowserContext() const { return delegate_->GetBrowserContext(); } @@ -813,6 +820,11 @@ opened_ = opened; } +void DownloadItemImpl::SetLastAccessTime(base::Time last_access_time) { + last_access_time_ = last_access_time; + UpdateObservers(); +} + void DownloadItemImpl::SetDisplayName(const base::FilePath& name) { display_name_ = name; }
diff --git a/content/browser/download/download_item_impl.h b/content/browser/download/download_item_impl.h index a444c6f..86f9f432 100644 --- a/content/browser/download/download_item_impl.h +++ b/content/browser/download/download_item_impl.h
@@ -65,8 +65,8 @@ const GURL& tab_referrer_url, const std::string& mime_type, const std::string& original_mime_type, - const base::Time& start_time, - const base::Time& end_time, + base::Time start_time, + base::Time end_time, const std::string& etag, const std::string& last_modified, int64_t received_bytes, @@ -76,6 +76,7 @@ DownloadDangerType danger_type, DownloadInterruptReason interrupt_reason, bool opened, + base::Time last_access_time, const std::vector<DownloadItem::ReceivedSlice>& received_slices, const net::NetLogWithSource& net_log); @@ -163,11 +164,13 @@ bool GetOpenWhenComplete() const override; bool GetAutoOpened() override; bool GetOpened() const override; + base::Time GetLastAccessTime() const override; BrowserContext* GetBrowserContext() const override; WebContents* GetWebContents() const override; void OnContentCheckCompleted(DownloadDangerType danger_type) override; void SetOpenWhenComplete(bool open) override; void SetOpened(bool opened) override; + void SetLastAccessTime(base::Time last_access_time) override; void SetDisplayName(const base::FilePath& name) override; std::string DebugString(bool verbose) const override; @@ -623,6 +626,9 @@ // be treated as though the user opened it. bool opened_ = false; + // Time when the download was last accessed. + base::Time last_access_time_; + // Did the delegate delay calling Complete on this download? bool delegate_delayed_complete_ = false;
diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc index ae778d2..8cb2b11 100644 --- a/content/browser/download/download_manager_impl.cc +++ b/content/browser/download/download_manager_impl.cc
@@ -130,8 +130,8 @@ const GURL& tab_refererr_url, const std::string& mime_type, const std::string& original_mime_type, - const base::Time& start_time, - const base::Time& end_time, + base::Time start_time, + base::Time end_time, const std::string& etag, const std::string& last_modified, int64_t received_bytes, @@ -141,6 +141,7 @@ DownloadDangerType danger_type, DownloadInterruptReason interrupt_reason, bool opened, + base::Time last_access_time, const std::vector<DownloadItem::ReceivedSlice>& received_slices, const net::NetLogWithSource& net_log) override { return new DownloadItemImpl( @@ -148,7 +149,7 @@ referrer_url, site_url, tab_url, tab_refererr_url, mime_type, original_mime_type, start_time, end_time, etag, last_modified, received_bytes, total_bytes, hash, state, danger_type, interrupt_reason, - opened, received_slices, net_log); + opened, last_access_time, received_slices, net_log); } DownloadItemImpl* CreateActiveItem( @@ -639,8 +640,8 @@ const GURL& tab_refererr_url, const std::string& mime_type, const std::string& original_mime_type, - const base::Time& start_time, - const base::Time& end_time, + base::Time start_time, + base::Time end_time, const std::string& etag, const std::string& last_modified, int64_t received_bytes, @@ -650,6 +651,7 @@ DownloadDangerType danger_type, DownloadInterruptReason interrupt_reason, bool opened, + base::Time last_access_time, const std::vector<DownloadItem::ReceivedSlice>& received_slices) { if (base::ContainsKey(downloads_, id)) { NOTREACHED(); @@ -660,7 +662,8 @@ this, guid, id, current_path, target_path, url_chain, referrer_url, site_url, tab_url, tab_refererr_url, mime_type, original_mime_type, start_time, end_time, etag, last_modified, received_bytes, total_bytes, - hash, state, danger_type, interrupt_reason, opened, received_slices, + hash, state, danger_type, interrupt_reason, opened, last_access_time, + received_slices, net::NetLogWithSource::Make(net_log_, net::NetLogSourceType::DOWNLOAD)); downloads_[id] = base::WrapUnique(item); downloads_by_guid_[guid] = item;
diff --git a/content/browser/download/download_manager_impl.h b/content/browser/download/download_manager_impl.h index b035d90..eb8ddca2 100644 --- a/content/browser/download/download_manager_impl.h +++ b/content/browser/download/download_manager_impl.h
@@ -94,8 +94,8 @@ const GURL& tab_refererr_url, const std::string& mime_type, const std::string& original_mime_type, - const base::Time& start_time, - const base::Time& end_time, + base::Time start_time, + base::Time end_time, const std::string& etag, const std::string& last_modified, int64_t received_bytes, @@ -105,6 +105,7 @@ DownloadDangerType danger_type, DownloadInterruptReason interrupt_reason, bool opened, + base::Time last_access_time, const std::vector<DownloadItem::ReceivedSlice>& received_slices) override; int InProgressCount() const override; int NonMaliciousInProgressCount() const override;
diff --git a/content/browser/download/download_manager_impl_unittest.cc b/content/browser/download/download_manager_impl_unittest.cc index d13298f..ede7691de 100644 --- a/content/browser/download/download_manager_impl_unittest.cc +++ b/content/browser/download/download_manager_impl_unittest.cc
@@ -139,8 +139,8 @@ const GURL& tab_referrer_url, const std::string& mime_type, const std::string& original_mime_type, - const base::Time& start_time, - const base::Time& end_time, + base::Time start_time, + base::Time end_time, const std::string& etag, const std::string& last_modofied, int64_t received_bytes, @@ -150,6 +150,7 @@ DownloadDangerType danger_type, DownloadInterruptReason interrupt_reason, bool opened, + base::Time last_access_time, const std::vector<DownloadItem::ReceivedSlice>& received_slices, const net::NetLogWithSource& net_log) override; DownloadItemImpl* CreateActiveItem( @@ -212,8 +213,8 @@ const GURL& tab_referrer_url, const std::string& mime_type, const std::string& original_mime_type, - const base::Time& start_time, - const base::Time& end_time, + base::Time start_time, + base::Time end_time, const std::string& etag, const std::string& last_modified, int64_t received_bytes, @@ -223,6 +224,7 @@ DownloadDangerType danger_type, DownloadInterruptReason interrupt_reason, bool opened, + base::Time last_access_time, const std::vector<DownloadItem::ReceivedSlice>& received_slices, const net::NetLogWithSource& net_log) { DCHECK(items_.find(download_id) == items_.end()); @@ -607,7 +609,7 @@ "application/octet-stream", "application/octet-stream", base::Time::Now(), base::Time::Now(), std::string(), std::string(), 10, 10, std::string(), DownloadItem::INTERRUPTED, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, - DOWNLOAD_INTERRUPT_REASON_SERVER_FAILED, false, + DOWNLOAD_INTERRUPT_REASON_SERVER_FAILED, false, base::Time::Now(), std::vector<DownloadItem::ReceivedSlice>()); ASSERT_TRUE(persisted_item);
diff --git a/content/browser/download/mock_download_item_impl.cc b/content/browser/download/mock_download_item_impl.cc index 92795c0..621c7f8 100644 --- a/content/browser/download/mock_download_item_impl.cc +++ b/content/browser/download/mock_download_item_impl.cc
@@ -30,6 +30,7 @@ DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, DOWNLOAD_INTERRUPT_REASON_NONE, false, + base::Time(), std::vector<DownloadItem::ReceivedSlice>(), net::NetLogWithSource()) {}
diff --git a/content/browser/download/mock_download_item_impl.h b/content/browser/download/mock_download_item_impl.h index 205d84a9..289c5ce 100644 --- a/content/browser/download/mock_download_item_impl.h +++ b/content/browser/download/mock_download_item_impl.h
@@ -101,6 +101,7 @@ MOCK_CONST_METHOD0(IsTemporary, bool()); MOCK_METHOD1(SetOpened, void(bool)); MOCK_CONST_METHOD0(GetOpened, bool()); + MOCK_CONST_METHOD0(GetLastAccessTime, base::Time()); MOCK_CONST_METHOD0(GetLastModifiedTime, const std::string&()); MOCK_CONST_METHOD0(GetETag, const std::string&()); MOCK_CONST_METHOD0(GetLastReason, DownloadInterruptReason());
diff --git a/content/public/browser/download_item.h b/content/public/browser/download_item.h index 33ce114..80e0c63c 100644 --- a/content/public/browser/download_item.h +++ b/content/public/browser/download_item.h
@@ -396,6 +396,10 @@ // Returns true if the download has been opened. virtual bool GetOpened() const = 0; + // Time the download was last accessed. Returns NULL if the download has never + // been opened. + virtual base::Time GetLastAccessTime() const = 0; + // Misc State accessors --------------------------------------------------- // BrowserContext that indirectly owns this download. Always valid. @@ -422,6 +426,9 @@ // Mark the download as having been opened (without actually opening it). virtual void SetOpened(bool opened) = 0; + // Updates the last access time of the download. + virtual void SetLastAccessTime(base::Time last_access_time) = 0; + // Set a display name for the download that will be independent of the target // filename. If |name| is not empty, then GetFileNameToReportUser() will // return |name|. Has no effect on the final target filename.
diff --git a/content/public/browser/download_manager.h b/content/public/browser/download_manager.h index 7a72e1b..477963e 100644 --- a/content/public/browser/download_manager.h +++ b/content/public/browser/download_manager.h
@@ -140,8 +140,8 @@ const GURL& tab_referrer_url, const std::string& mime_type, const std::string& original_mime_type, - const base::Time& start_time, - const base::Time& end_time, + base::Time start_time, + base::Time end_time, const std::string& etag, const std::string& last_modified, int64_t received_bytes, @@ -151,6 +151,7 @@ DownloadDangerType danger_type, DownloadInterruptReason interrupt_reason, bool opened, + base::Time last_access_time, const std::vector<DownloadItem::ReceivedSlice>& received_slices) = 0; // The number of in progress (including paused) downloads.
diff --git a/content/public/test/mock_download_item.h b/content/public/test/mock_download_item.h index d99b6a9..ee5508e 100644 --- a/content/public/test/mock_download_item.h +++ b/content/public/test/mock_download_item.h
@@ -98,11 +98,13 @@ MOCK_CONST_METHOD0(GetOpenWhenComplete, bool()); MOCK_METHOD0(GetAutoOpened, bool()); MOCK_CONST_METHOD0(GetOpened, bool()); + MOCK_CONST_METHOD0(GetLastAccessTime, base::Time()); MOCK_CONST_METHOD0(GetBrowserContext, BrowserContext*()); MOCK_CONST_METHOD0(GetWebContents, WebContents*()); MOCK_METHOD1(OnContentCheckCompleted, void(DownloadDangerType)); MOCK_METHOD1(SetOpenWhenComplete, void(bool)); MOCK_METHOD1(SetOpened, void(bool)); + MOCK_METHOD1(SetLastAccessTime, void(base::Time)); MOCK_METHOD1(SetDisplayName, void(const base::FilePath&)); MOCK_CONST_METHOD1(DebugString, std::string(bool));
diff --git a/content/public/test/mock_download_manager.cc b/content/public/test/mock_download_manager.cc index 8dca9b2..24a76e2 100644 --- a/content/public/test/mock_download_manager.cc +++ b/content/public/test/mock_download_manager.cc
@@ -21,8 +21,8 @@ const GURL& tab_referrer_url, const std::string& mime_type, const std::string& original_mime_type, - const base::Time& start_time, - const base::Time& end_time, + base::Time start_time, + base::Time end_time, const std::string& etag, const std::string& last_modified, int64_t received_bytes, @@ -32,6 +32,7 @@ DownloadDangerType danger_type, DownloadInterruptReason interrupt_reason, bool opened, + base::Time last_access_time, const std::vector<DownloadItem::ReceivedSlice>& received_slices) : guid(guid), id(id), @@ -53,6 +54,7 @@ danger_type(danger_type), interrupt_reason(interrupt_reason), opened(opened), + last_access_time(last_access_time), received_slices(received_slices) {} MockDownloadManager::CreateDownloadItemAdapter::CreateDownloadItemAdapter( @@ -76,6 +78,7 @@ danger_type(rhs.danger_type), interrupt_reason(rhs.interrupt_reason), opened(rhs.opened), + last_access_time(rhs.last_access_time), received_slices(rhs.received_slices) {} MockDownloadManager::CreateDownloadItemAdapter::~CreateDownloadItemAdapter() {} @@ -94,6 +97,7 @@ received_bytes == rhs.received_bytes && total_bytes == rhs.total_bytes && state == rhs.state && danger_type == rhs.danger_type && interrupt_reason == rhs.interrupt_reason && opened == rhs.opened && + last_access_time == rhs.last_access_time && received_slices == rhs.received_slices); } @@ -120,8 +124,8 @@ const GURL& tab_referrer_url, const std::string& mime_type, const std::string& original_mime_type, - const base::Time& start_time, - const base::Time& end_time, + base::Time start_time, + base::Time end_time, const std::string& etag, const std::string& last_modified, int64_t received_bytes, @@ -131,12 +135,13 @@ DownloadDangerType danger_type, DownloadInterruptReason interrupt_reason, bool opened, + base::Time last_access_time, const std::vector<DownloadItem::ReceivedSlice>& received_slices) { CreateDownloadItemAdapter adapter( guid, id, current_path, target_path, url_chain, referrer_url, site_url, tab_url, tab_referrer_url, mime_type, original_mime_type, start_time, end_time, etag, last_modified, received_bytes, total_bytes, hash, state, - danger_type, interrupt_reason, opened, received_slices); + danger_type, interrupt_reason, opened, last_access_time, received_slices); return MockCreateDownloadItem(adapter); }
diff --git a/content/public/test/mock_download_manager.h b/content/public/test/mock_download_manager.h index 1bf0164..4da5ff5b 100644 --- a/content/public/test/mock_download_manager.h +++ b/content/public/test/mock_download_manager.h
@@ -52,6 +52,7 @@ DownloadDangerType danger_type; DownloadInterruptReason interrupt_reason; bool opened; + base::Time last_access_time; std::vector<DownloadItem::ReceivedSlice> received_slices; CreateDownloadItemAdapter( @@ -66,8 +67,8 @@ const GURL& tab_refererr_url, const std::string& mime_type, const std::string& original_mime_type, - const base::Time& start_time, - const base::Time& end_time, + base::Time start_time, + base::Time end_time, const std::string& etag, const std::string& last_modified, int64_t received_bytes, @@ -77,6 +78,7 @@ DownloadDangerType danger_type, DownloadInterruptReason interrupt_reason, bool opened, + base::Time last_access_time, const std::vector<DownloadItem::ReceivedSlice>& received_slices); // Required by clang compiler. CreateDownloadItemAdapter(const CreateDownloadItemAdapter& rhs); @@ -127,8 +129,8 @@ const GURL& tab_refererr_url, const std::string& mime_type, const std::string& original_mime_type, - const base::Time& start_time, - const base::Time& end_time, + base::Time start_time, + base::Time end_time, const std::string& etag, const std::string& last_modified, int64_t received_bytes, @@ -138,6 +140,7 @@ DownloadDangerType danger_type, DownloadInterruptReason interrupt_reason, bool opened, + base::Time last_access_time, const std::vector<DownloadItem::ReceivedSlice>& received_slices) override; MOCK_METHOD1(MockCreateDownloadItem,
diff --git a/ios/chrome/browser/ui/omnibox/page_info_view_controller.mm b/ios/chrome/browser/ui/omnibox/page_info_view_controller.mm index e977b05..06fc333 100644 --- a/ios/chrome/browser/ui/omnibox/page_info_view_controller.mm +++ b/ios/chrome/browser/ui/omnibox/page_info_view_controller.mm
@@ -508,6 +508,9 @@ CGSize sizeWithFont = [[[button titleLabel] text] cr_pixelAlignedSizeWithFont:font]; frame.size = sizeWithFont; + // According to iOS Human Interface Guidelines, minimal size of UIButton + // should be 44x44. + frame.size.height = std::max<CGFloat>(44, frame.size.height); [button setFrame:frame];
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index c6f49b80..8ddf277c 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -736,10 +736,6 @@ crbug.com/638693 virtual/threaded/animations/display-inline-style-adjust.html [ Pass Crash Failure ] crbug.com/421283 html/marquee/marquee-scrollamount.html [ Pass Failure ] -crbug.com/590095 virtual/disable-spinvalidation/paint/invalidation/invalidation-with-scale-transform.html [ NeedsRebaseline ] -crbug.com/590095 virtual/disable-spinvalidation/paint/invalidation/transform-layout-repaint.html [ NeedsRebaseline ] -crbug.com/590095 virtual/disable-spinvalidation/paint/invalidation/reflection-repaint-test.html [ NeedsRebaseline ] - # TODO(oshima): Mac Android are currently not supported. crbug.com/567837 [ Mac Android ] virtual/scalefactor200withzoom/fast/hidpi/static [ Skip ] @@ -850,6 +846,8 @@ Bug(github) external/wpt/uievents/keyboard/key-manual.js [ Skip ] crbug.com/698135 external/wpt/editing/run/delete.html [ Crash Failure Timeout ] +crbug.com/698165 external/wpt/editing/run/forwarddelete.html [ Pass Timeout ] +crbug.com/698165 external/wpt/editing/run/justifycenter.html [ Pass Timeout ] crbug.com/688613 external/wpt/mediacapture-streams/MediaStreamTrack-MediaElement-disabled-audio-is-silence.https.html [ Skip ] @@ -2490,7 +2488,6 @@ # Sheriff failures 2017-02-21 crbug.com/73609 http/tests/media/video-play-stall.html [ Pass Timeout ] crbug.com/73609 virtual/mojo-loading/http/tests/media/video-play-stall.html [ Pass Timeout ] -crbug.com/694467 fast/text/line-break-8bit-after-16bit.html [ Pass Failure ] # Sheriff failures 2017-02-23 crbug.com/695352 [ Mac ] virtual/threaded/compositing/visibility/overlays-persist-on-navigation.html [ Pass Crash ]
diff --git a/third_party/WebKit/LayoutTests/fast/text/line-break-8bit-after-16bit-expected.html b/third_party/WebKit/LayoutTests/fast/text/line-break-8bit-after-16bit-expected.html index a274316..6915d4e 100644 --- a/third_party/WebKit/LayoutTests/fast/text/line-break-8bit-after-16bit-expected.html +++ b/third_party/WebKit/LayoutTests/fast/text/line-break-8bit-after-16bit-expected.html
@@ -17,3 +17,11 @@ <div>あああbar</div> <div>国国、bar</div> </body> +<script> +if (window.testRunner) { + testRunner.waitUntilDone(); + window.onload = function () { + document.fonts.ready.then(function () { testRunner.notifyDone(); }); + }; +} +</script>
diff --git a/third_party/WebKit/LayoutTests/fast/text/line-break-8bit-after-16bit.html b/third_party/WebKit/LayoutTests/fast/text/line-break-8bit-after-16bit.html index 78c0d71..1dd90eb42 100644 --- a/third_party/WebKit/LayoutTests/fast/text/line-break-8bit-after-16bit.html +++ b/third_party/WebKit/LayoutTests/fast/text/line-break-8bit-after-16bit.html
@@ -17,3 +17,11 @@ <div>あああ<span>bar</span></div> <div><span>国国、</span>bar</div> </body> +<script> +if (window.testRunner) { + testRunner.waitUntilDone(); + window.onload = function () { + document.fonts.ready.then(function () { testRunner.notifyDone(); }); + }; +} +</script>
diff --git a/third_party/WebKit/LayoutTests/http/tests/webfont/font-face-revalidation-expected.html b/third_party/WebKit/LayoutTests/http/tests/webfont/font-face-revalidation-expected.html new file mode 100644 index 0000000..cf6f087b --- /dev/null +++ b/third_party/WebKit/LayoutTests/http/tests/webfont/font-face-revalidation-expected.html
@@ -0,0 +1,8 @@ +<!DOCTYPE html> +<script src="/js-test-resources/ahem.js"></script> +<p> +This tests that web font is displayed correctly after revalidation of font resource. + +You should see a black square below. +</p> +<div style="font-family: Ahem">test</span>
diff --git a/third_party/WebKit/LayoutTests/http/tests/webfont/font-face-revalidation.html b/third_party/WebKit/LayoutTests/http/tests/webfont/font-face-revalidation.html new file mode 100644 index 0000000..16f868af --- /dev/null +++ b/third_party/WebKit/LayoutTests/http/tests/webfont/font-face-revalidation.html
@@ -0,0 +1,28 @@ +<!DOCTYPE html> +<style></style> +<script> +if (window.testRunner) + testRunner.waitUntilDone(); + +function addFontFaceRule() { + let font = 'cachable-slow-ahem-loading.cgi?delay=50'; + document.styleSheets[0].insertRule( + '@font-face { font-family: ahem; src: url(' + font + '); }', + document.styleSheets[0].cssRules.length); +} + +addFontFaceRule(); + +document.fonts.ready.then(() => { + addFontFaceRule(); + document.fonts.onloadingdone = function() { + testRunner.notifyDone(); + }; +}); +</script> +<p> +This tests that web font is displayed correctly after revalidation of font resource. + +You should see a black square below. +</p> +<div style="font-family: ahem">test</span>
diff --git a/third_party/WebKit/LayoutTests/platform/linux/virtual/disable-spinvalidation/paint/invalidation/reflection-repaint-test-expected.txt b/third_party/WebKit/LayoutTests/platform/linux/virtual/disable-spinvalidation/paint/invalidation/reflection-repaint-test-expected.txt deleted file mode 100644 index 19181a7..0000000 --- a/third_party/WebKit/LayoutTests/platform/linux/virtual/disable-spinvalidation/paint/invalidation/reflection-repaint-test-expected.txt +++ /dev/null
@@ -1,41 +0,0 @@ -{ - "layers": [ - { - "name": "LayoutView #document", - "bounds": [800, 600], - "contentsOpaque": true, - "drawsContent": true, - "paintInvalidations": [ - { - "object": "LayoutBlockFlow DIV id='target'", - "rect": [22, 50, 226, 167], - "reason": "forced by layout" - }, - { - "object": "LayoutText #text", - "rect": [23, 51, 72, 110], - "reason": "full" - } - ] - } - ], - "objectPaintInvalidations": [ - { - "object": "LayoutBlockFlow DIV id='target'", - "reason": "forced by layout" - }, - { - "object": "RootInlineBox", - "reason": "forced by layout" - }, - { - "object": "LayoutText #text", - "reason": "full" - }, - { - "object": "InlineTextBox 'PASS'", - "reason": "full" - } - ] -} -
diff --git a/third_party/WebKit/LayoutTests/platform/linux/virtual/disable-spinvalidation/paint/invalidation/transform-layout-repaint-expected.txt b/third_party/WebKit/LayoutTests/platform/linux/virtual/disable-spinvalidation/paint/invalidation/transform-layout-repaint-expected.txt index 48ff40a..1565faa6 100644 --- a/third_party/WebKit/LayoutTests/platform/linux/virtual/disable-spinvalidation/paint/invalidation/transform-layout-repaint-expected.txt +++ b/third_party/WebKit/LayoutTests/platform/linux/virtual/disable-spinvalidation/paint/invalidation/transform-layout-repaint-expected.txt
@@ -13,7 +13,7 @@ }, { "object": "LayoutText #text", - "rect": [52, 51, 43, 32], + "rect": [52, 51, 42, 31], "reason": "full" } ]
diff --git a/third_party/WebKit/LayoutTests/virtual/disable-spinvalidation/paint/invalidation/invalidation-with-scale-transform-expected.txt b/third_party/WebKit/LayoutTests/virtual/disable-spinvalidation/paint/invalidation/invalidation-with-scale-transform-expected.txt new file mode 100644 index 0000000..6d795a45 --- /dev/null +++ b/third_party/WebKit/LayoutTests/virtual/disable-spinvalidation/paint/invalidation/invalidation-with-scale-transform-expected.txt
@@ -0,0 +1,29 @@ +{ + "layers": [ + { + "name": "LayoutView #document", + "bounds": [800, 600], + "contentsOpaque": true, + "drawsContent": true, + "paintInvalidations": [ + { + "object": "LayoutBlockFlow (positioned) DIV id='target'", + "rect": [85, 70, 91, 91], + "reason": "bounds change" + }, + { + "object": "LayoutBlockFlow (positioned) DIV id='target'", + "rect": [84, 70, 91, 91], + "reason": "bounds change" + } + ] + } + ], + "objectPaintInvalidations": [ + { + "object": "LayoutBlockFlow (positioned) DIV id='target'", + "reason": "bounds change" + } + ] +} +
diff --git a/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp b/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp index 4aab69c8..6e0b70d24 100644 --- a/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp +++ b/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp
@@ -133,8 +133,8 @@ SecurityOrigin* origin = 0; if (m_world->isMainWorld()) { - // ActivityLogger for main world is updated within updateDocument(). - updateDocument(); + // ActivityLogger for main world is updated within updateDocumentInternal(). + updateDocumentInternal(); origin = frame()->document()->getSecurityOrigin(); // FIXME: Can this be removed when CSP moves to browser? ContentSecurityPolicy* csp = frame()->document()->contentSecurityPolicy(); @@ -316,12 +316,22 @@ // to update. The update is done when the window proxy gets initialized later. if (m_lifecycle == Lifecycle::ContextUninitialized) return; - // TODO(yukishiino): Is it okay to not update document when the context - // is detached? It's not trivial to fix this because udpateDocumentProperty - // requires a not-yet-detached context to instantiate a document wrapper. - if (m_lifecycle == Lifecycle::ContextDetached) - return; + // If this WindowProxy was previously initialized, reinitialize it now to + // preserve JS object identity. Otherwise, extant references to the + // WindowProxy will be broken. + if (m_lifecycle == Lifecycle::ContextDetached) { + initialize(); + DCHECK_EQ(Lifecycle::ContextInitialized, m_lifecycle); + // Initialization internally updates the document properties, so just + // return afterwards. + return; + } + + updateDocumentInternal(); +} + +void LocalWindowProxy::updateDocumentInternal() { updateActivityLogger(); updateDocumentProperty(); updateSecurityOrigin(frame()->document()->getSecurityOrigin());
diff --git a/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.h b/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.h index c6b8cd6..a9b68c9d 100644 --- a/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.h +++ b/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.h
@@ -85,6 +85,12 @@ void setSecurityToken(SecurityOrigin*); + // Triggers updates of objects that are associated with a Document: + // - the activity logger + // - the document DOM wrapper + // - the security origin + void updateDocumentInternal(); + // The JavaScript wrapper for the document object is cached on the global // object for fast access. UpdateDocumentProperty sets the wrapper // for the current document on the global object.
diff --git a/third_party/WebKit/Source/core/css/BinaryDataFontFaceSource.h b/third_party/WebKit/Source/core/css/BinaryDataFontFaceSource.h index 438bcecf..0b248e0 100644 --- a/third_party/WebKit/Source/core/css/BinaryDataFontFaceSource.h +++ b/third_party/WebKit/Source/core/css/BinaryDataFontFaceSource.h
@@ -6,7 +6,7 @@ #define BinaryDataFontFaceSource_h #include "core/css/CSSFontFaceSource.h" -#include <memory> +#include "wtf/RefPtr.h" namespace blink { @@ -22,7 +22,7 @@ private: PassRefPtr<SimpleFontData> createFontData(const FontDescription&) override; - std::unique_ptr<FontCustomPlatformData> m_customPlatformData; + RefPtr<FontCustomPlatformData> m_customPlatformData; }; } // namespace blink
diff --git a/third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp b/third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp index 2052be42..c9d12e30a 100644 --- a/third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp +++ b/third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp
@@ -14,6 +14,7 @@ #include "platform/Histogram.h" #include "platform/RuntimeEnabledFeatures.h" #include "platform/fonts/FontCache.h" +#include "platform/fonts/FontCustomPlatformData.h" #include "platform/fonts/FontDescription.h" #include "platform/fonts/SimpleFontData.h" #include "platform/loader/fetch/ResourceFetcher.h" @@ -79,8 +80,10 @@ RemoteFontFaceSource::~RemoteFontFaceSource() {} void RemoteFontFaceSource::dispose() { - m_font->removeClient(this); - m_font = nullptr; + if (m_font) { + m_font->removeClient(this); + m_font = nullptr; + } pruneTable(); } @@ -97,18 +100,19 @@ } bool RemoteFontFaceSource::isLoading() const { - return m_font->isLoading(); + return m_font && m_font->isLoading(); } bool RemoteFontFaceSource::isLoaded() const { - return m_font->isLoaded(); + return !m_font; } bool RemoteFontFaceSource::isValid() const { - return !m_font->errorOccurred(); + return m_font || m_customFontData; } -void RemoteFontFaceSource::notifyFinished(Resource*) { +void RemoteFontFaceSource::notifyFinished(Resource* unusedResource) { + DCHECK_EQ(unusedResource, m_font); m_histograms.maySetDataSource(m_font->response().wasCached() ? FontLoadHistograms::FromDiskCache : FontLoadHistograms::FromNetwork); @@ -117,7 +121,8 @@ m_font->getStatus() == ResourceStatus::LoadError, m_isInterventionTriggered); - m_font->ensureCustomFontData(); + m_customFontData = m_font->getCustomFontData(); + // FIXME: Provide more useful message such as OTS rejection reason. // See crbug.com/97467 if (m_font->getStatus() == ResourceStatus::DecodeError && @@ -131,6 +136,9 @@ "OTS parsing error: " + m_font->otsParsingMessage())); } + m_font->removeClient(this); + m_font = nullptr; + pruneTable(); if (m_face) { m_fontSelector->fontFaceInvalidated(); @@ -139,7 +147,7 @@ } void RemoteFontFaceSource::fontLoadShortLimitExceeded(FontResource*) { - if (m_font->isLoaded()) + if (isLoaded()) return; if (m_display == FontDisplayFallback) @@ -149,7 +157,7 @@ } void RemoteFontFaceSource::fontLoadLongLimitExceeded(FontResource*) { - if (m_font->isLoaded()) + if (isLoaded()) return; if (m_display == FontDisplayBlock || @@ -171,7 +179,7 @@ m_face->didBecomeVisibleFallback(this); } - m_histograms.recordFallbackTime(m_font.get()); + m_histograms.recordFallbackTime(); } void RemoteFontFaceSource::switchToFailurePeriod() { @@ -206,16 +214,16 @@ PassRefPtr<SimpleFontData> RemoteFontFaceSource::createFontData( const FontDescription& fontDescription) { + if (m_period == FailurePeriod || !isValid()) + return nullptr; if (!isLoaded()) return createLoadingFallbackFontData(fontDescription); + DCHECK(m_customFontData); - if (!m_font->ensureCustomFontData() || m_period == FailurePeriod) - return nullptr; - - m_histograms.recordFallbackTime(m_font.get()); + m_histograms.recordFallbackTime(); return SimpleFontData::create( - m_font->platformDataFromCustomData(fontDescription.effectiveFontSize(), + m_customFontData->fontPlatformData(fontDescription.effectiveFontSize(), fontDescription.isSyntheticBold(), fontDescription.isSyntheticItalic(), fontDescription.orientation(), @@ -241,6 +249,10 @@ } void RemoteFontFaceSource::beginLoadIfNeeded() { + if (isLoaded()) + return; + DCHECK(m_font); + if (m_fontSelector->document() && m_font->stillNeedsLoad()) { if (!m_font->url().protocolIsData() && !m_font->isLoaded() && m_display == FontDisplayAuto && @@ -297,8 +309,7 @@ recordInterventionResult(isInterventionTriggered); } -void RemoteFontFaceSource::FontLoadHistograms::recordFallbackTime( - const FontResource* font) { +void RemoteFontFaceSource::FontLoadHistograms::recordFallbackTime() { if (m_blankPaintTime <= 0) return; int duration = static_cast<int>(currentTimeMS() - m_blankPaintTime);
diff --git a/third_party/WebKit/Source/core/css/RemoteFontFaceSource.h b/third_party/WebKit/Source/core/css/RemoteFontFaceSource.h index beb46b5..c068907 100644 --- a/third_party/WebKit/Source/core/css/RemoteFontFaceSource.h +++ b/third_party/WebKit/Source/core/css/RemoteFontFaceSource.h
@@ -12,6 +12,7 @@ namespace blink { class CSSFontSelector; +class FontCustomPlatformData; enum FontDisplay { FontDisplayAuto, @@ -88,7 +89,7 @@ bool loadError, bool isInterventionTriggered); void longLimitExceeded(bool isInterventionTriggered); - void recordFallbackTime(const FontResource*); + void recordFallbackTime(); void recordRemoteFont(const FontResource*, bool isInterventionTriggered); bool hadBlankText() { return m_blankPaintTime; } DataSource dataSource() { return m_dataSource; } @@ -112,8 +113,14 @@ bool shouldTriggerWebFontsIntervention(); bool isLowPriorityLoadingAllowedForRemoteFont() const override; + // Cleared once load is finished. Member<FontResource> m_font; + Member<CSSFontSelector> m_fontSelector; + + // |nullptr| if font is not loaded or failed to decode. + RefPtr<FontCustomPlatformData> m_customFontData; + const FontDisplay m_display; DisplayPeriod m_period; FontLoadHistograms m_histograms;
diff --git a/third_party/WebKit/Source/core/editing/EditingUtilities.cpp b/third_party/WebKit/Source/core/editing/EditingUtilities.cpp index 329a39f..008907f2 100644 --- a/third_party/WebKit/Source/core/editing/EditingUtilities.cpp +++ b/third_party/WebKit/Source/core/editing/EditingUtilities.cpp
@@ -2082,11 +2082,11 @@ if (!hasRichlyEditableStyle(node)) return nullptr; return new StaticRangeVector( - 1, StaticRange::create( - firstRangeOf(node.document() - .frame() - ->selection() - .computeVisibleSelectionInDOMTreeDeprecated()))); + 1, StaticRange::create(createRange(firstEphemeralRangeOf( + node.document() + .frame() + ->selection() + .computeVisibleSelectionInDOMTreeDeprecated())))); } DispatchEventResult dispatchBeforeInputInsertText(Node* target,
diff --git a/third_party/WebKit/Source/core/editing/SelectionEditor.cpp b/third_party/WebKit/Source/core/editing/SelectionEditor.cpp index cbf5174bb..a06bc771 100644 --- a/third_party/WebKit/Source/core/editing/SelectionEditor.cpp +++ b/third_party/WebKit/Source/core/editing/SelectionEditor.cpp
@@ -368,7 +368,7 @@ Range* SelectionEditor::firstRange() const { if (m_logicalRange) return m_logicalRange->cloneRange(); - return firstRangeOf(computeVisibleSelectionInDOMTree()); + return createRange(firstEphemeralRangeOf(computeVisibleSelectionInDOMTree())); } bool SelectionEditor::shouldAlwaysUseDirectionalSelection() const {
diff --git a/third_party/WebKit/Source/core/editing/SurroundingTextTest.cpp b/third_party/WebKit/Source/core/editing/SurroundingTextTest.cpp index 8f848f9..f0546a92 100644 --- a/third_party/WebKit/Source/core/editing/SurroundingTextTest.cpp +++ b/third_party/WebKit/Source/core/editing/SurroundingTextTest.cpp
@@ -113,7 +113,8 @@ { VisibleSelection selection = select(0, 5); - SurroundingText surroundingText(*firstRangeOf(selection), 1); + SurroundingText surroundingText( + *createRange(firstEphemeralRangeOf(selection)), 1); EXPECT_EQ("Lorem ", surroundingText.content()); EXPECT_EQ(0u, surroundingText.startOffsetInContent()); @@ -122,7 +123,8 @@ { VisibleSelection selection = select(0, 5); - SurroundingText surroundingText(*firstRangeOf(selection), 5); + SurroundingText surroundingText( + *createRange(firstEphemeralRangeOf(selection)), 5); EXPECT_EQ("Lorem ip", surroundingText.content().simplifyWhiteSpace()); EXPECT_EQ(1u, surroundingText.startOffsetInContent()); @@ -131,7 +133,8 @@ { VisibleSelection selection = select(0, 5); - SurroundingText surroundingText(*firstRangeOf(selection), 42); + SurroundingText surroundingText( + *createRange(firstEphemeralRangeOf(selection)), 42); EXPECT_EQ("Lorem ipsum dolor sit amet", surroundingText.content().simplifyWhiteSpace()); @@ -141,7 +144,8 @@ { VisibleSelection selection = select(6, 11); - SurroundingText surroundingText(*firstRangeOf(selection), 2); + SurroundingText surroundingText( + *createRange(firstEphemeralRangeOf(selection)), 2); EXPECT_EQ(" ipsum ", surroundingText.content()); EXPECT_EQ(1u, surroundingText.startOffsetInContent()); @@ -150,7 +154,8 @@ { VisibleSelection selection = select(6, 11); - SurroundingText surroundingText(*firstRangeOf(selection), 42); + SurroundingText surroundingText( + *createRange(firstEphemeralRangeOf(selection)), 42); EXPECT_EQ("Lorem ipsum dolor sit amet", surroundingText.content().simplifyWhiteSpace()); @@ -219,7 +224,8 @@ { VisibleSelection selection = select(0, 1); - SurroundingText surroundingText(*firstRangeOf(selection), 1); + SurroundingText surroundingText( + *createRange(firstEphemeralRangeOf(selection)), 1); EXPECT_EQ("fo", surroundingText.content().simplifyWhiteSpace()); EXPECT_EQ(0u, surroundingText.startOffsetInContent()); @@ -228,7 +234,8 @@ { VisibleSelection selection = select(0, 3); - SurroundingText surroundingText(*firstRangeOf(selection), 12); + SurroundingText surroundingText( + *createRange(firstEphemeralRangeOf(selection)), 12); EXPECT_EQ("e of foo bar", surroundingText.content().simplifyWhiteSpace()); EXPECT_EQ(5u, surroundingText.startOffsetInContent()); @@ -237,7 +244,8 @@ { VisibleSelection selection = select(0, 3); - SurroundingText surroundingText(*firstRangeOf(selection), 1337); + SurroundingText surroundingText( + *createRange(firstEphemeralRangeOf(selection)), 1337); EXPECT_EQ("This is outside of foo bar the selected node", surroundingText.content().simplifyWhiteSpace()); @@ -247,7 +255,8 @@ { VisibleSelection selection = select(4, 7); - SurroundingText surroundingText(*firstRangeOf(selection), 12); + SurroundingText surroundingText( + *createRange(firstEphemeralRangeOf(selection)), 12); EXPECT_EQ("foo bar the se", surroundingText.content().simplifyWhiteSpace()); EXPECT_EQ(5u, surroundingText.startOffsetInContent()); @@ -256,7 +265,8 @@ { VisibleSelection selection = select(0, 7); - SurroundingText surroundingText(*firstRangeOf(selection), 1337); + SurroundingText surroundingText( + *createRange(firstEphemeralRangeOf(selection)), 1337); EXPECT_EQ("This is outside of foo bar the selected node", surroundingText.content().simplifyWhiteSpace());
diff --git a/third_party/WebKit/Source/core/editing/VisibleSelection.cpp b/third_party/WebKit/Source/core/editing/VisibleSelection.cpp index 0db1b80..0fe46f85 100644 --- a/third_party/WebKit/Source/core/editing/VisibleSelection.cpp +++ b/third_party/WebKit/Source/core/editing/VisibleSelection.cpp
@@ -144,10 +144,6 @@ return EphemeralRange(start, end); } -Range* firstRangeOf(const VisibleSelection& selection) { - return createRange(firstEphemeralRangeOf(selection)); -} - template <typename Strategy> EphemeralRangeTemplate<Strategy> VisibleSelectionTemplate<Strategy>::toNormalizedEphemeralRange() const {
diff --git a/third_party/WebKit/Source/core/editing/VisibleSelection.h b/third_party/WebKit/Source/core/editing/VisibleSelection.h index b423261..09a430a 100644 --- a/third_party/WebKit/Source/core/editing/VisibleSelection.h +++ b/third_party/WebKit/Source/core/editing/VisibleSelection.h
@@ -196,9 +196,6 @@ // to return. CORE_EXPORT EphemeralRange firstEphemeralRangeOf(const VisibleSelection&); -// TODO(sof): move more firstRangeOf() uses to be over EphemeralRange instead. -CORE_EXPORT Range* firstRangeOf(const VisibleSelection&); - CORE_EXPORT std::ostream& operator<<(std::ostream&, const VisibleSelection&); CORE_EXPORT std::ostream& operator<<(std::ostream&, const VisibleSelectionInFlatTree&);
diff --git a/third_party/WebKit/Source/core/editing/VisibleSelectionTest.cpp b/third_party/WebKit/Source/core/editing/VisibleSelectionTest.cpp index 28b63331b..e0c1ebcc 100644 --- a/third_party/WebKit/Source/core/editing/VisibleSelectionTest.cpp +++ b/third_party/WebKit/Source/core/editing/VisibleSelectionTest.cpp
@@ -208,7 +208,7 @@ EXPECT_TRUE(selection.isCaret()); EXPECT_TRUE(selectionInFlatTree.isCaret()); - Range* range = firstRangeOf(selection); + Range* range = createRange(firstEphemeralRangeOf(selection)); EXPECT_EQ(0u, range->startOffset()); EXPECT_EQ(0u, range->endOffset()); EXPECT_EQ("", range->text()); @@ -381,7 +381,7 @@ selectionInFlatTree = expandUsingGranularity(selectionInFlatTree, WordGranularity); - Range* range = firstRangeOf(selection); + Range* range = createRange(firstEphemeralRangeOf(selection)); EXPECT_EQ(0u, range->startOffset()); EXPECT_EQ(5u, range->endOffset()); EXPECT_EQ("Lorem", range->text()); @@ -397,7 +397,7 @@ selectionInFlatTree = expandUsingGranularity(selectionInFlatTree, WordGranularity); - Range* range = firstRangeOf(selection); + Range* range = createRange(firstEphemeralRangeOf(selection)); EXPECT_EQ(6u, range->startOffset()); EXPECT_EQ(11u, range->endOffset()); EXPECT_EQ("ipsum", range->text()); @@ -415,7 +415,7 @@ selectionInFlatTree = expandUsingGranularity(selectionInFlatTree, WordGranularity); - Range* range = firstRangeOf(selection); + Range* range = createRange(firstEphemeralRangeOf(selection)); EXPECT_EQ(5u, range->startOffset()); EXPECT_EQ(6u, range->endOffset()); EXPECT_EQ(" ", range->text()); @@ -433,7 +433,7 @@ selectionInFlatTree = expandUsingGranularity(selectionInFlatTree, WordGranularity); - Range* range = firstRangeOf(selection); + Range* range = createRange(firstEphemeralRangeOf(selection)); EXPECT_EQ(26u, range->startOffset()); EXPECT_EQ(27u, range->endOffset()); EXPECT_EQ(",", range->text()); @@ -449,7 +449,7 @@ selectionInFlatTree = expandUsingGranularity(selectionInFlatTree, WordGranularity); - Range* range = firstRangeOf(selection); + Range* range = createRange(firstEphemeralRangeOf(selection)); EXPECT_EQ(27u, range->startOffset()); EXPECT_EQ(28u, range->endOffset()); EXPECT_EQ(" ", range->text()); @@ -465,7 +465,7 @@ selectionInFlatTree = expandUsingGranularity(selectionInFlatTree, WordGranularity); - Range* range = firstRangeOf(selection); + Range* range = createRange(firstEphemeralRangeOf(selection)); EXPECT_EQ(0u, range->startOffset()); EXPECT_EQ(5u, range->endOffset()); EXPECT_EQ("Lorem", range->text()); @@ -481,7 +481,7 @@ selectionInFlatTree = expandUsingGranularity(selectionInFlatTree, WordGranularity); - Range* range = firstRangeOf(selection); + Range* range = createRange(firstEphemeralRangeOf(selection)); EXPECT_EQ(0u, range->startOffset()); EXPECT_EQ(11u, range->endOffset()); EXPECT_EQ("Lorem ipsum", range->text());
diff --git a/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp b/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp index 756813b..2f2478ea 100644 --- a/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp +++ b/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp
@@ -205,8 +205,8 @@ // We only supports single selections. if (selectionModifier.selection().isNone()) return ranges; - ranges->push_back( - StaticRange::create(firstRangeOf(selectionModifier.selection()))); + ranges->push_back(StaticRange::create( + createRange(firstEphemeralRangeOf(selectionModifier.selection())))); return ranges; } @@ -2076,7 +2076,8 @@ if (!selection.isNonOrphanedCaretOrRange() || !selection.isContentEditable()) return ""; Element* formatBlockElement = - FormatBlockCommand::elementForFormatBlockCommand(firstRangeOf(selection)); + FormatBlockCommand::elementForFormatBlockCommand( + createRange(firstEphemeralRangeOf(selection))); if (!formatBlockElement) return ""; return formatBlockElement->localName();
diff --git a/third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp b/third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp index 4a6ad4eb..bef6978 100644 --- a/third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp +++ b/third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp
@@ -184,7 +184,8 @@ startOfParagraph(visibleEndOfSelection, CanSkipOverEditingBoundary) .deepEquivalent(); - Range* currentSelection = firstRangeOf(endingSelection()); + Range* currentSelection = + createRange(firstEphemeralRangeOf(endingSelection())); ContainerNode* scopeForStartOfSelection = nullptr; ContainerNode* scopeForEndOfSelection = nullptr; // FIXME: This is an inefficient way to keep selection alive because @@ -296,9 +297,9 @@ return; } - DCHECK(firstRangeOf(endingSelection())); - doApplyForSingleParagraph(false, listTag, *firstRangeOf(endingSelection()), - editingState); + Range* const range = createRange(firstEphemeralRangeOf(endingSelection())); + DCHECK(range); + doApplyForSingleParagraph(false, listTag, *range, editingState); } InputEvent::InputType InsertListCommand::inputType() const {
diff --git a/third_party/WebKit/Source/core/loader/resource/FontResource.cpp b/third_party/WebKit/Source/core/loader/resource/FontResource.cpp index 6401832..1488e55 100644 --- a/third_party/WebKit/Source/core/loader/resource/FontResource.cpp +++ b/third_party/WebKit/Source/core/loader/resource/FontResource.cpp
@@ -131,7 +131,7 @@ BLINK_FROM_HERE); } -bool FontResource::ensureCustomFontData() { +PassRefPtr<FontCustomPlatformData> FontResource::getCustomFontData() { if (!m_fontData && !errorOccurred() && !isLoading()) { if (data()) m_fontData = FontCustomPlatformData::create(data(), m_otsParsingMessage); @@ -143,18 +143,7 @@ recordPackageFormatHistogram(PackageFormatUnknown); } } - return m_fontData.get(); -} - -FontPlatformData FontResource::platformDataFromCustomData( - float size, - bool bold, - bool italic, - FontOrientation orientation, - FontVariationSettings* fontVariationSettings) { - DCHECK(m_fontData); - return m_fontData->fontPlatformData(size, bold, italic, orientation, - fontVariationSettings); + return m_fontData; } void FontResource::willReloadAfterDiskCacheMiss() { @@ -210,7 +199,7 @@ } void FontResource::allClientsAndObserversRemoved() { - m_fontData.reset(); + m_fontData.clear(); Resource::allClientsAndObserversRemoved(); }
diff --git a/third_party/WebKit/Source/core/loader/resource/FontResource.h b/third_party/WebKit/Source/core/loader/resource/FontResource.h index cbe83d7..11e80151 100644 --- a/third_party/WebKit/Source/core/loader/resource/FontResource.h +++ b/third_party/WebKit/Source/core/loader/resource/FontResource.h
@@ -29,20 +29,17 @@ #include "base/gtest_prod_util.h" #include "core/CoreExport.h" #include "platform/Timer.h" -#include "platform/fonts/FontOrientation.h" #include "platform/heap/Handle.h" #include "platform/loader/fetch/Resource.h" #include "platform/loader/fetch/ResourceClient.h" -#include <memory> +#include "wtf/RefPtr.h" namespace blink { class FetchRequest; class ResourceFetcher; -class FontPlatformData; class FontCustomPlatformData; class FontResourceClient; -class FontVariationSettings; class CORE_EXPORT FontResource final : public Resource { public: @@ -62,13 +59,7 @@ bool isCORSFailed() const { return m_corsFailed; } String otsParsingMessage() const { return m_otsParsingMessage; } - bool ensureCustomFontData(); - FontPlatformData platformDataFromCustomData( - float size, - bool bold, - bool italic, - FontOrientation = FontOrientation::Horizontal, - FontVariationSettings* = nullptr); + PassRefPtr<FontCustomPlatformData> getCustomFontData(); // Returns true if the loading priority of the remote font resource can be // lowered. The loading priority of the font can be lowered only if the @@ -108,7 +99,7 @@ LoadLimitStateEnumMax }; - std::unique_ptr<FontCustomPlatformData> m_fontData; + RefPtr<FontCustomPlatformData> m_fontData; String m_otsParsingMessage; LoadLimitState m_loadLimitState; bool m_corsFailed;
diff --git a/third_party/WebKit/Source/devtools/front_end/perf_ui/FlameChart.js b/third_party/WebKit/Source/devtools/front_end/perf_ui/FlameChart.js index 57ddf918..49249c4 100644 --- a/third_party/WebKit/Source/devtools/front_end/perf_ui/FlameChart.js +++ b/third_party/WebKit/Source/devtools/front_end/perf_ui/FlameChart.js
@@ -353,7 +353,7 @@ var timelineData = this._timelineData(); var level = timelineData.entryLevels[this._selectedEntryIndex]; if (this._selectedEntryIndex >= 0 && level >= group.startLevel && - (groupIndex === groups.length || groups[groupIndex + 1].startLevel > level)) + (groupIndex >= groups.length - 1 || groups[groupIndex + 1].startLevel > level)) this._selectedEntryIndex = -1; }
diff --git a/third_party/WebKit/Source/devtools/front_end/quick_open/CommandMenu.js b/third_party/WebKit/Source/devtools/front_end/quick_open/CommandMenu.js index 9f11fdd5..4a84fc3 100644 --- a/third_party/WebKit/Source/devtools/front_end/quick_open/CommandMenu.js +++ b/third_party/WebKit/Source/devtools/front_end/quick_open/CommandMenu.js
@@ -221,14 +221,6 @@ /** * @override - * @return {boolean} - */ - renderMonospace() { - return false; - } - - /** - * @override * @return {string} */ notFoundText() {
diff --git a/third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js b/third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js index be222d573..0a2eb140 100644 --- a/third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js +++ b/third_party/WebKit/Source/devtools/front_end/quick_open/FilteredListWidget.js
@@ -41,11 +41,6 @@ this._itemElementsContainer.classList.add('container'); this._bottomElementsContainer.appendChild(this._itemElementsContainer); - if (delegate.renderMonospace()) { - this._list.element.classList.add('monospace'); - this._promptElement.classList.add('monospace'); - } - this._notFoundElement = this._bottomElementsContainer.createChild('div', 'not-found-text'); this._notFoundElement.classList.add('hidden'); @@ -503,13 +498,6 @@ /** * @return {boolean} */ - renderMonospace() { - return true; - } - - /** - * @return {boolean} - */ renderAsTwoRows() { return false; }
diff --git a/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp b/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp index 25c2a68..abc30a13 100644 --- a/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp +++ b/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp
@@ -1791,7 +1791,7 @@ ->frame() ->selection() .computeVisibleSelectionInDOMTreeDeprecated(); - Range* selectionRange = firstRangeOf(selection); + Range* selectionRange = createRange(firstEphemeralRangeOf(selection)); ContainerNode* parentNode = getNode()->parentNode(); int nodeIndex = getNode()->nodeIndex(); if (!selectionRange
diff --git a/third_party/WebKit/Source/platform/fonts/FontCustomPlatformData.cpp b/third_party/WebKit/Source/platform/fonts/FontCustomPlatformData.cpp index 77b8fa5..d600bff9 100644 --- a/third_party/WebKit/Source/platform/fonts/FontCustomPlatformData.cpp +++ b/third_party/WebKit/Source/platform/fonts/FontCustomPlatformData.cpp
@@ -41,7 +41,6 @@ #include "third_party/skia/include/core/SkStream.h" #include "third_party/skia/include/core/SkTypeface.h" #include "wtf/PtrUtil.h" -#include <memory> namespace blink { @@ -98,7 +97,7 @@ italic && !m_baseTypeface->isItalic(), orientation); } -std::unique_ptr<FontCustomPlatformData> FontCustomPlatformData::create( +PassRefPtr<FontCustomPlatformData> FontCustomPlatformData::create( SharedBuffer* buffer, String& otsParseMessage) { DCHECK(buffer); @@ -108,7 +107,7 @@ otsParseMessage = decoder.getErrorString(); return nullptr; } - return WTF::wrapUnique( + return adoptRef( new FontCustomPlatformData(std::move(typeface), decoder.decodedSize())); }
diff --git a/third_party/WebKit/Source/platform/fonts/FontCustomPlatformData.h b/third_party/WebKit/Source/platform/fonts/FontCustomPlatformData.h index 80eb0cf..9864c3bd 100644 --- a/third_party/WebKit/Source/platform/fonts/FontCustomPlatformData.h +++ b/third_party/WebKit/Source/platform/fonts/FontCustomPlatformData.h
@@ -38,8 +38,8 @@ #include "wtf/Allocator.h" #include "wtf/Forward.h" #include "wtf/Noncopyable.h" +#include "wtf/RefCounted.h" #include "wtf/text/WTFString.h" -#include <memory> class SkTypeface; @@ -49,14 +49,14 @@ class SharedBuffer; class FontVariationSettings; -class PLATFORM_EXPORT FontCustomPlatformData { +class PLATFORM_EXPORT FontCustomPlatformData + : public RefCounted<FontCustomPlatformData> { USING_FAST_MALLOC(FontCustomPlatformData); WTF_MAKE_NONCOPYABLE(FontCustomPlatformData); public: - static std::unique_ptr<FontCustomPlatformData> create( - SharedBuffer*, - String& otsParseMessage); + static PassRefPtr<FontCustomPlatformData> create(SharedBuffer*, + String& otsParseMessage); ~FontCustomPlatformData(); FontPlatformData fontPlatformData(
diff --git a/third_party/WebKit/Source/platform/testing/FontTestHelpers.cpp b/third_party/WebKit/Source/platform/testing/FontTestHelpers.cpp index 0753de7..11c3f4d 100644 --- a/third_party/WebKit/Source/platform/testing/FontTestHelpers.cpp +++ b/third_party/WebKit/Source/platform/testing/FontTestHelpers.cpp
@@ -11,7 +11,6 @@ #include "platform/testing/UnitTestHelpers.h" #include "wtf/PassRefPtr.h" #include "wtf/RefPtr.h" -#include <memory> namespace blink { namespace testing { @@ -48,10 +47,10 @@ void fontCacheInvalidated() override {} private: - TestFontSelector(std::unique_ptr<FontCustomPlatformData> customPlatformData) - : m_customPlatformData(std::move(customPlatformData)) {} + TestFontSelector(PassRefPtr<FontCustomPlatformData> customPlatformData) + : m_customPlatformData(customPlatformData) {} - std::unique_ptr<FontCustomPlatformData> m_customPlatformData; + RefPtr<FontCustomPlatformData> m_customPlatformData; }; } // namespace
diff --git a/third_party/WebKit/Source/web/BUILD.gn b/third_party/WebKit/Source/web/BUILD.gn index 4953c20..02ee268 100644 --- a/third_party/WebKit/Source/web/BUILD.gn +++ b/third_party/WebKit/Source/web/BUILD.gn
@@ -391,6 +391,7 @@ "tests/WebURLResponseTest.cpp", "tests/WebUserGestureTokenTest.cpp", "tests/WebViewTest.cpp", + "tests/WindowProxyTest.cpp", "tests/sim/SimCanvas.cpp", "tests/sim/SimCanvas.h", "tests/sim/SimCompositor.cpp", @@ -405,6 +406,8 @@ "tests/sim/SimRequest.h", "tests/sim/SimTest.cpp", "tests/sim/SimTest.h", + "tests/sim/SimWebFrameClient.cpp", + "tests/sim/SimWebFrameClient.h", "tests/sim/SimWebViewClient.cpp", "tests/sim/SimWebViewClient.h", ]
diff --git a/third_party/WebKit/Source/web/TextFinder.cpp b/third_party/WebKit/Source/web/TextFinder.cpp index 65e476d2..7fd1db6 100644 --- a/third_party/WebKit/Source/web/TextFinder.cpp +++ b/third_party/WebKit/Source/web/TextFinder.cpp
@@ -131,7 +131,7 @@ .computeVisibleSelectionInDOMTreeDeprecated()); bool activeSelection = !selection.isNone(); if (activeSelection) { - m_activeMatch = firstRangeOf(selection); + m_activeMatch = createRange(firstEphemeralRangeOf(selection)); ownerFrame().frame()->selection().clear(); }
diff --git a/third_party/WebKit/Source/web/tests/WindowProxyTest.cpp b/third_party/WebKit/Source/web/tests/WindowProxyTest.cpp new file mode 100644 index 0000000..c81c9ba8 --- /dev/null +++ b/third_party/WebKit/Source/web/tests/WindowProxyTest.cpp
@@ -0,0 +1,51 @@ +// Copyright 2017 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 "platform/testing/UnitTestHelpers.h" +#include "web/tests/sim/SimRequest.h" +#include "web/tests/sim/SimTest.h" + +namespace blink { + +using WindowProxyTest = SimTest; + +// Tests that a WindowProxy is reinitialized after a navigation, even if the new +// Document does not use any scripting. +TEST_F(WindowProxyTest, ReinitializedAfterNavigation) { + // TODO(dcheng): It's nicer to use TestingPlatformSupportWithMockScheduler, + // but that leads to random DCHECKs in loading code. + + SimRequest mainResource("https://example.com/index.html", "text/html"); + loadURL("https://example.com/index.html"); + mainResource.complete( + "<!DOCTYPE html>" + "<html><head><script>" + "var childWindow;" + "function runTest() {" + " childWindow = window[0];" + " document.querySelector('iframe').onload = runTest2;" + " childWindow.location = 'data:text/plain,Initial.';" + "}" + "function runTest2() {" + " try {" + " childWindow.location = 'data:text/plain,Final.';" + " console.log('PASSED');" + " } catch (e) {" + " console.log('FAILED');" + " }" + " document.querySelector('iframe').onload = null;" + "}" + "</script></head><body onload='runTest()'>" + "<iframe></iframe></body></html>"); + + // Wait for the first data: URL to load + testing::runPendingTasks(); + + // Wait for the second data: URL to load. + testing::runPendingTasks(); + + ASSERT_GT(consoleMessages().size(), 0U); + EXPECT_EQ("PASSED", consoleMessages()[0]); +} +}
diff --git a/third_party/WebKit/Source/web/tests/sim/SimTest.cpp b/third_party/WebKit/Source/web/tests/sim/SimTest.cpp index 0ec6383..de789a3 100644 --- a/third_party/WebKit/Source/web/tests/sim/SimTest.cpp +++ b/third_party/WebKit/Source/web/tests/sim/SimTest.cpp
@@ -15,7 +15,7 @@ namespace blink { -SimTest::SimTest() : m_webViewClient(m_compositor) { +SimTest::SimTest() : m_webViewClient(m_compositor), m_webFrameClient(*this) { Document::setThreadedParsingEnabledForTesting(false); // Use the mock theme to get more predictable code paths, this also avoids // the OS callbacks in ScrollAnimatorMac which can schedule frames @@ -25,7 +25,7 @@ // in the middle of a test. LayoutTestSupport::setMockThemeEnabledForTest(true); ScrollbarTheme::setMockScrollbarsEnabled(true); - m_webViewHelper.initialize(true, nullptr, &m_webViewClient); + m_webViewHelper.initialize(true, &m_webFrameClient, &m_webViewClient); m_compositor.setWebViewImpl(webView()); m_page.setPage(webView().page()); } @@ -73,4 +73,8 @@ return m_compositor; } +void SimTest::addConsoleMessage(const String& message) { + m_consoleMessages.push_back(message); +} + } // namespace blink
diff --git a/third_party/WebKit/Source/web/tests/sim/SimTest.h b/third_party/WebKit/Source/web/tests/sim/SimTest.h index d3a84a7..a0ffbc80 100644 --- a/third_party/WebKit/Source/web/tests/sim/SimTest.h +++ b/third_party/WebKit/Source/web/tests/sim/SimTest.h
@@ -5,12 +5,13 @@ #ifndef SimTest_h #define SimTest_h +#include <gtest/gtest.h> #include "web/tests/FrameTestHelpers.h" #include "web/tests/sim/SimCompositor.h" #include "web/tests/sim/SimNetwork.h" #include "web/tests/sim/SimPage.h" +#include "web/tests/sim/SimWebFrameClient.h" #include "web/tests/sim/SimWebViewClient.h" -#include <gtest/gtest.h> namespace blink { @@ -34,12 +35,21 @@ const SimWebViewClient& webViewClient() const; SimCompositor& compositor(); + Vector<String>& consoleMessages() { return m_consoleMessages; } + private: + friend class SimWebFrameClient; + + void addConsoleMessage(const String&); + SimNetwork m_network; SimCompositor m_compositor; SimWebViewClient m_webViewClient; + SimWebFrameClient m_webFrameClient; SimPage m_page; FrameTestHelpers::WebViewHelper m_webViewHelper; + + Vector<String> m_consoleMessages; }; } // namespace blink
diff --git a/third_party/WebKit/Source/web/tests/sim/SimWebFrameClient.cpp b/third_party/WebKit/Source/web/tests/sim/SimWebFrameClient.cpp new file mode 100644 index 0000000..2cc36dd6 --- /dev/null +++ b/third_party/WebKit/Source/web/tests/sim/SimWebFrameClient.cpp
@@ -0,0 +1,21 @@ +// Copyright 2017 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 "web/tests/sim/SimWebFrameClient.h" + +#include "public/web/WebConsoleMessage.h" +#include "web/tests/sim/SimTest.h" + +namespace blink { + +SimWebFrameClient::SimWebFrameClient(SimTest& test) : m_test(&test) {} + +void SimWebFrameClient::didAddMessageToConsole(const WebConsoleMessage& message, + const WebString& sourceName, + unsigned sourceLine, + const WebString& stackTrace) { + m_test->addConsoleMessage(message.text); +} + +} // namespace blink
diff --git a/third_party/WebKit/Source/web/tests/sim/SimWebFrameClient.h b/third_party/WebKit/Source/web/tests/sim/SimWebFrameClient.h new file mode 100644 index 0000000..7be7f2a --- /dev/null +++ b/third_party/WebKit/Source/web/tests/sim/SimWebFrameClient.h
@@ -0,0 +1,30 @@ +// Copyright 2017 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 SimWebFrameClient_h +#define SimWebFrameClient_h + +#include "web/tests/FrameTestHelpers.h" + +namespace blink { + +class SimTest; + +class SimWebFrameClient final : public FrameTestHelpers::TestWebFrameClient { + public: + explicit SimWebFrameClient(SimTest&); + + // WebFrameClient overrides: + void didAddMessageToConsole(const WebConsoleMessage&, + const WebString& sourceName, + unsigned sourceLine, + const WebString& stackTrace) override; + + private: + SimTest* m_test; +}; + +} // namespace blink + +#endif
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index e652ad92..3c6c35c6 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml
@@ -3015,6 +3015,157 @@ </summary> </histogram> +<histogram name="AuthPolicy.ErrorTypeOfAuthenticateUser" + enum="AuthPolicyErrorType"> + <owner>ljusten@chromium.org</owner> + <summary> + Result from an attempt to authenticate a user to an Active Directory domain. + </summary> +</histogram> + +<histogram name="AuthPolicy.ErrorTypeOfJoinADDomain" enum="AuthPolicyErrorType"> + <owner>ljusten@chromium.org</owner> + <summary> + Result from an attempt to join a machine to an Active Directory domain. + </summary> +</histogram> + +<histogram name="AuthPolicy.ErrorTypeOfRefreshDevicePolicy" + enum="AuthPolicyErrorType"> + <owner>ljusten@chromium.org</owner> + <summary> + Result from an attempt to fetch device policy from an Active Directory + domain and store it on disk. + </summary> +</histogram> + +<histogram name="AuthPolicy.ErrorTypeOfRefreshUserPolicy" + enum="AuthPolicyErrorType"> + <owner>ljusten@chromium.org</owner> + <summary> + Result from an attempt to fetch user policy from an Active Directory domain + and store it on disk. + </summary> +</histogram> + +<histogram name="AuthPolicy.FailedTriesOfKinit"> + <owner>ljusten@chromium.org</owner> + <summary> + Number of times 'kinit' failed until the next try succeeded or the method + gave up because a maximum number of tries was exceeded. 'kinit' is run for + Active Directory enrolled devices during user authentication and device + policy fetch. + </summary> +</histogram> + +<histogram name="AuthPolicy.FailedTriesOfSmbClient"> + <owner>ljusten@chromium.org</owner> + <summary> + Number of times 'smbclient' failed until it the next try succeeded or the + method gave up because a maximum number of tries was exceeded. 'smbclient' + is run for Active Directory enrolled devices during policy fetch. + </summary> +</histogram> + +<histogram name="AuthPolicy.NumGposToDownload"> + <owner>ljusten@chromium.org</owner> + <summary> + Number of group policy objects attached to a specific user or machine on an + Active Directory domain. This value is recorded when user or device policy + is fetched from an Active Directory server. + </summary> +</histogram> + +<histogram name="AuthPolicy.TimeToAuthenticateUser" units="ms"> + <owner>ljusten@chromium.org</owner> + <summary> + Time in milliseconds to authenticate a user to an Active Directory domain. + The value is recorded no matter if the operation was successful or not. + </summary> +</histogram> + +<histogram name="AuthPolicy.TimeToJoinADDomain" units="ms"> + <owner>ljusten@chromium.org</owner> + <summary> + Time in milliseconds to join a machine to an Active Directory domain. Domain + join is part of the Active Directory enrollment process. The value is + recorded no matter if the operation was successful or not. + </summary> +</histogram> + +<histogram name="AuthPolicy.TimeToRefreshDevicePolicy" units="ms"> + <owner>ljusten@chromium.org</owner> + <summary> + Time in milliseconds to fetch device policy from an Active Directory domain + and store it on disk. The value is recorded no matter if the operation was + successful or not. + </summary> +</histogram> + +<histogram name="AuthPolicy.TimeToRefreshUserPolicy" units="ms"> + <owner>ljusten@chromium.org</owner> + <summary> + Time in milliseconds to fetch user policy from an Active Directory domain + and store it on disk. The value is recorded no matter if the operation was + successful or not. + </summary> +</histogram> + +<histogram name="AuthPolicy.TimeToRunKinit" units="ms"> + <owner>ljusten@chromium.org</owner> + <summary> + Time in milliseconds to call 'kinit' (request Kerberos ticket-granting- + ticket). TGTs are requested regularly for accessing services on Active + Directory domains. The value is recorded no matter if the operation was + successful or not. + </summary> +</histogram> + +<histogram name="AuthPolicy.TimeToRunNetAdsInfo" units="ms"> + <owner>ljusten@chromium.org</owner> + <summary> + Time in milliseconds to call 'net ads info' (query Active Directory + information). The value is recorded no matter if the operation was + successful or not. + </summary> +</histogram> + +<histogram name="AuthPolicy.TimeToRunNetAdsJoin" units="ms"> + <owner>ljusten@chromium.org</owner> + <summary> + Time in milliseconds to call 'net ads join' (join machine to an Active + Directory domain). The value is recorded no matter if the operation was + successful or not. + </summary> +</histogram> + +<histogram name="AuthPolicy.TimeToRunNetAdsSearch" units="ms"> + <owner>ljusten@chromium.org</owner> + <summary> + Time in milliseconds to call 'net ads search' (query information about an + Active Directory account). The value is recorded no matter if the operation + was successful or not. + </summary> +</histogram> + +<histogram name="AuthPolicy.TimeToRunNetAdsWorkgroup" units="ms"> + <owner>ljusten@chromium.org</owner> + <summary> + Time in milliseconds to call 'net ads workgroup' (query Active Directory + workgroup). The value is recorded no matter if the operation was successful + or not. + </summary> +</histogram> + +<histogram name="AuthPolicy.TimeToRunSmbclient" units="ms"> + <owner>ljusten@chromium.org</owner> + <summary> + Time in milliseconds to call 'smbclient' (download policy from an Active + Directory domain). The value is recorded no matter if the operation was + successful or not. + </summary> +</histogram> + <histogram name="Autocheckout.Bubble" enum="AutocheckoutBubble"> <obsolete> Deprecated as of 8/2013. @@ -43870,59 +44021,6 @@ </summary> </histogram> -<histogram name="Omnibox.ZeroSuggest.Eligible.OnFocus" - enum="ZeroSuggestEligibleOnFocus"> - <owner>mpearson@chromium.org</owner> - <summary> - Whether the user has settings configured so that the current page URL can be - sent to the suggest server to request contextual suggestions. For example, - this is only supported for users who have Google as their default search - engine (unmodified version of Google), have search suggest enabled, are - signed-in and syncing without a custom passphrase, and don't have an - incognito window open. There are other criteria too. Recorded on focus in - the omnibox if there is default search provider and we've constructed a - suggest URL. - - Some additional guidelines: if an incognito window is open, all focus events - will go into the "generally ineligible" bucket. Likewise, if the - current page is a search results page, we don't allow contextual suggestions - either so focus events on those pages go in the "generally - ineligible" bucket. The difference between "eligible" and - "generally eligible but not this time" depends only the properties - of the current URL. - - Recorded regardless of whether contextual or non-contextual zero suggest is - currently enabled on the user's platform. However, if zero suggest (in all - forms) is entirely disabled, the user will be perpetually ineligible. - </summary> -</histogram> - -<histogram name="Omnibox.ZeroSuggest.Eligible.OnProfileOpen" - enum="BooleanSupported"> - <owner>mpearson@chromium.org</owner> - <summary> - Whether the user has settings configured so that the current page URL could - be sent to the suggest server to request contextual suggestions. For - example, this is only supported for users who have Google as their default - search engine (unmodified version of Google), have search suggest enabled, - and are signed-in and syncing without a custom passphrase. There are other - criteria too. Recorded on profile open. Note that opening an incognito - window (if none are currently open under the given profile) counts as - opening a new profile. - - Some additional guidelines: unlike Omnibox.ZeroSuggest.Eligible.OnFocus, - because this is recorded on profile open, users cannot be declared - ineligible because they have an incognito window open (it's impossible to - have an incognito window open for a given profile at the time of profile - open) and also cannot be declared ineligible because the user is viewing a - search results page. (We test this on-profile-open using an arbitrary URL.) - - Recorded regardless of whether contextual or non-contextual zero suggest is - currently enabled on the user's platform. However, if zero suggest (in all - forms) is entirely disabled, the user will be perpetually ineligible. - </summary> -</histogram> - <histogram name="Omnibox.ZeroSuggest.MostVisitedResultsCounterfactual"> <owner>hfung@chromium.org</owner> <summary> @@ -80989,6 +81087,35 @@ <int value="2" label="Processing in WebRTC"/> </enum> +<enum name="AuthPolicyErrorType" type="int"> + <int value="0" label="Success"/> + <int value="1" label="Unspecified error"/> + <int value="2" label="Unspecified D-Bus error"/> + <int value="3" label="Badly formatted user principal name"/> + <int value="4" label="Auth failed because of bad user name"/> + <int value="5" label="Auth failed because of bad password"/> + <int value="6" label="Auth failed because of expired password"/> + <int value="7" label="Auth failed because of bad realm or network"/> + <int value="8" label="kinit exited with unspecified error"/> + <int value="9" label="net exited with unspecified error"/> + <int value="10" label="smdclient exited with unspecified error"/> + <int value="11" label="authpolicy_parser exited with unknown error"/> + <int value="12" label="Parsing GPOs failed"/> + <int value="13" label="GPO data is bad"/> + <int value="14" label="Some local IO operation failed"/> + <int value="15" label="Machine is not joined to AD domain yet"/> + <int value="16" label="User is not logged in yet"/> + <int value="17" label="Failed to send policy to Session Manager"/> + <int value="18" + label="User doesn't have the right to join machines to the domain"/> + <int value="19" label="General network problem"/> + <int value="20" label="Machine name contains restricted characters"/> + <int value="21" label="Machine name too long"/> + <int value="22" label="User joined maximum number of machines to the domain"/> + <int value="23" + label="kinit or smbclient failed to contact Key Distribution Center"/> +</enum> + <enum name="AutocheckoutBubble" type="int"> <obsolete> Deprecated as of 8/2013. @@ -108234,6 +108361,8 @@ <int value="5" label="Bad domain id"/> <int value="6" label="Duplicate domain hash"/> <int value="7" label="Bad delay_before_prompt_seconds param"/> + <int value="8" label="Bad prompt_wave param"/> + <int value="9" label="Bad time_between_prompts_seconds param"/> </enum> <enum name="SettingsSections" type="int"> @@ -111197,6 +111326,10 @@ <int value="22" label="kSwReporterPromptSeed"/> <int value="23" label="kGoogleServicesAccountId"/> <int value="24" label="kGoogleServicesLastAccountId"/> + <int value="25" label="kSettingsResetPromptPromptWave"/> + <int value="26" label="kSettingsResetPromptLastTriggeredForDefaultSearch"/> + <int value="27" label="kSettingsResetPromptLastTriggeredForStartupUrls"/> + <int value="28" label="kSettingsResetPromptLastTriggeredForHomepage"/> </enum> <enum name="TranslateBubbleUiEvent" type="int"> @@ -113612,21 +113745,6 @@ </int> </enum> -<enum name="ZeroSuggestEligibleOnFocus" type="int"> - <int value="0" label="Eligible"> - URL can be currently sent to the suggest server. - </int> - <int value="1" - label="Generally eligible in current context but particular URL - ineligible"> - URL cannot be sent to the suggest server but another URL would be eligible - at this time. - </int> - <int value="2" label="Generally ineligible in current context"> - No URL can be sent to the suggest server at this time. - </int> -</enum> - </enums> <!-- Histogram suffixes list -->
diff --git a/ui/message_center/views/custom_notification_view.cc b/ui/message_center/views/custom_notification_view.cc index ce671e0..90a946b 100644 --- a/ui/message_center/views/custom_notification_view.cc +++ b/ui/message_center/views/custom_notification_view.cc
@@ -87,12 +87,7 @@ const gfx::Insets insets = GetInsets(); const int contents_width = kNotificationWidth - insets.width(); const int contents_height = contents_view_->GetHeightForWidth(contents_width); - - // This is union of min height for M (64) and N (92). - constexpr int kMinContentHeight = 64; - return gfx::Size(kNotificationWidth, - std::max(kMinContentHeight + insets.height(), - contents_height + insets.height())); + return gfx::Size(kNotificationWidth, contents_height + insets.height()); } void CustomNotificationView::Layout() {
diff --git a/ui/message_center/views/custom_notification_view_unittest.cc b/ui/message_center/views/custom_notification_view_unittest.cc index 6da7c55..4b9e13d5 100644 --- a/ui/message_center/views/custom_notification_view_unittest.cc +++ b/ui/message_center/views/custom_notification_view_unittest.cc
@@ -384,11 +384,11 @@ size.Enlarge(0, -notification_view()->GetInsets().height()); EXPECT_EQ("360x100", size.ToString()); - // The minimum height is 64px. + // Allow small notifications. custom_view()->set_preferred_size(gfx::Size(10, 10)); size = notification_view()->GetPreferredSize(); size.Enlarge(0, -notification_view()->GetInsets().height()); - EXPECT_EQ("360x64", size.ToString()); + EXPECT_EQ("360x10", size.ToString()); // The long notification. custom_view()->set_preferred_size(gfx::Size(1000, 1000));
diff --git a/ui/views/view.cc b/ui/views/view.cc index 04c5f2f..897dacd 100644 --- a/ui/views/view.cc +++ b/ui/views/view.cc
@@ -195,9 +195,11 @@ // If |view| has a parent, remove it from its parent. View* parent = view->parent_; - ui::NativeTheme* old_theme = NULL; + ui::NativeTheme* old_theme = nullptr; + Widget* old_widget = nullptr; if (parent) { old_theme = view->GetNativeTheme(); + old_widget = view->GetWidget(); if (parent == this) { ReorderChildView(view, index); return; @@ -241,7 +243,7 @@ for (View* v = this; v; v = v->parent_) v->ViewHierarchyChangedImpl(false, details); - view->PropagateAddNotifications(details); + view->PropagateAddNotifications(details, widget && widget != old_widget); UpdateTooltip(); @@ -1516,6 +1518,10 @@ } } +void View::AddedToWidget() {} + +void View::RemovedFromWidget() {} + // Painting -------------------------------------------------------------------- void View::PaintChildren(const ui::PaintContext& context) { @@ -1929,12 +1935,14 @@ } Widget* widget = GetWidget(); + bool is_removed_from_widget = false; if (widget) { UnregisterChildrenForVisibleBoundsNotification(view); if (view->visible()) view->SchedulePaint(); - if (!new_parent || new_parent->GetWidget() != widget) + is_removed_from_widget = !new_parent || new_parent->GetWidget() != widget; + if (is_removed_from_widget) widget->NotifyWillRemoveView(view); } @@ -1944,7 +1952,7 @@ if (widget) widget->LayerTreeChanged(); - view->PropagateRemoveNotifications(this, new_parent); + view->PropagateRemoveNotifications(this, new_parent, is_removed_from_widget); view->parent_ = nullptr; if (delete_removed_view && !view->owned_by_client_) @@ -1965,26 +1973,35 @@ observer.OnChildViewRemoved(view, this); } -void View::PropagateRemoveNotifications(View* old_parent, View* new_parent) { +void View::PropagateRemoveNotifications(View* old_parent, + View* new_parent, + bool is_removed_from_widget) { { internal::ScopedChildrenLock lock(this); - for (auto* child : children_) - child->PropagateRemoveNotifications(old_parent, new_parent); + for (auto* child : children_) { + child->PropagateRemoveNotifications(old_parent, new_parent, + is_removed_from_widget); + } } ViewHierarchyChangedDetails details(false, old_parent, this, new_parent); for (View* v = this; v; v = v->parent_) v->ViewHierarchyChangedImpl(true, details); + + if (is_removed_from_widget) + RemovedFromWidget(); } -void View::PropagateAddNotifications( - const ViewHierarchyChangedDetails& details) { +void View::PropagateAddNotifications(const ViewHierarchyChangedDetails& details, + bool is_added_to_widget) { { internal::ScopedChildrenLock lock(this); for (auto* child : children_) - child->PropagateAddNotifications(details); + child->PropagateAddNotifications(details, is_added_to_widget); } ViewHierarchyChangedImpl(true, details); + if (is_added_to_widget) + AddedToWidget(); } void View::PropagateNativeViewHierarchyChanged() {
diff --git a/ui/views/view.h b/ui/views/view.h index fcbee85..2d23890 100644 --- a/ui/views/view.h +++ b/ui/views/view.h
@@ -1102,6 +1102,9 @@ // required when a view is added or removed from a view hierarchy // // Refer to comments in struct |ViewHierarchyChangedDetails| for |details|. + // + // See also AddedToWidget() and RemovedFromWidget() for detecting when the + // view is added to/removed from a widget. virtual void ViewHierarchyChanged(const ViewHierarchyChangedDetails& details); // When SetVisible() changes the visibility of a view, this method is @@ -1116,6 +1119,15 @@ // FocusManager manages this view. virtual void NativeViewHierarchyChanged(); + // This method is invoked for a view when it is attached to a hierarchy with + // a widget, i.e. GetWidget() starts returning a non-null result. + // It is also called when the view is moved to a different widget. + virtual void AddedToWidget(); + + // This method is invoked for a view when it is removed from a hierarchy with + // a widget or moved to a different widget. + virtual void RemovedFromWidget(); + // Painting ------------------------------------------------------------------ // Responsible for calling Paint() on child Views. Override to control the @@ -1298,10 +1310,16 @@ // |old_parent| is the original parent of the View that was removed. // If |new_parent| is not NULL, the View that was removed will be reparented // to |new_parent| after the remove operation. - void PropagateRemoveNotifications(View* old_parent, View* new_parent); + // If is_removed_from_widget is true, calls RemovedFromWidget for all + // children. + void PropagateRemoveNotifications(View* old_parent, + View* new_parent, + bool is_removed_from_widget); // Call ViewHierarchyChanged() for all children. - void PropagateAddNotifications(const ViewHierarchyChangedDetails& details); + // If is_added_to_widget is true, calls AddedToWidget for all children. + void PropagateAddNotifications(const ViewHierarchyChangedDetails& details, + bool is_added_to_widget); // Propagates NativeViewHierarchyChanged() notification through all the // children.
diff --git a/ui/views/view_unittest.cc b/ui/views/view_unittest.cc index 6fe82775..96cd62a 100644 --- a/ui/views/view_unittest.cc +++ b/ui/views/view_unittest.cc
@@ -3335,6 +3335,146 @@ EXPECT_EQ(v2.get(), v4->add_details().move_view); } +class WidgetObserverView : public View { + public: + WidgetObserverView(); + ~WidgetObserverView() override; + + void ResetTestState(); + + int added_to_widget_count() { return added_to_widget_count_; } + int removed_from_widget_count() { return removed_from_widget_count_; } + + private: + void AddedToWidget() override; + void RemovedFromWidget() override; + + int added_to_widget_count_ = 0; + int removed_from_widget_count_ = 0; + + DISALLOW_COPY_AND_ASSIGN(WidgetObserverView); +}; + +WidgetObserverView::WidgetObserverView() { + ResetTestState(); +} + +WidgetObserverView::~WidgetObserverView() {} + +void WidgetObserverView::ResetTestState() { + added_to_widget_count_ = 0; + removed_from_widget_count_ = 0; +} + +void WidgetObserverView::AddedToWidget() { + ++added_to_widget_count_; +} + +void WidgetObserverView::RemovedFromWidget() { + ++removed_from_widget_count_; +} + +// Verifies that AddedToWidget and RemovedFromWidget are called for a view when +// it is added to hierarchy. +// The tree looks like this: +// widget +// +-- root +// +// then v1 is added to root: +// +// v1 +// +-- v2 +// +// finally v1 is removed from root. +TEST_F(ViewTest, AddedToRemovedFromWidget) { + Widget widget; + Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP); + params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET; + params.bounds = gfx::Rect(50, 50, 650, 650); + widget.Init(params); + + View* root = widget.GetRootView(); + + WidgetObserverView v1; + WidgetObserverView v2; + WidgetObserverView v3; + v1.set_owned_by_client(); + v2.set_owned_by_client(); + v3.set_owned_by_client(); + + v1.AddChildView(&v2); + EXPECT_EQ(0, v2.added_to_widget_count()); + EXPECT_EQ(0, v2.removed_from_widget_count()); + + root->AddChildView(&v1); + EXPECT_EQ(1, v1.added_to_widget_count()); + EXPECT_EQ(0, v1.removed_from_widget_count()); + EXPECT_EQ(1, v2.added_to_widget_count()); + EXPECT_EQ(0, v2.removed_from_widget_count()); + + v1.ResetTestState(); + v2.ResetTestState(); + + v2.AddChildView(&v3); + EXPECT_EQ(0, v1.added_to_widget_count()); + EXPECT_EQ(0, v1.removed_from_widget_count()); + EXPECT_EQ(0, v2.added_to_widget_count()); + EXPECT_EQ(0, v2.removed_from_widget_count()); + + v1.ResetTestState(); + v2.ResetTestState(); + + root->RemoveChildView(&v1); + EXPECT_EQ(0, v1.added_to_widget_count()); + EXPECT_EQ(1, v1.removed_from_widget_count()); + EXPECT_EQ(0, v2.added_to_widget_count()); + EXPECT_EQ(1, v2.removed_from_widget_count()); + + v2.ResetTestState(); + v1.RemoveChildView(&v2); + EXPECT_EQ(0, v2.removed_from_widget_count()); + + // Test move between parents in a single Widget. + v2.RemoveChildView(&v3); + v1.ResetTestState(); + v2.ResetTestState(); + v3.ResetTestState(); + + v1.AddChildView(&v2); + root->AddChildView(&v1); + root->AddChildView(&v3); + EXPECT_EQ(1, v1.added_to_widget_count()); + EXPECT_EQ(1, v2.added_to_widget_count()); + EXPECT_EQ(1, v3.added_to_widget_count()); + + v3.AddChildView(&v1); + EXPECT_EQ(1, v1.added_to_widget_count()); + EXPECT_EQ(0, v1.removed_from_widget_count()); + EXPECT_EQ(1, v2.added_to_widget_count()); + EXPECT_EQ(0, v2.removed_from_widget_count()); + EXPECT_EQ(1, v3.added_to_widget_count()); + EXPECT_EQ(0, v3.removed_from_widget_count()); + + // Test move between widgets. + Widget second_widget; + params.bounds = gfx::Rect(150, 150, 650, 650); + second_widget.Init(params); + + View* second_root = second_widget.GetRootView(); + + v1.ResetTestState(); + v2.ResetTestState(); + v3.ResetTestState(); + + second_root->AddChildView(&v1); + EXPECT_EQ(1, v1.removed_from_widget_count()); + EXPECT_EQ(1, v1.added_to_widget_count()); + EXPECT_EQ(1, v2.added_to_widget_count()); + EXPECT_EQ(1, v2.removed_from_widget_count()); + EXPECT_EQ(0, v3.added_to_widget_count()); + EXPECT_EQ(0, v3.removed_from_widget_count()); +} + // Verifies if the child views added under the root are all deleted when calling // RemoveAllChildViews. // The tree looks like this: