diff --git a/DEPS b/DEPS index b1565750..7aeee83 100644 --- a/DEPS +++ b/DEPS
@@ -40,11 +40,11 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling Skia # and whatever else without interference from each other. - 'skia_revision': '95a34fe28a9de960ab9821c6e21fdfc3038f79c8', + 'skia_revision': '26acbe8a89250a7c06f43d1668c3a0dea9da289d', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other. - 'v8_revision': '2d0a8587e23d3dd7b7f7218707df718357b36a67', + 'v8_revision': '5ec87e41b4b216167e449f2bf477459e93d72907', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling swarming_client # and whatever else without interference from each other.
diff --git a/chrome/android/java/res/layout/password_entry_editor_interactive.xml b/chrome/android/java/res/layout/password_entry_editor_interactive.xml index 7fe27b99..e7bdd3a 100644 --- a/chrome/android/java/res/layout/password_entry_editor_interactive.xml +++ b/chrome/android/java/res/layout/password_entry_editor_interactive.xml
@@ -83,6 +83,15 @@ </LinearLayout> + <TextView + android:id="@+id/passwords_link" + android:textAppearance="?android:attr/textAppearanceMedium" + android:textColor="@color/default_text_color" + android:layout_width="fill_parent" + android:layout_height="wrap_content" + android:layout_marginTop="15dp" + android:visibility="gone"/> + </LinearLayout> </ScrollView>
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java index bd9e1a2..5f83108 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEntryEditor.java
@@ -11,9 +11,17 @@ import android.content.ClipData; import android.content.ClipboardManager; import android.content.Context; +import android.content.Intent; +import android.net.Uri; import android.os.Build; import android.os.Bundle; import android.text.InputType; +import android.text.SpannableString; +import android.text.Spanned; +import android.text.TextPaint; +import android.text.method.LinkMovementMethod; +import android.text.style.ClickableSpan; +import android.text.style.ForegroundColorSpan; import android.view.LayoutInflater; import android.view.Menu; import android.view.MenuInflater; @@ -25,12 +33,16 @@ import android.widget.ImageButton; import android.widget.TextView; +import org.chromium.base.ApiCompatibilityUtils; import org.chromium.base.VisibleForTesting; import org.chromium.base.metrics.RecordHistogram; import org.chromium.chrome.R; import org.chromium.chrome.browser.ChromeFeatureList; import org.chromium.chrome.browser.PasswordUIView; import org.chromium.chrome.browser.PasswordUIView.PasswordListObserver; +import org.chromium.chrome.browser.sync.ProfileSyncService; +import org.chromium.components.sync.AndroidSyncSettings; +import org.chromium.ui.text.SpanApplier; import org.chromium.ui.widget.Toast; /** @@ -133,8 +145,35 @@ hidePassword(); hookupPasswordButtons(); } else { - mView.findViewById(R.id.password_title).setVisibility(View.GONE); mView.findViewById(R.id.password_data).setVisibility(View.GONE); + if (isPasswordSyncingUser()) { + ForegroundColorSpan colorSpan = + new ForegroundColorSpan(ApiCompatibilityUtils.getColor( + getResources(), R.color.pref_accent_color)); + SpannableString passwordLink = + SpanApplier.applySpans(getString(R.string.manage_passwords_text), + new SpanApplier.SpanInfo("<link>", "</link>", colorSpan)); + ClickableSpan clickableLink = new ClickableSpan() { + @Override + public void onClick(View textView) { + Intent intent = new Intent(Intent.ACTION_VIEW, + Uri.parse(PasswordUIView.getAccountDashboardURL())); + intent.setPackage(getActivity().getPackageName()); + getActivity().startActivity(intent); + } + + @Override + public void updateDrawState(TextPaint ds) {} + }; + passwordLink.setSpan(clickableLink, 0, passwordLink.length(), + Spanned.SPAN_INCLUSIVE_EXCLUSIVE); + TextView passwordsLinkTextView = mView.findViewById(R.id.passwords_link); + passwordsLinkTextView.setVisibility(View.VISIBLE); + passwordsLinkTextView.setText(passwordLink); + passwordsLinkTextView.setMovementMethod(LinkMovementMethod.getInstance()); + } else { + mView.findViewById(R.id.password_title).setVisibility(View.GONE); + } } } } else { @@ -191,6 +230,12 @@ return Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP; } + private boolean isPasswordSyncingUser() { + ProfileSyncService syncService = ProfileSyncService.get(); + return (AndroidSyncSettings.isSyncEnabled(getActivity().getApplicationContext()) + && syncService.isEngineInitialized() && !syncService.isUsingSecondaryPassphrase()); + } + @Override public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) { inflater.inflate(R.menu.password_entry_editor_action_bar_menu, menu);
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn index 34885f2..1ad51ad 100644 --- a/chrome/browser/BUILD.gn +++ b/chrome/browser/BUILD.gn
@@ -2566,6 +2566,7 @@ "service_process/service_process_control.h", "service_process/service_process_control_mac.mm", ] + deps += [ "//chrome/common:service_process_mojom" ] } else { # Partial-only printing support. sources += [
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc index 1a9c42c..d9baddc 100644 --- a/chrome/browser/chrome_browser_main.cc +++ b/chrome/browser/chrome_browser_main.cc
@@ -81,7 +81,6 @@ #include "chrome/browser/prefs/chrome_pref_service_factory.h" #include "chrome/browser/prefs/incognito_mode_prefs.h" #include "chrome/browser/prefs/pref_metrics_service.h" -#include "chrome/browser/printing/cloud_print/cloud_print_proxy_service.h" #include "chrome/browser/printing/cloud_print/cloud_print_proxy_service_factory.h" #include "chrome/browser/process_singleton.h" #include "chrome/browser/profiles/profile.h" @@ -253,9 +252,12 @@ #include "extensions/components/javascript_dialog_extensions_client/javascript_dialog_extension_client_impl.h" #endif // BUILDFLAG(ENABLE_EXTENSIONS) -#if BUILDFLAG(ENABLE_PRINT_PREVIEW) && !defined(OFFICIAL_BUILD) +#if BUILDFLAG(ENABLE_PRINT_PREVIEW) +#include "chrome/browser/printing/cloud_print/cloud_print_proxy_service.h" +#if !defined(OFFICIAL_BUILD) #include "printing/printed_document.h" -#endif // BUILDFLAG(ENABLE_PRINT_PREVIEW) && !defined(OFFICIAL_BUILD) +#endif // !defined(OFFICIAL_BUILD) +#endif // BUILDFLAG(ENABLE_PRINT_PREVIEW) #if BUILDFLAG(ENABLE_RLZ) #include "chrome/browser/rlz/chrome_rlz_tracker_delegate.h"
diff --git a/chrome/browser/password_manager/chrome_password_manager_client.cc b/chrome/browser/password_manager/chrome_password_manager_client.cc index 39be8b9e..b99df4b 100644 --- a/chrome/browser/password_manager/chrome_password_manager_client.cc +++ b/chrome/browser/password_manager/chrome_password_manager_client.cc
@@ -246,6 +246,12 @@ IsPasswordManagementEnabledForCurrentPage(); } +bool ChromePasswordManagerClient::IsFillingFallbackEnabledForCurrentPage() + const { + return !Profile::FromBrowserContext(web_contents()->GetBrowserContext()) + ->IsGuestSession(); +} + void ChromePasswordManagerClient::PostHSTSQueryForHost( const GURL& origin, const HSTSCallback& callback) const {
diff --git a/chrome/browser/password_manager/chrome_password_manager_client.h b/chrome/browser/password_manager/chrome_password_manager_client.h index 2d279116..7f0aa884 100644 --- a/chrome/browser/password_manager/chrome_password_manager_client.h +++ b/chrome/browser/password_manager/chrome_password_manager_client.h
@@ -51,6 +51,7 @@ // PasswordManagerClient implementation. bool IsSavingAndFillingEnabledForCurrentPage() const override; bool IsFillingEnabledForCurrentPage() const override; + bool IsFillingFallbackEnabledForCurrentPage() const override; void PostHSTSQueryForHost(const GURL& origin, const HSTSCallback& callback) const override; bool OnCredentialManagerUsed() override;
diff --git a/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc b/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc index ac18905..05898e3 100644 --- a/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc +++ b/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc
@@ -28,15 +28,27 @@ #include "chrome/browser/profiles/profile.h" #include "chrome/browser/service_process/service_process_control.h" #include "chrome/common/chrome_switches.h" -#include "chrome/common/cloud_print/cloud_print_proxy_info.h" #include "chrome/common/pref_names.h" -#include "chrome/common/service_messages.h" #include "components/prefs/pref_service.h" #include "content/public/browser/browser_thread.h" #include "printing/backend/print_backend.h" using content::BrowserThread; +namespace { + +void ForwardGetPrintersResult( + const CloudPrintProxyService::PrintersCallback& callback, + const std::vector<std::string>& printers) { + UMA_HISTOGRAM_ENUMERATION("CloudPrint.ServiceEvents", + ServiceProcessControl::SERVICE_PRINTERS_REPLY, + ServiceProcessControl::SERVICE_EVENT_MAX); + UMA_HISTOGRAM_COUNTS_10000("CloudPrint.AvailablePrinters", printers.size()); + callback.Run(printers); +} + +} // namespace + CloudPrintProxyService::CloudPrintProxyService(Profile* profile) : profile_(profile), weak_factory_(this) { @@ -161,18 +173,25 @@ void CloudPrintProxyService::GetCloudPrintProxyPrinters( const PrintersCallback& callback) { DCHECK_CURRENTLY_ON(BrowserThread::UI); + UMA_HISTOGRAM_ENUMERATION("CloudPrint.ServiceEvents", + ServiceProcessControl::SERVICE_PRINTERS_REQUEST, + ServiceProcessControl::SERVICE_EVENT_MAX); ServiceProcessControl* process_control = GetServiceProcessControl(); DCHECK(process_control->IsConnected()); - process_control->GetPrinters(callback); + GetCloudPrintProxy().GetPrinters( + base::BindOnce(&ForwardGetPrintersResult, callback)); } void CloudPrintProxyService::RefreshCloudPrintProxyStatus() { DCHECK_CURRENTLY_ON(BrowserThread::UI); + UMA_HISTOGRAM_ENUMERATION("CloudPrint.ServiceEvents", + ServiceProcessControl::SERVICE_EVENT_INFO_REQUEST, + ServiceProcessControl::SERVICE_EVENT_MAX); ServiceProcessControl* process_control = GetServiceProcessControl(); DCHECK(process_control->IsConnected()); - ServiceProcessControl::CloudPrintProxyInfoCallback callback = base::Bind( - &CloudPrintProxyService::ProxyInfoCallback, base::Unretained(this)); - process_control->GetCloudPrintProxyInfo(callback); + auto callback = base::BindOnce(&CloudPrintProxyService::ProxyInfoCallback, + base::Unretained(this)); + GetCloudPrintProxy().GetCloudPrintProxyInfo(std::move(callback)); } void CloudPrintProxyService::EnableCloudPrintProxyWithRobot( @@ -182,9 +201,9 @@ const base::DictionaryValue* user_preferences) { ServiceProcessControl* process_control = GetServiceProcessControl(); DCHECK(process_control->IsConnected()); - process_control->Send( - new ServiceMsg_EnableCloudPrintProxyWithRobot( - robot_auth_code, robot_email, user_email, *user_preferences)); + GetCloudPrintProxy().EnableCloudPrintProxyWithRobot( + robot_auth_code, robot_email, user_email, + user_preferences->CreateDeepCopy()); // Assume the IPC worked. profile_->GetPrefs()->SetString(prefs::kCloudPrintEmail, user_email); } @@ -192,17 +211,20 @@ void CloudPrintProxyService::DisableCloudPrintProxy() { ServiceProcessControl* process_control = GetServiceProcessControl(); DCHECK(process_control->IsConnected()); - process_control->Send(new ServiceMsg_DisableCloudPrintProxy); + GetCloudPrintProxy().DisableCloudPrintProxy(); // Assume the IPC worked. profile_->GetPrefs()->SetString(prefs::kCloudPrintEmail, std::string()); } -void CloudPrintProxyService::ProxyInfoCallback( - const cloud_print::CloudPrintProxyInfo& proxy_info) { - proxy_id_ = proxy_info.proxy_id; - profile_->GetPrefs()->SetString( - prefs::kCloudPrintEmail, - proxy_info.enabled ? proxy_info.email : std::string()); +void CloudPrintProxyService::ProxyInfoCallback(bool enabled, + const std::string& email, + const std::string& proxy_id) { + UMA_HISTOGRAM_ENUMERATION("CloudPrint.ServiceEvents", + ServiceProcessControl::SERVICE_EVENT_INFO_REPLY, + ServiceProcessControl::SERVICE_EVENT_MAX); + proxy_id_ = proxy_id; + profile_->GetPrefs()->SetString(prefs::kCloudPrintEmail, + enabled ? email : std::string()); ApplyCloudPrintConnectorPolicy(); } @@ -214,3 +236,11 @@ ServiceProcessControl* CloudPrintProxyService::GetServiceProcessControl() { return ServiceProcessControl::GetInstance(); } + +cloud_print::mojom::CloudPrint& CloudPrintProxyService::GetCloudPrintProxy() { + if (!cloud_print_proxy_ || cloud_print_proxy_.encountered_error()) { + GetServiceProcessControl()->remote_interfaces().GetInterface( + &cloud_print_proxy_); + } + return *cloud_print_proxy_; +}
diff --git a/chrome/browser/printing/cloud_print/cloud_print_proxy_service.h b/chrome/browser/printing/cloud_print/cloud_print_proxy_service.h index ac30e5c..776db58 100644 --- a/chrome/browser/printing/cloud_print/cloud_print_proxy_service.h +++ b/chrome/browser/printing/cloud_print/cloud_print_proxy_service.h
@@ -12,6 +12,7 @@ #include "base/macros.h" #include "base/memory/weak_ptr.h" #include "base/observer_list.h" +#include "chrome/common/cloud_print.mojom.h" #include "components/keyed_service/core/keyed_service.h" #include "components/prefs/pref_change_registrar.h" @@ -22,10 +23,6 @@ class DictionaryValue; } // namespace base -namespace cloud_print { -struct CloudPrintProxyInfo; -} // namespace cloud_print - // Layer between the browser user interface and the cloud print proxy code // running in the service process. class CloudPrintProxyService : public KeyedService { @@ -73,8 +70,9 @@ void DisableCloudPrintProxy(); // Callback that gets the cloud print proxy info. - void ProxyInfoCallback( - const cloud_print::CloudPrintProxyInfo& proxy_info); + void ProxyInfoCallback(bool enabled, + const std::string& email, + const std::string& proxy_id); // Invoke a task that gets run after the service process successfully // launches. The task typically involves sending an IPC to the service @@ -85,15 +83,19 @@ // not set or the connector is not enabled). bool ApplyCloudPrintConnectorPolicy(); - Profile* profile_; - std::string proxy_id_; + cloud_print::mojom::CloudPrint& GetCloudPrintProxy(); // Virtual for testing. virtual ServiceProcessControl* GetServiceProcessControl(); + Profile* profile_; + std::string proxy_id_; + // For watching for connector policy changes. PrefChangeRegistrar pref_change_registrar_; + cloud_print::mojom::CloudPrintPtr cloud_print_proxy_; + base::WeakPtrFactory<CloudPrintProxyService> weak_factory_; DISALLOW_COPY_AND_ASSIGN(CloudPrintProxyService);
diff --git a/chrome/browser/printing/cloud_print/cloud_print_proxy_service_unittest.cc b/chrome/browser/printing/cloud_print/cloud_print_proxy_service_unittest.cc index 1075eacd..6de22a51 100644 --- a/chrome/browser/printing/cloud_print/cloud_print_proxy_service_unittest.cc +++ b/chrome/browser/printing/cloud_print/cloud_print_proxy_service_unittest.cc
@@ -19,9 +19,9 @@ #include "chrome/browser/service_process/service_process_control.h" #include "chrome/browser/ui/startup/startup_browser_creator.h" #include "chrome/common/chrome_switches.h" +#include "chrome/common/cloud_print.mojom.h" #include "chrome/common/cloud_print/cloud_print_proxy_info.h" #include "chrome/common/pref_names.h" -#include "chrome/common/service_messages.h" #include "chrome/test/base/testing_browser_process.h" #include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile_manager.h" @@ -29,6 +29,7 @@ #include "components/sync_preferences/testing_pref_service_syncable.h" #include "content/public/browser/browser_thread.h" #include "content/public/test/test_browser_thread_bundle.h" +#include "mojo/public/cpp/bindings/binding_set.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -55,27 +56,7 @@ MOCK_METHOD2(Launch, void(const base::Closure&, const base::Closure&)); MOCK_METHOD0(Disconnect, void()); - MOCK_METHOD1(OnMessageReceived, bool(const IPC::Message&)); - MOCK_METHOD1(OnChannelConnected, void(int32_t peer_pid)); - MOCK_METHOD0(OnChannelError, void()); - - MOCK_METHOD1(Send, bool(IPC::Message*)); - - typedef enum { - kServiceStateDisabled, - kServiceStateEnabled, - kServiceStateNone - } ServiceState; - - void SetConnectSuccessMockExpectations(ServiceState state, bool post_task); - - void SetServiceEnabledExpectations(); - void SetServiceDisabledExpectations(); - void SetWillBeEnabledExpectations(); - void SetWillBeDisabledExpectations(); - - bool SendEnabledInfo(); - bool SendDisabledInfo(); + void SetConnectSuccessMockExpectations(bool post_task); private: bool connected_; @@ -98,7 +79,6 @@ } void MockServiceProcessControl::SetConnectSuccessMockExpectations( - ServiceState service_state, bool post_task) { EXPECT_CALL(*this, IsConnected()).WillRepeatedly(ReturnPointee(&connected_)); @@ -110,71 +90,82 @@ EXPECT_CALL(*this, Disconnect()).Times(AtMost(1)) .WillRepeatedly(Assign(&connected_, false)); - EXPECT_CALL(*this, Send(_)).Times(0); - - if (service_state == kServiceStateEnabled) - SetServiceEnabledExpectations(); - else if (service_state == kServiceStateDisabled) - SetServiceDisabledExpectations(); } -void MockServiceProcessControl::SetServiceEnabledExpectations() { - EXPECT_CALL(*this, Send(Property(&IPC::Message::type, - static_cast<int32_t>( - ServiceMsg_GetCloudPrintProxyInfo::ID)))) - .Times(1) - .WillOnce(DoAll(DeleteArg<0>(), - WithoutArgs(Invoke( - this, &MockServiceProcessControl::SendEnabledInfo)))); -} +class MockCloudPrintProxy : public cloud_print::mojom::CloudPrint { + public: + void AddBinding(cloud_print::mojom::CloudPrintRequest request) { + bindings_.AddBinding(this, std::move(request)); + } -void MockServiceProcessControl::SetServiceDisabledExpectations() { - EXPECT_CALL(*this, Send(Property(&IPC::Message::type, - static_cast<int32_t>( - ServiceMsg_GetCloudPrintProxyInfo::ID)))) - .Times(1) - .WillOnce( - DoAll(DeleteArg<0>(), - WithoutArgs(Invoke( - this, &MockServiceProcessControl::SendDisabledInfo)))); -} + void ReturnDisabledInfo() { + cloud_proxy_info_expectation_set_ = true; + cloud_proxy_info_.enabled = false; + cloud_proxy_info_.email.clear(); + } -void MockServiceProcessControl::SetWillBeEnabledExpectations() { - int32_t message_id = ServiceMsg_EnableCloudPrintProxyWithRobot::ID; - EXPECT_CALL( - *this, - Send(Property(&IPC::Message::type, message_id))) - .Times(1).WillOnce(DoAll(DeleteArg<0>(), Return(true))); -} + void ReturnEnabledInfo() { + cloud_proxy_info_expectation_set_ = true; + cloud_proxy_info_.enabled = true; + cloud_proxy_info_.email = MockServiceProcessControl::EnabledUserId(); + } -void MockServiceProcessControl::SetWillBeDisabledExpectations() { - EXPECT_CALL(*this, Send(Property(&IPC::Message::type, - static_cast<int32_t>( - ServiceMsg_DisableCloudPrintProxy::ID)))) - .Times(1) - .WillOnce(DoAll(DeleteArg<0>(), Return(true))); -} + bool has_been_enabled() { + bindings_.FlushForTesting(); + return enabled_; + } + bool has_been_disabled() { + bindings_.FlushForTesting(); + return disabled_; + } -bool MockServiceProcessControl::SendEnabledInfo() { - info_.enabled = true; - info_.email = EnabledUserId(); - PostTask(base::Bind(&MockServiceProcessControl::OnCloudPrintProxyInfo, - base::Unretained(this), info_)); - return true; -} + private: + void GetCloudPrintProxyInfo( + GetCloudPrintProxyInfoCallback callback) override { + EXPECT_TRUE(cloud_proxy_info_expectation_set_); + std::move(callback).Run(cloud_proxy_info_.enabled, cloud_proxy_info_.email, + cloud_proxy_info_.proxy_id); + } + void GetPrinters(GetPrintersCallback callback) override { NOTREACHED(); } + void DisableCloudPrintProxy() override { disabled_ = true; } -bool MockServiceProcessControl::SendDisabledInfo() { - info_.enabled = false; - info_.email = std::string(); - PostTask(base::Bind(&MockServiceProcessControl::OnCloudPrintProxyInfo, - base::Unretained(this), info_)); - return true; -} + void EnableCloudPrintProxyWithRobot( + const std::string& robot_auth_code, + const std::string& robot_email, + const std::string& user_email, + std::unique_ptr<base::DictionaryValue> user_settings) override { + enabled_ = true; + } + + mojo::BindingSet<cloud_print::mojom::CloudPrint> bindings_; + + bool cloud_proxy_info_expectation_set_ = false; + cloud_print::CloudPrintProxyInfo cloud_proxy_info_; + + bool disabled_ = false; + bool enabled_ = false; +}; class TestCloudPrintProxyService : public CloudPrintProxyService { public: explicit TestCloudPrintProxyService(Profile* profile) - : CloudPrintProxyService(profile) { } + : CloudPrintProxyService(profile) { + service_manager::InterfaceProvider::TestApi test_api( + &process_control_.remote_interfaces()); + test_api.SetBinderForName( + "cloud_print::mojom::CloudPrint", + base::Bind(&TestCloudPrintProxyService::HandleCloudPrintProxyRequest, + base::Unretained(this))); + service_manager::mojom::InterfaceProviderPtr handle; + mojo::MakeRequest(&handle); + process_control_.SetMojoHandle(std::move(handle)); + } + + ~TestCloudPrintProxyService() override { + service_manager::InterfaceProvider::TestApi test_api( + &ServiceProcessControl::GetInstance()->remote_interfaces()); + test_api.ClearBinderForName("cloud_print::mojom::CloudPrint"); + } void Initialize() { CloudPrintProxyService::Initialize(); @@ -189,10 +180,13 @@ ServiceProcessControl* GetServiceProcessControl() override { return &process_control_; } + MockServiceProcessControl* GetMockServiceProcessControl() { return &process_control_; } + MockCloudPrintProxy& GetMockCloudPrintProxy() { return mock_proxy_; } + void EnableForUser() { EnableForUserWithRobot("123", "123@gmail.com", MockServiceProcessControl::EnabledUserId(), @@ -200,7 +194,13 @@ } private: + void HandleCloudPrintProxyRequest(mojo::ScopedMessagePipeHandle handle) { + mock_proxy_.AddBinding( + cloud_print::mojom::CloudPrintRequest(std::move(handle))); + } + MockServiceProcessControl process_control_; + MockCloudPrintProxy mock_proxy_; }; class CloudPrintProxyPolicyTest : public ::testing::Test { @@ -221,8 +221,7 @@ TEST_F(CloudPrintProxyPolicyTest, VerifyExpectations) { MockServiceProcessControl mock_control; - mock_control.SetConnectSuccessMockExpectations( - MockServiceProcessControl::kServiceStateNone, false); + mock_control.SetConnectSuccessMockExpectations(false); EXPECT_FALSE(mock_control.IsConnected()); mock_control.Launch(base::Closure(), base::Closure()); @@ -236,8 +235,9 @@ TEST_F(CloudPrintProxyPolicyTest, StartWithNoPolicyProxyDisabled) { TestCloudPrintProxyService service(&profile_); + service.GetMockCloudPrintProxy().ReturnDisabledInfo(); service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations( - MockServiceProcessControl::kServiceStateDisabled, false); + false); sync_preferences::TestingPrefServiceSyncable* prefs = profile_.GetTestingPrefService(); @@ -254,7 +254,8 @@ TestCloudPrintProxyService service(&profile_); service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations( - MockServiceProcessControl::kServiceStateEnabled, false); + false); + service.GetMockCloudPrintProxy().ReturnEnabledInfo(); sync_preferences::TestingPrefServiceSyncable* prefs = profile_.GetTestingPrefService(); @@ -272,7 +273,8 @@ TestCloudPrintProxyService service(&profile_); service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations( - MockServiceProcessControl::kServiceStateDisabled, false); + false); + service.GetMockCloudPrintProxy().ReturnDisabledInfo(); sync_preferences::TestingPrefServiceSyncable* prefs = profile_.GetTestingPrefService(); @@ -290,8 +292,8 @@ TestCloudPrintProxyService service(&profile_); service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations( - MockServiceProcessControl::kServiceStateEnabled, false); - service.GetMockServiceProcessControl()->SetWillBeDisabledExpectations(); + false); + service.GetMockCloudPrintProxy().ReturnEnabledInfo(); sync_preferences::TestingPrefServiceSyncable* prefs = profile_.GetTestingPrefService(); @@ -303,13 +305,15 @@ service.Initialize(); EXPECT_EQ(std::string(), prefs->GetString(prefs::kCloudPrintEmail)); + EXPECT_TRUE(service.GetMockCloudPrintProxy().has_been_disabled()); } TEST_F(CloudPrintProxyPolicyTest, StartWithNoPolicyProxyDisabledThenSetPolicy) { TestCloudPrintProxyService service(&profile_); service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations( - MockServiceProcessControl::kServiceStateDisabled, false); + false); + service.GetMockCloudPrintProxy().ReturnDisabledInfo(); sync_preferences::TestingPrefServiceSyncable* prefs = profile_.GetTestingPrefService(); @@ -331,7 +335,8 @@ TestCloudPrintProxyService service(&profile_); service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations( - MockServiceProcessControl::kServiceStateEnabled, false); + false); + service.GetMockCloudPrintProxy().ReturnEnabledInfo(); sync_preferences::TestingPrefServiceSyncable* prefs = profile_.GetTestingPrefService(); @@ -344,11 +349,11 @@ EXPECT_EQ(MockServiceProcessControl::EnabledUserId(), prefs->GetString(prefs::kCloudPrintEmail)); - service.GetMockServiceProcessControl()->SetWillBeDisabledExpectations(); prefs->SetManagedPref(prefs::kCloudPrintProxyEnabled, base::MakeUnique<base::Value>(false)); EXPECT_EQ(std::string(), prefs->GetString(prefs::kCloudPrintEmail)); + EXPECT_TRUE(service.GetMockCloudPrintProxy().has_been_disabled()); } TEST_F(CloudPrintProxyPolicyTest, @@ -356,7 +361,8 @@ TestCloudPrintProxyService service(&profile_); service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations( - MockServiceProcessControl::kServiceStateDisabled, false); + false); + service.GetMockCloudPrintProxy().ReturnDisabledInfo(); sync_preferences::TestingPrefServiceSyncable* prefs = profile_.GetTestingPrefService(); @@ -377,8 +383,8 @@ TestCloudPrintProxyService service(&profile_); service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations( - MockServiceProcessControl::kServiceStateEnabled, false); - service.GetMockServiceProcessControl()->SetWillBeDisabledExpectations(); + false); + service.GetMockCloudPrintProxy().ReturnEnabledInfo(); sync_preferences::TestingPrefServiceSyncable* prefs = profile_.GetTestingPrefService(); @@ -392,13 +398,15 @@ EXPECT_EQ(std::string(), prefs->GetString(prefs::kCloudPrintEmail)); prefs->RemoveManagedPref(prefs::kCloudPrintProxyEnabled); EXPECT_EQ(std::string(), prefs->GetString(prefs::kCloudPrintEmail)); + EXPECT_TRUE(service.GetMockCloudPrintProxy().has_been_disabled()); } TEST_F(CloudPrintProxyPolicyTest, StartWithNoPolicyProxyDisabledThenEnable) { TestCloudPrintProxyService service(&profile_); service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations( - MockServiceProcessControl::kServiceStateDisabled, false); + false); + service.GetMockCloudPrintProxy().ReturnDisabledInfo(); sync_preferences::TestingPrefServiceSyncable* prefs = profile_.GetTestingPrefService(); @@ -409,11 +417,11 @@ service.Initialize(); EXPECT_EQ(std::string(), prefs->GetString(prefs::kCloudPrintEmail)); - service.GetMockServiceProcessControl()->SetWillBeEnabledExpectations(); service.EnableForUser(); EXPECT_EQ(MockServiceProcessControl::EnabledUserId(), prefs->GetString(prefs::kCloudPrintEmail)); + EXPECT_TRUE(service.GetMockCloudPrintProxy().has_been_enabled()); } TEST_F(CloudPrintProxyPolicyTest, @@ -421,8 +429,8 @@ TestCloudPrintProxyService service(&profile_); service.GetMockServiceProcessControl()->SetConnectSuccessMockExpectations( - MockServiceProcessControl::kServiceStateEnabled, false); - service.GetMockServiceProcessControl()->SetWillBeDisabledExpectations(); + false); + service.GetMockCloudPrintProxy().ReturnEnabledInfo(); sync_preferences::TestingPrefServiceSyncable* prefs = profile_.GetTestingPrefService(); @@ -440,9 +448,9 @@ prefs->RemoveManagedPref(prefs::kCloudPrintProxyEnabled); EXPECT_EQ(std::string(), prefs->GetString(prefs::kCloudPrintEmail)); - service.GetMockServiceProcessControl()->SetWillBeEnabledExpectations(); service.EnableForUser(); EXPECT_EQ(MockServiceProcessControl::EnabledUserId(), prefs->GetString(prefs::kCloudPrintEmail)); + EXPECT_TRUE(service.GetMockCloudPrintProxy().has_been_enabled()); }
diff --git a/chrome/browser/printing/cloud_print/test/cloud_print_proxy_process_browsertest.cc b/chrome/browser/printing/cloud_print/test/cloud_print_proxy_process_browsertest.cc index 15ae0a6..a1394919 100644 --- a/chrome/browser/printing/cloud_print/test/cloud_print_proxy_process_browsertest.cc +++ b/chrome/browser/printing/cloud_print/test/cloud_print_proxy_process_browsertest.cc
@@ -34,9 +34,10 @@ #include "chrome/browser/ui/startup/startup_browser_creator.h" #include "chrome/common/chrome_content_client.h" #include "chrome/common/chrome_switches.h" +#include "chrome/common/cloud_print/cloud_print_proxy_info.h" #include "chrome/common/pref_names.h" -#include "chrome/common/service_messages.h" #include "chrome/common/service_process_util.h" +#include "chrome/service/cloud_print/cloud_print_message_handler.h" #include "chrome/service/service_ipc_server.h" #include "chrome/service/service_process.h" #include "chrome/test/base/chrome_unit_test_suite.h" @@ -49,7 +50,9 @@ #include "content/public/common/content_paths.h" #include "content/public/test/test_browser_thread_bundle.h" #include "content/public/test/test_utils.h" +#include "ipc/ipc.mojom.h" #include "ipc/ipc_channel_mojo.h" +#include "ipc/ipc_channel_proxy.h" #include "mojo/edk/embedder/embedder.h" #include "mojo/edk/embedder/named_platform_handle.h" #include "mojo/edk/embedder/named_platform_handle_utils.h" @@ -148,25 +151,16 @@ ServiceIPCServer::Client* client, const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner, base::WaitableEvent* shutdown_event) - : ServiceIPCServer(client, io_task_runner, shutdown_event), - enabled_(true) {} + : ServiceIPCServer(client, io_task_runner, shutdown_event) {} - MOCK_METHOD1(OnMessageReceived, bool(const IPC::Message& message)); MOCK_METHOD1(OnChannelConnected, void(int32_t peer_pid)); MOCK_METHOD0(OnChannelError, void()); + MOCK_METHOD0(ShutDown, void()); void SetServiceEnabledExpectations(); - void SetWillBeDisabledExpectations(); - - void CallServiceOnChannelConnected(int32_t peer_pid) { - ServiceIPCServer::OnChannelConnected(peer_pid); - } - - bool SendInfo(); private: cloud_print::CloudPrintProxyInfo info_; - bool enabled_; }; // static @@ -175,53 +169,12 @@ } void MockServiceIPCServer::SetServiceEnabledExpectations() { - EXPECT_CALL(*this, OnChannelConnected(_)).Times(1) - .WillRepeatedly( - Invoke(this, &MockServiceIPCServer::CallServiceOnChannelConnected)); - EXPECT_CALL(*this, OnChannelError()).Times(0); - EXPECT_CALL(*this, OnMessageReceived(_)).Times(0); - - EXPECT_CALL(*this, - OnMessageReceived(Property( - &IPC::Message::type, - static_cast<int32_t>(ServiceMsg_GetCloudPrintProxyInfo::ID)))) - .Times(AnyNumber()) - .WillRepeatedly( - WithoutArgs(Invoke(this, &MockServiceIPCServer::SendInfo))); - - EXPECT_CALL(*this, OnMessageReceived(Property( - &IPC::Message::type, - static_cast<int32_t>(ServiceMsg_Shutdown::ID)))) + EXPECT_CALL(*this, ShutDown()) .Times(1) .WillOnce(DoAll( Assign(&g_good_shutdown, true), - WithoutArgs(Invoke(g_service_process, &ServiceProcess::Shutdown)), - Return(true))); -} - -void MockServiceIPCServer::SetWillBeDisabledExpectations() { - SetServiceEnabledExpectations(); - - EXPECT_CALL(*this, - OnMessageReceived(Property( - &IPC::Message::type, - static_cast<int32_t>(ServiceMsg_DisableCloudPrintProxy::ID)))) - .Times(AtLeast(1)) - .WillRepeatedly(DoAll(Assign(&enabled_, false), Return(true))); -} - -bool MockServiceIPCServer::SendInfo() { - if (enabled_) { - info_.enabled = true; - info_.email = EnabledUserId(); - EXPECT_TRUE(Send(new ServiceHostMsg_CloudPrintProxy_Info(info_))); - } else { - info_.enabled = false; - info_.email = std::string(); - EXPECT_TRUE(Send(new ServiceHostMsg_CloudPrintProxy_Info(info_))); - } - return true; + WithoutArgs(Invoke(g_service_process, &::ServiceProcess::Shutdown)))); } typedef base::Callback<void(MockServiceIPCServer* server)> @@ -274,6 +227,8 @@ MockServiceIPCServer server(&service_process, service_process.io_task_runner(), service_process.GetShutdownEventForTesting()); + server.binder_registry().AddInterface(base::Bind( + &cloud_print::CloudPrintMessageHandler::Create, &service_process)); // Here is where the expectations/mock responses need to be set up. set_expectations.Run(&server); @@ -322,15 +277,6 @@ base::Bind(&SetServiceEnabledExpectations)); } -void SetServiceWillBeDisabledExpectations(MockServiceIPCServer* server) { - server->SetWillBeDisabledExpectations(); -} - -MULTIPROCESS_TEST_MAIN(CloudPrintMockService_StartEnabledExpectDisabled) { - return CloudPrintMockService_Main( - base::Bind(&SetServiceWillBeDisabledExpectations)); -} - class CloudPrintProxyPolicyStartupTest : public base::MultiProcessTest, public IPC::Listener { public: @@ -345,7 +291,6 @@ } base::Process Launch(const std::string& name); void WaitForConnect(mojo::edk::PeerConnection* peer_connection); - bool Send(IPC::Message* message); void ShutdownAndWaitForExitWithTimeout(base::Process process); // IPC::Listener implementation @@ -495,25 +440,23 @@ FROM_HERE, {base::MayBlock(), base::TaskPriority::BACKGROUND}, base::BindOnce(&ConnectAsync, base::Passed(&pipe.handle1), GetServiceProcessChannel(), peer_connection)); - ServiceProcessControl::GetInstance()->SetChannel( - IPC::ChannelProxy::Create(IPC::ChannelMojo::CreateClientFactory( - std::move(pipe.handle0), IOTaskRunner()), - this, IOTaskRunner())); -} - -bool CloudPrintProxyPolicyStartupTest::Send(IPC::Message* message) { - return ServiceProcessControl::GetInstance()->Send(message); + ServiceProcessControl::GetInstance()->SetMojoHandle( + mojo::MakeProxy(service_manager::mojom::InterfaceProviderPtrInfo( + std::move(pipe.handle0), 0U))); } void CloudPrintProxyPolicyStartupTest::ShutdownAndWaitForExitWithTimeout( base::Process process) { - ASSERT_TRUE(Send(new ServiceMsg_Shutdown())); + chrome::mojom::ServiceProcessPtr service_process; + ServiceProcessControl::GetInstance()->remote_interfaces().GetInterface( + &service_process); + service_process->ShutDown(); int exit_code = -100; bool exited = process.WaitForExitWithTimeout(TestTimeouts::action_timeout(), &exit_code); EXPECT_TRUE(exited); - EXPECT_EQ(exit_code, 0); + EXPECT_EQ(0, exit_code); } void CloudPrintProxyPolicyStartupTest::OnChannelConnected(int32_t peer_pid) {
diff --git a/chrome/browser/service_process/service_process_control.cc b/chrome/browser/service_process/service_process_control.cc index 887ca122..2fcaee9 100644 --- a/chrome/browser/service_process/service_process_control.cc +++ b/chrome/browser/service_process/service_process_control.cc
@@ -26,10 +26,8 @@ #include "build/build_config.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/upgrade_detector.h" -#include "chrome/common/service_messages.h" #include "chrome/common/service_process_util.h" #include "content/public/browser/browser_thread.h" -#include "ipc/ipc_channel_mojo.h" #include "mojo/edk/embedder/embedder.h" #include "mojo/edk/embedder/named_platform_handle.h" #include "mojo/edk/embedder/named_platform_handle_utils.h" @@ -48,7 +46,7 @@ base::TimeDelta::FromMilliseconds(20); void ConnectAsyncWithBackoff( - mojo::ScopedMessagePipeHandle handle, + service_manager::mojom::InterfaceProviderRequest interface_provider_request, mojo::edk::NamedPlatformHandle os_pipe, size_t num_retries_left, base::TimeDelta retry_delay, @@ -64,10 +62,10 @@ } else { base::PostDelayedTaskWithTraits( FROM_HERE, {base::MayBlock(), base::TaskPriority::BACKGROUND}, - base::BindOnce(&ConnectAsyncWithBackoff, std::move(handle), - std::move(os_pipe), num_retries_left - 1, - retry_delay * 2, std::move(response_task_runner), - std::move(response_callback)), + base::BindOnce( + &ConnectAsyncWithBackoff, std::move(interface_provider_request), + std::move(os_pipe), num_retries_left - 1, retry_delay * 2, + std::move(response_task_runner), std::move(response_callback)), retry_delay); } } else { @@ -75,7 +73,7 @@ mojo::FuseMessagePipes( peer_connection->Connect(mojo::edk::ConnectionParams( mojo::edk::TransportProtocol::kLegacy, std::move(os_pipe_handle))), - std::move(handle)); + interface_provider_request.PassMessagePipe()); response_task_runner->PostTask(FROM_HERE, base::BindOnce(std::move(response_callback), std::move(peer_connection))); @@ -97,7 +95,7 @@ void ServiceProcessControl::ConnectInternal() { // If the channel has already been established then we run the task // and return. - if (channel_.get()) { + if (service_process_) { RunConnectDoneTasks(); return; } @@ -105,22 +103,17 @@ // Actually going to connect. DVLOG(1) << "Connecting to Service Process IPC Server"; - mojo::MessagePipe pipe; + service_manager::mojom::InterfaceProviderPtr remote_interfaces; + auto interface_provider_request = mojo::MakeRequest(&remote_interfaces); + SetMojoHandle(std::move(remote_interfaces)); base::PostTaskWithTraits( FROM_HERE, {base::MayBlock(), base::TaskPriority::BACKGROUND}, base::BindOnce( - &ConnectAsyncWithBackoff, std::move(pipe.handle1), + &ConnectAsyncWithBackoff, std::move(interface_provider_request), GetServiceProcessChannel(), kMaxConnectionAttempts, kInitialConnectionRetryDelay, base::ThreadTaskRunnerHandle::Get(), base::BindOnce(&ServiceProcessControl::OnPeerConnectionComplete, weak_factory_.GetWeakPtr()))); - // TODO(hclam): Handle error connecting to channel. - auto io_task_runner = - BrowserThread::GetTaskRunnerForThread(BrowserThread::IO); - SetChannel( - IPC::ChannelProxy::Create(IPC::ChannelMojo::CreateClientFactory( - std::move(pipe.handle0), io_task_runner), - this, io_task_runner)); } void ServiceProcessControl::OnPeerConnectionComplete( @@ -129,9 +122,17 @@ peer_connection_ = std::move(peer_connection); } -void ServiceProcessControl::SetChannel( - std::unique_ptr<IPC::ChannelProxy> channel) { - channel_ = std::move(channel); +void ServiceProcessControl::SetMojoHandle( + service_manager::mojom::InterfaceProviderPtr handle) { + remote_interfaces_.Close(); + remote_interfaces_.Bind(std::move(handle)); + remote_interfaces_.SetConnectionLostClosure(base::Bind( + &ServiceProcessControl::OnChannelError, base::Unretained(this))); + + // TODO(hclam): Handle error connecting to channel. + remote_interfaces_.GetInterface(&service_process_); + service_process_->Hello(base::BindOnce( + &ServiceProcessControl::OnChannelConnected, base::Unretained(this))); } void ServiceProcessControl::RunConnectDoneTasks() { @@ -163,7 +164,7 @@ } bool ServiceProcessControl::IsConnected() const { - return channel_ != NULL; + return !!service_process_; } void ServiceProcessControl::Launch(const base::Closure& success_task, @@ -200,8 +201,9 @@ void ServiceProcessControl::Disconnect() { DCHECK_CURRENTLY_ON(BrowserThread::UI); - channel_.reset(); peer_connection_.reset(); + remote_interfaces_.Close(); + service_process_.reset(); } void ServiceProcessControl::OnProcessLaunched() { @@ -224,19 +226,12 @@ launcher_ = NULL; } -bool ServiceProcessControl::OnMessageReceived(const IPC::Message& message) { - bool handled = true; - IPC_BEGIN_MESSAGE_MAP(ServiceProcessControl, message) - IPC_MESSAGE_HANDLER(ServiceHostMsg_CloudPrintProxy_Info, - OnCloudPrintProxyInfo) - IPC_MESSAGE_HANDLER(ServiceHostMsg_Histograms, OnHistograms) - IPC_MESSAGE_HANDLER(ServiceHostMsg_Printers, OnPrinters) - IPC_MESSAGE_UNHANDLED(handled = false) - IPC_END_MESSAGE_MAP() - return handled; +void ServiceProcessControl::OnUpgradeRecommended() { + if (apply_changes_from_upgrade_observer_) + service_process_->UpdateAvailable(); } -void ServiceProcessControl::OnChannelConnected(int32_t peer_pid) { +void ServiceProcessControl::OnChannelConnected() { DCHECK_CURRENTLY_ON(BrowserThread::UI); UMA_HISTOGRAM_ENUMERATION("CloudPrint.ServiceEvents", @@ -245,7 +240,7 @@ // We just established a channel with the service process. Notify it if an // upgrade is available. if (UpgradeDetector::GetInstance()->notify_upgrade()) - Send(new ServiceMsg_UpdateAvailable); + service_process_->UpdateAvailable(); else apply_changes_from_upgrade_observer_ = true; @@ -258,34 +253,10 @@ UMA_HISTOGRAM_ENUMERATION("CloudPrint.ServiceEvents", SERVICE_EVENT_CHANNEL_ERROR, SERVICE_EVENT_MAX); - channel_.reset(); - peer_connection_.reset(); + Disconnect(); RunConnectDoneTasks(); } -bool ServiceProcessControl::Send(IPC::Message* message) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); - if (!channel_.get()) - return false; - return channel_->Send(message); -} - -void ServiceProcessControl::OnUpgradeRecommended() { - if (apply_changes_from_upgrade_observer_) - Send(new ServiceMsg_UpdateAvailable); -} - -void ServiceProcessControl::OnCloudPrintProxyInfo( - const cloud_print::CloudPrintProxyInfo& proxy_info) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); - UMA_HISTOGRAM_ENUMERATION("CloudPrint.ServiceEvents", - SERVICE_EVENT_INFO_REPLY, SERVICE_EVENT_MAX); - if (!cloud_print_info_callback_.is_null()) { - cloud_print_info_callback_.Run(proxy_info); - cloud_print_info_callback_.Reset(); - } -} - void ServiceProcessControl::OnHistograms( const std::vector<std::string>& pickled_histograms) { UMA_HISTOGRAM_ENUMERATION("CloudPrint.ServiceEvents", @@ -305,31 +276,6 @@ histograms_timeout_callback_.Cancel(); } -void ServiceProcessControl::OnPrinters( - const std::vector<std::string>& printers) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); - UMA_HISTOGRAM_ENUMERATION( - "CloudPrint.ServiceEvents", SERVICE_PRINTERS_REPLY, SERVICE_EVENT_MAX); - UMA_HISTOGRAM_COUNTS_10000("CloudPrint.AvailablePrinters", printers.size()); - if (!printers_callback_.is_null()) { - printers_callback_.Run(printers); - printers_callback_.Reset(); - } -} - -bool ServiceProcessControl::GetCloudPrintProxyInfo( - const CloudPrintProxyInfoCallback& cloud_print_info_callback) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); - DCHECK(!cloud_print_info_callback.is_null()); - cloud_print_info_callback_.Reset(); - UMA_HISTOGRAM_ENUMERATION("CloudPrint.ServiceEvents", - SERVICE_EVENT_INFO_REQUEST, SERVICE_EVENT_MAX); - if (!Send(new ServiceMsg_GetCloudPrintProxyInfo())) - return false; - cloud_print_info_callback_ = cloud_print_info_callback; - return true; -} - bool ServiceProcessControl::GetHistograms( const base::Closure& histograms_callback, const base::TimeDelta& timeout) { @@ -351,13 +297,15 @@ SERVICE_EVENT_HISTOGRAMS_REQUEST, SERVICE_EVENT_MAX); - if (!Send(new ServiceMsg_GetHistograms())) + if (!service_process_) return false; + service_process_->GetHistograms(base::BindOnce( + &ServiceProcessControl::OnHistograms, base::Unretained(this))); + // Run timeout task to make sure |histograms_callback| is called. - histograms_timeout_callback_.Reset( - base::Bind(&ServiceProcessControl::RunHistogramsCallback, - base::Unretained(this))); + histograms_timeout_callback_.Reset(base::Bind( + &ServiceProcessControl::RunHistogramsCallback, base::Unretained(this))); BrowserThread::PostDelayedTask(BrowserThread::UI, FROM_HERE, histograms_timeout_callback_.callback(), timeout); @@ -366,25 +314,13 @@ return true; } -bool ServiceProcessControl::GetPrinters( - const PrintersCallback& printers_callback) { - DCHECK_CURRENTLY_ON(BrowserThread::UI); - DCHECK(!printers_callback.is_null()); - printers_callback_.Reset(); - UMA_HISTOGRAM_ENUMERATION( - "CloudPrint.ServiceEvents", SERVICE_PRINTERS_REQUEST, SERVICE_EVENT_MAX); - if (!Send(new ServiceMsg_GetPrinters())) - return false; - printers_callback_ = printers_callback; - return true; -} - bool ServiceProcessControl::Shutdown() { DCHECK_CURRENTLY_ON(BrowserThread::UI); - bool ret = Send(new ServiceMsg_Shutdown()); - channel_.reset(); - peer_connection_.reset(); - return ret; + if (!service_process_) + return false; + service_process_->ShutDown(); + Disconnect(); + return true; } // static
diff --git a/chrome/browser/service_process/service_process_control.h b/chrome/browser/service_process/service_process_control.h index 5daf693..e11a7a5a 100644 --- a/chrome/browser/service_process/service_process_control.h +++ b/chrome/browser/service_process/service_process_control.h
@@ -21,18 +21,13 @@ #include "base/process/process.h" #include "build/build_config.h" #include "chrome/browser/upgrade_observer.h" -#include "ipc/ipc_channel_proxy.h" -#include "ipc/ipc_listener.h" -#include "ipc/ipc_sender.h" +#include "chrome/common/service_process.mojom.h" +#include "services/service_manager/public/cpp/interface_provider.h" namespace base { class CommandLine; } -namespace cloud_print { -struct CloudPrintProxyInfo; -} // namespace cloud_print - namespace mojo { namespace edk { class PeerConnection; @@ -47,11 +42,8 @@ // // THREADING // -// This class is accessed on the UI thread through some UI actions. It then -// talks to the IPC channel on the IO thread. -class ServiceProcessControl : public IPC::Sender, - public IPC::Listener, - public UpgradeObserver { +// This class is accessed on the UI thread through some UI actions. +class ServiceProcessControl : public UpgradeObserver { public: enum ServiceProcessEvent { SERVICE_EVENT_INITIALIZE, @@ -73,13 +65,6 @@ SERVICE_EVENT_MAX, }; - using iterator = base::IDMap<ServiceProcessControl*>::iterator; - using MessageQueue = std::queue<IPC::Message>; - using CloudPrintProxyInfoCallback = - base::Callback<void(const cloud_print::CloudPrintProxyInfo&)>; - using PrintersCallback = - base::Callback<void(const std::vector<std::string>&)>; - // Returns the singleton instance of this class. static ServiceProcessControl* GetInstance(); @@ -105,14 +90,6 @@ // Virtual for testing. virtual void Disconnect(); - // IPC::Listener implementation. - bool OnMessageReceived(const IPC::Message& message) override; - void OnChannelConnected(int32_t peer_pid) override; - void OnChannelError() override; - - // IPC::Sender implementation - bool Send(IPC::Message* message) override; - // UpgradeObserver implementation. void OnUpgradeRecommended() override; @@ -122,14 +99,6 @@ // Virtual for testing. virtual bool Shutdown(); - // Send request for cloud print proxy info (enabled state, email, proxy id). - // The callback gets the information when received. - // Returns true if request was sent. Callback will be called only in case of - // reply from service. The method resets any previous callback. - // This call starts service if needed. - bool GetCloudPrintProxyInfo( - const CloudPrintProxyInfoCallback& cloud_print_status_callback); - // Send request for histograms collected in service process. // Returns true if request was sent, and callback will be called in case of // success or timeout. The method resets any previous callback. @@ -138,12 +107,9 @@ bool GetHistograms(const base::Closure& cloud_print_status_callback, const base::TimeDelta& timeout); - // Send request for printers available for cloud print proxy. - // The callback gets the information when received. - // Returns true if request was sent. Callback will be called only in case of - // reply from service. The method resets any previous callback. - // This call starts service if needed. - bool GetPrinters(const PrintersCallback& enumerate_printers_callback); + service_manager::InterfaceProvider& remote_interfaces() { + return remote_interfaces_; + } private: // This class is responsible for launching the service process on the @@ -178,6 +144,7 @@ friend class MockServiceProcessControl; friend class CloudPrintProxyPolicyStartupTest; + friend class TestCloudPrintProxyService; ServiceProcessControl(); ~ServiceProcessControl() override; @@ -186,11 +153,10 @@ typedef std::vector<base::Closure> TaskList; - // Message handlers - void OnCloudPrintProxyInfo( - const cloud_print::CloudPrintProxyInfo& proxy_info); + void OnChannelConnected(); + void OnChannelError(); + void OnHistograms(const std::vector<std::string>& pickled_histograms); - void OnPrinters(const std::vector<std::string>& printers); // Runs callback provided in |GetHistograms()|. void RunHistogramsCallback(); @@ -208,15 +174,15 @@ void OnPeerConnectionComplete( std::unique_ptr<mojo::edk::PeerConnection> connection); - // Takes ownership of the pointer. Split out for testing. - void SetChannel(std::unique_ptr<IPC::ChannelProxy> channel); + // Split out for testing. + void SetMojoHandle(service_manager::mojom::InterfaceProviderPtr handle); static void RunAllTasksHelper(TaskList* task_list); std::unique_ptr<mojo::edk::PeerConnection> peer_connection_; - // IPC channel to the service process. - std::unique_ptr<IPC::ChannelProxy> channel_; + service_manager::InterfaceProvider remote_interfaces_; + chrome::mojom::ServiceProcessPtr service_process_; // Service process launcher. scoped_refptr<Launcher> launcher_; @@ -226,14 +192,6 @@ // Callbacks that get invoked when there was a connection failure. TaskList connect_failure_tasks_; - // Callback that gets invoked when a printers is received from - // the cloud print proxy. - PrintersCallback printers_callback_; - - // Callback that gets invoked when a status message is received from - // the cloud print proxy. - CloudPrintProxyInfoCallback cloud_print_info_callback_; - // Callback that gets invoked when a message with histograms is received from // the service process. base::Closure histograms_callback_;
diff --git a/chrome/browser/service_process/service_process_control_browsertest.cc b/chrome/browser/service_process/service_process_control_browsertest.cc index b5bbae6d..7526994 100644 --- a/chrome/browser/service_process/service_process_control_browsertest.cc +++ b/chrome/browser/service_process/service_process_control_browsertest.cc
@@ -21,7 +21,7 @@ #include "build/build_config.h" #include "chrome/browser/ui/browser.h" #include "chrome/common/chrome_constants.h" -#include "chrome/common/service_messages.h" +#include "chrome/common/cloud_print.mojom.h" #include "chrome/common/service_process_util.h" #include "chrome/test/base/in_process_browser_test.h" #include "components/version_info/version_info.h" @@ -64,8 +64,9 @@ base::RunLoop::QuitCurrentWhenIdleDeprecated(); } - static void CloudPrintInfoCallback( - const cloud_print::CloudPrintProxyInfo& proxy_info) { + static void CloudPrintInfoCallback(bool enabled, + const std::string& email, + const std::string& proxy_id) { QuitMessageLoop(); } @@ -156,8 +157,11 @@ // Make sure we are connected to the service process. ASSERT_TRUE(ServiceProcessControl::GetInstance()->IsConnected()); - ServiceProcessControl::GetInstance()->GetCloudPrintProxyInfo( - base::Bind(&ServiceProcessControlBrowserTest::CloudPrintInfoCallback)); + cloud_print::mojom::CloudPrintPtr cloud_print_proxy; + ServiceProcessControl::GetInstance()->remote_interfaces().GetInterface( + &cloud_print_proxy); + cloud_print_proxy->GetCloudPrintProxyInfo( + base::Bind(&ServiceProcessControlBrowserTest::CloudPrintInfoCallback)); content::RunMessageLoop(); // And then shutdown the service process. @@ -169,8 +173,11 @@ // Make sure we are connected to the service process. ASSERT_TRUE(ServiceProcessControl::GetInstance()->IsConnected()); - ServiceProcessControl::GetInstance()->GetCloudPrintProxyInfo( - base::Bind(&ServiceProcessControlBrowserTest::CloudPrintInfoCallback)); + cloud_print::mojom::CloudPrintPtr cloud_print_proxy; + ServiceProcessControl::GetInstance()->remote_interfaces().GetInterface( + &cloud_print_proxy); + cloud_print_proxy->GetCloudPrintProxyInfo( + base::Bind(&ServiceProcessControlBrowserTest::CloudPrintInfoCallback)); content::RunMessageLoop(); // And then shutdown the service process. @@ -183,10 +190,15 @@ // Make sure we are connected to the service process. ASSERT_TRUE(ServiceProcessControl::GetInstance()->IsConnected()); // Send an IPC that will keep the service process alive after we disconnect. - ServiceProcessControl::GetInstance()->Send( - new ServiceMsg_EnableCloudPrintProxyWithRobot( - "", "", "", base::DictionaryValue())); - ServiceProcessControl::GetInstance()->GetCloudPrintProxyInfo( + cloud_print::mojom::CloudPrintPtr cloud_print_proxy; + ServiceProcessControl::GetInstance()->remote_interfaces().GetInterface( + &cloud_print_proxy); + cloud_print_proxy->EnableCloudPrintProxyWithRobot( + "", "", "", base::MakeUnique<base::DictionaryValue>()); + + ServiceProcessControl::GetInstance()->remote_interfaces().GetInterface( + &cloud_print_proxy); + cloud_print_proxy->GetCloudPrintProxyInfo( base::Bind(&ServiceProcessControlBrowserTest::CloudPrintInfoCallback)); content::RunMessageLoop(); Disconnect(); @@ -196,7 +208,9 @@ ASSERT_TRUE(ServiceProcessControl::GetInstance()->IsConnected()); content::RunMessageLoop(); - ServiceProcessControl::GetInstance()->GetCloudPrintProxyInfo( + ServiceProcessControl::GetInstance()->remote_interfaces().GetInterface( + &cloud_print_proxy); + cloud_print_proxy->GetCloudPrintProxyInfo( base::Bind(&ServiceProcessControlBrowserTest::CloudPrintInfoCallback)); content::RunMessageLoop(); @@ -218,15 +232,20 @@ // Make sure we are connected to the service process. ASSERT_TRUE(ServiceProcessControl::GetInstance()->IsConnected()); - EXPECT_TRUE(ServiceProcessControl::GetInstance()->GetCloudPrintProxyInfo( - base::Bind(&ServiceProcessControlBrowserTest::CloudPrintInfoCallback))); + cloud_print::mojom::CloudPrintPtr cloud_print_proxy; + ServiceProcessControl::GetInstance()->remote_interfaces().GetInterface( + &cloud_print_proxy); + cloud_print_proxy->GetCloudPrintProxyInfo( + base::Bind(&ServiceProcessControlBrowserTest::CloudPrintInfoCallback)); content::RunMessageLoop(); // Launch the service process again. LaunchServiceProcessControl(true); ASSERT_TRUE(ServiceProcessControl::GetInstance()->IsConnected()); - EXPECT_TRUE(ServiceProcessControl::GetInstance()->GetCloudPrintProxyInfo( - base::Bind(&ServiceProcessControlBrowserTest::CloudPrintInfoCallback))); + ServiceProcessControl::GetInstance()->remote_interfaces().GetInterface( + &cloud_print_proxy); + cloud_print_proxy->GetCloudPrintProxyInfo( + base::Bind(&ServiceProcessControlBrowserTest::CloudPrintInfoCallback)); content::RunMessageLoop(); }
diff --git a/chrome/common/BUILD.gn b/chrome/common/BUILD.gn index 5c4d97e..c6992ca 100644 --- a/chrome/common/BUILD.gn +++ b/chrome/common/BUILD.gn
@@ -418,7 +418,6 @@ if (enable_print_preview) { # Full printing support. sources += [ - "service_messages.h", "service_process_util.cc", "service_process_util.h", "service_process_util_linux.cc", @@ -759,3 +758,13 @@ "//url/mojo:url_mojom_origin", ] } + +mojom("service_process_mojom") { + sources = [ + "cloud_print.mojom", + "service_process.mojom", + ] + public_deps = [ + "//mojo/common:common_custom_types", + ] +}
diff --git a/chrome/common/cloud_print.mojom b/chrome/common/cloud_print.mojom new file mode 100644 index 0000000..5bbbae8 --- /dev/null +++ b/chrome/common/cloud_print.mojom
@@ -0,0 +1,28 @@ +// 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. + +module cloud_print.mojom; + +import "mojo/common/values.mojom"; + +// An interface for controlling cloud print running in a service process. +interface CloudPrint { + // Tell the service process to enable the cloud proxy passing in the OAuth2 + // auth code of a robot account. + EnableCloudPrintProxyWithRobot( + string robot_auth_code, + string robot_email, + string user_email, + mojo.common.mojom.DictionaryValue user_settings); + + // Tell the service process to disable the cloud proxy. + DisableCloudPrintProxy(); + + // Gets the current status of the cloud print proxy + // (whether it is enabled, the email address and the proxy id). + GetCloudPrintProxyInfo() => (bool enabled, string email, string proxy_id); + + // Gets all available printers. + GetPrinters() => (array<string> printers); +};
diff --git a/chrome/common/common_message_generator.h b/chrome/common/common_message_generator.h index fc776be8..4e18db0 100644 --- a/chrome/common/common_message_generator.h +++ b/chrome/common/common_message_generator.h
@@ -22,10 +22,6 @@ #include "chrome/common/extensions/mojom/inline_install_traits.h" #endif -#if BUILDFLAG(ENABLE_PRINT_PREVIEW) -#include "chrome/common/service_messages.h" -#endif - #if BUILDFLAG(ENABLE_PRINTING) #include "chrome/common/chrome_utility_printing_messages.h" #endif
diff --git a/chrome/common/service_messages.h b/chrome/common/service_messages.h deleted file mode 100644 index c1b598b..0000000 --- a/chrome/common/service_messages.h +++ /dev/null
@@ -1,62 +0,0 @@ -// Copyright (c) 2011 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. - -// Multiply-included message file, no traditional include guard. -#include <string> -#include <vector> - -#include "base/values.h" -#include "chrome/common/cloud_print/cloud_print_proxy_info.h" -#include "ipc/ipc_channel_handle.h" -#include "ipc/ipc_message_macros.h" - -#define IPC_MESSAGE_START ServiceMsgStart - -IPC_STRUCT_TRAITS_BEGIN(cloud_print::CloudPrintProxyInfo) - IPC_STRUCT_TRAITS_MEMBER(enabled) - IPC_STRUCT_TRAITS_MEMBER(email) - IPC_STRUCT_TRAITS_MEMBER(proxy_id) -IPC_STRUCT_TRAITS_END() - -// Tell the service process to enable the cloud proxy passing in the OAuth2 -// auth code of a robot account. -IPC_MESSAGE_CONTROL4(ServiceMsg_EnableCloudPrintProxyWithRobot, - std::string /* robot_auth_code */, - std::string /* robot_email */, - std::string /* user_email */, - base::DictionaryValue /* user_settings */) - -// Tell the service process to disable the cloud proxy. -IPC_MESSAGE_CONTROL0(ServiceMsg_DisableCloudPrintProxy) - -// Requests a message back on the current status of the cloud print proxy -// (whether it is enabled, the email address and the proxy id). -IPC_MESSAGE_CONTROL0(ServiceMsg_GetCloudPrintProxyInfo) - -// Requests a message back with serialized UMA histograms. -IPC_MESSAGE_CONTROL0(ServiceMsg_GetHistograms) - -// Requests a message back with all available printers. -IPC_MESSAGE_CONTROL0(ServiceMsg_GetPrinters) - -// Tell the service process to shutdown. -IPC_MESSAGE_CONTROL0(ServiceMsg_Shutdown) - -// Tell the service process that an update is available. -IPC_MESSAGE_CONTROL0(ServiceMsg_UpdateAvailable) - -//----------------------------------------------------------------------------- -// Service process host messages: -// These are messages from the service process to the browser. -// Sent as a response to a request for cloud print proxy info -IPC_MESSAGE_CONTROL1(ServiceHostMsg_CloudPrintProxy_Info, - cloud_print::CloudPrintProxyInfo /* proxy info */) - -// Sent as a response to ServiceMsg_GetHistograms. -IPC_MESSAGE_CONTROL1(ServiceHostMsg_Histograms, - std::vector<std::string> /* pickled_histograms */) - -// Sent as a response to ServiceMsg_GetPrinters. -IPC_MESSAGE_CONTROL1(ServiceHostMsg_Printers, - std::vector<std::string> /* printers */)
diff --git a/chrome/common/service_process.mojom b/chrome/common/service_process.mojom new file mode 100644 index 0000000..ccec7d7a --- /dev/null +++ b/chrome/common/service_process.mojom
@@ -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. + +module chrome.mojom; + +// A control interface for a service process +// (https://www.chromium.org/developers/design-documents/service-processes). +interface ServiceProcess { + // A message for ensuring the connection is established. + Hello() => (); + + // Gets serialized UMA histograms. + GetHistograms() => (array<string> histograms); + + // Tell the service process that an update is available. + UpdateAvailable(); + + // Tell the service process to shutdown. + ShutDown(); +};
diff --git a/chrome/installer/zucchini/mapped_file_unittest.cc b/chrome/installer/zucchini/mapped_file_unittest.cc index a3d6d82..927fb40d 100644 --- a/chrome/installer/zucchini/mapped_file_unittest.cc +++ b/chrome/installer/zucchini/mapped_file_unittest.cc
@@ -26,11 +26,9 @@ }; TEST_F(MappedFileWriterTest, ErrorCreating) { - // Opening |file| in write mode causes failure of |file_writer| with same - // path. - base::File file(file_path_, base::File::FLAG_CREATE_ALWAYS | - base::File::FLAG_READ | - base::File::FLAG_WRITE); + // Create a directory |file_path_|, so |file_writer| fails when it tries to + // open a file with the same name for write. + ASSERT_TRUE(base::CreateDirectory(file_path_)); { MappedFileWriter file_writer(file_path_, 10); EXPECT_FALSE(file_writer.IsValid());
diff --git a/chrome/service/BUILD.gn b/chrome/service/BUILD.gn index d4d8661..eded8ac1 100644 --- a/chrome/service/BUILD.gn +++ b/chrome/service/BUILD.gn
@@ -55,6 +55,7 @@ "//base", "//chrome:strings", "//chrome/common", + "//chrome/common:service_process_mojom", "//components/cloud_devices/common", "//components/data_use_measurement/core", "//components/network_session_configurator/browser",
diff --git a/chrome/service/cloud_print/cloud_print_message_handler.cc b/chrome/service/cloud_print/cloud_print_message_handler.cc index babcad7..58b541aba 100644 --- a/chrome/service/cloud_print/cloud_print_message_handler.cc +++ b/chrome/service/cloud_print/cloud_print_message_handler.cc
@@ -6,59 +6,51 @@ #include <vector> -#include "chrome/common/service_messages.h" -#include "ipc/ipc_sender.h" +#include "chrome/common/cloud_print/cloud_print_proxy_info.h" +#include "mojo/public/cpp/bindings/strong_binding.h" namespace cloud_print { CloudPrintMessageHandler::CloudPrintMessageHandler( - IPC::Sender* ipc_sender, CloudPrintProxy::Provider* proxy_provider) - : ipc_sender_(ipc_sender), proxy_provider_(proxy_provider) { - DCHECK(ipc_sender); + : proxy_provider_(proxy_provider) { DCHECK(proxy_provider); } -CloudPrintMessageHandler::~CloudPrintMessageHandler() { +CloudPrintMessageHandler::~CloudPrintMessageHandler() = default; + +// static +void CloudPrintMessageHandler::Create( + CloudPrintProxy::Provider* proxy_provider, + cloud_print::mojom::CloudPrintRequest request) { + mojo::MakeStrongBinding( + base::MakeUnique<CloudPrintMessageHandler>(proxy_provider), + std::move(request)); } -bool CloudPrintMessageHandler::HandleMessage(const IPC::Message& message) { - bool handled = true; - IPC_BEGIN_MESSAGE_MAP(CloudPrintMessageHandler, message) - IPC_MESSAGE_HANDLER(ServiceMsg_EnableCloudPrintProxyWithRobot, - OnEnableCloudPrintProxyWithRobot) - IPC_MESSAGE_HANDLER(ServiceMsg_DisableCloudPrintProxy, - OnDisableCloudPrintProxy) - IPC_MESSAGE_HANDLER(ServiceMsg_GetCloudPrintProxyInfo, - OnGetCloudPrintProxyInfo) - IPC_MESSAGE_HANDLER(ServiceMsg_GetPrinters, OnGetPrinters) - IPC_MESSAGE_UNHANDLED(handled = false) - IPC_END_MESSAGE_MAP() - return handled; -} - -void CloudPrintMessageHandler::OnEnableCloudPrintProxyWithRobot( +void CloudPrintMessageHandler::EnableCloudPrintProxyWithRobot( const std::string& robot_auth_code, const std::string& robot_email, const std::string& user_email, - const base::DictionaryValue& user_settings) { + std::unique_ptr<base::DictionaryValue> user_settings) { proxy_provider_->GetCloudPrintProxy()->EnableForUserWithRobot( - robot_auth_code, robot_email, user_email, user_settings); + robot_auth_code, robot_email, user_email, *user_settings); } -void CloudPrintMessageHandler::OnGetCloudPrintProxyInfo() { +void CloudPrintMessageHandler::GetCloudPrintProxyInfo( + GetCloudPrintProxyInfoCallback callback) { CloudPrintProxyInfo info; proxy_provider_->GetCloudPrintProxy()->GetProxyInfo(&info); - ipc_sender_->Send(new ServiceHostMsg_CloudPrintProxy_Info(info)); + std::move(callback).Run(info.enabled, info.email, info.proxy_id); } -void CloudPrintMessageHandler::OnGetPrinters() { +void CloudPrintMessageHandler::GetPrinters(GetPrintersCallback callback) { std::vector<std::string> printers; proxy_provider_->GetCloudPrintProxy()->GetPrinters(&printers); - ipc_sender_->Send(new ServiceHostMsg_Printers(printers)); + std::move(callback).Run(printers); } -void CloudPrintMessageHandler::OnDisableCloudPrintProxy() { +void CloudPrintMessageHandler::DisableCloudPrintProxy() { proxy_provider_->GetCloudPrintProxy()->UnregisterPrintersAndDisableForUser(); }
diff --git a/chrome/service/cloud_print/cloud_print_message_handler.h b/chrome/service/cloud_print/cloud_print_message_handler.h index 1a1cb13..81e209ef 100644 --- a/chrome/service/cloud_print/cloud_print_message_handler.h +++ b/chrome/service/cloud_print/cloud_print_message_handler.h
@@ -8,42 +8,35 @@ #include <string> #include "base/macros.h" +#include "chrome/common/cloud_print.mojom.h" #include "chrome/service/cloud_print/cloud_print_proxy.h" -#include "chrome/service/service_ipc_server.h" namespace base { class DictionaryValue; } -namespace IPC { -class Message; -class Sender; -} - namespace cloud_print { // Handles IPC messages for Cloud Print. Lives on the main thread. -class CloudPrintMessageHandler : public ServiceIPCServer::MessageHandler { +class CloudPrintMessageHandler : public cloud_print::mojom::CloudPrint { public: - CloudPrintMessageHandler(IPC::Sender* ipc_sender, - CloudPrintProxy::Provider* proxy_provider); + explicit CloudPrintMessageHandler(CloudPrintProxy::Provider* proxy_provider); ~CloudPrintMessageHandler() override; - // ServiceIPCServer::MessageHandler implementation. - bool HandleMessage(const IPC::Message& message) override; + static void Create(CloudPrintProxy::Provider* proxy_provider, + cloud_print::mojom::CloudPrintRequest request); private: - // IPC message handlers. - void OnEnableCloudPrintProxyWithRobot( + // cloud_print::mojom::CloudPrintProxy. + void EnableCloudPrintProxyWithRobot( const std::string& robot_auth_code, const std::string& robot_email, const std::string& user_email, - const base::DictionaryValue& user_settings); - void OnGetCloudPrintProxyInfo(); - void OnGetPrinters(); - void OnDisableCloudPrintProxy(); + std::unique_ptr<base::DictionaryValue> user_settings) override; + void GetCloudPrintProxyInfo(GetCloudPrintProxyInfoCallback callback) override; + void GetPrinters(GetPrintersCallback callback) override; + void DisableCloudPrintProxy() override; - IPC::Sender* ipc_sender_; // Owned by our owner. CloudPrintProxy::Provider* proxy_provider_; // Owned by our owner. DISALLOW_COPY_AND_ASSIGN(CloudPrintMessageHandler);
diff --git a/chrome/service/service_ipc_server.cc b/chrome/service/service_ipc_server.cc index 9514bd8..6d55cbb 100644 --- a/chrome/service/service_ipc_server.cc +++ b/chrome/service/service_ipc_server.cc
@@ -7,10 +7,6 @@ #include <algorithm> #include "base/metrics/histogram_delta_serialization.h" -#include "build/build_config.h" -#include "chrome/common/service_messages.h" -#include "ipc/ipc_channel_mojo.h" -#include "ipc/ipc_logging.h" ServiceIPCServer::ServiceIPCServer( Client* client, @@ -18,38 +14,30 @@ base::WaitableEvent* shutdown_event) : client_(client), io_task_runner_(io_task_runner), - shutdown_event_(shutdown_event) { + shutdown_event_(shutdown_event), + binding_(this) { DCHECK(client); DCHECK(shutdown_event); + binder_registry_.AddInterface( + base::Bind(&ServiceIPCServer::HandleServiceProcessConnection, + base::Unretained(this))); } bool ServiceIPCServer::Init() { -#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED) - IPC::Logging::GetInstance()->SetIPCSender(this); -#endif CreateChannel(); return true; } void ServiceIPCServer::CreateChannel() { - channel_.reset(); // Tear down the existing channel, if any. - channel_ = IPC::SyncChannel::Create( - IPC::ChannelMojo::CreateServerFactory(client_->CreateChannelMessagePipe(), - io_task_runner_), - this /* listener */, io_task_runner_, true /* create_pipe_now */, - shutdown_event_); + binding_.Close(); + + binding_.Bind(service_manager::mojom::InterfaceProviderRequest( + client_->CreateChannelMessagePipe())); + binding_.set_connection_error_handler( + base::Bind(&ServiceIPCServer::OnChannelError, base::Unretained(this))); } -ServiceIPCServer::~ServiceIPCServer() { -#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED) - IPC::Logging::GetInstance()->SetIPCSender(NULL); -#endif -} - -void ServiceIPCServer::OnChannelConnected(int32_t peer_pid) { - DCHECK(!ipc_client_connected_); - ipc_client_connected_ = true; -} +ServiceIPCServer::~ServiceIPCServer() = default; void ServiceIPCServer::OnChannelError() { // When an IPC client (typically a browser process) disconnects, the pipe is @@ -66,48 +54,12 @@ } } -bool ServiceIPCServer::Send(IPC::Message* msg) { - if (!channel_.get()) { - delete msg; - return false; - } - - return channel_->Send(msg); +void ServiceIPCServer::Hello(HelloCallback callback) { + ipc_client_connected_ = true; + std::move(callback).Run(); } -void ServiceIPCServer::AddMessageHandler( - std::unique_ptr<MessageHandler> handler) { - message_handlers_.push_back(std::move(handler)); -} - -bool ServiceIPCServer::OnMessageReceived(const IPC::Message& msg) { - DCHECK(ipc_client_connected_); - bool handled = true; - IPC_BEGIN_MESSAGE_MAP(ServiceIPCServer, msg) - IPC_MESSAGE_HANDLER(ServiceMsg_GetHistograms, OnGetHistograms) - IPC_MESSAGE_HANDLER(ServiceMsg_Shutdown, OnShutdown); - IPC_MESSAGE_HANDLER(ServiceMsg_UpdateAvailable, OnUpdateAvailable); - IPC_MESSAGE_UNHANDLED(handled = false) - IPC_END_MESSAGE_MAP() - - if (!handled) { - // Make a copy of the handlers to prevent modification during iteration. - std::vector<MessageHandler*> temp_handlers; - temp_handlers.reserve(message_handlers_.size()); - for (const auto& handler : message_handlers_) - temp_handlers.push_back(handler.get()); - - for (auto* handler : temp_handlers) { - handled = handler->HandleMessage(msg); - if (handled) - break; - } - } - - return handled; -} - -void ServiceIPCServer::OnGetHistograms() { +void ServiceIPCServer::GetHistograms(GetHistogramsCallback callback) { if (!histogram_delta_serializer_) { histogram_delta_serializer_.reset( new base::HistogramDeltaSerialization("ServiceProcess")); @@ -117,13 +69,23 @@ // histograms held in persistent storage on the assumption that they will be // visible to the recipient through other means. histogram_delta_serializer_->PrepareAndSerializeDeltas(&deltas, false); - channel_->Send(new ServiceHostMsg_Histograms(deltas)); + std::move(callback).Run(deltas); } -void ServiceIPCServer::OnShutdown() { +void ServiceIPCServer::ShutDown() { client_->OnShutdown(); } -void ServiceIPCServer::OnUpdateAvailable() { +void ServiceIPCServer::UpdateAvailable() { client_->OnUpdateAvailable(); } + +void ServiceIPCServer::GetInterface(const std::string& interface_name, + mojo::ScopedMessagePipeHandle pipe) { + binder_registry_.BindInterface(interface_name, std::move(pipe)); +} + +void ServiceIPCServer::HandleServiceProcessConnection( + chrome::mojom::ServiceProcessRequest request) { + service_process_bindings_.AddBinding(this, std::move(request)); +}
diff --git a/chrome/service/service_ipc_server.h b/chrome/service/service_ipc_server.h index eb25ba2..b48fe50 100644 --- a/chrome/service/service_ipc_server.h +++ b/chrome/service/service_ipc_server.h
@@ -12,10 +12,12 @@ #include "base/macros.h" #include "base/single_thread_task_runner.h" -#include "ipc/ipc_listener.h" -#include "ipc/ipc_sender.h" -#include "ipc/ipc_sync_channel.h" +#include "chrome/common/service_process.mojom.h" +#include "mojo/public/cpp/bindings/binding.h" +#include "mojo/public/cpp/bindings/binding_set.h" #include "mojo/public/cpp/system/message_pipe.h" +#include "services/service_manager/public/cpp/binder_registry.h" +#include "services/service_manager/public/interfaces/interface_provider.mojom.h" namespace base { @@ -25,15 +27,9 @@ } // namespace base // This class handles IPC commands for the service process. -class ServiceIPCServer : public IPC::Listener, public IPC::Sender { +class ServiceIPCServer : public service_manager::mojom::InterfaceProvider, + public chrome::mojom::ServiceProcess { public: - class MessageHandler { - public: - virtual ~MessageHandler() {} - // Must return true if the message is handled. - virtual bool HandleMessage(const IPC::Message& message) = 0; - }; - class Client { public: virtual ~Client() {} @@ -60,14 +56,9 @@ bool Init(); - // IPC::Sender implementation. - bool Send(IPC::Message* msg) override; - - // Registers a MessageHandler with the ServiceIPCServer. When an IPC message - // is received that is not handled by the ServiceIPCServer itself, the - // handlers will be called to handle the message in first-add first-call order - // until it is handled or there are no more handlers. - void AddMessageHandler(std::unique_ptr<MessageHandler> handler); + service_manager::BinderRegistry& binder_registry() { + return binder_registry_; + } bool is_ipc_client_connected() const { return ipc_client_connected_; } @@ -75,24 +66,27 @@ friend class ServiceIPCServerTest; friend class MockServiceIPCServer; - // IPC::Listener implementation. - bool OnMessageReceived(const IPC::Message& msg) override; - void OnChannelConnected(int32_t peer_pid) override; - void OnChannelError() override; - - // IPC message handlers. - void OnGetHistograms(); - void OnShutdown(); - void OnUpdateAvailable(); - // Helper method to create the sync channel. void CreateChannel(); + void OnChannelError(); + + // chrome::mojom::ServiceProcess: + void Hello(HelloCallback callback) override; + void GetHistograms(GetHistogramsCallback callback) override; + void UpdateAvailable() override; + void ShutDown() override; + + // service_manager::mojom::InterfaceProvider: + void GetInterface(const std::string& interface_name, + mojo::ScopedMessagePipeHandle pipe) override; + + void HandleServiceProcessConnection( + chrome::mojom::ServiceProcessRequest request); + Client* client_; scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_; - std::unique_ptr<IPC::SyncChannel> channel_; base::WaitableEvent* shutdown_event_; - std::vector<std::unique_ptr<MessageHandler>> message_handlers_; // Indicates whether an IPC client is currently connected to the channel. bool ipc_client_connected_ = false; @@ -101,6 +95,11 @@ std::unique_ptr<base::HistogramDeltaSerialization> histogram_delta_serializer_; + mojo::Binding<service_manager::mojom::InterfaceProvider> binding_; + mojo::BindingSet<chrome::mojom::ServiceProcess> service_process_bindings_; + + service_manager::BinderRegistry binder_registry_; + DISALLOW_COPY_AND_ASSIGN(ServiceIPCServer); };
diff --git a/chrome/service/service_ipc_server_unittest.cc b/chrome/service/service_ipc_server_unittest.cc index 1585257..8a4efd45 100644 --- a/chrome/service/service_ipc_server_unittest.cc +++ b/chrome/service/service_ipc_server_unittest.cc
@@ -15,10 +15,8 @@ #include "base/synchronization/waitable_event.h" #include "base/threading/thread.h" #include "build/build_config.h" -#include "chrome/common/service_messages.h" -#include "ipc/ipc_channel.h" -#include "ipc/ipc_channel_mojo.h" #include "mojo/public/cpp/system/message_pipe.h" +#include "services/service_manager/public/cpp/interface_provider.h" #include "testing/gtest/include/gtest/gtest.h" namespace { @@ -41,7 +39,7 @@ int shutdown_calls_ = 0; int update_available_calls_ = 0; int ipc_client_disconnect_calls_ = 0; - mojo::ScopedMessagePipeHandle channel_handle_; + service_manager::mojom::InterfaceProviderPtr interface_provider_; }; void FakeServiceIPCServerClient::OnShutdown() { @@ -62,33 +60,7 @@ mojo::ScopedMessagePipeHandle FakeServiceIPCServerClient::CreateChannelMessagePipe() { - mojo::MessagePipe channel; - channel_handle_ = std::move(channel.handle0); - return std::move(channel.handle1); -} - -class FakeChannelListener : public IPC::Listener { - public: - FakeChannelListener() {} - ~FakeChannelListener() override {} - bool OnMessageReceived(const IPC::Message& message) override { return true; } -}; - -class FakeMessageHandler : public ServiceIPCServer::MessageHandler { - public: - explicit FakeMessageHandler(bool should_handle); - bool HandleMessage(const IPC::Message& message) override; - bool should_handle_; - int handle_message_calls_; -}; - -FakeMessageHandler::FakeMessageHandler(bool should_handle) - : should_handle_(should_handle), handle_message_calls_(0) { -} - -bool FakeMessageHandler::HandleMessage(const IPC::Message& message) { - handle_message_calls_++; - return should_handle_; + return mojo::MakeRequest(&interface_provider_).PassMessagePipe(); } } // namespace @@ -107,21 +79,14 @@ // Simulates the browser process shutting down. void DestroyClientChannel(); - // Sends |message| to the ServiceIPCServer. - void SendToServiceProcess(IPC::Message* message); - - IPC::SyncChannel* GetServerChannel() { - return server_->channel_.get(); - } - protected: FakeServiceIPCServerClient service_process_client_; base::MessageLoopForUI main_message_loop_; base::Thread io_thread_; base::WaitableEvent shutdown_event_; std::unique_ptr<ServiceIPCServer> server_; - FakeChannelListener client_process_channel_listener_; - std::unique_ptr<IPC::SyncChannel> client_process_channel_; + service_manager::InterfaceProvider remote_interfaces_; + chrome::mojom::ServiceProcessPtr service_process_; }; ServiceIPCServerTest::ServiceIPCServerTest() @@ -143,12 +108,9 @@ void ServiceIPCServerTest::TearDown() { // Close the ipc channels to prevent memory leaks. - if (client_process_channel_) { - client_process_channel_->Close(); - PumpLoops(); - } - if (GetServerChannel()) { - GetServerChannel()->Close(); + if (service_process_) { + remote_interfaces_.Close(); + service_process_.reset(); PumpLoops(); } io_thread_.Stop(); @@ -164,22 +126,18 @@ } void ServiceIPCServerTest::ConnectClientChannel() { - client_process_channel_ = IPC::SyncChannel::Create( - IPC::ChannelMojo::CreateClientFactory( - std::move(service_process_client_.channel_handle_), - io_thread_.task_runner()), - &client_process_channel_listener_, io_thread_.task_runner(), - true /* create_pipe_now */, &shutdown_event_); + remote_interfaces_.Close(); + remote_interfaces_.Bind( + std::move(service_process_client_.interface_provider_)); + + remote_interfaces_.GetInterface(&service_process_); + service_process_->Hello(base::BindOnce(&base::DoNothing)); PumpLoops(); } void ServiceIPCServerTest::DestroyClientChannel() { - client_process_channel_.reset(); - PumpLoops(); -} - -void ServiceIPCServerTest::SendToServiceProcess(IPC::Message* message) { - client_process_channel_->Send(message); + remote_interfaces_.Close(); + service_process_.reset(); PumpLoops(); } @@ -199,7 +157,8 @@ ConnectClientChannel(); ASSERT_TRUE(server_->is_ipc_client_connected()); - SendToServiceProcess(new ServiceMsg_UpdateAvailable()); + service_process_->UpdateAvailable(); + PumpLoops(); ASSERT_TRUE(server_->is_ipc_client_connected()); // Destroy the client process channel again to verify the @@ -216,7 +175,8 @@ // When a shutdown message is received, the ServiceIPCServer::Client is // notified. - SendToServiceProcess(new ServiceMsg_Shutdown()); + service_process_->ShutDown(); + PumpLoops(); ASSERT_EQ(1, service_process_client_.shutdown_calls_); } @@ -226,40 +186,7 @@ // When a product update message is received, the ServiceIPCServer::Client is // notified. - SendToServiceProcess(new ServiceMsg_UpdateAvailable()); + service_process_->UpdateAvailable(); + PumpLoops(); ASSERT_EQ(1, service_process_client_.update_available_calls_); } - -TEST_F(ServiceIPCServerTest, SingleMessageHandler) { - ConnectClientChannel(); - ASSERT_TRUE(server_->is_ipc_client_connected()); - - // Verify that a message handler is offered messages not handled by the server - // itself. - FakeMessageHandler* handler = - new FakeMessageHandler(true /* should_handle */); - server_->AddMessageHandler(base::WrapUnique(handler)); - SendToServiceProcess(new ServiceMsg_DisableCloudPrintProxy()); - ASSERT_EQ(1, handler->handle_message_calls_); -} - -TEST_F(ServiceIPCServerTest, MultipleMessageHandlers) { - ConnectClientChannel(); - ASSERT_TRUE(server_->is_ipc_client_connected()); - - // If there are multiple handlers they are offered the message in order of - // being added until it is handled. - FakeMessageHandler* handler1 = - new FakeMessageHandler(false /* should_handle */); - server_->AddMessageHandler(base::WrapUnique(handler1)); - FakeMessageHandler* handler2 = - new FakeMessageHandler(true /* should_handle */); - server_->AddMessageHandler(base::WrapUnique(handler2)); - FakeMessageHandler* handler3 = - new FakeMessageHandler(true /* should_handle */); - server_->AddMessageHandler(base::WrapUnique(handler3)); - SendToServiceProcess(new ServiceMsg_DisableCloudPrintProxy()); - ASSERT_EQ(1, handler1->handle_message_calls_); - ASSERT_EQ(1, handler2->handle_message_calls_); - ASSERT_EQ(0, handler3->handle_message_calls_); -}
diff --git a/chrome/service/service_process.cc b/chrome/service/service_process.cc index 095afcd..b6fa102a 100644 --- a/chrome/service/service_process.cc +++ b/chrome/service/service_process.cc
@@ -243,9 +243,8 @@ ipc_server_.reset(new ServiceIPCServer(this /* client */, io_task_runner(), &shutdown_event_)); - ipc_server_->AddMessageHandler( - base::MakeUnique<cloud_print::CloudPrintMessageHandler>(ipc_server_.get(), - this)); + ipc_server_->binder_registry().AddInterface( + base::Bind(&cloud_print::CloudPrintMessageHandler::Create, this)); ipc_server_->Init(); // After the IPC server has started we signal that the service process is
diff --git a/components/ntp_snippets/features.cc b/components/ntp_snippets/features.cc index 985f5d6..5522fd9 100644 --- a/components/ntp_snippets/features.cc +++ b/components/ntp_snippets/features.cc
@@ -13,6 +13,12 @@ namespace ntp_snippets { +// Holds an experiment ID. So long as the feature is set through a server-side +// variations config, this feature should exist on the client. This ensures that +// the experiment ID is visible in chrome://snippets-internals. +const base::Feature kRemoteSuggestionsBackendFeature{ + "NTPRemoteSuggestionsBackend", base::FEATURE_DISABLED_BY_DEFAULT}; + // Keep sorted, and keep nullptr at the end. const base::Feature* const kAllFeatures[] = { &kArticleSuggestionsFeature, @@ -28,6 +34,7 @@ &kPhysicalWebPageSuggestionsFeature, &kPublisherFaviconsFromNewServerFeature, &kRecentOfflineTabSuggestionsFeature, + &kRemoteSuggestionsBackendFeature, nullptr}; const base::Feature kArticleSuggestionsFeature{
diff --git a/components/password_manager/content/browser/content_password_manager_driver.cc b/components/password_manager/content/browser/content_password_manager_driver.cc index 39b39db..59733b6 100644 --- a/components/password_manager/content/browser/content_password_manager_driver.cc +++ b/components/password_manager/content/browser/content_password_manager_driver.cc
@@ -38,12 +38,11 @@ : render_frame_host_(render_frame_host), client_(client), password_generation_manager_(client, this), - password_autofill_manager_(this, autofill_client), + password_autofill_manager_(this, autofill_client, client), next_free_key_(0), is_main_frame_(render_frame_host->GetParent() == nullptr), password_manager_binding_(this), weak_factory_(this) { - // For some frames |this| may be instantiated before log manager creation, so // here we can not send logging state to renderer process for them. For such // cases, after the log manager got ready later,
diff --git a/components/password_manager/core/browser/password_autofill_manager.cc b/components/password_manager/core/browser/password_autofill_manager.cc index 941cfa28..892527c3 100644 --- a/components/password_manager/core/browser/password_autofill_manager.cc +++ b/components/password_manager/core/browser/password_autofill_manager.cc
@@ -26,7 +26,6 @@ #include "components/autofill/core/common/autofill_data_validation.h" #include "components/autofill/core/common/autofill_util.h" #include "components/password_manager/core/browser/android_affiliation/affiliation_utils.h" -#include "components/password_manager/core/browser/password_manager.h" #include "components/password_manager/core/browser/password_manager_client.h" #include "components/password_manager/core/browser/password_manager_driver.h" #include "components/password_manager/core/browser/password_manager_metrics_recorder.h" @@ -139,9 +138,12 @@ PasswordAutofillManager::PasswordAutofillManager( PasswordManagerDriver* password_manager_driver, - autofill::AutofillClient* autofill_client) - : password_manager_driver_(password_manager_driver), + autofill::AutofillClient* autofill_client, + PasswordManagerClient* password_client) + : form_data_key_(-1), + password_manager_driver_(password_manager_driver), autofill_client_(autofill_client), + password_client_(password_client), weak_ptr_factory_(this) {} PasswordAutofillManager::~PasswordAutofillManager() { @@ -264,7 +266,8 @@ if (base::FeatureList::IsEnabled( password_manager::features::kEnableManualFallbacksFilling) && - (options & autofill::IS_PASSWORD_FIELD)) { + (options & autofill::IS_PASSWORD_FIELD) && password_client_ && + password_client_->IsFillingFallbackEnabledForCurrentPage()) { #if !defined(OS_ANDROID) suggestions.push_back(autofill::Suggestion()); suggestions.back().frontend_id = autofill::POPUP_ITEM_ID_SEPARATOR; @@ -319,6 +322,9 @@ // is NULL too. if (!autofill_client_) return; + if (!password_client_ || + !password_client_->IsFillingFallbackEnabledForCurrentPage()) + return; std::vector<autofill::Suggestion> suggestions; autofill::Suggestion all_saved_passwords( l10n_util::GetStringUTF8(IDS_AUTOFILL_SHOW_ALL_SAVED_FALLBACK), @@ -387,20 +393,16 @@ metrics_util::LogContextOfShowAllSavedPasswordsAccepted( show_all_saved_passwords_shown_context_); - PasswordManager* password_manager = - password_manager_driver_->GetPasswordManager(); - PasswordManagerClient* client = - password_manager ? password_manager->client() : nullptr; - if (client) { + if (password_client_) { using UserAction = password_manager::PasswordManagerMetricsRecorder::PageLevelUserAction; switch (show_all_saved_passwords_shown_context_) { case metrics_util::SHOW_ALL_SAVED_PASSWORDS_CONTEXT_PASSWORD: - client->GetMetricsRecorder().RecordPageLevelUserAction( + password_client_->GetMetricsRecorder().RecordPageLevelUserAction( UserAction::kShowAllPasswordsWhileSomeAreSuggested); break; case metrics_util::SHOW_ALL_SAVED_PASSWORDS_CONTEXT_MANUAL_FALLBACK: - client->GetMetricsRecorder().RecordPageLevelUserAction( + password_client_->GetMetricsRecorder().RecordPageLevelUserAction( UserAction::kShowAllPasswordsWhileNoneAreSuggested); break; case metrics_util::SHOW_ALL_SAVED_PASSWORDS_CONTEXT_NONE:
diff --git a/components/password_manager/core/browser/password_autofill_manager.h b/components/password_manager/core/browser/password_autofill_manager.h index 4483d03..1b05f88 100644 --- a/components/password_manager/core/browser/password_autofill_manager.h +++ b/components/password_manager/core/browser/password_autofill_manager.h
@@ -20,13 +20,15 @@ namespace password_manager { +class PasswordManagerClient; class PasswordManagerDriver; // This class is responsible for filling password forms. class PasswordAutofillManager : public autofill::AutofillPopupDelegate { public: PasswordAutofillManager(PasswordManagerDriver* password_manager_driver, - autofill::AutofillClient* autofill_client); + autofill::AutofillClient* autofill_client, + PasswordManagerClient* password_client); virtual ~PasswordAutofillManager(); // AutofillPopupDelegate implementation. @@ -135,6 +137,8 @@ autofill::AutofillClient* autofill_client_; // weak + PasswordManagerClient* password_client_; + base::WeakPtrFactory<PasswordAutofillManager> weak_ptr_factory_; DISALLOW_COPY_AND_ASSIGN(PasswordAutofillManager);
diff --git a/components/password_manager/core/browser/password_autofill_manager_unittest.cc b/components/password_manager/core/browser/password_autofill_manager_unittest.cc index 000553f..51a3274 100644 --- a/components/password_manager/core/browser/password_autofill_manager_unittest.cc +++ b/components/password_manager/core/browser/password_autofill_manager_unittest.cc
@@ -116,8 +116,8 @@ void InitializePasswordAutofillManager( TestPasswordManagerClient* client, autofill::AutofillClient* autofill_client) { - password_autofill_manager_.reset( - new PasswordAutofillManager(client->mock_driver(), autofill_client)); + password_autofill_manager_.reset(new PasswordAutofillManager( + client->mock_driver(), autofill_client, client)); password_autofill_manager_->OnAddPasswordFormMapping(fill_data_id_, fill_data_); }
diff --git a/components/password_manager/core/browser/password_generation_manager_unittest.cc b/components/password_manager/core/browser/password_generation_manager_unittest.cc index 32b667e..05093fc 100644 --- a/components/password_manager/core/browser/password_generation_manager_unittest.cc +++ b/components/password_manager/core/browser/password_generation_manager_unittest.cc
@@ -46,7 +46,7 @@ explicit TestPasswordManagerDriver(PasswordManagerClient* client) : password_manager_(client), password_generation_manager_(client, this), - password_autofill_manager_(this, nullptr) {} + password_autofill_manager_(this, nullptr, client) {} ~TestPasswordManagerDriver() override {} // PasswordManagerDriver implementation.
diff --git a/components/password_manager/core/browser/password_manager_client.cc b/components/password_manager/core/browser/password_manager_client.cc index 6ed6f1e..ead9e48 100644 --- a/components/password_manager/core/browser/password_manager_client.cc +++ b/components/password_manager/core/browser/password_manager_client.cc
@@ -15,6 +15,10 @@ return true; } +bool PasswordManagerClient::IsFillingFallbackEnabledForCurrentPage() const { + return true; +} + void PasswordManagerClient::PostHSTSQueryForHost( const GURL& origin, const HSTSCallback& callback) const {
diff --git a/components/password_manager/core/browser/password_manager_client.h b/components/password_manager/core/browser/password_manager_client.h index 068b63e..7d45719b 100644 --- a/components/password_manager/core/browser/password_manager_client.h +++ b/components/password_manager/core/browser/password_manager_client.h
@@ -62,6 +62,9 @@ // password manager is disabled, or in the presence of SSL errors on a page. virtual bool IsFillingEnabledForCurrentPage() const; + // Checks if manual filling fallback is enabled for the current page. + virtual bool IsFillingFallbackEnabledForCurrentPage() const; + // Checks asynchronously whether HTTP Strict Transport Security (HSTS) is // active for the host of the given origin. Notifies |callback| with the // result on the calling thread.
diff --git a/components/password_manager/core/browser/password_manager_unittest.cc b/components/password_manager/core/browser/password_manager_unittest.cc index 408e82c9..536e505 100644 --- a/components/password_manager/core/browser/password_manager_unittest.cc +++ b/components/password_manager/core/browser/password_manager_unittest.cc
@@ -144,7 +144,7 @@ manager_.reset(new PasswordManager(&client_)); password_autofill_manager_.reset( - new PasswordAutofillManager(client_.GetDriver(), nullptr)); + new PasswordAutofillManager(client_.GetDriver(), nullptr, &client_)); EXPECT_CALL(driver_, GetPasswordManager()) .WillRepeatedly(Return(manager_.get()));
diff --git a/content/browser/service_worker/service_worker_url_loader_job_unittest.cc b/content/browser/service_worker/service_worker_url_loader_job_unittest.cc index 4a5aacc..977bfe8 100644 --- a/content/browser/service_worker/service_worker_url_loader_job_unittest.cc +++ b/content/browser/service_worker/service_worker_url_loader_job_unittest.cc
@@ -262,10 +262,10 @@ // Start a ServiceWorkerURLLoaderJob. It should return a // StartLoaderCallback. StartLoaderCallback callback; - auto job = base::MakeUnique<ServiceWorkerURLLoaderJob>( + job_ = base::MakeUnique<ServiceWorkerURLLoaderJob>( base::BindOnce(&ReceiveStartLoaderCallback, &callback), this, request, GetBlobStorageContext()); - job->ForwardToServiceWorker(); + job_->ForwardToServiceWorker(); base::RunLoop().RunUntilIdle(); if (!callback) return JobResult::kDidNotHandleRequest; @@ -318,6 +318,7 @@ storage::BlobStorageContext blob_context_; TestURLLoaderClient client_; bool was_main_resource_load_failed_called_ = false; + std::unique_ptr<ServiceWorkerURLLoaderJob> job_; }; TEST_F(ServiceWorkerURLLoaderJobTest, Basic) { @@ -383,6 +384,10 @@ EXPECT_EQ(200, info.headers->response_code()); ExpectFetchedViaServiceWorker(info); + // TODO(falken): This should be true since the worker is still streaming the + // response body. See https://crbug.com/758455 + EXPECT_FALSE(version_->HasWork()); + // Write the body stream. uint32_t written_bytes = sizeof(kResponseBody) - 1; MojoResult mojo_result = data_pipe.producer_handle->WriteData( @@ -436,6 +441,53 @@ EXPECT_EQ(kResponseBody, response); } +// Test when the job is cancelled while a stream response is being written. +TEST_F(ServiceWorkerURLLoaderJobTest, StreamResponseAndCancel) { + // Construct the Stream to respond with. + const char kResponseBody[] = "Here is sample text for the Stream."; + blink::mojom::ServiceWorkerStreamCallbackPtr stream_callback; + mojo::DataPipe data_pipe; + helper_->RespondWithStream(mojo::MakeRequest(&stream_callback), + std::move(data_pipe.consumer_handle)); + + // Perform the request. + JobResult result = TestRequest(); + EXPECT_EQ(JobResult::kHandledRequest, result); + const ResourceResponseHead& info = client_.response_head(); + EXPECT_EQ(200, info.headers->response_code()); + ExpectFetchedViaServiceWorker(info); + + // Start writing the body stream, then cancel the job before finishing. + uint32_t written_bytes = sizeof(kResponseBody) - 1; + MojoResult mojo_result = data_pipe.producer_handle->WriteData( + kResponseBody, &written_bytes, MOJO_WRITE_DATA_FLAG_NONE); + ASSERT_EQ(MOJO_RESULT_OK, mojo_result); + EXPECT_EQ(sizeof(kResponseBody) - 1, written_bytes); + EXPECT_TRUE(data_pipe.producer_handle.is_valid()); + EXPECT_FALSE(job_->WasCanceled()); + // TODO(falken): This should be true since the worker is still streaming the + // response body. See https://crbug.com/758455 + EXPECT_FALSE(version_->HasWork()); + job_->Cancel(); + EXPECT_TRUE(job_->WasCanceled()); + EXPECT_FALSE(version_->HasWork()); + + // Although ServiceworkerURLLoaderJob resets its URLLoaderClient pointer in + // Cancel(), the URLLoaderClient still exists. In this test, it is |client_| + // which owns the data pipe, so it's still valid to write data to it. + mojo_result = data_pipe.producer_handle->WriteData( + kResponseBody, &written_bytes, MOJO_WRITE_DATA_FLAG_NONE); + // TODO(falken): This should probably be an error. + EXPECT_EQ(MOJO_RESULT_OK, mojo_result); + + stream_callback->OnAborted(); + + base::RunLoop().RunUntilIdle(); + EXPECT_FALSE(data_pipe.consumer_handle.is_valid()); + // TODO(falken): This should be an error, see https://crbug.com/758455 + EXPECT_EQ(net::OK, client_.completion_status().error_code); +} + // Test when the service worker responds with network fallback. // i.e., does not call respondWith(). TEST_F(ServiceWorkerURLLoaderJobTest, FallbackResponse) {
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index 943fea5..ff9f5a7d 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc
@@ -3007,8 +3007,8 @@ ServiceWorkerNetworkProvider::FromWebServiceWorkerNetworkProvider( web_provider); mojom::ServiceWorkerWorkerClientRequest service_worker_client_request; - // Sandboxed iframes are not allowed to use service worker so don't have a - // real service worker provider, so the provider context is null. + // Some sandboxed iframes are not allowed to use service worker so don't have + // a real service worker provider, so the provider context is null. if (provider->context()) { service_worker_client_request = provider->context()->CreateWorkerClientRequest();
diff --git a/ios/chrome/browser/prefs/BUILD.gn b/ios/chrome/browser/prefs/BUILD.gn index e7f59a7..3b10894 100644 --- a/ios/chrome/browser/prefs/BUILD.gn +++ b/ios/chrome/browser/prefs/BUILD.gn
@@ -60,7 +60,6 @@ "//components/variations/service", "//components/web_resource", "//ios/chrome/browser", - "//ios/chrome/browser/bookmarks:features", "//ios/chrome/browser/browser_state", "//ios/chrome/browser/desktop_promotion", "//ios/chrome/browser/first_run",
diff --git a/ios/chrome/browser/prefs/browser_prefs.mm b/ios/chrome/browser/prefs/browser_prefs.mm index cbd3841..e3bbee0 100644 --- a/ios/chrome/browser/prefs/browser_prefs.mm +++ b/ios/chrome/browser/prefs/browser_prefs.mm
@@ -39,7 +39,6 @@ #include "components/update_client/update_client.h" #include "components/variations/service/variations_service.h" #include "components/web_resource/web_resource_pref_names.h" -#include "ios/chrome/browser/bookmarks/bookmark_new_generation_features.h" #include "ios/chrome/browser/browser_state/browser_state_info_cache.h" #include "ios/chrome/browser/desktop_promotion/desktop_promotion_sync_service.h" #include "ios/chrome/browser/first_run/first_run.h" @@ -54,7 +53,6 @@ #import "ios/chrome/browser/ui/bookmarks/bookmark_collection_view.h" #import "ios/chrome/browser/ui/bookmarks/bookmark_mediator.h" #import "ios/chrome/browser/ui/bookmarks/bookmark_promo_controller.h" -#import "ios/chrome/browser/ui/bookmarks/bookmark_table_view.h" #include "ios/chrome/browser/voice/voice_search_prefs_registration.h" #include "ios/public/provider/chrome/browser/chrome_browser_provider.h" #include "ui/base/l10n/l10n_util.h" @@ -126,12 +124,7 @@ DesktopPromotionSyncService::RegisterDesktopPromotionUserPrefs(registry); RegisterVoiceSearchBrowserStatePrefs(registry); - if (base::FeatureList::IsEnabled( - bookmark_new_generation::features::kBookmarkNewGeneration)) { - [BookmarkTableView registerBrowserStatePrefs:registry]; - } else { - [BookmarkCollectionView registerBrowserStatePrefs:registry]; - } + [BookmarkCollectionView registerBrowserStatePrefs:registry]; [BookmarkMediator registerBrowserStatePrefs:registry]; [BookmarkPromoController registerBrowserStatePrefs:registry]; [HandoffManager registerBrowserStatePrefs:registry];
diff --git a/ios/chrome/browser/ui/bookmarks/BUILD.gn b/ios/chrome/browser/ui/bookmarks/BUILD.gn index 861f6c8..2116a14 100644 --- a/ios/chrome/browser/ui/bookmarks/BUILD.gn +++ b/ios/chrome/browser/ui/bookmarks/BUILD.gn
@@ -180,7 +180,6 @@ testonly = true sources = [ "bookmarks_egtest.mm", - "bookmarks_new_generation_egtest.mm", ] deps = [ "//base",
diff --git a/ios/chrome/browser/ui/bookmarks/bars/bookmark_editing_bar.h b/ios/chrome/browser/ui/bookmarks/bars/bookmark_editing_bar.h index 0f117ce..dd15eec 100644 --- a/ios/chrome/browser/ui/bookmarks/bars/bookmark_editing_bar.h +++ b/ios/chrome/browser/ui/bookmarks/bars/bookmark_editing_bar.h
@@ -9,8 +9,6 @@ #import "ios/chrome/browser/ui/bookmarks/bars/bookmark_top_bar.h" -// TODO(crbug.com/753599) : Delete this file after new bookmarks ui is launched. - // The top bar that shows up when a user is editing bookmarks. @interface BookmarkEditingBar : BookmarkTopBar // Set the target/action of the respective buttons.
diff --git a/ios/chrome/browser/ui/bookmarks/bars/bookmark_navigation_bar.h b/ios/chrome/browser/ui/bookmarks/bars/bookmark_navigation_bar.h index aa7f54c3..38b04f3 100644 --- a/ios/chrome/browser/ui/bookmarks/bars/bookmark_navigation_bar.h +++ b/ios/chrome/browser/ui/bookmarks/bars/bookmark_navigation_bar.h
@@ -7,8 +7,6 @@ #import <UIKit/UIKit.h> -// TODO(crbug.com/753599) : Delete this file after new bookmarks ui is launched. - // The navigation bar on the bookmark home page. @interface BookmarkNavigationBar : UIView
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_collection_cells.h b/ios/chrome/browser/ui/bookmarks/bookmark_collection_cells.h index 23fe989..196d108 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_collection_cells.h +++ b/ios/chrome/browser/ui/bookmarks/bookmark_collection_cells.h
@@ -7,8 +7,6 @@ #import <UIKit/UIKit.h> -// TODO(crbug.com/753599) : Delete this file after new bookmarks ui is launched. - namespace bookmark_cell { // The types get cached, which means that their values must not change. typedef enum {
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_collection_view.h b/ios/chrome/browser/ui/bookmarks/bookmark_collection_view.h index f8b9a5a..d07f383 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_collection_view.h +++ b/ios/chrome/browser/ui/bookmarks/bookmark_collection_view.h
@@ -15,8 +15,6 @@ #include "ios/chrome/browser/ui/bookmarks/bookmark_model_bridge_observer.h" #import "ios/chrome/browser/ui/bookmarks/bookmark_utils_ios.h" -// TODO(crbug.com/753599) : Delete this file after new bookmarks ui is launched. - @class BookmarkCollectionView; class GURL; @protocol UrlLoader;
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller.mm b/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller.mm index 6c56fd0..c6631ab 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller.mm +++ b/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller.mm
@@ -317,12 +317,7 @@ #pragma mark - BookmarkPromoControllerDelegate - (void)promoStateChanged:(BOOL)promoEnabled { - if (base::FeatureList::IsEnabled( - bookmark_new_generation::features::kBookmarkNewGeneration)) { - [self.bookmarksTableView promoStateChangedAnimated:YES]; - } else { - [self.folderView promoStateChangedAnimated:YES]; - } + [self.folderView promoStateChangedAnimated:YES]; } #pragma mark Action sheet callbacks @@ -456,18 +451,6 @@ [self deleteNodes:nodes]; } -- (BOOL)bookmarkTableViewShouldShowPromoCell:(BookmarkTableView*)tableView { - return self.bookmarkPromoController.promoState; -} - -- (void)bookmarkTableViewShowSignIn:(BookmarkTableView*)view { - [self.bookmarkPromoController showSignIn]; -} - -- (void)bookmarkTableViewDismissPromo:(BookmarkTableView*)view { - [self.bookmarkPromoController hidePromoCell]; -} - #pragma mark - BookmarkFolderViewControllerDelegate - (void)folderPicker:(BookmarkFolderViewController*)folderPicker
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_menu_cell.h b/ios/chrome/browser/ui/bookmarks/bookmark_menu_cell.h index 07dddc7..034265fb 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_menu_cell.h +++ b/ios/chrome/browser/ui/bookmarks/bookmark_menu_cell.h
@@ -7,8 +7,6 @@ #import <UIKit/UIKit.h> -// TODO(crbug.com/753599) : Delete this file after new bookmarks ui is launched. - @class BookmarkMenuItem; @class MDCInkView;
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_menu_view.h b/ios/chrome/browser/ui/bookmarks/bookmark_menu_view.h index 6ee0c18..a35363c 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_menu_view.h +++ b/ios/chrome/browser/ui/bookmarks/bookmark_menu_view.h
@@ -7,8 +7,6 @@ #import <UIKit/UIKit.h> -// TODO(crbug.com/753599) : Delete this file after new bookmarks ui is launched. - @class BookmarkMenuItem; @class BookmarkMenuView;
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_navigation_controller.h b/ios/chrome/browser/ui/bookmarks/bookmark_navigation_controller.h index 6281b791..062307f5d 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_navigation_controller.h +++ b/ios/chrome/browser/ui/bookmarks/bookmark_navigation_controller.h
@@ -7,8 +7,6 @@ #import <UIKit/UIKit.h> -// TODO(crbug.com/753599) : Delete this file after new bookmarks ui is launched. - @interface BookmarkNavigationController : UINavigationController @end
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_promo_cell.h b/ios/chrome/browser/ui/bookmarks/bookmark_promo_cell.h index cd8adbad..11e4c6f 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_promo_cell.h +++ b/ios/chrome/browser/ui/bookmarks/bookmark_promo_cell.h
@@ -7,8 +7,6 @@ #import <UIKit/UIKit.h> -// TODO(crbug.com/753599) : Delete this file after new bookmarks ui is launched. - @class BookmarkPromoCell; @protocol BookmarkPromoCellDelegate
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_signin_promo_cell.h b/ios/chrome/browser/ui/bookmarks/bookmark_signin_promo_cell.h index f0c20e2..8e08d99 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_signin_promo_cell.h +++ b/ios/chrome/browser/ui/bookmarks/bookmark_signin_promo_cell.h
@@ -7,8 +7,6 @@ #import <UIKit/UIKit.h> -// TODO(crbug.com/753599) : Delete this file after new bookmarks ui is launched. - @class SigninPromoView; typedef void (^CloseButtonCallback)(void);
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_table_view.h b/ios/chrome/browser/ui/bookmarks/bookmark_table_view.h index 84c5c4c..8361a59 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_table_view.h +++ b/ios/chrome/browser/ui/bookmarks/bookmark_table_view.h
@@ -18,10 +18,6 @@ class ChromeBrowserState; } -namespace user_prefs { -class PrefRegistrySyncable; -} // namespace user_prefs - @class BookmarkTableView; // Delegate to handle actions on the table. @@ -39,15 +35,6 @@ selectedNodesForDeletion: (const std::set<const bookmarks::BookmarkNode*>&)nodes; -// Returns true if a bookmarks promo cell should be shown. -- (BOOL)bookmarkTableViewShouldShowPromoCell:(BookmarkTableView*)view; - -// Shows a sign-in view controller. -- (void)bookmarkTableViewShowSignIn:(BookmarkTableView*)view; - -// Dismisses the promo. -- (void)bookmarkTableViewDismissPromo:(BookmarkTableView*)view; - @end @interface BookmarkTableView : UIView @@ -68,12 +55,6 @@ - (instancetype)init NS_UNAVAILABLE; + (instancetype) new NS_UNAVAILABLE; -// Registers the feature preferences. -+ (void)registerBrowserStatePrefs:(user_prefs::PrefRegistrySyncable*)registry; - -// Called when something outside the view causes the promo state to change. -- (void)promoStateChangedAnimated:(BOOL)animated; - @end #endif // IOS_CHROME_BROWSER_UI_BOOKMARKS_BOOKMARK_TABLE_VIEW_H_
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_table_view.mm b/ios/chrome/browser/ui/bookmarks/bookmark_table_view.mm index 3f786b7..88ccf2a 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_table_view.mm +++ b/ios/chrome/browser/ui/bookmarks/bookmark_table_view.mm
@@ -11,23 +11,13 @@ #include "components/favicon/core/large_icon_service.h" #include "components/favicon_base/fallback_icon_style.h" #include "components/favicon_base/favicon_types.h" -#include "components/pref_registry/pref_registry_syncable.h" #include "ios/chrome/browser/bookmarks/bookmark_model_factory.h" #include "ios/chrome/browser/bookmarks/bookmarks_utils.h" -#include "ios/chrome/browser/experimental_flags.h" #include "ios/chrome/browser/favicon/ios_chrome_large_icon_service_factory.h" -#include "ios/chrome/browser/pref_names.h" -#import "ios/chrome/browser/ui/authentication/signin_promo_view.h" -#import "ios/chrome/browser/ui/authentication/signin_promo_view_configurator.h" -#import "ios/chrome/browser/ui/authentication/signin_promo_view_consumer.h" -#import "ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h" #include "ios/chrome/browser/ui/bookmarks/bookmark_collection_view_background.h" #include "ios/chrome/browser/ui/bookmarks/bookmark_model_bridge_observer.h" #import "ios/chrome/browser/ui/bookmarks/bookmark_utils_ios.h" #import "ios/chrome/browser/ui/bookmarks/cells/bookmark_table_cell.h" -#import "ios/chrome/browser/ui/bookmarks/cells/bookmark_table_promo_cell.h" -#import "ios/chrome/browser/ui/bookmarks/cells/bookmark_table_signin_promo_cell.h" -#import "ios/chrome/browser/ui/sync/synced_sessions_bridge.h" #include "ios/chrome/grit/ios_strings.h" #include "skia/ext/skia_utils_ios.h" #include "ui/base/l10n/l10n_util_mac.h" @@ -38,10 +28,10 @@ namespace { // Minimal acceptable favicon size, in points. -CGFloat kMinFaviconSizePt = 16; +CGFloat minFaviconSizePt = 16; // Cell height, in points. -CGFloat kCellHeightPt = 56.0; +CGFloat cellHeightPt = 56.0; } using bookmarks::BookmarkNode; @@ -50,15 +40,12 @@ // collections. using IntegerPair = std::pair<NSInteger, NSInteger>; -@interface BookmarkTableView ()<BookmarkTablePromoCellDelegate, - SigninPromoViewConsumer, - UITableViewDataSource, +@interface BookmarkTableView ()<UITableViewDataSource, UITableViewDelegate, BookmarkModelBridgeObserver> { // A vector of bookmark nodes to display in the table view. std::vector<const BookmarkNode*> _bookmarkItems; const BookmarkNode* _currentRootNode; - // Bridge to register for bookmark changes. std::unique_ptr<bookmarks::BookmarkModelBridge> _modelBridge; // Map of favicon load tasks for each index path. Used to keep track of @@ -67,12 +54,6 @@ std::map<IntegerPair, base::CancelableTaskTracker::TaskId> _faviconLoadTasks; // Task tracker used for async favicon loads. base::CancelableTaskTracker _faviconTaskTracker; - - // Mediator, helper for the sign-in promo view. - SigninPromoViewMediator* _signinPromoViewMediator; - - // True if the promo is visible. - BOOL _promoVisible; } // The UITableView to show bookmarks. @@ -87,11 +68,6 @@ @property(nonatomic, strong) BookmarkCollectionViewBackground* emptyTableBackgroundView; -// Section indices. -@property(nonatomic, readonly, assign) NSInteger promoSection; -@property(nonatomic, readonly, assign) NSInteger bookmarksSection; -@property(nonatomic, readonly, assign) NSInteger sectionCount; - @end @implementation BookmarkTableView @@ -102,10 +78,8 @@ @synthesize delegate = _delegate; @synthesize emptyTableBackgroundView = _emptyTableBackgroundView; -+ (void)registerBrowserStatePrefs:(user_prefs::PrefRegistrySyncable*)registry { - registry->RegisterIntegerPref(prefs::kIosBookmarkSigninPromoDisplayedCount, - 0); -} +// TODO(crbug.com/695749) Add promo section, bottom context bar, +// promo view and register kIosBookmarkSigninPromoDisplayedCount. - (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState delegate:(id<BookmarkTableViewDelegate>)delegate @@ -128,21 +102,10 @@ [self computeBookmarkTableViewData]; - // Set promo state before the tableview is created. - [self promoStateChangedAnimated:NO]; - - // Create and setup tableview. self.tableView = [[UITableView alloc] initWithFrame:frame style:UITableViewStylePlain]; self.tableView.dataSource = self; self.tableView.delegate = self; - - // Use iOS8's self sizing feature to compute row height. However, - // this reduces the row height of bookmarks section from 56 to 45 - // TODO(crbug.com/695749): Fix the bookmark section row height to 56. - self.tableView.estimatedRowHeight = kCellHeightPt; - self.tableView.rowHeight = UITableViewAutomaticDimension; - // Remove extra rows. self.tableView.tableFooterView = [[UIView alloc] initWithFrame:CGRectZero]; self.tableView.autoresizingMask = @@ -164,117 +127,42 @@ } - (void)dealloc { - [_signinPromoViewMediator signinPromoViewRemoved]; _tableView.dataSource = nil; _tableView.delegate = nil; _faviconTaskTracker.TryCancelAll(); } -#pragma mark - Public - -- (void)promoStateChangedAnimated:(BOOL)animated { - // We show promo cell only on the root view, that is when showing - // the permanent nodes. - BOOL promoVisible = - ((_currentRootNode == self.bookmarkModel->root_node()) && - [self.delegate bookmarkTableViewShouldShowPromoCell:self]) || - (_signinPromoViewMediator && - _signinPromoViewMediator.signinPromoViewState == - ios::SigninPromoViewState::SigninStarted); - - if (promoVisible == _promoVisible) - return; - - _promoVisible = promoVisible; - - if (experimental_flags::IsSigninPromoEnabled()) { - if (!promoVisible) { - _signinPromoViewMediator.consumer = nil; - [_signinPromoViewMediator signinPromoViewRemoved]; - _signinPromoViewMediator = nil; - } else { - _signinPromoViewMediator = [[SigninPromoViewMediator alloc] - initWithBrowserState:_browserState - accessPoint:signin_metrics::AccessPoint:: - ACCESS_POINT_BOOKMARK_MANAGER]; - _signinPromoViewMediator.consumer = self; - [_signinPromoViewMediator signinPromoViewVisible]; - } - } - [self.tableView reloadData]; -} - #pragma mark - UITableViewDataSource - (NSInteger)numberOfSectionsInTableView:(UITableView*)tableView { - return self.sectionCount; + // TODO(crbug.com/695749) Add promo section check here. + return 1; } - (NSInteger)tableView:(UITableView*)tableView numberOfRowsInSection:(NSInteger)section { - if (section == self.bookmarksSection) - return _bookmarkItems.size(); - if (section == self.promoSection) - return 1; - - NOTREACHED(); - return -1; + // TODO(crbug.com/695749) Add promo section check here. + return _bookmarkItems.size(); } - (UITableViewCell*)tableView:(UITableView*)tableView cellForRowAtIndexPath:(NSIndexPath*)indexPath { - // TODO(crbug.com/695749): Introduce a custom separator for bookmarks - // section, so that we don't show a separator after promo section. - if (indexPath.section == self.promoSection) { - if (experimental_flags::IsSigninPromoEnabled()) { - BookmarkTableSigninPromoCell* signinPromoCell = [self.tableView - dequeueReusableCellWithIdentifier:[BookmarkTableSigninPromoCell - reuseIdentifier]]; - if (signinPromoCell == nil) { - signinPromoCell = - [[BookmarkTableSigninPromoCell alloc] initWithFrame:CGRectZero]; - } - signinPromoCell.signinPromoView.delegate = _signinPromoViewMediator; - [[_signinPromoViewMediator createConfigurator] - configureSigninPromoView:signinPromoCell.signinPromoView]; - __weak BookmarkTableView* weakSelf = self; - signinPromoCell.closeButtonAction = ^() { - [weakSelf signinPromoCloseButtonAction]; - }; - return signinPromoCell; - } else { - BookmarkTablePromoCell* promoCell = [self.tableView - dequeueReusableCellWithIdentifier:[BookmarkTablePromoCell - reuseIdentifier]]; - if (promoCell == nil) { - promoCell = [[BookmarkTablePromoCell alloc] initWithFrame:CGRectZero]; - } - promoCell.delegate = self; - return promoCell; - } - } - const BookmarkNode* node = [self nodeAtIndexPath:indexPath]; - BookmarkTableCell* cell = [tableView - dequeueReusableCellWithIdentifier:[BookmarkTableCell reuseIdentifier]]; + static NSString* bookmarkCellIdentifier = @"bookmarkCellIdentifier"; + + BookmarkTableCell* cell = + [tableView dequeueReusableCellWithIdentifier:bookmarkCellIdentifier]; if (cell == nil) { - cell = [[BookmarkTableCell alloc] - initWithReuseIdentifier:[BookmarkTableCell reuseIdentifier]]; + cell = [[BookmarkTableCell alloc] initWithNode:node + reuseIdentifier:bookmarkCellIdentifier]; } - [cell setNode:node]; - [self loadFaviconAtIndexPath:indexPath]; return cell; } - (BOOL)tableView:(UITableView*)tableView canEditRowAtIndexPath:(NSIndexPath*)indexPath { - if (indexPath.section == self.promoSection) { - // Ignore promo section edit. - return NO; - } - // We enable the swipe-to-delete gesture and reordering control for nodes of // type URL or Folder, and not the permanent ones. const BookmarkNode* node = [self nodeAtIndexPath:indexPath]; @@ -285,11 +173,6 @@ - (void)tableView:(UITableView*)tableView commitEditingStyle:(UITableViewCellEditingStyle)editingStyle forRowAtIndexPath:(NSIndexPath*)indexPath { - if (indexPath.section == self.promoSection) { - // Ignore promo section editing style. - return; - } - if (editingStyle == UITableViewCellEditingStyleDelete) { const BookmarkNode* node = [self nodeAtIndexPath:indexPath]; std::set<const BookmarkNode*> nodes; @@ -302,62 +185,25 @@ - (void)tableView:(UITableView*)tableView didSelectRowAtIndexPath:(NSIndexPath*)indexPath { - if (indexPath.section == self.bookmarksSection) { - const BookmarkNode* node = [self nodeAtIndexPath:indexPath]; - DCHECK(node); - if (node->is_folder()) { - [self.delegate bookmarkTableView:self selectedFolderForNavigation:node]; - } else { - // Open URL. Pass this to the delegate. - [self.delegate bookmarkTableView:self - selectedUrlForNavigation:node->url()]; - } + // TODO(crbug.com/695749) Add promo section check here. + const BookmarkNode* node = [self nodeAtIndexPath:indexPath]; + DCHECK(node); + if (node->is_folder()) { + [self.delegate bookmarkTableView:self selectedFolderForNavigation:node]; + } else { + // Open URL. Pass this to the delegate. + [self.delegate bookmarkTableView:self selectedUrlForNavigation:node->url()]; } // Deselect row. [tableView deselectRowAtIndexPath:indexPath animated:YES]; } -#pragma mark - BookmarkTablePromoCellDelegate - -- (void)bookmarkTablePromoCellDidTapSignIn: - (BookmarkTablePromoCell*)bookmarkTablePromoCell { - [self.delegate bookmarkTableViewShowSignIn:self]; -} - -- (void)bookmarkTablePromoCellDidTapDismiss: - (BookmarkTablePromoCell*)bookmarkTablePromoCell { - [self.delegate bookmarkTableViewDismissPromo:self]; -} - -#pragma mark - SigninPromoViewConsumer - -- (void)configureSigninPromoWithConfigurator: - (SigninPromoViewConfigurator*)configurator - identityChanged:(BOOL)identityChanged { - DCHECK(_signinPromoViewMediator); - NSIndexPath* indexPath = - [NSIndexPath indexPathForRow:0 inSection:self.promoSection]; - BookmarkTableSigninPromoCell* signinPromoCell = - static_cast<BookmarkTableSigninPromoCell*>( - [self.tableView cellForRowAtIndexPath:indexPath]); - if (!signinPromoCell) - return; - // Should always reconfigure the cell size even if it has to be reloaded. - [configurator configureSigninPromoView:signinPromoCell.signinPromoView]; - if (identityChanged) { - // The section should be reload to update the cell height. - NSIndexSet* indexSet = [NSIndexSet indexSetWithIndex:self.promoSection]; - [self.tableView reloadSections:indexSet - withRowAnimation:UITableViewRowAnimationNone]; - } -} - -- (void)signinDidFinish { - [self promoStateChangedAnimated:NO]; +- (CGFloat)tableView:(UITableView*)tableView + heightForRowAtIndexPath:(NSIndexPath*)indexPath { + return cellHeightPt; } #pragma mark - BookmarkModelBridgeObserver Callbacks - // BookmarkModelBridgeObserver Callbacks // Instances of this class automatically observe the bookmark model. // The bookmark model has loaded. @@ -430,32 +276,8 @@ [self loadFaviconAtIndexPath:indexPath]; } -#pragma mark - Sections - -- (NSInteger)promoSection { - return [self shouldShowPromoCell] ? 0 : -1; -} - -- (NSInteger)bookmarksSection { - return [self shouldShowPromoCell] ? 1 : 0; -} - -- (NSInteger)sectionCount { - return [self shouldShowPromoCell] ? 2 : 1; -} - #pragma mark - Private -// Removes the sign-in promo view. -- (void)signinPromoCloseButtonAction { - [_signinPromoViewMediator signinPromoViewClosed]; - [_delegate bookmarkTableViewDismissPromo:self]; -} - -- (BOOL)shouldShowPromoCell { - return _promoVisible; -} - - (void)refreshContents { [self computeBookmarkTableViewData]; [self updateEmptyBackground]; @@ -465,9 +287,8 @@ // Returns the bookmark node associated with |indexPath|. - (const BookmarkNode*)nodeAtIndexPath:(NSIndexPath*)indexPath { - if (indexPath.section == self.bookmarksSection) { - return _bookmarkItems[indexPath.row]; - } + // TODO(crbug.com/695749) Add check if section is bookmarks. + return _bookmarkItems[indexPath.row]; NOTREACHED(); return nullptr; @@ -581,7 +402,7 @@ CGFloat scale = [UIScreen mainScreen].scale; CGFloat preferredSize = scale * [BookmarkTableCell preferredImageSize]; - CGFloat minSize = scale * kMinFaviconSizePt; + CGFloat minSize = scale * minFaviconSizePt; base::CancelableTaskTracker::TaskId taskId = IOSChromeLargeIconServiceFactory::GetForBrowserState(self.browserState)
diff --git a/ios/chrome/browser/ui/bookmarks/bookmarks_egtest.mm b/ios/chrome/browser/ui/bookmarks/bookmarks_egtest.mm index a75be4c..8e86fe0 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmarks_egtest.mm +++ b/ios/chrome/browser/ui/bookmarks/bookmarks_egtest.mm
@@ -9,7 +9,6 @@ #import <XCTest/XCTest.h> #include "base/strings/sys_string_conversions.h" -#include "base/test/scoped_feature_list.h" #include "components/bookmarks/browser/bookmark_model.h" #include "components/bookmarks/browser/titled_url_match.h" #include "components/prefs/pref_service.h" @@ -93,6 +92,11 @@ grey_not(grey_accessibilityTrait(UIAccessibilityTraitKeyboardKey)), nil); } +// Matcher for the Delete button on the bookmarks UI. +id<GREYMatcher> BookmarksDeleteSwipeButton() { + return ButtonWithAccessibilityLabel(@"Delete"); +} + // Matcher for the More Menu. id<GREYMatcher> MoreMenuButton() { return ButtonWithAccessibilityLabelId( @@ -355,6 +359,34 @@ assertWithMatcher:grey_notNil()]; } +- (void)testUndoDeleteBookmarkFromSwipe { + if (!base::FeatureList::IsEnabled( + bookmark_new_generation::features::kBookmarkNewGeneration)) { + EARL_GREY_TEST_SKIPPED(@"Only enabled with new UI."); + } + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openMobileBookmarks]; + + // Swipe action on the URL. + [[EarlGrey selectElementWithMatcher:grey_accessibilityLabel(@"Second URL")] + performAction:grey_swipeFastInDirection(kGREYDirectionLeft)]; + + // Delete it. + [[EarlGrey selectElementWithMatcher:BookmarksDeleteSwipeButton()] + performAction:grey_tap()]; + + // Wait until it's gone. + [BookmarksTestCase waitForDeletionOfBookmarkWithTitle:@"Second URL"]; + + // Press undo + [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Undo")] + performAction:grey_tap()]; + + // Verify it's back. + [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Second URL")] + assertWithMatcher:grey_notNil()]; +} + // Try moving bookmarks, then undoing that move. - (void)testUndoMoveBookmark { if (base::FeatureList::IsEnabled( @@ -1115,6 +1147,10 @@ // Tests that tapping No thanks on the promo make it disappear. - (void)testPromoNoThanksMakeItDisappear { + if (base::FeatureList::IsEnabled( + bookmark_new_generation::features::kBookmarkNewGeneration)) { + EARL_GREY_TEST_SKIPPED(@"Only enabled with old UI."); + } [BookmarksTestCase setupStandardBookmarks]; [BookmarksTestCase openTopLevelBookmarksFolder]; @@ -1144,9 +1180,12 @@ // Tests that tapping Sign in on the promo make the Sign in sheet appear and // the promo still appears after dismissing the Sign in sheet. - (void)testUIPromoSignIn { + if (base::FeatureList::IsEnabled( + bookmark_new_generation::features::kBookmarkNewGeneration)) { + EARL_GREY_TEST_SKIPPED(@"Only enabled with old UI."); + } [BookmarksTestCase setupStandardBookmarks]; [BookmarksTestCase openTopLevelBookmarksFolder]; - // Set up a fake identity. ChromeIdentity* identity = [FakeChromeIdentity identityWithEmail:@"fakefoo@egmail.com" @@ -1324,6 +1363,28 @@ performAction:grey_tap()]; } +// Tests that the bookmark context bar is shown in MobileBookmarks. +- (void)testBookmarkContextBarShown { + if (!base::FeatureList::IsEnabled( + bookmark_new_generation::features::kBookmarkNewGeneration)) { + EARL_GREY_TEST_SKIPPED(@"Only enabled with new UI."); + } + [BookmarksTestCase setupStandardBookmarks]; + [BookmarksTestCase openMobileBookmarks]; + + // Verify the context bar is shown. + [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Context Bar")] + assertWithMatcher:grey_notNil()]; + + // Verify the context bar's leading and trailing buttons are shown. + [[EarlGrey selectElementWithMatcher:grey_accessibilityID( + @"Context Bar Leading Button")] + assertWithMatcher:grey_notNil()]; + [[EarlGrey selectElementWithMatcher:grey_accessibilityID( + @"Context Bar Trailing Button")] + assertWithMatcher:grey_notNil()]; +} + #pragma mark Helper Methods // Navigates to the bookmark manager UI. @@ -1344,6 +1405,16 @@ // Navigates to the bookmark manager UI, and selects |bookmarkFolder|. + (void)openBookmarkFolder:(NSString*)bookmarkFolder { [BookmarksTestCase openBookmarks]; + if (base::FeatureList::IsEnabled( + bookmark_new_generation::features::kBookmarkNewGeneration)) { + [[EarlGrey + selectElementWithMatcher:grey_allOf(grey_kindOfClass(NSClassFromString( + @"UITableViewCell")), + grey_descendant( + grey_text(@"Mobile Bookmarks")), + nil)] performAction:grey_tap()]; + return; + } if (IsCompact()) { // Opens the bookmark manager sidebar on handsets. [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Menu")]
diff --git a/ios/chrome/browser/ui/bookmarks/bookmarks_new_generation_egtest.mm b/ios/chrome/browser/ui/bookmarks/bookmarks_new_generation_egtest.mm deleted file mode 100644 index e6b0cfa..0000000 --- a/ios/chrome/browser/ui/bookmarks/bookmarks_new_generation_egtest.mm +++ /dev/null
@@ -1,359 +0,0 @@ -// 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. - -#import <EarlGrey/EarlGrey.h> -#import <UIKit/UIKit.h> -#import <XCTest/XCTest.h> -#include <vector> - -#include "base/strings/sys_string_conversions.h" -#include "base/test/scoped_feature_list.h" -#include "components/bookmarks/browser/bookmark_model.h" -#include "components/prefs/pref_service.h" -#include "components/strings/grit/components_strings.h" -#include "ios/chrome/browser/bookmarks/bookmark_model_factory.h" -#include "ios/chrome/browser/bookmarks/bookmark_new_generation_features.h" -#include "ios/chrome/browser/browser_state/chrome_browser_state.h" -#include "ios/chrome/browser/pref_names.h" -#import "ios/chrome/browser/ui/authentication/signin_promo_view.h" -#include "ios/chrome/browser/ui/tools_menu/tools_menu_constants.h" -#import "ios/chrome/browser/ui/uikit_ui_util.h" -#include "ios/chrome/grit/ios_strings.h" -#import "ios/chrome/test/app/bookmarks_test_util.h" -#import "ios/chrome/test/app/chrome_test_util.h" -#import "ios/chrome/test/earl_grey/accessibility_util.h" -#import "ios/chrome/test/earl_grey/chrome_earl_grey.h" -#import "ios/chrome/test/earl_grey/chrome_earl_grey_ui.h" -#import "ios/chrome/test/earl_grey/chrome_matchers.h" -#import "ios/chrome/test/earl_grey/chrome_test_case.h" -#import "ios/public/provider/chrome/browser/signin/fake_chrome_identity.h" -#import "ios/public/provider/chrome/browser/signin/fake_chrome_identity_service.h" -#import "ios/testing/wait_util.h" -#import "ios/web/public/test/http_server/http_server.h" -#include "ios/web/public/test/http_server/http_server_util.h" -#include "ui/base/l10n/l10n_util.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - -using chrome_test_util::ButtonWithAccessibilityLabel; -using chrome_test_util::ButtonWithAccessibilityLabelId; -using chrome_test_util::PrimarySignInButton; -using chrome_test_util::SecondarySignInButton; - -// Matcher for the Delete button on the bookmarks UI. -id<GREYMatcher> BookmarksDeleteSwipeButton() { - return ButtonWithAccessibilityLabel(@"Delete"); -} - -// Matcher for the Back button on the bookmarks UI. -id<GREYMatcher> BookmarksBackButton() { - return ButtonWithAccessibilityLabel(@"Back"); -} - -// Bookmark integration tests for Chrome. -@interface BookmarksNewGenTestCase : ChromeTestCase { - base::test::ScopedFeatureList _scoped_feature_list; -} -@end - -@implementation BookmarksNewGenTestCase - -- (void)setUp { - [super setUp]; - _scoped_feature_list.InitAndEnableFeature( - bookmark_new_generation::features::kBookmarkNewGeneration); - - // Wait for bookmark model to be loaded. - GREYAssert(testing::WaitUntilConditionOrTimeout( - testing::kWaitForUIElementTimeout, - ^{ - return chrome_test_util::BookmarksLoaded(); - }), - @"Bookmark model did not load"); - GREYAssert(chrome_test_util::ClearBookmarks(), - @"Not all bookmarks were removed."); -} - -// Tear down called once per test. -- (void)tearDown { - [super tearDown]; - GREYAssert(chrome_test_util::ClearBookmarks(), - @"Not all bookmarks were removed."); -} - -#pragma mark - Tests - -- (void)testUndoDeleteBookmarkFromSwipe { - [BookmarksNewGenTestCase setupStandardBookmarks]; - [BookmarksNewGenTestCase openBookmarks]; - [BookmarksNewGenTestCase openMobileBookmarks]; - - // Swipe action on the URL. - [[EarlGrey selectElementWithMatcher:grey_accessibilityLabel(@"Second URL")] - performAction:grey_swipeFastInDirection(kGREYDirectionLeft)]; - - // Delete it. - [[EarlGrey selectElementWithMatcher:BookmarksDeleteSwipeButton()] - performAction:grey_tap()]; - - // Wait until it's gone. - [BookmarksNewGenTestCase waitForDeletionOfBookmarkWithTitle:@"Second URL"]; - - // Press undo - [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Undo")] - performAction:grey_tap()]; - - // Verify it's back. - [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Second URL")] - assertWithMatcher:grey_notNil()]; -} - -// Tests that the bookmark context bar is shown in MobileBookmarks. -- (void)testBookmarkContextBarShown { - [BookmarksNewGenTestCase setupStandardBookmarks]; - [BookmarksNewGenTestCase openBookmarks]; - [BookmarksNewGenTestCase openMobileBookmarks]; - - // Verify the context bar is shown. - [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Context Bar")] - assertWithMatcher:grey_notNil()]; - - // Verify the context bar's leading and trailing buttons are shown. - [[EarlGrey selectElementWithMatcher:grey_accessibilityID( - @"Context Bar Leading Button")] - assertWithMatcher:grey_notNil()]; - [[EarlGrey selectElementWithMatcher:grey_accessibilityID( - @"Context Bar Trailing Button")] - assertWithMatcher:grey_notNil()]; -} - -// Tests that the promo view is only seen at root level and not in any of the -// child nodes. -- (void)testPromoViewIsSeenOnlyInRootNode { - [BookmarksNewGenTestCase setupStandardBookmarks]; - [BookmarksNewGenTestCase openBookmarks]; - - // We are going to set the PromoAlreadySeen preference. Set a teardown handler - // to reset it. - [self setTearDownHandler:^{ - [BookmarksNewGenTestCase setPromoAlreadySeen:NO]; - }]; - // Check that promo is visible. - [BookmarksNewGenTestCase verifyPromoAlreadySeen:NO]; - [[EarlGrey selectElementWithMatcher:PrimarySignInButton()] - assertWithMatcher:grey_sufficientlyVisible()]; - - // Go to child node. - [BookmarksNewGenTestCase openMobileBookmarks]; - - // Wait until promo is gone. - [[EarlGrey selectElementWithMatcher:SecondarySignInButton()] - assertWithMatcher:grey_notVisible()]; - - // Check that the promo already seen state is not updated. - [BookmarksNewGenTestCase verifyPromoAlreadySeen:NO]; - - // Come back to root node, and the promo view should appear. - [[EarlGrey selectElementWithMatcher:BookmarksBackButton()] - performAction:grey_tap()]; - - // Check promo view is still visible. - [[EarlGrey selectElementWithMatcher:PrimarySignInButton()] - assertWithMatcher:grey_sufficientlyVisible()]; -} - -// Tests that tapping No thanks on the promo make it disappear. -- (void)testPromoNoThanksMakeItDisappear { - [BookmarksNewGenTestCase setupStandardBookmarks]; - [BookmarksNewGenTestCase openBookmarks]; - - // We are going to set the PromoAlreadySeen preference. Set a teardown handler - // to reset it. - [self setTearDownHandler:^{ - [BookmarksNewGenTestCase setPromoAlreadySeen:NO]; - }]; - // Check that promo is visible. - [BookmarksNewGenTestCase verifyPromoAlreadySeen:NO]; - [[EarlGrey selectElementWithMatcher:PrimarySignInButton()] - assertWithMatcher:grey_sufficientlyVisible()]; - - // Tap the dismiss button. - [[EarlGrey - selectElementWithMatcher:grey_accessibilityID(kSigninPromoCloseButtonId)] - performAction:grey_tap()]; - - // Wait until promo is gone. - [[EarlGrey selectElementWithMatcher:SecondarySignInButton()] - assertWithMatcher:grey_notVisible()]; - - // Check that the promo already seen state is updated. - [BookmarksNewGenTestCase verifyPromoAlreadySeen:YES]; -} - -// Tests that tapping Sign in on the promo make the Sign in sheet appear and -// the promo still appears after dismissing the Sign in sheet. -- (void)testUIPromoSignIn { - [BookmarksNewGenTestCase setupStandardBookmarks]; - [BookmarksNewGenTestCase openBookmarks]; - - // Set up a fake identity. - ChromeIdentity* identity = - [FakeChromeIdentity identityWithEmail:@"fakefoo@egmail.com" - gaiaID:@"fakefoopassword" - name:@"Fake Foo"]; - ios::FakeChromeIdentityService::GetInstanceFromChromeProvider()->AddIdentity( - identity); - - // Check that promo is visible. - [BookmarksNewGenTestCase verifyPromoAlreadySeen:NO]; - [[EarlGrey - selectElementWithMatcher:grey_allOf(SecondarySignInButton(), - grey_sufficientlyVisible(), nil)] - assertWithMatcher:grey_notNil()]; - - // Tap the Sign in button. - [[EarlGrey - selectElementWithMatcher:grey_allOf(grey_accessibilityID( - kSigninPromoSecondaryButtonId), - grey_sufficientlyVisible(), nil)] - performAction:grey_tap()]; - - // Tap the CANCEL button. - [[EarlGrey selectElementWithMatcher: - grey_buttonTitle([l10n_util::GetNSString( - IDS_IOS_ACCOUNT_CONSISTENCY_SETUP_SKIP_BUTTON) - uppercaseString])] performAction:grey_tap()]; - - // Check that the bookmarks UI reappeared and the cell is still here. - [[EarlGrey - selectElementWithMatcher:grey_allOf(SecondarySignInButton(), - grey_sufficientlyVisible(), nil)] - assertWithMatcher:grey_notNil()]; - - [BookmarksNewGenTestCase verifyPromoAlreadySeen:NO]; -} - -#pragma mark - Helpers - -// Navigates to the bookmark manager UI. -+ (void)openBookmarks { - [ChromeEarlGreyUI openToolsMenu]; - - // Opens the bookmark manager. - [[EarlGrey - selectElementWithMatcher:grey_accessibilityID(kToolsMenuBookmarksId)] - performAction:grey_tap()]; - - // Wait for it to load, and the menu to go away. - [[EarlGrey - selectElementWithMatcher:grey_accessibilityID(kToolsMenuBookmarksId)] - assertWithMatcher:grey_nil()]; -} - -// Selects |bookmarkFolder| to open. -+ (void)openBookmarkFolder:(NSString*)bookmarkFolder { - [[EarlGrey - selectElementWithMatcher:grey_allOf( - grey_kindOfClass( - NSClassFromString(@"UITableViewCell")), - grey_descendant(grey_text(bookmarkFolder)), - nil)] performAction:grey_tap()]; -} - -// Loads a set of default bookmarks in the model for the tests to use. -+ (void)setupStandardBookmarks { - [BookmarksNewGenTestCase waitForBookmarkModelLoaded:YES]; - - bookmarks::BookmarkModel* bookmark_model = - ios::BookmarkModelFactory::GetForBrowserState( - chrome_test_util::GetOriginalBrowserState()); - - const GURL firstURL = web::test::HttpServer::MakeUrl( - "http://ios/testing/data/http_server_files/pony.html"); - NSString* firstTitle = @"First URL"; - bookmark_model->AddURL(bookmark_model->mobile_node(), 0, - base::SysNSStringToUTF16(firstTitle), firstURL); - - const GURL secondURL = web::test::HttpServer::MakeUrl( - "http://ios/testing/data/http_server_files/destination.html"); - NSString* secondTitle = @"Second URL"; - bookmark_model->AddURL(bookmark_model->mobile_node(), 0, - base::SysNSStringToUTF16(secondTitle), secondURL); - - NSString* folderTitle = @"Folder 1"; - const bookmarks::BookmarkNode* folder1 = bookmark_model->AddFolder( - bookmark_model->mobile_node(), 0, base::SysNSStringToUTF16(folderTitle)); - - folderTitle = @"Folder 2"; - const bookmarks::BookmarkNode* folder2 = bookmark_model->AddFolder( - folder1, 0, base::SysNSStringToUTF16(folderTitle)); - - folderTitle = @"Folder 3"; - const bookmarks::BookmarkNode* folder3 = bookmark_model->AddFolder( - folder2, 0, base::SysNSStringToUTF16(folderTitle)); - - const GURL thirdURL = web::test::HttpServer::MakeUrl( - "http://ios/testing/data/http_server_files/chromium_logo_page.html"); - NSString* thirdTitle = @"Third URL"; - bookmark_model->AddURL(folder3, 0, base::SysNSStringToUTF16(thirdTitle), - thirdURL); -} - -// Selects MobileBookmarks to open. -+ (void)openMobileBookmarks { - [BookmarksNewGenTestCase openBookmarkFolder:@"Mobile Bookmarks"]; -} - -// Checks that the promo has already been seen or not. -+ (void)verifyPromoAlreadySeen:(BOOL)seen { - ios::ChromeBrowserState* browserState = - chrome_test_util::GetOriginalBrowserState(); - PrefService* prefs = browserState->GetPrefs(); - if (prefs->GetBoolean(prefs::kIosBookmarkPromoAlreadySeen) == seen) { - return; - } - NSString* errorDesc = (seen) - ? @"Expected promo already seen, but it wasn't." - : @"Expected promo not already seen, but it was."; - GREYFail(errorDesc); -} - -// Checks that the promo has already been seen or not. -+ (void)setPromoAlreadySeen:(BOOL)seen { - ios::ChromeBrowserState* browserState = - chrome_test_util::GetOriginalBrowserState(); - PrefService* prefs = browserState->GetPrefs(); - prefs->SetBoolean(prefs::kIosBookmarkPromoAlreadySeen, seen); -} - -// Waits for the disparition of the given |title| in the UI. -+ (void)waitForDeletionOfBookmarkWithTitle:(NSString*)title { - // Wait until it's gone. - ConditionBlock condition = ^{ - NSError* error = nil; - [[EarlGrey selectElementWithMatcher:grey_accessibilityID(title)] - assertWithMatcher:grey_notVisible() - error:&error]; - return error == nil; - }; - GREYAssert(testing::WaitUntilConditionOrTimeout(10, condition), - @"Waiting for bookmark to go away"); -} - -// Waits for the bookmark model to be loaded in memory. -+ (void)waitForBookmarkModelLoaded:(BOOL)loaded { - bookmarks::BookmarkModel* bookmarkModel = - ios::BookmarkModelFactory::GetForBrowserState( - chrome_test_util::GetOriginalBrowserState()); - GREYAssert(testing::WaitUntilConditionOrTimeout( - testing::kWaitForUIElementTimeout, - ^{ - return bookmarkModel->loaded() == loaded; - }), - @"Bookmark model was not loaded"); -} - -@end
diff --git a/ios/chrome/browser/ui/bookmarks/cells/BUILD.gn b/ios/chrome/browser/ui/bookmarks/cells/BUILD.gn index 944f4c4..21f7a9cd 100644 --- a/ios/chrome/browser/ui/bookmarks/cells/BUILD.gn +++ b/ios/chrome/browser/ui/bookmarks/cells/BUILD.gn
@@ -8,10 +8,6 @@ "bookmark_parent_folder_item.mm", "bookmark_table_cell.h", "bookmark_table_cell.mm", - "bookmark_table_promo_cell.h", - "bookmark_table_promo_cell.mm", - "bookmark_table_signin_promo_cell.h", - "bookmark_table_signin_promo_cell.mm", "bookmark_text_field_item.h", "bookmark_text_field_item.mm", ] @@ -21,9 +17,7 @@ "//components/bookmarks/browser:browser", "//ios/chrome/app/strings", "//ios/chrome/browser/ui", - "//ios/chrome/browser/ui/authentication:authentication_ui", "//ios/chrome/browser/ui/collection_view/cells", - "//ios/chrome/browser/ui/colors", "//ios/chrome/browser/ui/icons", "//ios/public/provider/chrome/browser", "//ios/public/provider/chrome/browser/ui",
diff --git a/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_cell.h b/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_cell.h index 74a8674..f689ef4 100644 --- a/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_cell.h +++ b/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_cell.h
@@ -21,7 +21,8 @@ // @interface BookmarkTableCell : UITableViewCell -- (instancetype)initWithReuseIdentifier:(NSString*)bookmarkCellIdentifier +- (instancetype)initWithNode:(const bookmarks::BookmarkNode*)node + reuseIdentifier:(NSString*)bookmarkCellIdentifier NS_DESIGNATED_INITIALIZER; - (instancetype)initWithStyle:(UITableViewCellStyle)style @@ -32,9 +33,6 @@ // Returns the preferred image size for favicons. + (CGFloat)preferredImageSize; -// Identifier for -[UITableView registerClass:forCellWithReuseIdentifier:]. -+ (NSString*)reuseIdentifier; - // Sets the favicon image. - (void)setImage:(UIImage*)image; @@ -42,9 +40,6 @@ - (void)setPlaceholderText:(NSString*)text textColor:(UIColor*)textColor backgroundColor:(UIColor*)backgroundColor; - -// Set the bookmark node this cell shows. -- (void)setNode:(const bookmarks::BookmarkNode*)node; @end #endif // IOS_CHROME_BROWSER_UI_BOOKMARKS_CELLS_BOOKMARK_TABLE_CELL_H_
diff --git a/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_cell.mm b/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_cell.mm index 4fdb7b9a..1b60687 100644 --- a/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_cell.mm +++ b/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_cell.mm
@@ -28,41 +28,35 @@ #pragma mark - Initializer -- (instancetype)initWithReuseIdentifier:(NSString*)bookmarkCellIdentifier { +- (instancetype)initWithNode:(const bookmarks::BookmarkNode*)node + reuseIdentifier:(NSString*)bookmarkCellIdentifier { self = [super initWithStyle:UITableViewCellStyleDefault reuseIdentifier:bookmarkCellIdentifier]; if (self) { + self.textLabel.text = bookmark_utils_ios::TitleForBookmarkNode(node); + self.textLabel.accessibilityIdentifier = self.textLabel.text; self.textLabel.font = [MDCTypography subheadFont]; self.imageView.clipsToBounds = YES; + self.imageView.image = [UIImage imageNamed:@"bookmark_gray_folder"]; [self.imageView setHidden:NO]; _placeholderLabel = [[UILabel alloc] initWithFrame:CGRectZero]; _placeholderLabel.textAlignment = NSTextAlignmentCenter; [_placeholderLabel setHidden:YES]; [self.contentView addSubview:_placeholderLabel]; + + if (node->is_folder()) { + [self setAccessoryType:UITableViewCellAccessoryDisclosureIndicator]; + } else { + [self setAccessoryType:UITableViewCellAccessoryNone]; + } } return self; } #pragma mark - Public -- (void)setNode:(const bookmarks::BookmarkNode*)node { - self.textLabel.text = bookmark_utils_ios::TitleForBookmarkNode(node); - self.textLabel.accessibilityIdentifier = self.textLabel.text; - - self.imageView.image = [UIImage imageNamed:@"bookmark_gray_folder"]; - if (node->is_folder()) { - [self setAccessoryType:UITableViewCellAccessoryDisclosureIndicator]; - } else { - [self setAccessoryType:UITableViewCellAccessoryNone]; - } -} - -+ (NSString*)reuseIdentifier { - return @"BookmarkTableCellIdentifier"; -} - + (CGFloat)preferredImageSize { return kBookmarkTableCellDefaultImageSize; } @@ -90,15 +84,6 @@ #pragma mark - Layout -- (void)prepareForReuse { - self.imageView.image = nil; - self.placeholderLabel.hidden = YES; - self.imageView.hidden = NO; - self.textLabel.text = nil; - self.textLabel.accessibilityIdentifier = nil; - [super prepareForReuse]; -} - - (void)layoutSubviews { [super layoutSubviews]; CGFloat textLabelStart =
diff --git a/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_promo_cell.h b/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_promo_cell.h deleted file mode 100644 index dcf29e9..0000000 --- a/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_promo_cell.h +++ /dev/null
@@ -1,33 +0,0 @@ -// 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 IOS_CHROME_BROWSER_UI_BOOKMARKS_CELLS_BOOKMARK_TABLE_PROMO_CELL_H_ -#define IOS_CHROME_BROWSER_UI_BOOKMARKS_CELLS_BOOKMARK_TABLE_PROMO_CELL_H_ - -#import <UIKit/UIKit.h> - -@class BookmarkTablePromoCell; - -@protocol BookmarkTablePromoCellDelegate - -// Called when the SIGN IN button is tapped. -- (void)bookmarkTablePromoCellDidTapSignIn: - (BookmarkTablePromoCell*)bookmarkPromoCell; - -// Called when the NO THANKS button is tapped. -- (void)bookmarkTablePromoCellDidTapDismiss: - (BookmarkTablePromoCell*)bookmarkPromoCell; - -@end - -@interface BookmarkTablePromoCell : UITableViewCell - -// Identifier for -[UITableView registerClass:forCellWithReuseIdentifier:]. -+ (NSString*)reuseIdentifier; - -@property(nonatomic, weak) id<BookmarkTablePromoCellDelegate> delegate; - -@end - -#endif // IOS_CHROME_BROWSER_UI_BOOKMARKS_CELLS_BOOKMARK_PROMO_CELL_H_
diff --git a/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_promo_cell.mm b/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_promo_cell.mm deleted file mode 100644 index 99cb685c..0000000 --- a/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_promo_cell.mm +++ /dev/null
@@ -1,258 +0,0 @@ -// 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. - -#import "ios/chrome/browser/ui/bookmarks/cells/bookmark_table_promo_cell.h" - -#import <QuartzCore/QuartzCore.h> - -#include "base/logging.h" - -#import "ios/chrome/browser/ui/bookmarks/bookmark_utils_ios.h" -#import "ios/chrome/browser/ui/colors/MDCPalette+CrAdditions.h" -#import "ios/chrome/browser/ui/rtl_geometry.h" -#import "ios/chrome/browser/ui/uikit_ui_util.h" -#import "ios/chrome/browser/ui/util/constraints_ui_util.h" -#include "ios/chrome/grit/ios_chromium_strings.h" -#include "ios/chrome/grit/ios_strings.h" -#import "ios/third_party/material_components_ios/src/components/Buttons/src/MaterialButtons.h" -#import "ios/third_party/material_components_ios/src/components/Typography/src/MaterialTypography.h" -#import "ui/base/l10n/l10n_util_mac.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - -namespace { - -const CGFloat kPadding = 16; -const CGFloat kButtonHeight = 36; -const CGFloat kTitleSubtitleSpace = 8; -const CGFloat kSubtitleButtonsSpace = 8; -const CGFloat kButtonsSpace = 8; - -void SetTextWithLineHeight(UILabel* label, NSString* text, CGFloat lineHeight) { - NSMutableParagraphStyle* paragraphStyle = - [[NSMutableParagraphStyle alloc] init]; - [paragraphStyle setMinimumLineHeight:lineHeight]; - [paragraphStyle setMaximumLineHeight:lineHeight]; - NSDictionary* attributes = @{NSParagraphStyleAttributeName : paragraphStyle}; - NSAttributedString* attributedString = - [[NSAttributedString alloc] initWithString:text attributes:attributes]; - label.attributedText = attributedString; -} - -} // namespace - -// The view that contains the promo UI: the title, the subtitle, the dismiss and -// the sign in buttons. It is common to all size classes, but can be embedded -// differently within the BookmarkTablePromoCell. -@interface BookmarkTablePromoView : UIView - -@property(nonatomic, weak) UILabel* titleLabel; -@property(nonatomic, weak) UILabel* subtitleLabel; -@property(nonatomic, weak) MDCFlatButton* signInButton; -@property(nonatomic, weak) MDCFlatButton* dismissButton; - -@end - -@implementation BookmarkTablePromoView - -@synthesize titleLabel = _titleLabel; -@synthesize subtitleLabel = _subtitleLabel; -@synthesize signInButton = _signInButton; -@synthesize dismissButton = _dismissButton; - -+ (BOOL)requiresConstraintBasedLayout { - return YES; -} - -- (instancetype)initWithFrame:(CGRect)frame { - self = [super initWithFrame:frame]; - if (self) { - self.backgroundColor = [UIColor whiteColor]; - self.accessibilityIdentifier = @"promo_view"; - - // The title. - UILabel* titleLabel = [[UILabel alloc] init]; - _titleLabel = titleLabel; - _titleLabel.textColor = bookmark_utils_ios::darkTextColor(); - _titleLabel.font = [[MDCTypography fontLoader] mediumFontOfSize:16]; - _titleLabel.numberOfLines = 0; - SetTextWithLineHeight(_titleLabel, - l10n_util::GetNSString(IDS_IOS_BOOKMARK_PROMO_TITLE), - 20.f); - _titleLabel.translatesAutoresizingMaskIntoConstraints = NO; - [self addSubview:_titleLabel]; - - // The subtitle. - UILabel* subtitleLabel = [[UILabel alloc] init]; - _subtitleLabel = subtitleLabel; - _subtitleLabel.textColor = bookmark_utils_ios::darkTextColor(); - _subtitleLabel.font = [[MDCTypography fontLoader] regularFontOfSize:14]; - _subtitleLabel.numberOfLines = 0; - SetTextWithLineHeight( - _subtitleLabel, l10n_util::GetNSString(IDS_IOS_BOOKMARK_PROMO_MESSAGE), - 20.f); - _subtitleLabel.translatesAutoresizingMaskIntoConstraints = NO; - [self addSubview:_subtitleLabel]; - - // The sign-in button. - MDCFlatButton* signInButton = [[MDCFlatButton alloc] init]; - _signInButton = signInButton; - [_signInButton setBackgroundColor:[[MDCPalette cr_bluePalette] tint500] - forState:UIControlStateNormal]; - [_signInButton setTitleColor:[UIColor whiteColor] - forState:UIControlStateNormal]; - _signInButton.inkColor = [UIColor colorWithWhite:1 alpha:0.2]; - [_signInButton - setTitle:l10n_util::GetNSString(IDS_IOS_BOOKMARK_PROMO_SIGN_IN_BUTTON) - forState:UIControlStateNormal]; - _signInButton.accessibilityIdentifier = @"promo_sign_in_button"; - _signInButton.translatesAutoresizingMaskIntoConstraints = NO; - [self addSubview:_signInButton]; - - // The dismiss button. - MDCFlatButton* dismissButton = [[MDCFlatButton alloc] init]; - _dismissButton = dismissButton; - [_dismissButton - setTitle:l10n_util::GetNSString(IDS_IOS_BOOKMARK_PROMO_DISMISS_BUTTON) - forState:UIControlStateNormal]; - [_dismissButton setTitleColor:[[MDCPalette cr_bluePalette] tint500] - forState:UIControlStateNormal]; - _dismissButton.accessibilityIdentifier = @"promo_no_thanks_button"; - _dismissButton.translatesAutoresizingMaskIntoConstraints = NO; - [self addSubview:_dismissButton]; - } - return self; -} - -- (void)updateConstraints { - if ([[self constraints] count] == 0) { - NSDictionary* views = @{ - @"title" : self.titleLabel, - @"subtitle" : self.subtitleLabel, - @"signIn" : self.signInButton, - @"dismiss" : self.dismissButton, - }; - NSDictionary* metrics = @{ - @"p" : @(kPadding), - @"b" : @(kButtonHeight), - @"s1" : @(kTitleSubtitleSpace), - @"s2" : @(kSubtitleButtonsSpace), - @"s3" : @(kButtonsSpace), - }; - // clang-format off - NSArray* constraints = @[ - @"V:|-(p)-[title]-(s1)-[subtitle]-(s2)-[signIn(==b)]-(p)-|", - @"H:|-(p)-[title]-(>=p)-|", - @"H:|-(p)-[subtitle]-(>=p)-|", - @"H:|-(>=p)-[dismiss]-(s3)-[signIn]-(p)-|" - ]; - // clang-format on - ApplyVisualConstraintsWithMetricsAndOptions( - constraints, views, metrics, LayoutOptionForRTLSupport(), self); - AddSameCenterYConstraint(self, self.signInButton, self.dismissButton); - } - [super updateConstraints]; -} - -@end - -@interface BookmarkTablePromoCell () { -} -@property(nonatomic, weak) BookmarkTablePromoView* promoView; -@property(nonatomic, strong) NSArray* compactContentViewConstraints; -@end - -@implementation BookmarkTablePromoCell - -@synthesize delegate = _delegate; -@synthesize promoView = _promoView; -@synthesize compactContentViewConstraints = _compactContentViewConstraints; - -+ (NSString*)reuseIdentifier { - return @"BookmarkTablePromoCell"; -} - -+ (BOOL)requiresConstraintBasedLayout { - return YES; -} - -- (instancetype)initWithFrame:(CGRect)frame { - self = [super initWithFrame:frame]; - if (self) { - self.contentView.translatesAutoresizingMaskIntoConstraints = NO; - - BookmarkTablePromoView* promoView = - [[BookmarkTablePromoView alloc] initWithFrame:frame]; - _promoView = promoView; - [_promoView.signInButton addTarget:self - action:@selector(signIn:) - forControlEvents:UIControlEventTouchUpInside]; - [_promoView.dismissButton addTarget:self - action:@selector(dismiss:) - forControlEvents:UIControlEventTouchUpInside]; - _promoView.translatesAutoresizingMaskIntoConstraints = NO; - [self.contentView addSubview:_promoView]; - [self updatePromoView]; - } - return self; -} - -- (void)updateConstraints { - if ([[self constraints] count] == 0) { - NSArray* constraints = @[ @"H:|[contentView]|" ]; - NSDictionary* views = @{@"contentView" : self.contentView}; - ApplyVisualConstraintsWithMetricsAndOptions( - constraints, views, nil, LayoutOptionForRTLSupport(), self); - } - [super updateConstraints]; -} - -- (void)prepareForReuse { - [super prepareForReuse]; - _delegate = nil; -} - -#pragma mark - Private - -- (void)updatePromoView { - self.promoView.layer.cornerRadius = 0; - self.promoView.layer.borderWidth = 0; - self.promoView.layer.borderColor = NULL; - [self.contentView addConstraints:self.compactContentViewConstraints]; -} - -- (NSArray*)compactContentViewConstraints { - if (!_compactContentViewConstraints) { - NSMutableArray* array = [NSMutableArray array]; - NSDictionary* views = @{@"promoView" : self.promoView}; - [array - addObjectsFromArray:[NSLayoutConstraint - constraintsWithVisualFormat:@"H:|[promoView]|" - options:0 - metrics:nil - views:views]]; - [array - addObjectsFromArray:[NSLayoutConstraint - constraintsWithVisualFormat:@"V:|[promoView]|" - options:0 - metrics:nil - views:views]]; - _compactContentViewConstraints = [array copy]; - } - return _compactContentViewConstraints; -} - -#pragma mark - Private actions - -- (void)signIn:(id)sender { - [self.delegate bookmarkTablePromoCellDidTapSignIn:self]; -} - -- (void)dismiss:(id)sender { - [self.delegate bookmarkTablePromoCellDidTapDismiss:self]; -} - -@end
diff --git a/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_signin_promo_cell.h b/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_signin_promo_cell.h deleted file mode 100644 index b81718e..0000000 --- a/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_signin_promo_cell.h +++ /dev/null
@@ -1,28 +0,0 @@ -// 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 IOS_CHROME_BROWSER_UI_BOOKMARKS_CELLS_BOOKMARK_TABLE_SIGNIN_PROMO_CELL_H_ -#define IOS_CHROME_BROWSER_UI_BOOKMARKS_CELLS_BOOKMARK_TABLE_SIGNIN_PROMO_CELL_H_ - -#import <UIKit/UIKit.h> - -@class SigninPromoView; - -typedef void (^CloseButtonCallback)(void); - -// Sign-in promo cell based on SigninPromoView. This cell invites the user to -// login without typing their password. -@interface BookmarkTableSigninPromoCell : UITableViewCell - -// Identifier for -[UITableView registerClass:forCellWithReuseIdentifier:]. -+ (NSString*)reuseIdentifier; - -@property(nonatomic, readonly) SigninPromoView* signinPromoView; -// Called when the user taps on the close button. If not set, the close button -// is hidden. -@property(nonatomic, copy) CloseButtonCallback closeButtonAction; - -@end - -#endif // IOS_CHROME_BROWSER_UI_BOOKMARKS_CELLS_BOOKMARK_TABLE_SIGNIN_PROMO_CELL_H_
diff --git a/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_signin_promo_cell.mm b/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_signin_promo_cell.mm deleted file mode 100644 index 2cc4374..0000000 --- a/ios/chrome/browser/ui/bookmarks/cells/bookmark_table_signin_promo_cell.mm +++ /dev/null
@@ -1,89 +0,0 @@ -// 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. - -#import "ios/chrome/browser/ui/bookmarks/cells/bookmark_table_signin_promo_cell.h" - -#import "ios/chrome/browser/ui/authentication/signin_promo_view.h" -#include "ios/chrome/browser/ui/ui_util.h" -#import "ios/chrome/browser/ui/util/constraints_ui_util.h" -#include "ios/chrome/grit/ios_chromium_strings.h" -#include "ui/base/l10n/l10n_util.h" - -#if !defined(__has_feature) || !__has_feature(objc_arc) -#error "This file requires ARC support." -#endif - -namespace { -const NSInteger kSigninPromoMargin = 8; -} - -@implementation BookmarkTableSigninPromoCell { - SigninPromoView* _signinPromoView; - UIButton* _closeButton; -} - -@synthesize signinPromoView = _signinPromoView; -@synthesize closeButtonAction = _closeButtonAction; - -+ (NSString*)reuseIdentifier { - return @"BookmarkTableSigninPromoCell"; -} - -+ (BOOL)requiresConstraintBasedLayout { - return YES; -} - -- (instancetype)initWithFrame:(CGRect)frame { - self = [super initWithFrame:frame]; - if (self) { - UIView* contentView = self.contentView; - _signinPromoView = [[SigninPromoView alloc] initWithFrame:self.bounds]; - _signinPromoView.translatesAutoresizingMaskIntoConstraints = NO; - [contentView addSubview:_signinPromoView]; - _signinPromoView.layer.borderColor = - [UIColor colorWithWhite:0.0 alpha:0.08].CGColor; - _signinPromoView.layer.borderWidth = 1.0f; - NSArray* visualConstraints = @[ - @"V:|-0-[signin_promo_view]-(margin)-|", - @"H:|-(margin)-[signin_promo_view]-(margin)-|", - ]; - NSDictionary* views = @{@"signin_promo_view" : _signinPromoView}; - NSDictionary* metrics = @{ @"margin" : @(kSigninPromoMargin) }; - ApplyVisualConstraintsWithMetrics(visualConstraints, views, metrics); - _signinPromoView.closeButton.hidden = NO; - [_signinPromoView.closeButton addTarget:self - action:@selector(closeButtonAction:) - forControlEvents:UIControlEventTouchUpInside]; - - _signinPromoView.backgroundColor = [UIColor whiteColor]; - _signinPromoView.textLabel.text = - l10n_util::GetNSString(IDS_IOS_SIGNIN_PROMO_BOOKMARKS); - } - return self; -} - -- (void)layoutSubviews { - // Adjust the text label preferredMaxLayoutWidth according self.frame.width, - // so the text will adjust its height and not its width. - CGFloat parentWidth = CGRectGetWidth(self.bounds); - _signinPromoView.textLabel.preferredMaxLayoutWidth = - parentWidth - 2 * _signinPromoView.horizontalPadding; - - // Re-layout with the new preferred width to allow the label to adjust its - // height. - [super layoutSubviews]; -} - -- (void)prepareForReuse { - [super prepareForReuse]; - _closeButtonAction = nil; - _signinPromoView.delegate = nil; -} - -- (void)closeButtonAction:(id)sender { - DCHECK(_closeButtonAction); - _closeButtonAction(); -} - -@end
diff --git a/ipc/ipc_message_start.h b/ipc/ipc_message_start.h index cd31e63..6359b3f 100644 --- a/ipc/ipc_message_start.h +++ b/ipc/ipc_message_start.h
@@ -23,7 +23,6 @@ NaClMsgStart, GpuChannelMsgStart, MediaMsgStart, - ServiceMsgStart, PpapiMsgStart, FirefoxImporterUnittestMsgStart, FileUtilitiesMsgStart,
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index 0c57108..60a3d87 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -2887,7 +2887,6 @@ crbug.com/736548 [ Mac ] css2.1/t040304-c64-uri-00-a-g.html [ Failure Pass ] -crbug.com/v8/6529 http/tests/devtools/console/console-format.html [ NeedsManualRebaseline ] crbug.com/667560 virtual/mojo-loading/http/tests/devtools/console/console-format.html [ NeedsManualRebaseline ] crbug.com/737959 http/tests/misc/object-image-load-outlives-gc-without-crashing.html [ Failure Pass ]
diff --git a/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-format-expected.txt b/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-format-expected.txt index a974b4b..99da3d48 100644 --- a/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-format-expected.txt +++ b/third_party/WebKit/LayoutTests/http/tests/devtools/console/console-format-expected.txt
@@ -212,10 +212,10 @@ console-format.html:8 [-Infinity] globals[20] -Infinity -console-format.html:7 (10) ["test", "test2", undefined × 2, "test4", undefined × 5, foo: {…}] +console-format.html:7 (10) ["test", "test2", empty × 2, "test4", empty × 5, foo: {…}] console-format.html:8 [Array(10)] globals[21] -(10) ["test", "test2", undefined × 2, "test4", undefined × 5, foo: {…}] +(10) ["test", "test2", empty × 2, "test4", empty × 5, foo: {…}] console-format.html:7 {} console-format.html:8 [{…}] globals[22] @@ -249,10 +249,10 @@ console-format.html:8 [ƒ] globals[29] ƒ Object() { [native code] } -console-format.html:7 {__defineGetter__: ƒ, __defineSetter__: ƒ, hasOwnProperty: ƒ, __lookupGetter__: ƒ, __lookupSetter__: ƒ, …} +console-format.html:7 {constructor: ƒ, __defineGetter__: ƒ, __defineSetter__: ƒ, hasOwnProperty: ƒ, __lookupGetter__: ƒ, …} console-format.html:8 [{…}] globals[30] -{__defineGetter__: ƒ, __defineSetter__: ƒ, hasOwnProperty: ƒ, __lookupGetter__: ƒ, __lookupSetter__: ƒ, …} +{constructor: ƒ, __defineGetter__: ƒ, __defineSetter__: ƒ, hasOwnProperty: ƒ, __lookupGetter__: ƒ, …} console-format.html:7 ƒ ( /**/ foo/**/, /*/**/bar, /**/baz) {} console-format.html:8 [ƒ] @@ -263,10 +263,10 @@ console-format.html:8 [Number] globals[32] Number {[[PrimitiveValue]]: 42} -console-format.html:7 String {0: "a", 1: "b", 2: "c", length: 3, [[PrimitiveValue]]: "abc"} +console-format.html:7 String {[[PrimitiveValue]]: "abc"} console-format.html:8 [String] globals[33] -String {0: "a", 1: "b", 2: "c", length: 3, [[PrimitiveValue]]: "abc"} +String {[[PrimitiveValue]]: "abc"} console-format.html:7 Uint16Array(3) [1, 2, 3] console-format.html:8 [Uint16Array(3)] globals[34] @@ -500,7 +500,7 @@ __proto__: Array(0) globals[20] -Infinity -console-format.html:7 (10) ["test", "test2", undefined × 2, "test4", undefined × 5, foo: {…}] +console-format.html:7 (10) ["test", "test2", empty × 2, "test4", empty × 5, foo: {…}] 0: "test" 1: "test2" 4: "test4" @@ -508,11 +508,11 @@ length: 10 __proto__: Array(0) console-format.html:8 [Array(10)] - 0: (10) ["test", "test2", undefined × 2, "test4", undefined × 5, foo: {…}] + 0: (10) ["test", "test2", empty × 2, "test4", empty × 5, foo: {…}] length: 1 __proto__: Array(0) globals[21] -(10) ["test", "test2", undefined × 2, "test4", undefined × 5, foo: {…}] +(10) ["test", "test2", empty × 2, "test4", empty × 5, foo: {…}] 0: "test" 1: "test2" 4: "test4" @@ -608,7 +608,7 @@ __proto__: Array(0) globals[29] ƒ Object() { [native code] } -console-format.html:7 {__defineGetter__: ƒ, __defineSetter__: ƒ, hasOwnProperty: ƒ, __lookupGetter__: ƒ, __lookupSetter__: ƒ, …} +console-format.html:7 {constructor: ƒ, __defineGetter__: ƒ, __defineSetter__: ƒ, hasOwnProperty: ƒ, __lookupGetter__: ƒ, …} constructor: ƒ Object() hasOwnProperty: ƒ hasOwnProperty() isPrototypeOf: ƒ isPrototypeOf() @@ -623,11 +623,11 @@ get __proto__: ƒ __proto__() set __proto__: ƒ __proto__() console-format.html:8 [{…}] - 0: {__defineGetter__: ƒ, __defineSetter__: ƒ, hasOwnProperty: ƒ, __lookupGetter__: ƒ, __lookupSetter__: ƒ, …} + 0: {constructor: ƒ, __defineGetter__: ƒ, __defineSetter__: ƒ, hasOwnProperty: ƒ, __lookupGetter__: ƒ, …} length: 1 __proto__: Array(0) globals[30] -{__defineGetter__: ƒ, __defineSetter__: ƒ, hasOwnProperty: ƒ, __lookupGetter__: ƒ, __lookupSetter__: ƒ, …} +{constructor: ƒ, __defineGetter__: ƒ, __defineSetter__: ƒ, hasOwnProperty: ƒ, __lookupGetter__: ƒ, …} constructor: ƒ Object() hasOwnProperty: ƒ hasOwnProperty() isPrototypeOf: ƒ isPrototypeOf() @@ -661,7 +661,7 @@ Number {[[PrimitiveValue]]: 42} __proto__: Number [[PrimitiveValue]]: 42 -console-format.html:7 String {0: "a", 1: "b", 2: "c", length: 3, [[PrimitiveValue]]: "abc"} +console-format.html:7 String {[[PrimitiveValue]]: "abc"} 0: "a" 1: "b" 2: "c" @@ -669,11 +669,11 @@ __proto__: String [[PrimitiveValue]]: "abc" console-format.html:8 [String] - 0: String {0: "a", 1: "b", 2: "c", length: 3, [[PrimitiveValue]]: "abc"} + 0: String {[[PrimitiveValue]]: "abc"} length: 1 __proto__: Array(0) globals[33] -String {0: "a", 1: "b", 2: "c", length: 3, [[PrimitiveValue]]: "abc"} +String {[[PrimitiveValue]]: "abc"} 0: "a" 1: "b" 2: "c"
diff --git a/third_party/WebKit/Source/build/scripts/core/css/make_css_property_metadata.py b/third_party/WebKit/Source/build/scripts/core/css/make_css_property_metadata.py index 1f48354..12e5c10 100755 --- a/third_party/WebKit/Source/build/scripts/core/css/make_css_property_metadata.py +++ b/third_party/WebKit/Source/build/scripts/core/css/make_css_property_metadata.py
@@ -24,17 +24,13 @@ 'CSSPropertyMetadata.cpp': self.generate_css_property_metadata_cpp } - for property_value in self._properties.values(): - property_value['supports_percentage'] = ( - 'Percent' in property_value['typedom_types']) @template_expander.use_jinja('core/css/templates/CSSPropertyMetadata.cpp.tmpl', filters=filters) def generate_css_property_metadata_cpp(self): return { 'input_files': self._input_files, 'properties_including_aliases': self._properties_including_aliases, - 'switches': [('is_property', 'IsProperty'), - ('supports_percentage', 'PropertySupportsPercentage')], + 'switches': [('is_property', 'IsProperty')], 'first_enum_value': self._first_enum_value, }
diff --git a/third_party/WebKit/Source/build/scripts/core/css/properties/make_css_property_api_headers.py b/third_party/WebKit/Source/build/scripts/core/css/properties/make_css_property_api_headers.py index 0b44a852..1082750 100755 --- a/third_party/WebKit/Source/build/scripts/core/css/properties/make_css_property_api_headers.py +++ b/third_party/WebKit/Source/build/scripts/core/css/properties/make_css_property_api_headers.py
@@ -62,6 +62,7 @@ 'separator': property_['separator'], 'is_repeated': True if property_['separator'] else False, 'is_descriptor': property_['is_descriptor'], + 'supports_percentage': 'Percent' in property_['typedom_types'], } return generate_property_api_h
diff --git a/third_party/WebKit/Source/build/scripts/core/css/properties/templates/CSSPropertyAPI.h.tmpl b/third_party/WebKit/Source/build/scripts/core/css/properties/templates/CSSPropertyAPI.h.tmpl index 4f5b05e..094c88e 100644 --- a/third_party/WebKit/Source/build/scripts/core/css/properties/templates/CSSPropertyAPI.h.tmpl +++ b/third_party/WebKit/Source/build/scripts/core/css/properties/templates/CSSPropertyAPI.h.tmpl
@@ -46,6 +46,7 @@ return 0; } virtual bool IsDescriptor() const { return false; } + virtual bool SupportsPercentage() const { return false; } }; {% for api_class_data in api_classes_by_property_id %}
diff --git a/third_party/WebKit/Source/build/scripts/core/css/properties/templates/CSSPropertyAPISubclass.h.tmpl b/third_party/WebKit/Source/build/scripts/core/css/properties/templates/CSSPropertyAPISubclass.h.tmpl index aaf03fb..50c29d3 100644 --- a/third_party/WebKit/Source/build/scripts/core/css/properties/templates/CSSPropertyAPISubclass.h.tmpl +++ b/third_party/WebKit/Source/build/scripts/core/css/properties/templates/CSSPropertyAPISubclass.h.tmpl
@@ -39,6 +39,9 @@ {% if is_descriptor %} bool IsDescriptor() const override { return true; } {% endif %} + {% if supports_percentage %} + bool SupportsPercentage() const override { return true; } + {% endif %} }; } // namespace blink
diff --git a/third_party/WebKit/Source/build/scripts/core/css/templates/CSSOMTypes.cpp.tmpl b/third_party/WebKit/Source/build/scripts/core/css/templates/CSSOMTypes.cpp.tmpl index e84394b..34f295c 100644 --- a/third_party/WebKit/Source/build/scripts/core/css/templates/CSSOMTypes.cpp.tmpl +++ b/third_party/WebKit/Source/build/scripts/core/css/templates/CSSOMTypes.cpp.tmpl
@@ -11,6 +11,7 @@ #include "core/css/cssom/CSSOMKeywords.h" #include "core/css/cssom/CSSKeywordValue.h" #include "core/css/cssom/CSSStyleValue.h" +#include "core/css/properties/CSSPropertyAPI.h" namespace blink { @@ -21,7 +22,7 @@ id, ToCSSKeywordValue(styleValue)); } if (styleValue.ContainsPercent() && - !CSSPropertyMetadata::PropertySupportsPercentage(id)) { + !CSSPropertyAPI::Get(id).SupportsPercentage()) { return false; } if (styleValue.GetType() == CSSStyleValue::kUnknownType) {
diff --git a/ui/webui/resources/cr_elements/cr_dialog/cr_dialog.html b/ui/webui/resources/cr_elements/cr_dialog/cr_dialog.html index bb556e4..6e99999 100644 --- a/ui/webui/resources/cr_elements/cr_dialog/cr_dialog.html +++ b/ui/webui/resources/cr_elements/cr_dialog/cr_dialog.html
@@ -55,6 +55,7 @@ font-size: calc(15 / 13 * 100%); line-height: 1; padding: 16px 16px; + @apply(--cr-dialog-title); } :host ::slotted([slot=button-container]) { @@ -95,7 +96,7 @@ .top-container { align-items: flex-start; display: flex; - min-height: 47px; + min-height: var(--cr-dialog-top-container-min-height, 47px); } .title-container { @@ -127,6 +128,7 @@ on-keypress="onCloseKeypress_"> </button> </div> + <slot name="header"></slot> <div class="body-container"> <span id="bodyTopMarker"></span> <slot name="body"></slot>