diff --git a/DEPS b/DEPS index 5023cef..d8ecf8d 100644 --- a/DEPS +++ b/DEPS
@@ -44,7 +44,7 @@ # 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': '02777351e7ebb36db9477e225ca6e7689cf99a9e', + 'v8_revision': '2e13db8d39d701f1b4dc3d351babf7b8fb7643f9', # 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. @@ -96,7 +96,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling catapult # and whatever else without interference from each other. - 'catapult_revision': 'deb4a2aa8f11cb520e9b8bc7413296d899a3af47', + 'catapult_revision': '29612b077d93e63d1c79645fdece5b04c9724682', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling libFuzzer # and whatever else without interference from each other.
diff --git a/android_webview/browser/aw_browser_context.cc b/android_webview/browser/aw_browser_context.cc index 5130641..0835b14 100644 --- a/android_webview/browser/aw_browser_context.cc +++ b/android_webview/browser/aw_browser_context.cc
@@ -323,6 +323,11 @@ return nullptr; } +content::BrowsingDataRemoverDelegate* +AwBrowserContext::GetBrowsingDataRemoverDelegate() { + return nullptr; +} + net::URLRequestContextGetter* AwBrowserContext::CreateRequestContext( content::ProtocolHandlerMap* protocol_handlers, content::URLRequestInterceptorScopedVector request_interceptors) {
diff --git a/android_webview/browser/aw_browser_context.h b/android_webview/browser/aw_browser_context.h index c298c41..7e907d0 100644 --- a/android_webview/browser/aw_browser_context.h +++ b/android_webview/browser/aw_browser_context.h
@@ -97,6 +97,8 @@ content::SSLHostStateDelegate* GetSSLHostStateDelegate() override; content::PermissionManager* GetPermissionManager() override; content::BackgroundSyncController* GetBackgroundSyncController() override; + content::BrowsingDataRemoverDelegate* GetBrowsingDataRemoverDelegate() + override; net::URLRequestContextGetter* CreateRequestContext( content::ProtocolHandlerMap* protocol_handlers, content::URLRequestInterceptorScopedVector request_interceptors) override;
diff --git a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc index 9bec3ce..127397c 100644 --- a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc +++ b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
@@ -755,21 +755,6 @@ DISALLOW_COPY_AND_ASSIGN(RemoveDownloadsTester); }; -// TestingProfile does not contain ChromeBrowsingDataRemoverDelegate. Add it -// for the purpose of this test. -class TestingProfileWithDelegate : public TestingProfile { - public: - TestingProfileWithDelegate() : delegate_(this) {} - - content::BrowsingDataRemoverDelegate* GetBrowsingDataRemoverDelegate() - override { - return &delegate_; - } - - private: - ChromeBrowsingDataRemoverDelegate delegate_; -}; - } // namespace // RemoveAutofillTester is not a part of the anonymous namespace above, as @@ -947,7 +932,7 @@ class ChromeBrowsingDataRemoverDelegateTest : public testing::Test { public: ChromeBrowsingDataRemoverDelegateTest() - : profile_(new TestingProfileWithDelegate()), + : profile_(new TestingProfile()), clear_domain_reliability_tester_(profile_.get()) { remover_ = content::BrowserContext::GetBrowsingDataRemover(profile_.get());
diff --git a/chrome/browser/page_load_metrics/metrics_web_contents_observer.cc b/chrome/browser/page_load_metrics/metrics_web_contents_observer.cc index fe9f2818..c3d51772 100644 --- a/chrome/browser/page_load_metrics/metrics_web_contents_observer.cc +++ b/chrome/browser/page_load_metrics/metrics_web_contents_observer.cc
@@ -103,10 +103,20 @@ return metrics; } -MetricsWebContentsObserver::~MetricsWebContentsObserver() { +MetricsWebContentsObserver::~MetricsWebContentsObserver() {} + +void MetricsWebContentsObserver::WebContentsDestroyed() { // TODO(csharrison): Use a more user-initiated signal for CLOSE. NotifyPageEndAllLoads(END_CLOSE, UserInitiatedInfo::NotUserInitiated()); + // We tear down PageLoadTrackers in WebContentsDestroyed, rather than in the + // destructor, since |web_contents()| returns nullptr in the destructor, and + // PageLoadMetricsObservers can cause code to execute that wants to be able to + // access the current WebContents. + committed_load_ = nullptr; + provisional_loads_.clear(); + aborted_provisional_loads_.clear(); + for (auto& observer : testing_observers_) observer.OnGoingAway(); }
diff --git a/chrome/browser/page_load_metrics/metrics_web_contents_observer.h b/chrome/browser/page_load_metrics/metrics_web_contents_observer.h index ac5c21b..27bc8c8 100644 --- a/chrome/browser/page_load_metrics/metrics_web_contents_observer.h +++ b/chrome/browser/page_load_metrics/metrics_web_contents_observer.h
@@ -97,6 +97,7 @@ void MediaStartedPlaying( const content::WebContentsObserver::MediaPlayerInfo& video_type, const content::WebContentsObserver::MediaPlayerId& id) override; + void WebContentsDestroyed() override; // These methods are forwarded from the MetricsNavigationThrottle. void WillStartNavigationRequest(content::NavigationHandle* navigation_handle);
diff --git a/chrome/browser/page_load_metrics/metrics_web_contents_observer_unittest.cc b/chrome/browser/page_load_metrics/metrics_web_contents_observer_unittest.cc index 3a1b844d8..4c38023 100644 --- a/chrome/browser/page_load_metrics/metrics_web_contents_observer_unittest.cc +++ b/chrome/browser/page_load_metrics/metrics_web_contents_observer_unittest.cc
@@ -9,14 +9,17 @@ #include "base/macros.h" #include "base/memory/ptr_util.h" +#include "base/memory/weak_ptr.h" #include "base/process/kill.h" #include "base/test/histogram_tester.h" #include "base/time/time.h" +#include "base/timer/mock_timer.h" #include "chrome/browser/page_load_metrics/metrics_navigation_throttle.h" #include "chrome/browser/page_load_metrics/page_load_metrics_embedder_interface.h" #include "chrome/browser/page_load_metrics/page_load_metrics_observer.h" #include "chrome/browser/page_load_metrics/page_load_tracker.h" #include "chrome/common/page_load_metrics/page_load_metrics_messages.h" +#include "chrome/common/page_load_metrics/test/weak_mock_timer.h" #include "chrome/common/url_constants.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h" #include "content/public/browser/navigation_handle.h" @@ -117,7 +120,8 @@ }; class TestPageLoadMetricsEmbedderInterface - : public PageLoadMetricsEmbedderInterface { + : public PageLoadMetricsEmbedderInterface, + public test::WeakMockTimerProvider { public: TestPageLoadMetricsEmbedderInterface() : is_ntp_(false) {} @@ -130,6 +134,11 @@ tracker->AddObserver(base::MakeUnique<FilteringPageLoadMetricsObserver>( &completed_filtered_urls_)); } + std::unique_ptr<base::Timer> CreateTimer() override { + auto timer = base::MakeUnique<test::WeakMockTimer>(); + SetMockTimer(timer->AsWeakPtr()); + return std::move(timer); + } const std::vector<mojom::PageLoadTimingPtr>& updated_timings() const { return updated_timings_; } @@ -160,6 +169,14 @@ bool is_ntp_; }; +void PopulatePageLoadTiming(mojom::PageLoadTiming* timing) { + page_load_metrics::InitPageLoadTimingForTest(timing); + timing->navigation_start = base::Time::FromDoubleT(1); + timing->response_start = base::TimeDelta::FromMilliseconds(10); + timing->parse_timing->parse_start = base::TimeDelta::FromMilliseconds(20); + timing->document_timing->first_layout = base::TimeDelta::FromMilliseconds(30); +} + } // namespace class MetricsWebContentsObserverTest : public ChromeRenderViewHostTestHarness { @@ -176,12 +193,30 @@ ->NavigateAndCommit(GURL(url::kAboutBlankURL)); } + // Returns the mock timer used for buffering updates in the + // PageLoadMetricsUpdateDispatcher. + base::MockTimer* GetMostRecentTimer() { + return embedder_interface_->GetMockTimer(); + } + void SimulateTimingUpdate(const mojom::PageLoadTiming& timing) { SimulateTimingUpdate(timing, web_contents()->GetMainFrame()); } void SimulateTimingUpdate(const mojom::PageLoadTiming& timing, content::RenderFrameHost* render_frame_host) { + SimulateTimingUpdateWithoutFiringDispatchTimer(timing, render_frame_host); + // If sending the timing update caused the PageLoadMetricsUpdateDispatcher + // to schedule a buffering timer, then fire it now so metrics are dispatched + // to observers. + base::MockTimer* mock_timer = GetMostRecentTimer(); + if (mock_timer && mock_timer->IsRunning()) + mock_timer->Fire(); + } + + void SimulateTimingUpdateWithoutFiringDispatchTimer( + const mojom::PageLoadTiming& timing, + content::RenderFrameHost* render_frame_host) { observer()->OnTimingUpdated(render_frame_host, timing, mojom::PageLoadMetadata()); } @@ -736,13 +771,7 @@ // Dispatch a timing update for the child frame that includes a first paint. mojom::PageLoadTiming subframe_timing; - page_load_metrics::InitPageLoadTimingForTest(&subframe_timing); - subframe_timing.navigation_start = base::Time::FromDoubleT(2); - subframe_timing.response_start = base::TimeDelta::FromMilliseconds(10); - subframe_timing.parse_timing->parse_start = - base::TimeDelta::FromMilliseconds(20); - subframe_timing.document_timing->first_layout = - base::TimeDelta::FromMilliseconds(30); + PopulatePageLoadTiming(&subframe_timing); subframe_timing.paint_timing->first_paint = base::TimeDelta::FromMilliseconds(40); content::RenderFrameHostTester* subframe_tester = @@ -798,4 +827,125 @@ CheckNoErrorEvents(); } +// We buffer cross-frame paint updates to account for paint timings from +// different frames arriving out of order. +TEST_F(MetricsWebContentsObserverTest, OutOfOrderCrossFrameTiming2) { + // Dispatch a timing update for the main frame that includes a first + // paint. This should be buffered, with the dispatch timer running. + mojom::PageLoadTiming timing; + PopulatePageLoadTiming(&timing); + timing.paint_timing->first_paint = base::TimeDelta::FromMilliseconds(1000); + content::WebContentsTester* web_contents_tester = + content::WebContentsTester::For(web_contents()); + web_contents_tester->NavigateAndCommit(GURL(kDefaultTestUrl)); + SimulateTimingUpdateWithoutFiringDispatchTimer(timing, main_rfh()); + + EXPECT_TRUE(GetMostRecentTimer()->IsRunning()); + ASSERT_EQ(0, CountUpdatedTimingReported()); + + content::RenderFrameHostTester* rfh_tester = + content::RenderFrameHostTester::For(main_rfh()); + + // Dispatch a timing update for a child frame that includes a first paint. + mojom::PageLoadTiming subframe_timing; + PopulatePageLoadTiming(&subframe_timing); + subframe_timing.paint_timing->first_paint = + base::TimeDelta::FromMilliseconds(500); + content::RenderFrameHost* subframe = rfh_tester->AppendChild("subframe"); + content::RenderFrameHostTester* subframe_tester = + content::RenderFrameHostTester::For(subframe); + subframe_tester->SimulateNavigationStart(GURL(kDefaultTestUrl2)); + subframe_tester->SimulateNavigationCommit(GURL(kDefaultTestUrl2)); + SimulateTimingUpdateWithoutFiringDispatchTimer(subframe_timing, subframe); + subframe_tester->SimulateNavigationStop(); + + histogram_tester_.ExpectTotalCount( + page_load_metrics::internal::kHistogramOutOfOrderTiming, 1); + + EXPECT_TRUE(GetMostRecentTimer()->IsRunning()); + ASSERT_EQ(0, CountUpdatedTimingReported()); + + // At this point, the timing update is buffered, waiting for the timer to + // fire. + GetMostRecentTimer()->Fire(); + + // Firing the timer should produce a timing update. The update should be a + // merged view of the main frame timing, with a first paint timestamp from the + // subframe. + ASSERT_EQ(1, CountUpdatedTimingReported()); + EXPECT_FALSE(timing.Equals(*updated_timings().back())); + EXPECT_TRUE( + updated_timings().back()->parse_timing->Equals(*timing.parse_timing)); + EXPECT_TRUE(updated_timings().back()->document_timing->Equals( + *timing.document_timing)); + EXPECT_FALSE( + updated_timings().back()->paint_timing->Equals(*timing.paint_timing)); + EXPECT_TRUE(updated_timings().back()->paint_timing->first_paint); + + // The first paint value should be the min of all received first paints, which + // in this case is the first paint from the subframe. Since it is offset by + // the subframe's navigation start, the received value should be >= the first + // paint value specified in the subframe. + EXPECT_GE(updated_timings().back()->paint_timing->first_paint, + subframe_timing.paint_timing->first_paint); + EXPECT_LT(updated_timings().back()->paint_timing->first_paint, + timing.paint_timing->first_paint); + + base::TimeDelta initial_first_paint = + updated_timings().back()->paint_timing->first_paint.value(); + + // Dispatch a timing update for an additional child frame, with an earlier + // first paint time. This should cause an immediate update, without a timer + // delay. + subframe_timing.paint_timing->first_paint = + base::TimeDelta::FromMilliseconds(50); + content::RenderFrameHost* subframe2 = rfh_tester->AppendChild("subframe"); + content::RenderFrameHostTester* subframe2_tester = + content::RenderFrameHostTester::For(subframe2); + subframe2_tester->SimulateNavigationStart(GURL(kDefaultTestUrl2)); + subframe2_tester->SimulateNavigationCommit(GURL(kDefaultTestUrl2)); + SimulateTimingUpdateWithoutFiringDispatchTimer(subframe_timing, subframe2); + subframe2_tester->SimulateNavigationStop(); + + base::TimeDelta updated_first_paint = + updated_timings().back()->paint_timing->first_paint.value(); + + EXPECT_FALSE(GetMostRecentTimer()->IsRunning()); + ASSERT_EQ(2, CountUpdatedTimingReported()); + EXPECT_LT(updated_first_paint, initial_first_paint); + + histogram_tester_.ExpectTotalCount( + page_load_metrics::internal::kHistogramOutOfOrderTimingBuffered, 1); + histogram_tester_.ExpectBucketCount( + page_load_metrics::internal::kHistogramOutOfOrderTimingBuffered, + (initial_first_paint - updated_first_paint).InMilliseconds(), 1); + + CheckNoErrorEvents(); +} + +TEST_F(MetricsWebContentsObserverTest, DispatchDelayedMetricsOnPageClose) { + mojom::PageLoadTiming timing; + PopulatePageLoadTiming(&timing); + timing.paint_timing->first_paint = base::TimeDelta::FromMilliseconds(1000); + content::WebContentsTester* web_contents_tester = + content::WebContentsTester::For(web_contents()); + web_contents_tester->NavigateAndCommit(GURL(kDefaultTestUrl)); + SimulateTimingUpdateWithoutFiringDispatchTimer(timing, main_rfh()); + + EXPECT_TRUE(GetMostRecentTimer()->IsRunning()); + ASSERT_EQ(0, CountUpdatedTimingReported()); + ASSERT_EQ(0, CountCompleteTimingReported()); + + // Navigate to a new page. This should force dispatch of the buffered timing + // update. + NavigateToUntrackedUrl(); + + ASSERT_EQ(1, CountUpdatedTimingReported()); + ASSERT_EQ(1, CountCompleteTimingReported()); + EXPECT_TRUE(timing.Equals(*updated_timings().back())); + EXPECT_TRUE(timing.Equals(*complete_timings().back())); + + CheckNoErrorEvents(); +} + } // namespace page_load_metrics
diff --git a/chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.cc b/chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.cc index bc777a0..37a51c54 100644 --- a/chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.cc +++ b/chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.cc
@@ -38,6 +38,12 @@ test_->RegisterObservers(tracker); } + std::unique_ptr<base::Timer> CreateTimer() override { + auto timer = base::MakeUnique<test::WeakMockTimer>(); + test_->SetMockTimer(timer->AsWeakPtr()); + return std::move(timer); + } + private: PageLoadMetricsObserverTestHarness* test_; @@ -51,76 +57,6 @@ PageLoadMetricsObserverTestHarness::~PageLoadMetricsObserverTestHarness() {} -// static -void PageLoadMetricsObserverTestHarness::PopulateRequiredTimingFields( - mojom::PageLoadTiming* inout_timing) { - if (inout_timing->paint_timing->first_meaningful_paint && - !inout_timing->paint_timing->first_contentful_paint) { - inout_timing->paint_timing->first_contentful_paint = - inout_timing->paint_timing->first_meaningful_paint; - } - if ((inout_timing->paint_timing->first_text_paint || - inout_timing->paint_timing->first_image_paint || - inout_timing->paint_timing->first_contentful_paint) && - !inout_timing->paint_timing->first_paint) { - inout_timing->paint_timing->first_paint = - OptionalMin(OptionalMin(inout_timing->paint_timing->first_text_paint, - inout_timing->paint_timing->first_image_paint), - inout_timing->paint_timing->first_contentful_paint); - } - if (inout_timing->paint_timing->first_paint && - !inout_timing->document_timing->first_layout) { - inout_timing->document_timing->first_layout = - inout_timing->paint_timing->first_paint; - } - if (inout_timing->document_timing->load_event_start && - !inout_timing->document_timing->dom_content_loaded_event_start) { - inout_timing->document_timing->dom_content_loaded_event_start = - inout_timing->document_timing->load_event_start; - } - if (inout_timing->document_timing->first_layout && - !inout_timing->parse_timing->parse_start) { - inout_timing->parse_timing->parse_start = - inout_timing->document_timing->first_layout; - } - if (inout_timing->document_timing->dom_content_loaded_event_start && - !inout_timing->parse_timing->parse_stop) { - inout_timing->parse_timing->parse_stop = - inout_timing->document_timing->dom_content_loaded_event_start; - } - if (inout_timing->parse_timing->parse_stop && - !inout_timing->parse_timing->parse_start) { - inout_timing->parse_timing->parse_start = - inout_timing->parse_timing->parse_stop; - } - if (inout_timing->parse_timing->parse_start && - !inout_timing->response_start) { - inout_timing->response_start = inout_timing->parse_timing->parse_start; - } - if (inout_timing->parse_timing->parse_start) { - if (!inout_timing->parse_timing->parse_blocked_on_script_load_duration) - inout_timing->parse_timing->parse_blocked_on_script_load_duration = - base::TimeDelta(); - if (!inout_timing->parse_timing - ->parse_blocked_on_script_execution_duration) { - inout_timing->parse_timing->parse_blocked_on_script_execution_duration = - base::TimeDelta(); - } - if (!inout_timing->parse_timing - ->parse_blocked_on_script_load_from_document_write_duration) { - inout_timing->parse_timing - ->parse_blocked_on_script_load_from_document_write_duration = - base::TimeDelta(); - } - if (!inout_timing->parse_timing - ->parse_blocked_on_script_execution_from_document_write_duration) { - inout_timing->parse_timing - ->parse_blocked_on_script_execution_from_document_write_duration = - base::TimeDelta(); - } - } -} - void PageLoadMetricsObserverTestHarness::SetUp() { ChromeRenderViewHostTestHarness::SetUp(); SetContents(CreateTestWebContents()); @@ -146,6 +82,12 @@ const mojom::PageLoadTiming& timing, const mojom::PageLoadMetadata& metadata) { observer_->OnTimingUpdated(web_contents()->GetMainFrame(), timing, metadata); + // If sending the timing update caused the PageLoadMetricsUpdateDispatcher to + // schedule a buffering timer, then fire it now so metrics are dispatched to + // observers. + base::MockTimer* mock_timer = GetMockTimer(); + if (mock_timer && mock_timer->IsRunning()) + mock_timer->Fire(); } void PageLoadMetricsObserverTestHarness::SimulateStartedResource(
diff --git a/chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.h b/chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.h index 5d4cd7da..79c2ce5 100644 --- a/chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.h +++ b/chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.h
@@ -9,6 +9,8 @@ #include "base/test/histogram_tester.h" #include "chrome/browser/page_load_metrics/metrics_web_contents_observer.h" #include "chrome/browser/page_load_metrics/page_load_tracker.h" +#include "chrome/common/page_load_metrics/test/page_load_metrics_test_util.h" +#include "chrome/common/page_load_metrics/test/weak_mock_timer.h" #include "chrome/common/url_constants.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h" #include "content/public/test/web_contents_tester.h" @@ -21,15 +23,12 @@ // an observer, override RegisterObservers and call tracker->AddObserver. This // will attach the observer to all main frame navigations. class PageLoadMetricsObserverTestHarness - : public ChromeRenderViewHostTestHarness { + : public ChromeRenderViewHostTestHarness, + public test::WeakMockTimerProvider { public: PageLoadMetricsObserverTestHarness(); ~PageLoadMetricsObserverTestHarness() override; - // Helper that fills in any timing fields that MWCO requires but that are - // currently missing. - static void PopulateRequiredTimingFields(mojom::PageLoadTiming* inout_timing); - void SetUp() override; virtual void RegisterObservers(PageLoadTracker* tracker) {}
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_embedder_interface.h b/chrome/browser/page_load_metrics/page_load_metrics_embedder_interface.h index 983a0145..000fa2e 100644 --- a/chrome/browser/page_load_metrics/page_load_metrics_embedder_interface.h +++ b/chrome/browser/page_load_metrics/page_load_metrics_embedder_interface.h
@@ -5,8 +5,14 @@ #ifndef CHROME_BROWSER_PAGE_LOAD_METRICS_PAGE_LOAD_METRICS_EMBEDDER_INTERFACE_H_ #define CHROME_BROWSER_PAGE_LOAD_METRICS_PAGE_LOAD_METRICS_EMBEDDER_INTERFACE_H_ +#include <memory> + class GURL; +namespace base { +class Timer; +} // namespace base + namespace page_load_metrics { class PageLoadTracker; @@ -18,6 +24,7 @@ virtual ~PageLoadMetricsEmbedderInterface() {} virtual bool IsNewTabPageUrl(const GURL& url) = 0; virtual void RegisterObservers(PageLoadTracker* metrics) = 0; + virtual std::unique_ptr<base::Timer> CreateTimer() = 0; }; } // namespace page_load_metrics
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc b/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc index ac44410..c0c3791c 100644 --- a/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc +++ b/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc
@@ -6,6 +6,7 @@ #include "base/macros.h" #include "base/memory/ptr_util.h" +#include "base/timer/timer.h" #include "chrome/browser/page_load_metrics/metrics_web_contents_observer.h" #if defined(OS_ANDROID) #include "chrome/browser/page_load_metrics/observers/android_page_load_metrics_observer.h" @@ -55,6 +56,7 @@ // page_load_metrics::PageLoadMetricsEmbedderInterface: bool IsNewTabPageUrl(const GURL& url) override; void RegisterObservers(page_load_metrics::PageLoadTracker* tracker) override; + std::unique_ptr<base::Timer> CreateTimer() override; private: bool IsPrerendering() const; @@ -142,6 +144,10 @@ nullptr; } +std::unique_ptr<base::Timer> PageLoadMetricsEmbedder::CreateTimer() { + return base::MakeUnique<base::OneShotTimer>(); +} + bool PageLoadMetricsEmbedder::IsNewTabPageUrl(const GURL& url) { Profile* profile = Profile::FromBrowserContext(web_contents_->GetBrowserContext());
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_update_dispatcher.cc b/chrome/browser/page_load_metrics/page_load_metrics_update_dispatcher.cc index f5e4ec5..0f47b82 100644 --- a/chrome/browser/page_load_metrics/page_load_metrics_update_dispatcher.cc +++ b/chrome/browser/page_load_metrics/page_load_metrics_update_dispatcher.cc
@@ -25,6 +25,10 @@ const char kPageLoadTimingStatus[] = "PageLoad.Internal.PageLoadTimingStatus"; const char kPageLoadTimingDispatchStatus[] = "PageLoad.Internal.PageLoadTimingStatus.AtTimingCallbackDispatch"; +const char kHistogramOutOfOrderTiming[] = + "PageLoad.Internal.OutOfOrderInterFrameTiming"; +const char kHistogramOutOfOrderTimingBuffered[] = + "PageLoad.Internal.OutOfOrderInterFrameTiming.AfterBuffering"; } // namespace internal @@ -200,10 +204,57 @@ return internal::VALID; } +// If the updated value has an earlier time than the current value, log so we +// can keep track of how often this happens. +void LogIfOutOfOrderTiming(const base::Optional<base::TimeDelta>& current, + const base::Optional<base::TimeDelta>& update) { + if (!current || !update) + return; + + if (update < current) { + PAGE_LOAD_HISTOGRAM(internal::kHistogramOutOfOrderTimingBuffered, + current.value() - update.value()); + } +} + +// PaintTimingMerger merges paint timing values received from different frames +// together. +class PaintTimingMerger { + public: + explicit PaintTimingMerger(mojom::PaintTiming* target) : target_(target) {} + + // Merge paint timing values from |new_paint_timing| into the target + // PaintTiming. + void Merge(base::TimeDelta navigation_start_offset, + const mojom::PaintTiming& new_paint_timing, + bool is_main_frame); + + // Whether we merged a new value, for a paint timing field we didn't + // previously have a value for in the target PaintTiming. + bool did_merge_new_timing_value() const { + return did_merge_new_timing_value_; + } + + private: + void MaybeUpdateTimeDelta( + base::Optional<base::TimeDelta>* inout_existing_value, + base::TimeDelta navigation_start_offset, + const base::Optional<base::TimeDelta>& optional_candidate_new_value); + + // The target PaintTiming we are merging values into. + mojom::PaintTiming* const target_; + + // Whether we merged a new value, for a paint timing field we didn't + // previously have a value for in |target_|. + bool did_merge_new_timing_value_ = false; + + DISALLOW_COPY_AND_ASSIGN(PaintTimingMerger); +}; + // Updates *|inout_existing_value| with |optional_candidate_new_value|, if // either *|inout_existing_value| isn't set, or |optional_candidate_new_value| < // |inout_existing_value|. -void MaybeUpdateTimeDelta( +void PaintTimingMerger::MaybeUpdateTimeDelta( base::Optional<base::TimeDelta>* inout_existing_value, base::TimeDelta navigation_start_offset, const base::Optional<base::TimeDelta>& optional_candidate_new_value) { @@ -228,13 +279,33 @@ // occasionally, as inter-frame updates can arrive out of order. Record a // histogram to track how frequently it happens, along with the magnitude // of the delta. - PAGE_LOAD_HISTOGRAM("PageLoad.Internal.OutOfOrderInterFrameTiming", + PAGE_LOAD_HISTOGRAM(internal::kHistogramOutOfOrderTiming, inout_existing_value->value() - candidate_new_value); + } else { + did_merge_new_timing_value_ = true; } *inout_existing_value = candidate_new_value; } +void PaintTimingMerger::Merge(base::TimeDelta navigation_start_offset, + const mojom::PaintTiming& new_paint_timing, + bool is_main_frame) { + MaybeUpdateTimeDelta(&target_->first_paint, navigation_start_offset, + new_paint_timing.first_paint); + MaybeUpdateTimeDelta(&target_->first_text_paint, navigation_start_offset, + new_paint_timing.first_text_paint); + MaybeUpdateTimeDelta(&target_->first_image_paint, navigation_start_offset, + new_paint_timing.first_image_paint); + MaybeUpdateTimeDelta(&target_->first_contentful_paint, + navigation_start_offset, + new_paint_timing.first_contentful_paint); + if (is_main_frame) { + // First meaningful paint is only tracked in the main frame. + target_->first_meaningful_paint = new_paint_timing.first_meaningful_paint; + } +} + } // namespace PageLoadMetricsUpdateDispatcher::PageLoadMetricsUpdateDispatcher( @@ -243,13 +314,24 @@ PageLoadMetricsEmbedderInterface* embedder_interface) : client_(client), embedder_interface_(embedder_interface), + timer_(embedder_interface->CreateTimer()), navigation_start_(navigation_handle->NavigationStart()), current_merged_page_timing_(CreatePageLoadTiming()), pending_merged_page_timing_(CreatePageLoadTiming()), main_frame_metadata_(mojom::PageLoadMetadata::New()), subframe_metadata_(mojom::PageLoadMetadata::New()) {} -PageLoadMetricsUpdateDispatcher::~PageLoadMetricsUpdateDispatcher() {} +PageLoadMetricsUpdateDispatcher::~PageLoadMetricsUpdateDispatcher() { + ShutDown(); +} + +void PageLoadMetricsUpdateDispatcher::ShutDown() { + if (timer_ && timer_->IsRunning()) { + timer_->Stop(); + DispatchTimingUpdates(); + } + timer_ = nullptr; +} void PageLoadMetricsUpdateDispatcher::UpdateMetrics( content::RenderFrameHost* render_frame_host, @@ -303,32 +385,11 @@ client_->OnSubFrameTimingChanged(new_timing); base::TimeDelta navigation_start_offset = it->second; - MergePaintTiming(navigation_start_offset, *(new_timing.paint_timing), - false /* is_main_frame */); + PaintTimingMerger merger(pending_merged_page_timing_->paint_timing.get()); + merger.Merge(navigation_start_offset, *new_timing.paint_timing, + false /* is_main_frame */); - DispatchTimingUpdates(); -} - -void PageLoadMetricsUpdateDispatcher::MergePaintTiming( - base::TimeDelta navigation_start_offset, - const mojom::PaintTiming& new_paint_timing, - bool is_main_frame) { - MaybeUpdateTimeDelta(&pending_merged_page_timing_->paint_timing->first_paint, - navigation_start_offset, new_paint_timing.first_paint); - MaybeUpdateTimeDelta( - &pending_merged_page_timing_->paint_timing->first_text_paint, - navigation_start_offset, new_paint_timing.first_text_paint); - MaybeUpdateTimeDelta( - &pending_merged_page_timing_->paint_timing->first_image_paint, - navigation_start_offset, new_paint_timing.first_image_paint); - MaybeUpdateTimeDelta( - &pending_merged_page_timing_->paint_timing->first_contentful_paint, - navigation_start_offset, new_paint_timing.first_contentful_paint); - if (is_main_frame) { - // first meaningful paint is only tracked in the main frame. - pending_merged_page_timing_->paint_timing->first_meaningful_paint = - new_paint_timing.first_meaningful_paint; - } + MaybeDispatchTimingUpdates(merger.did_merge_new_timing_value()); } void PageLoadMetricsUpdateDispatcher::UpdateSubFrameMetadata( @@ -374,10 +435,12 @@ // observed |paint_timing|, which is tracked across all frames in the page. pending_merged_page_timing_ = new_timing.Clone(); pending_merged_page_timing_->paint_timing = std::move(last_paint_timing); - MergePaintTiming(base::TimeDelta(), *new_timing.paint_timing, - true /* is_main_frame */); - DispatchTimingUpdates(); + PaintTimingMerger merger(pending_merged_page_timing_->paint_timing.get()); + merger.Merge(base::TimeDelta(), *new_timing.paint_timing, + true /* is_main_frame */); + + MaybeDispatchTimingUpdates(merger.did_merge_new_timing_value()); } void PageLoadMetricsUpdateDispatcher::UpdateMainFrameMetadata( @@ -398,7 +461,24 @@ client_->OnMainFrameMetadataChanged(); } +void PageLoadMetricsUpdateDispatcher::MaybeDispatchTimingUpdates( + bool did_merge_new_timing_value) { + // If we merged a new timing value, then we should buffer updates for + // |kBufferTimerDelayMillis|, to allow for any other out of order timings to + // arrive before we dispatch the minimum observed timings to observers. + if (did_merge_new_timing_value) { + timer_->Start( + FROM_HERE, base::TimeDelta::FromMilliseconds(kBufferTimerDelayMillis), + base::Bind(&PageLoadMetricsUpdateDispatcher::DispatchTimingUpdates, + base::Unretained(this))); + } else if (!timer_->IsRunning()) { + DispatchTimingUpdates(); + } +} + void PageLoadMetricsUpdateDispatcher::DispatchTimingUpdates() { + DCHECK(!timer_->IsRunning()); + if (pending_merged_page_timing_->paint_timing->first_paint) { if (!pending_merged_page_timing_->parse_timing->parse_start || !pending_merged_page_timing_->document_timing->first_layout) { @@ -418,6 +498,19 @@ if (current_merged_page_timing_->Equals(*pending_merged_page_timing_)) return; + + LogIfOutOfOrderTiming(current_merged_page_timing_->paint_timing->first_paint, + pending_merged_page_timing_->paint_timing->first_paint); + LogIfOutOfOrderTiming( + current_merged_page_timing_->paint_timing->first_text_paint, + pending_merged_page_timing_->paint_timing->first_text_paint); + LogIfOutOfOrderTiming( + current_merged_page_timing_->paint_timing->first_image_paint, + pending_merged_page_timing_->paint_timing->first_image_paint); + LogIfOutOfOrderTiming( + current_merged_page_timing_->paint_timing->first_contentful_paint, + pending_merged_page_timing_->paint_timing->first_contentful_paint); + current_merged_page_timing_ = pending_merged_page_timing_->Clone(); internal::PageLoadTimingStatus status =
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_update_dispatcher.h b/chrome/browser/page_load_metrics/page_load_metrics_update_dispatcher.h index 47f6e4f..7af5111c4 100644 --- a/chrome/browser/page_load_metrics/page_load_metrics_update_dispatcher.h +++ b/chrome/browser/page_load_metrics/page_load_metrics_update_dispatcher.h
@@ -6,9 +6,11 @@ #define CHROME_BROWSER_PAGE_LOAD_METRICS_PAGE_LOAD_METRICS_UPDATE_DISPATCHER_H_ #include <map> +#include <memory> #include "base/macros.h" #include "base/time/time.h" +#include "base/timer/timer.h" #include "chrome/common/page_load_metrics/page_load_metrics.mojom.h" namespace content { @@ -65,6 +67,8 @@ }; extern const char kPageLoadTimingStatus[]; +extern const char kHistogramOutOfOrderTiming[]; +extern const char kHistogramOutOfOrderTimingBuffered[]; } // namespace internal @@ -101,6 +105,8 @@ void DidFinishSubFrameNavigation( content::NavigationHandle* navigation_handle); + void ShutDown(); + const mojom::PageLoadTiming& timing() const { return *(current_merged_page_timing_.get()); } @@ -122,12 +128,7 @@ void UpdateMainFrameMetadata(const mojom::PageLoadMetadata& new_metadata); void UpdateSubFrameMetadata(const mojom::PageLoadMetadata& subframe_metadata); - // Merge values from |new_paint_timing| into |pending_merged_page_timing_|, - // offsetting any new timings by the |navigation_start_offset|. - void MergePaintTiming(base::TimeDelta navigation_start_offset, - const mojom::PaintTiming& new_paint_timing, - bool is_main_frame); - + void MaybeDispatchTimingUpdates(bool did_merge_new_timing_value); void DispatchTimingUpdates(); // The client is guaranteed to outlive this object. @@ -136,6 +137,8 @@ // Interface to chrome features. Must outlive the class. PageLoadMetricsEmbedderInterface* const embedder_interface_; + std::unique_ptr<base::Timer> timer_; + // Time the navigation for this page load was initiated. const base::TimeTicks navigation_start_;
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_util.cc b/chrome/browser/page_load_metrics/page_load_metrics_util.cc index 22ec5505..ab1acc1 100644 --- a/chrome/browser/page_load_metrics/page_load_metrics_util.cc +++ b/chrome/browser/page_load_metrics/page_load_metrics_util.cc
@@ -7,8 +7,6 @@ #include <algorithm> #include "chrome/common/page_load_metrics/page_load_timing.h" -#include "net/base/registry_controlled_domains/registry_controlled_domain.h" -#include "url/gurl.h" namespace page_load_metrics { @@ -45,45 +43,6 @@ } // namespace -base::Optional<std::string> GetGoogleHostnamePrefix(const GURL& url) { - const size_t registry_length = - net::registry_controlled_domains::GetRegistryLength( - url, - - // Do not include unknown registries (registries that don't have any - // matches in effective TLD names). - net::registry_controlled_domains::EXCLUDE_UNKNOWN_REGISTRIES, - - // Do not include private registries, such as appspot.com. We don't - // want to match URLs like www.google.appspot.com. - net::registry_controlled_domains::EXCLUDE_PRIVATE_REGISTRIES); - - const base::StringPiece hostname = url.host_piece(); - if (registry_length == 0 || registry_length == std::string::npos || - registry_length >= hostname.length()) { - return base::Optional<std::string>(); - } - - // Removes the tld and the preceding dot. - const base::StringPiece hostname_minus_registry = - hostname.substr(0, hostname.length() - (registry_length + 1)); - - if (hostname_minus_registry == "google") - return std::string(""); - - if (!base::EndsWith(hostname_minus_registry, ".google", - base::CompareCase::INSENSITIVE_ASCII)) { - return base::Optional<std::string>(); - } - - return std::string(hostname_minus_registry.substr( - 0, hostname_minus_registry.length() - strlen(".google"))); -} - -bool IsGoogleHostname(const GURL& url) { - return GetGoogleHostnamePrefix(url).has_value(); -} - bool WasStartedInForegroundOptionalEventInForeground( const base::Optional<base::TimeDelta>& event, const PageLoadExtraInfo& info) { @@ -132,18 +91,6 @@ return time_on_page; } -base::Optional<base::TimeDelta> OptionalMin( - const base::Optional<base::TimeDelta>& a, - const base::Optional<base::TimeDelta>& b) { - if (a && !b) - return a; - if (b && !a) - return b; - if (!a && !b) - return a; // doesn't matter which - return base::Optional<base::TimeDelta>(std::min(a.value(), b.value())); -} - bool DidObserveLoadingBehaviorInAnyFrame( const page_load_metrics::PageLoadExtraInfo& info, blink::WebLoadingBehaviorFlag behavior) {
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_util.h b/chrome/browser/page_load_metrics/page_load_metrics_util.h index 1e6af571..a81c0432 100644 --- a/chrome/browser/page_load_metrics/page_load_metrics_util.h +++ b/chrome/browser/page_load_metrics/page_load_metrics_util.h
@@ -9,6 +9,7 @@ #include "base/optional.h" #include "base/time/time.h" #include "chrome/browser/page_load_metrics/page_load_metrics_observer.h" +#include "chrome/common/page_load_metrics/page_load_metrics_util.h" #include "third_party/WebKit/public/platform/WebLoadingBehaviorFlag.h" // Up to 10 minutes, with 100 buckets. @@ -118,34 +119,12 @@ const PageLoadExtraInfo& info, base::TimeTicks app_background_time); -// Returns the minimum value of the optional TimeDeltas, if both values are -// set. Otherwise, if one value is set, returns that value. Otherwise, returns -// an unset value. -base::Optional<base::TimeDelta> OptionalMin( - const base::Optional<base::TimeDelta>& a, - const base::Optional<base::TimeDelta>& b); - // Whether the given loading behavior was observed in any frame (either the main // frame or a subframe). bool DidObserveLoadingBehaviorInAnyFrame( const page_load_metrics::PageLoadExtraInfo& info, blink::WebLoadingBehaviorFlag behavior); -// Whether the given url has a google hostname. -bool IsGoogleHostname(const GURL& url); - -// If the given hostname is a google hostname, returns the portion of the -// hostname before the google hostname. Otherwise, returns an unset optional -// value. -// -// For example: -// https://example.com/foo => returns an unset optional value -// https://google.com/foo => returns '' -// https://www.google.com/foo => returns 'www' -// https://news.google.com/foo => returns 'news' -// https://a.b.c.google.com/foo => returns 'a.b.c' -base::Optional<std::string> GetGoogleHostnamePrefix(const GURL& url); - } // namespace page_load_metrics #endif // CHROME_BROWSER_PAGE_LOAD_METRICS_PAGE_LOAD_METRICS_UTIL_H_
diff --git a/chrome/browser/page_load_metrics/page_load_tracker.cc b/chrome/browser/page_load_metrics/page_load_tracker.cc index 742c841..9f83e20f 100644 --- a/chrome/browser/page_load_metrics/page_load_tracker.cc +++ b/chrome/browser/page_load_metrics/page_load_tracker.cc
@@ -201,6 +201,8 @@ if (did_stop_tracking_) return; + metrics_update_dispatcher_.ShutDown(); + if (page_end_time_.is_null()) { // page_end_time_ can be unset in some cases, such as when a navigation is // aborted by a navigation that started before it. In these cases, set the
diff --git a/chrome/browser/password_manager/chrome_password_manager_client.cc b/chrome/browser/password_manager/chrome_password_manager_client.cc index b6c0edbd..19c012e16 100644 --- a/chrome/browser/password_manager/chrome_password_manager_client.cc +++ b/chrome/browser/password_manager/chrome_password_manager_client.cc
@@ -254,7 +254,7 @@ } bool ChromePasswordManagerClient::PromptUserToSaveOrUpdatePassword( - std::unique_ptr<password_manager::PasswordFormManager> form_to_save, + scoped_refptr<password_manager::PasswordFormManager> form_to_save, bool update_password) { // Save password infobar and the password bubble prompts in case of // "webby" URLs and do not prompt in case of "non-webby" URLS (e.g. file://). @@ -374,7 +374,7 @@ } void ChromePasswordManagerClient::AutomaticPasswordSave( - std::unique_ptr<password_manager::PasswordFormManager> saved_form) { + scoped_refptr<password_manager::PasswordFormManager> saved_form) { #if defined(OS_ANDROID) GeneratedPasswordSavedInfoBarDelegateAndroid::Create(web_contents()); #else
diff --git a/chrome/browser/password_manager/chrome_password_manager_client.h b/chrome/browser/password_manager/chrome_password_manager_client.h index 9ec30cf..017652e 100644 --- a/chrome/browser/password_manager/chrome_password_manager_client.h +++ b/chrome/browser/password_manager/chrome_password_manager_client.h
@@ -10,6 +10,7 @@ #include "base/compiler_specific.h" #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "components/autofill/content/common/autofill_driver.mojom.h" #include "components/password_manager/content/browser/content_password_manager_driver_factory.h" #include "components/password_manager/content/browser/credential_manager_impl.h" @@ -53,7 +54,7 @@ const HSTSCallback& callback) const override; bool OnCredentialManagerUsed() override; bool PromptUserToSaveOrUpdatePassword( - std::unique_ptr<password_manager::PasswordFormManager> form_to_save, + scoped_refptr<password_manager::PasswordFormManager> form_to_save, bool update_password) override; bool PromptUserToChooseCredentials( std::vector<std::unique_ptr<autofill::PasswordForm>> local_forms, @@ -70,7 +71,7 @@ const autofill::PasswordForm& form) override; void NotifyStorePasswordCalled() override; void AutomaticPasswordSave( - std::unique_ptr<password_manager::PasswordFormManager> saved_form_manager) + scoped_refptr<password_manager::PasswordFormManager> saved_form_manager) override; void PasswordWasAutofilled( const std::map<base::string16, const autofill::PasswordForm*>&
diff --git a/chrome/browser/password_manager/password_generation_interactive_uitest.cc b/chrome/browser/password_manager/password_generation_interactive_uitest.cc index d0fd8ff..e605a1f31 100644 --- a/chrome/browser/password_manager/password_generation_interactive_uitest.cc +++ b/chrome/browser/password_manager/password_generation_interactive_uitest.cc
@@ -181,17 +181,20 @@ EXPECT_FALSE(GenerationPopupShowing()); } -// Disabled due to flakiness due to resizes, see http://crbug.com/407998. IN_PROC_BROWSER_TEST_F(PasswordGenerationInteractiveTest, - DISABLED_GenerationTriggeredInIFrame) { + GenerationTriggeredInIFrame) { NavigateToFile("/password/framed_signup_form.html"); - std::string focus_script = - "var frame = document.getElementById('signup_iframe');" - "var frame_doc = frame.contentDocument;" - "frame_doc.getElementById('password_field').focus();"; + // Execute the script in the context of the iframe so that it kinda receives a + // user gesture. + std::vector<content::RenderFrameHost*> frames = WebContents()->GetAllFrames(); + ASSERT_EQ(2u, frames.size()); + ASSERT_TRUE(frames[0] == RenderFrameHost()); - ASSERT_TRUE(content::ExecuteScript(RenderViewHost(), focus_script)); + std::string focus_script = + "document.getElementById('password_field').focus();"; + + ASSERT_TRUE(content::ExecuteScript(frames[1], focus_script)); EXPECT_TRUE(GenerationPopupShowing()); }
diff --git a/chrome/browser/password_manager/password_manager_test_base.cc b/chrome/browser/password_manager/password_manager_test_base.cc index 5a5fa13..b1f3c17f 100644 --- a/chrome/browser/password_manager/password_manager_test_base.cc +++ b/chrome/browser/password_manager/password_manager_test_base.cc
@@ -65,9 +65,8 @@ private: // PasswordsClientUIDelegate: - void OnPasswordSubmitted( - std::unique_ptr<password_manager::PasswordFormManager> form_manager) - override; + void OnPasswordSubmitted(scoped_refptr<password_manager::PasswordFormManager> + form_manager) override; bool OnChooseCredentials( std::vector<std::unique_ptr<autofill::PasswordForm>> local_credentials, const GURL& origin, @@ -110,7 +109,7 @@ } void CustomManagePasswordsUIController::OnPasswordSubmitted( - std::unique_ptr<password_manager::PasswordFormManager> form_manager) { + scoped_refptr<password_manager::PasswordFormManager> form_manager) { if (target_state_ == password_manager::ui::PENDING_PASSWORD_STATE) { run_loop_->Quit(); run_loop_ = nullptr;
diff --git a/chrome/browser/password_manager/save_password_infobar_delegate_android.cc b/chrome/browser/password_manager/save_password_infobar_delegate_android.cc index 2595809..5282d24 100644 --- a/chrome/browser/password_manager/save_password_infobar_delegate_android.cc +++ b/chrome/browser/password_manager/save_password_infobar_delegate_android.cc
@@ -26,7 +26,7 @@ // static void SavePasswordInfoBarDelegate::Create( content::WebContents* web_contents, - std::unique_ptr<password_manager::PasswordFormManager> form_to_save) { + scoped_refptr<password_manager::PasswordFormManager> form_to_save) { Profile* profile = Profile::FromBrowserContext(web_contents->GetBrowserContext()); syncer::SyncService* sync_service = @@ -47,7 +47,7 @@ SavePasswordInfoBarDelegate::SavePasswordInfoBarDelegate( content::WebContents* web_contents, - std::unique_ptr<password_manager::PasswordFormManager> form_to_save, + scoped_refptr<password_manager::PasswordFormManager> form_to_save, bool is_smartlock_branding_enabled) : PasswordManagerInfoBarDelegate(), form_to_save_(std::move(form_to_save)),
diff --git a/chrome/browser/password_manager/save_password_infobar_delegate_android.h b/chrome/browser/password_manager/save_password_infobar_delegate_android.h index f805016..c6c1403 100644 --- a/chrome/browser/password_manager/save_password_infobar_delegate_android.h +++ b/chrome/browser/password_manager/save_password_infobar_delegate_android.h
@@ -8,6 +8,7 @@ #include <memory> #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "base/timer/elapsed_timer.h" #include "chrome/browser/password_manager/password_manager_infobar_delegate_android.h" #include "components/infobars/core/confirm_infobar_delegate.h" @@ -32,7 +33,7 @@ // for |web_contents|. static void Create( content::WebContents* web_contents, - std::unique_ptr<password_manager::PasswordFormManager> form_to_save); + scoped_refptr<password_manager::PasswordFormManager> form_to_save); ~SavePasswordInfoBarDelegate() override; @@ -47,13 +48,13 @@ // Makes a ctor available in tests. SavePasswordInfoBarDelegate( content::WebContents* web_contents, - std::unique_ptr<password_manager::PasswordFormManager> form_to_save, + scoped_refptr<password_manager::PasswordFormManager> form_to_save, bool is_smartlock_branding_enabled); private: // The PasswordFormManager managing the form we're asking the user about, // and should update as per their decision. - std::unique_ptr<password_manager::PasswordFormManager> form_to_save_; + scoped_refptr<password_manager::PasswordFormManager> form_to_save_; // Used to track the results we get from the info bar. password_manager::metrics_util::UIDismissalReason infobar_response_;
diff --git a/chrome/browser/password_manager/save_password_infobar_delegate_android_unittest.cc b/chrome/browser/password_manager/save_password_infobar_delegate_android_unittest.cc index 0ffd7a9..8bfe74f 100644 --- a/chrome/browser/password_manager/save_password_infobar_delegate_android_unittest.cc +++ b/chrome/browser/password_manager/save_password_infobar_delegate_android_unittest.cc
@@ -45,9 +45,9 @@ base::WrapUnique(new password_manager::StubFormSaver), form_fetcher) {} + private: ~MockPasswordFormManager() override {} - private: DISALLOW_COPY_AND_ASSIGN(MockPasswordFormManager); }; @@ -55,7 +55,7 @@ public: TestSavePasswordInfobarDelegate( content::WebContents* web_contents, - std::unique_ptr<password_manager::PasswordFormManager> form_to_save) + scoped_refptr<password_manager::PasswordFormManager> form_to_save) : SavePasswordInfoBarDelegate(web_contents, std::move(form_to_save), true /* is_smartlock_branding_enabled */) {} @@ -75,12 +75,12 @@ PrefService* prefs(); const autofill::PasswordForm& test_form() { return test_form_; } - std::unique_ptr<MockPasswordFormManager> CreateMockFormManager(); + scoped_refptr<MockPasswordFormManager> CreateMockFormManager(); protected: std::unique_ptr<ConfirmInfoBarDelegate> CreateDelegate( - std::unique_ptr<password_manager::PasswordFormManager> - password_form_manager); + scoped_refptr<password_manager::PasswordFormManager> + password_form_manager); password_manager::StubPasswordManagerClient client_; password_manager::StubPasswordManagerDriver driver_; @@ -108,16 +108,16 @@ return profile->GetPrefs(); } -std::unique_ptr<MockPasswordFormManager> +scoped_refptr<MockPasswordFormManager> SavePasswordInfoBarDelegateTest::CreateMockFormManager() { - return std::unique_ptr<MockPasswordFormManager>( + return scoped_refptr<MockPasswordFormManager>( new MockPasswordFormManager(&password_manager_, &client_, driver_.AsWeakPtr(), test_form(), &fetcher_)); } std::unique_ptr<ConfirmInfoBarDelegate> SavePasswordInfoBarDelegateTest::CreateDelegate( - std::unique_ptr<password_manager::PasswordFormManager> + scoped_refptr<password_manager::PasswordFormManager> password_form_manager) { std::unique_ptr<ConfirmInfoBarDelegate> delegate( new TestSavePasswordInfobarDelegate(web_contents(), @@ -134,7 +134,7 @@ } TEST_F(SavePasswordInfoBarDelegateTest, CancelTestCredentialSourceAPI) { - std::unique_ptr<MockPasswordFormManager> password_form_manager( + scoped_refptr<MockPasswordFormManager> password_form_manager( CreateMockFormManager()); EXPECT_CALL(*password_form_manager.get(), PermanentlyBlacklist()); std::unique_ptr<ConfirmInfoBarDelegate> infobar( @@ -144,7 +144,7 @@ TEST_F(SavePasswordInfoBarDelegateTest, CancelTestCredentialSourcePasswordManager) { - std::unique_ptr<MockPasswordFormManager> password_form_manager( + scoped_refptr<MockPasswordFormManager> password_form_manager( CreateMockFormManager()); EXPECT_CALL(*password_form_manager.get(), PermanentlyBlacklist()); std::unique_ptr<ConfirmInfoBarDelegate> infobar(
diff --git a/chrome/browser/password_manager/update_password_infobar_delegate_android.cc b/chrome/browser/password_manager/update_password_infobar_delegate_android.cc index ebc381ad..9e6e7a5 100644 --- a/chrome/browser/password_manager/update_password_infobar_delegate_android.cc +++ b/chrome/browser/password_manager/update_password_infobar_delegate_android.cc
@@ -24,7 +24,7 @@ // static void UpdatePasswordInfoBarDelegate::Create( content::WebContents* web_contents, - std::unique_ptr<password_manager::PasswordFormManager> form_to_save) { + scoped_refptr<password_manager::PasswordFormManager> form_to_save) { const bool is_smartlock_branding_enabled = password_bubble_experiment::IsSmartLockUser( ProfileSyncServiceFactory::GetForProfile( @@ -59,7 +59,7 @@ UpdatePasswordInfoBarDelegate::UpdatePasswordInfoBarDelegate( content::WebContents* web_contents, - std::unique_ptr<password_manager::PasswordFormManager> form_to_update, + scoped_refptr<password_manager::PasswordFormManager> form_to_update, bool is_smartlock_branding_enabled) : is_smartlock_branding_enabled_(is_smartlock_branding_enabled) { base::string16 message;
diff --git a/chrome/browser/password_manager/update_password_infobar_delegate_android.h b/chrome/browser/password_manager/update_password_infobar_delegate_android.h index 7bdf489..b759f80 100644 --- a/chrome/browser/password_manager/update_password_infobar_delegate_android.h +++ b/chrome/browser/password_manager/update_password_infobar_delegate_android.h
@@ -9,6 +9,7 @@ #include <vector> #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "chrome/browser/password_manager/password_manager_infobar_delegate_android.h" #include "chrome/browser/ui/passwords/manage_passwords_state.h" #include "components/password_manager/core/browser/password_form_manager.h" @@ -26,7 +27,7 @@ public: static void Create( content::WebContents* web_contents, - std::unique_ptr<password_manager::PasswordFormManager> form_to_update); + scoped_refptr<password_manager::PasswordFormManager> form_to_update); ~UpdatePasswordInfoBarDelegate() override; @@ -54,7 +55,7 @@ private: UpdatePasswordInfoBarDelegate( content::WebContents* web_contents, - std::unique_ptr<password_manager::PasswordFormManager> form_to_update, + scoped_refptr<password_manager::PasswordFormManager> form_to_update, bool is_smartlock_branding_enabled); // ConfirmInfoBarDelegate:
diff --git a/chrome/browser/prerender/prerender_browsertest.cc b/chrome/browser/prerender/prerender_browsertest.cc index 9736d35f1..0c6d628 100644 --- a/chrome/browser/prerender/prerender_browsertest.cc +++ b/chrome/browser/prerender/prerender_browsertest.cc
@@ -40,7 +40,6 @@ #include "chrome/browser/extensions/extension_apitest.h" #include "chrome/browser/external_protocol/external_protocol_handler.h" #include "chrome/browser/net/prediction_options.h" -#include "chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.h" #include "chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.h" #include "chrome/browser/password_manager/password_store_factory.h" #include "chrome/browser/predictors/autocomplete_action_predictor.h" @@ -71,6 +70,7 @@ #include "chrome/common/chrome_features.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_switches.h" +#include "chrome/common/page_load_metrics/test/page_load_metrics_test_util.h" #include "chrome/test/base/ui_test_utils.h" #include "components/content_settings/core/browser/host_content_settings_map.h" #include "components/favicon/content/content_favicon_driver.h" @@ -3439,8 +3439,7 @@ timing.navigation_start = base::Time::FromDoubleT(1); // Non-null time. timing.paint_timing->first_contentful_paint = base::TimeDelta::FromMilliseconds(2654); - page_load_metrics::PageLoadMetricsObserverTestHarness:: - PopulateRequiredTimingFields(&timing); + PopulateRequiredTimingFields(&timing); observer.OnFirstContentfulPaintInPage(timing, GenericPageLoadExtraInfo(dest_url())); @@ -3482,8 +3481,7 @@ timing.navigation_start = base::Time::FromDoubleT(1); // Non-null time. timing.paint_timing->first_contentful_paint = base::TimeDelta::FromMilliseconds(2361); - page_load_metrics::PageLoadMetricsObserverTestHarness:: - PopulateRequiredTimingFields(&timing); + PopulateRequiredTimingFields(&timing); observer.OnFirstContentfulPaintInPage(timing, GenericPageLoadExtraInfo(dest_url())); @@ -3525,8 +3523,7 @@ timing.navigation_start = base::Time::FromDoubleT(1); // Non-null time. timing.paint_timing->first_contentful_paint = base::TimeDelta::FromMilliseconds(2361); - page_load_metrics::PageLoadMetricsObserverTestHarness:: - PopulateRequiredTimingFields(&timing); + PopulateRequiredTimingFields(&timing); observer.OnFirstContentfulPaintInPage(timing, GenericPageLoadExtraInfo(dest_url())); @@ -3570,8 +3567,7 @@ timing.navigation_start = base::Time::FromDoubleT(1); // Non-null time. timing.paint_timing->first_contentful_paint = base::TimeDelta::FromMilliseconds(2362); - page_load_metrics::PageLoadMetricsObserverTestHarness:: - PopulateRequiredTimingFields(&timing); + PopulateRequiredTimingFields(&timing); observer.OnFirstContentfulPaintInPage(timing, GenericPageLoadExtraInfo(dest_url())); @@ -3622,8 +3618,7 @@ // The FCP time should end up on the edge of the bucket. timing.paint_timing->first_contentful_paint = base::TimeDelta::FromMilliseconds(2654); - page_load_metrics::PageLoadMetricsObserverTestHarness:: - PopulateRequiredTimingFields(&timing); + PopulateRequiredTimingFields(&timing); observer.OnFirstContentfulPaintInPage(timing, GenericPageLoadExtraInfo(dest_url())); @@ -3655,8 +3650,7 @@ timing.navigation_start = base::Time::FromDoubleT(1); // Non-null time. timing.paint_timing->first_contentful_paint = base::TimeDelta::FromMilliseconds(2654); - page_load_metrics::PageLoadMetricsObserverTestHarness:: - PopulateRequiredTimingFields(&timing); + PopulateRequiredTimingFields(&timing); observer.OnFirstContentfulPaintInPage(timing, GenericPageLoadExtraInfo(dest_url())); @@ -3702,8 +3696,7 @@ timing.navigation_start = base::Time::FromDoubleT(1); // Non-null time. timing.paint_timing->first_contentful_paint = base::TimeDelta::FromMilliseconds(2362); - page_load_metrics::PageLoadMetricsObserverTestHarness:: - PopulateRequiredTimingFields(&timing); + PopulateRequiredTimingFields(&timing); observer.OnFirstContentfulPaintInPage(timing, GenericPageLoadExtraInfo(dest_url()));
diff --git a/chrome/browser/profiles/off_the_record_profile_impl.cc b/chrome/browser/profiles/off_the_record_profile_impl.cc index 70b0b4a..ca7bf1f 100644 --- a/chrome/browser/profiles/off_the_record_profile_impl.cc +++ b/chrome/browser/profiles/off_the_record_profile_impl.cc
@@ -19,6 +19,8 @@ #include "chrome/browser/background_sync/background_sync_controller_factory.h" #include "chrome/browser/background_sync/background_sync_controller_impl.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h" +#include "chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_factory.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/dom_distiller/profile_utils.h" #include "chrome/browser/download/chrome_download_manager_delegate.h" @@ -414,6 +416,11 @@ return BackgroundSyncControllerFactory::GetForProfile(this); } +content::BrowsingDataRemoverDelegate* +OffTheRecordProfileImpl::GetBrowsingDataRemoverDelegate() { + return ChromeBrowsingDataRemoverDelegateFactory::GetForProfile(this); +} + bool OffTheRecordProfileImpl::IsSameProfile(Profile* profile) { return (profile == this) || (profile == profile_); }
diff --git a/chrome/browser/profiles/off_the_record_profile_impl.h b/chrome/browser/profiles/off_the_record_profile_impl.h index 77b93fd..46df7f1 100644 --- a/chrome/browser/profiles/off_the_record_profile_impl.h +++ b/chrome/browser/profiles/off_the_record_profile_impl.h
@@ -111,6 +111,8 @@ content::SSLHostStateDelegate* GetSSLHostStateDelegate() override; content::PermissionManager* GetPermissionManager() override; content::BackgroundSyncController* GetBackgroundSyncController() override; + content::BrowsingDataRemoverDelegate* GetBrowsingDataRemoverDelegate() + override; private: void InitIoData();
diff --git a/chrome/browser/ui/app_list/test/fake_profile.cc b/chrome/browser/ui/app_list/test/fake_profile.cc index 4d690496..81b94ef 100644 --- a/chrome/browser/ui/app_list/test/fake_profile.cc +++ b/chrome/browser/ui/app_list/test/fake_profile.cc
@@ -69,6 +69,11 @@ return nullptr; } +content::BrowsingDataRemoverDelegate* +FakeProfile::GetBrowsingDataRemoverDelegate() { + return nullptr; +} + net::URLRequestContextGetter* FakeProfile::CreateRequestContext( content::ProtocolHandlerMap* protocol_handlers, content::URLRequestInterceptorScopedVector request_interceptors) {
diff --git a/chrome/browser/ui/app_list/test/fake_profile.h b/chrome/browser/ui/app_list/test/fake_profile.h index e0e61b3..34a885a 100644 --- a/chrome/browser/ui/app_list/test/fake_profile.h +++ b/chrome/browser/ui/app_list/test/fake_profile.h
@@ -46,6 +46,8 @@ content::SSLHostStateDelegate* GetSSLHostStateDelegate() override; content::PermissionManager* GetPermissionManager() override; content::BackgroundSyncController* GetBackgroundSyncController() override; + content::BrowsingDataRemoverDelegate* GetBrowsingDataRemoverDelegate() + override; net::URLRequestContextGetter* CreateRequestContext( content::ProtocolHandlerMap* protocol_handlers, content::URLRequestInterceptorScopedVector request_interceptors) override;
diff --git a/chrome/browser/ui/passwords/manage_passwords_state.cc b/chrome/browser/ui/passwords/manage_passwords_state.cc index b99c9fca..81e1421 100644 --- a/chrome/browser/ui/passwords/manage_passwords_state.cc +++ b/chrome/browser/ui/passwords/manage_passwords_state.cc
@@ -80,7 +80,7 @@ ManagePasswordsState::~ManagePasswordsState() {} void ManagePasswordsState::OnPendingPassword( - std::unique_ptr<password_manager::PasswordFormManager> form_manager) { + scoped_refptr<password_manager::PasswordFormManager> form_manager) { ClearData(); form_manager_ = std::move(form_manager); local_credentials_forms_ = @@ -92,7 +92,7 @@ } void ManagePasswordsState::OnUpdatePassword( - std::unique_ptr<password_manager::PasswordFormManager> form_manager) { + scoped_refptr<password_manager::PasswordFormManager> form_manager) { ClearData(); form_manager_ = std::move(form_manager); local_credentials_forms_ = @@ -123,7 +123,7 @@ } void ManagePasswordsState::OnAutomaticPasswordSave( - std::unique_ptr<PasswordFormManager> form_manager) { + scoped_refptr<PasswordFormManager> form_manager) { ClearData(); form_manager_ = std::move(form_manager); local_credentials_forms_.reserve(form_manager_->best_matches().size()); @@ -222,7 +222,7 @@ } void ManagePasswordsState::ClearData() { - form_manager_.reset(); + form_manager_ = nullptr; local_credentials_forms_.clear(); credentials_callback_.Reset(); }
diff --git a/chrome/browser/ui/passwords/manage_passwords_state.h b/chrome/browser/ui/passwords/manage_passwords_state.h index 62de544..03a7a1b6 100644 --- a/chrome/browser/ui/passwords/manage_passwords_state.h +++ b/chrome/browser/ui/passwords/manage_passwords_state.h
@@ -11,6 +11,7 @@ #include "base/callback.h" #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "base/strings/string16.h" #include "components/autofill/core/common/password_form.h" #include "components/password_manager/core/browser/password_store_change.h" @@ -44,11 +45,11 @@ // Move to PENDING_PASSWORD_STATE. void OnPendingPassword( - std::unique_ptr<password_manager::PasswordFormManager> form_manager); + scoped_refptr<password_manager::PasswordFormManager> form_manager); // Move to PENDING_PASSWORD_UPDATE_STATE. void OnUpdatePassword( - std::unique_ptr<password_manager::PasswordFormManager> form_manager); + scoped_refptr<password_manager::PasswordFormManager> form_manager); // Move to CREDENTIAL_REQUEST_STATE. void OnRequestCredentials( @@ -62,7 +63,7 @@ // Move to CONFIRMATION_STATE. void OnAutomaticPasswordSave( - std::unique_ptr<password_manager::PasswordFormManager> form_manager); + scoped_refptr<password_manager::PasswordFormManager> form_manager); // Move to MANAGE_STATE or INACTIVE_STATE for PSL matched passwords. // |password_form_map| contains best matches from the password store for the @@ -127,7 +128,7 @@ GURL origin_; // Contains the password that was submitted. - std::unique_ptr<password_manager::PasswordFormManager> form_manager_; + scoped_refptr<password_manager::PasswordFormManager> form_manager_; // Contains all the current forms. std::vector<std::unique_ptr<autofill::PasswordForm>> local_credentials_forms_;
diff --git a/chrome/browser/ui/passwords/manage_passwords_state_unittest.cc b/chrome/browser/ui/passwords/manage_passwords_state_unittest.cc index de13dcf5..9b989a7 100644 --- a/chrome/browser/ui/passwords/manage_passwords_state_unittest.cc +++ b/chrome/browser/ui/passwords/manage_passwords_state_unittest.cc
@@ -86,11 +86,11 @@ // Returns a PasswordFormManager containing |test_stored_forms_| as the best // matches. - std::unique_ptr<password_manager::PasswordFormManager> CreateFormManager(); + scoped_refptr<password_manager::PasswordFormManager> CreateFormManager(); // Returns a PasswordFormManager containing test_local_federated_form() as a // stored federated credential. - std::unique_ptr<password_manager::PasswordFormManager> + scoped_refptr<password_manager::PasswordFormManager> CreateFormManagerWithFederation(); // Pushes irrelevant updates to |passwords_data_| and checks that they don't @@ -107,7 +107,7 @@ private: // Implements both CreateFormManager and CreateFormManagerWithFederation. - std::unique_ptr<password_manager::PasswordFormManager> + scoped_refptr<password_manager::PasswordFormManager> CreateFormManagerInternal(bool include_federated); password_manager::StubPasswordManagerClient stub_client_; @@ -123,19 +123,19 @@ std::vector<const autofill::PasswordForm*> test_stored_forms_; }; -std::unique_ptr<password_manager::PasswordFormManager> +scoped_refptr<password_manager::PasswordFormManager> ManagePasswordsStateTest::CreateFormManager() { return CreateFormManagerInternal(false); } -std::unique_ptr<password_manager::PasswordFormManager> +scoped_refptr<password_manager::PasswordFormManager> ManagePasswordsStateTest::CreateFormManagerWithFederation() { return CreateFormManagerInternal(true); } -std::unique_ptr<password_manager::PasswordFormManager> +scoped_refptr<password_manager::PasswordFormManager> ManagePasswordsStateTest::CreateFormManagerInternal(bool include_federated) { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( new password_manager::PasswordFormManager( &password_manager_, &stub_client_, driver_.AsWeakPtr(), test_local_form(), @@ -271,7 +271,7 @@ TEST_F(ManagePasswordsStateTest, PasswordSubmitted) { test_stored_forms().push_back(&test_local_form()); test_stored_forms().push_back(&test_psl_form()); - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); test_form_manager->ProvisionallySave( test_submitted_form(), @@ -291,7 +291,7 @@ TEST_F(ManagePasswordsStateTest, PasswordSaved) { test_stored_forms().push_back(&test_local_form()); - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); test_form_manager->ProvisionallySave( test_submitted_form(), @@ -310,7 +310,7 @@ } TEST_F(ManagePasswordsStateTest, PasswordSubmittedFederationsPresent) { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManagerWithFederation()); test_form_manager->ProvisionallySave( test_submitted_form(), @@ -366,7 +366,7 @@ TEST_F(ManagePasswordsStateTest, AutomaticPasswordSave) { test_stored_forms().push_back(&test_psl_form()); - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); test_form_manager->ProvisionallySave( test_submitted_form(), @@ -389,7 +389,7 @@ } TEST_F(ManagePasswordsStateTest, AutomaticPasswordSaveWithFederations) { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManagerWithFederation()); test_form_manager->ProvisionallySave( test_submitted_form(), @@ -462,7 +462,7 @@ } TEST_F(ManagePasswordsStateTest, OnInactive) { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); test_form_manager->ProvisionallySave( test_submitted_form(), @@ -479,7 +479,7 @@ } TEST_F(ManagePasswordsStateTest, PendingPasswordAddBlacklisted) { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); test_form_manager->ProvisionallySave( test_submitted_form(), @@ -515,7 +515,7 @@ } TEST_F(ManagePasswordsStateTest, AutomaticPasswordSaveAddBlacklisted) { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); test_form_manager->ProvisionallySave( test_submitted_form(), @@ -538,7 +538,7 @@ } TEST_F(ManagePasswordsStateTest, PasswordUpdateAddBlacklisted) { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); test_form_manager->ProvisionallySave( test_submitted_form(), @@ -553,7 +553,7 @@ TEST_F(ManagePasswordsStateTest, PasswordUpdateSubmitted) { test_stored_forms().push_back(&test_local_form()); test_stored_forms().push_back(&test_psl_form()); - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); test_form_manager->ProvisionallySave( test_submitted_form(), @@ -578,7 +578,7 @@ android_form.username_value = test_submitted_form().username_value; android_form.password_value = base::ASCIIToUTF16("old pass"); test_stored_forms().push_back(&android_form); - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); test_form_manager->ProvisionallySave( test_submitted_form(), @@ -599,7 +599,7 @@ TEST_F(ManagePasswordsStateTest, PasswordUpdateSubmittedWithFederations) { test_stored_forms().push_back(&test_local_form()); - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManagerWithFederation()); test_form_manager->ProvisionallySave( test_submitted_form(),
diff --git a/chrome/browser/ui/passwords/manage_passwords_test.cc b/chrome/browser/ui/passwords/manage_passwords_test.cc index 6e3ff50b..5970331 100644 --- a/chrome/browser/ui/passwords/manage_passwords_test.cc +++ b/chrome/browser/ui/passwords/manage_passwords_test.cc
@@ -9,6 +9,7 @@ #include "base/bind.h" #include "base/memory/ptr_util.h" +#include "base/memory/ref_counted.h" #include "base/strings/string16.h" #include "base/strings/utf_string_conversions.h" #include "chrome/app/chrome_command_ids.h" @@ -67,7 +68,7 @@ } void ManagePasswordsTest::SetupPendingPassword() { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( new password_manager::PasswordFormManager( nullptr, &client_, driver_.AsWeakPtr(), *test_form(), base::WrapUnique(new password_manager::StubFormSaver), &fetcher_)); @@ -76,7 +77,7 @@ } void ManagePasswordsTest::SetupAutomaticPassword() { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( new password_manager::PasswordFormManager( nullptr, &client_, driver_.AsWeakPtr(), *test_form(), base::WrapUnique(new password_manager::StubFormSaver), &fetcher_));
diff --git a/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc b/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc index 25c5cf770..a73c046 100644 --- a/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc +++ b/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc
@@ -69,7 +69,7 @@ ManagePasswordsUIController::~ManagePasswordsUIController() {} void ManagePasswordsUIController::OnPasswordSubmitted( - std::unique_ptr<PasswordFormManager> form_manager) { + scoped_refptr<PasswordFormManager> form_manager) { bool show_bubble = !form_manager->IsBlacklisted(); DestroyAccountChooser(); passwords_data_.OnPendingPassword(std::move(form_manager)); @@ -87,7 +87,7 @@ } void ManagePasswordsUIController::OnUpdatePasswordSubmitted( - std::unique_ptr<PasswordFormManager> form_manager) { + scoped_refptr<PasswordFormManager> form_manager) { DestroyAccountChooser(); passwords_data_.OnUpdatePassword(std::move(form_manager)); bubble_status_ = SHOULD_POP_UP; @@ -142,7 +142,7 @@ } void ManagePasswordsUIController::OnAutomaticPasswordSave( - std::unique_ptr<PasswordFormManager> form_manager) { + scoped_refptr<PasswordFormManager> form_manager) { DestroyAccountChooser(); passwords_data_.OnAutomaticPasswordSave(std::move(form_manager)); bubble_status_ = SHOULD_POP_UP;
diff --git a/chrome/browser/ui/passwords/manage_passwords_ui_controller.h b/chrome/browser/ui/passwords/manage_passwords_ui_controller.h index dfcfa146..4408e02 100644 --- a/chrome/browser/ui/passwords/manage_passwords_ui_controller.h +++ b/chrome/browser/ui/passwords/manage_passwords_ui_controller.h
@@ -9,6 +9,7 @@ #include <vector> #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "chrome/browser/ui/passwords/manage_passwords_state.h" #include "chrome/browser/ui/passwords/passwords_client_ui_delegate.h" #include "chrome/browser/ui/passwords/passwords_model_delegate.h" @@ -44,11 +45,10 @@ ~ManagePasswordsUIController() override; // PasswordsClientUIDelegate: - void OnPasswordSubmitted( - std::unique_ptr<password_manager::PasswordFormManager> form_manager) - override; + void OnPasswordSubmitted(scoped_refptr<password_manager::PasswordFormManager> + form_manager) override; void OnUpdatePasswordSubmitted( - std::unique_ptr<password_manager::PasswordFormManager> form_manager) + scoped_refptr<password_manager::PasswordFormManager> form_manager) override; bool OnChooseCredentials( std::vector<std::unique_ptr<autofill::PasswordForm>> local_credentials, @@ -59,7 +59,7 @@ const GURL& origin) override; void OnPromptEnableAutoSignin() override; void OnAutomaticPasswordSave( - std::unique_ptr<password_manager::PasswordFormManager> form_manager) + scoped_refptr<password_manager::PasswordFormManager> form_manager) override; void OnPasswordAutofilled( const std::map<base::string16, const autofill::PasswordForm*>&
diff --git a/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc b/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc index 2bf5bbf..a48451f 100644 --- a/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc +++ b/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc
@@ -180,12 +180,12 @@ void ExpectIconStateIs(password_manager::ui::State state); void ExpectIconAndControllerStateIs(password_manager::ui::State state); - std::unique_ptr<password_manager::PasswordFormManager> + scoped_refptr<password_manager::PasswordFormManager> CreateFormManagerWithBestMatches( const autofill::PasswordForm& observed_form, const std::vector<const autofill::PasswordForm*>& best_matches); - std::unique_ptr<password_manager::PasswordFormManager> CreateFormManager(); + scoped_refptr<password_manager::PasswordFormManager> CreateFormManager(); // Tests that the state is not changed when the password is autofilled. void TestNotChangingStateOnAutofill( @@ -241,11 +241,11 @@ EXPECT_EQ(state, controller()->GetState()); } -std::unique_ptr<password_manager::PasswordFormManager> +scoped_refptr<password_manager::PasswordFormManager> ManagePasswordsUIControllerTest::CreateFormManagerWithBestMatches( const autofill::PasswordForm& observed_form, const std::vector<const autofill::PasswordForm*>& best_matches) { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( new password_manager::PasswordFormManager( &password_manager_, &client_, driver_.AsWeakPtr(), observed_form, base::WrapUnique(new password_manager::StubFormSaver), &fetcher_)); @@ -253,7 +253,7 @@ return test_form_manager; } -std::unique_ptr<password_manager::PasswordFormManager> +scoped_refptr<password_manager::PasswordFormManager> ManagePasswordsUIControllerTest::CreateFormManager() { return CreateFormManagerWithBestMatches(test_local_form(), {&test_local_form()}); @@ -266,7 +266,7 @@ state == password_manager::ui::CONFIRMATION_STATE); // Set the bubble state to |state|. - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); test_form_manager->ProvisionallySave( test_local_form(), @@ -318,7 +318,7 @@ } TEST_F(ManagePasswordsUIControllerTest, PasswordSubmitted) { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); test_form_manager->ProvisionallySave( test_local_form(), @@ -338,7 +338,7 @@ blacklisted.origin = test_local_form().origin; blacklisted.signon_realm = blacklisted.origin.spec(); blacklisted.blacklisted_by_user = true; - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager = + scoped_refptr<password_manager::PasswordFormManager> test_form_manager = CreateFormManagerWithBestMatches(test_local_form(), {&blacklisted}); EXPECT_CALL(*controller(), OnUpdateBubbleAndIconVisibility()); controller()->OnPasswordSubmitted(std::move(test_form_manager)); @@ -351,7 +351,7 @@ TEST_F(ManagePasswordsUIControllerTest, PasswordSubmittedBubbleSuppressed) { CreateSmartBubbleFieldTrial(); - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); std::vector<password_manager::InteractionsStats> stats(1); stats[0].origin_domain = test_local_form().origin.GetOrigin(); @@ -375,7 +375,7 @@ TEST_F(ManagePasswordsUIControllerTest, PasswordSubmittedBubbleNotSuppressed) { CreateSmartBubbleFieldTrial(); - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); std::vector<password_manager::InteractionsStats> stats(1); stats[0].origin_domain = test_local_form().origin.GetOrigin(); @@ -397,7 +397,7 @@ } TEST_F(ManagePasswordsUIControllerTest, PasswordSaved) { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); test_form_manager->ProvisionallySave( test_local_form(), @@ -410,7 +410,7 @@ } TEST_F(ManagePasswordsUIControllerTest, PasswordBlacklisted) { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); test_form_manager->ProvisionallySave( test_local_form(), @@ -423,7 +423,7 @@ } TEST_F(ManagePasswordsUIControllerTest, NormalNavigations) { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); EXPECT_CALL(*controller(), OnUpdateBubbleAndIconVisibility()); controller()->OnPasswordSubmitted(std::move(test_form_manager)); @@ -442,7 +442,7 @@ } TEST_F(ManagePasswordsUIControllerTest, NormalNavigationsClosedBubble) { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); EXPECT_CALL(*controller(), OnUpdateBubbleAndIconVisibility()); controller()->OnPasswordSubmitted(std::move(test_form_manager)); @@ -467,7 +467,7 @@ content::WebContentsTester::For(web_contents()) ->NavigateAndCommit(GURL("chrome://sign-in")); - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); test_form_manager->ProvisionallySave( test_local_form(), @@ -500,7 +500,7 @@ } TEST_F(ManagePasswordsUIControllerTest, AutomaticPasswordSave) { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); EXPECT_CALL(*controller(), OnUpdateBubbleAndIconVisibility()); controller()->OnAutomaticPasswordSave(std::move(test_form_manager)); @@ -744,7 +744,7 @@ } TEST_F(ManagePasswordsUIControllerTest, UpdatePasswordSubmitted) { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); EXPECT_CALL(*controller(), OnUpdateBubbleAndIconVisibility()); controller()->OnUpdatePasswordSubmitted(std::move(test_form_manager)); @@ -755,7 +755,7 @@ } TEST_F(ManagePasswordsUIControllerTest, PasswordUpdated) { - std::unique_ptr<password_manager::PasswordFormManager> test_form_manager( + scoped_refptr<password_manager::PasswordFormManager> test_form_manager( CreateFormManager()); test_form_manager->ProvisionallySave( test_local_form(),
diff --git a/chrome/browser/ui/passwords/passwords_client_ui_delegate.h b/chrome/browser/ui/passwords/passwords_client_ui_delegate.h index b2b5928..2174249 100644 --- a/chrome/browser/ui/passwords/passwords_client_ui_delegate.h +++ b/chrome/browser/ui/passwords/passwords_client_ui_delegate.h
@@ -10,6 +10,7 @@ #include <vector> #include "base/callback.h" +#include "base/memory/ref_counted.h" #include "base/strings/string16.h" #include "components/autofill/core/common/password_form.h" @@ -30,13 +31,13 @@ // This stores the provided object and triggers the UI to prompt the user // about whether they would like to save the password. virtual void OnPasswordSubmitted( - std::unique_ptr<password_manager::PasswordFormManager> form_manager) = 0; + scoped_refptr<password_manager::PasswordFormManager> form_manager) = 0; // Called when the user submits a new password for an existing credential. // This stores the provided object and triggers the UI to prompt the user // about whether they would like to update the password. virtual void OnUpdatePasswordSubmitted( - std::unique_ptr<password_manager::PasswordFormManager> form_manager) = 0; + scoped_refptr<password_manager::PasswordFormManager> form_manager) = 0; // Called when the site asks user to choose from credentials. This triggers // the UI to prompt the user. |local_credentials| shouldn't be empty. |origin| @@ -61,7 +62,7 @@ // Called when the password will be saved automatically, but we still wish to // visually inform the user that the save has occured. virtual void OnAutomaticPasswordSave( - std::unique_ptr<password_manager::PasswordFormManager> form_manager) = 0; + scoped_refptr<password_manager::PasswordFormManager> form_manager) = 0; // Called when a form is autofilled with login information, so we can manage // password credentials for the current site which are stored in
diff --git a/chrome/common/BUILD.gn b/chrome/common/BUILD.gn index 47d8aff..eafd8cd 100644 --- a/chrome/common/BUILD.gn +++ b/chrome/common/BUILD.gn
@@ -113,6 +113,8 @@ "page_load_metrics/page_load_metrics_messages.h", "page_load_metrics/page_load_metrics_param_traits.cc", "page_load_metrics/page_load_metrics_param_traits.h", + "page_load_metrics/page_load_metrics_util.cc", + "page_load_metrics/page_load_metrics_util.h", "page_load_metrics/page_load_timing.cc", "page_load_metrics/page_load_timing.h", "page_load_metrics/page_track_decider.cc", @@ -654,6 +656,8 @@ visibility = [ "//chrome/test:*" ] sources = [ + "page_load_metrics/test/page_load_metrics_test_util.cc", + "page_load_metrics/test/page_load_metrics_test_util.h", "search/mock_searchbox.cc", "search/mock_searchbox.h", ]
diff --git a/chrome/common/page_load_metrics/page_load_metrics_util.cc b/chrome/common/page_load_metrics/page_load_metrics_util.cc new file mode 100644 index 0000000..43e65e9 --- /dev/null +++ b/chrome/common/page_load_metrics/page_load_metrics_util.cc
@@ -0,0 +1,65 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/common/page_load_metrics/page_load_metrics_util.h" + +#include <algorithm> + +#include "base/strings/string_util.h" +#include "net/base/registry_controlled_domains/registry_controlled_domain.h" + +namespace page_load_metrics { + +base::Optional<std::string> GetGoogleHostnamePrefix(const GURL& url) { + const size_t registry_length = + net::registry_controlled_domains::GetRegistryLength( + url, + + // Do not include unknown registries (registries that don't have any + // matches in effective TLD names). + net::registry_controlled_domains::EXCLUDE_UNKNOWN_REGISTRIES, + + // Do not include private registries, such as appspot.com. We don't + // want to match URLs like www.google.appspot.com. + net::registry_controlled_domains::EXCLUDE_PRIVATE_REGISTRIES); + + const base::StringPiece hostname = url.host_piece(); + if (registry_length == 0 || registry_length == std::string::npos || + registry_length >= hostname.length()) { + return base::Optional<std::string>(); + } + + // Removes the tld and the preceding dot. + const base::StringPiece hostname_minus_registry = + hostname.substr(0, hostname.length() - (registry_length + 1)); + + if (hostname_minus_registry == "google") + return std::string(""); + + if (!base::EndsWith(hostname_minus_registry, ".google", + base::CompareCase::INSENSITIVE_ASCII)) { + return base::Optional<std::string>(); + } + + return std::string(hostname_minus_registry.substr( + 0, hostname_minus_registry.length() - strlen(".google"))); +} + +bool IsGoogleHostname(const GURL& url) { + return GetGoogleHostnamePrefix(url).has_value(); +} + +base::Optional<base::TimeDelta> OptionalMin( + const base::Optional<base::TimeDelta>& a, + const base::Optional<base::TimeDelta>& b) { + if (a && !b) + return a; + if (b && !a) + return b; + if (!a && !b) + return a; // doesn't matter which + return base::Optional<base::TimeDelta>(std::min(a.value(), b.value())); +} + +} // namespace page_load_metrics
diff --git a/chrome/common/page_load_metrics/page_load_metrics_util.h b/chrome/common/page_load_metrics/page_load_metrics_util.h new file mode 100644 index 0000000..03e4b830 --- /dev/null +++ b/chrome/common/page_load_metrics/page_load_metrics_util.h
@@ -0,0 +1,40 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_COMMON_PAGE_LOAD_METRICS_PAGE_LOAD_METRICS_UTIL_H_ +#define CHROME_COMMON_PAGE_LOAD_METRICS_PAGE_LOAD_METRICS_UTIL_H_ + +#include <string> + +#include "base/optional.h" +#include "base/time/time.h" +#include "url/gurl.h" + +namespace page_load_metrics { + +// Returns the minimum value of the optional TimeDeltas, if both values are +// set. Otherwise, if one value is set, returns that value. Otherwise, returns +// an unset value. +base::Optional<base::TimeDelta> OptionalMin( + const base::Optional<base::TimeDelta>& a, + const base::Optional<base::TimeDelta>& b); + +// Whether the given url has a google hostname. +bool IsGoogleHostname(const GURL& url); + +// If the given hostname is a google hostname, returns the portion of the +// hostname before the google hostname. Otherwise, returns an unset optional +// value. +// +// For example: +// https://example.com/foo => returns an unset optional value +// https://google.com/foo => returns '' +// https://www.google.com/foo => returns 'www' +// https://news.google.com/foo => returns 'news' +// https://a.b.c.google.com/foo => returns 'a.b.c' +base::Optional<std::string> GetGoogleHostnamePrefix(const GURL& url); + +} // namespace page_load_metrics + +#endif // CHROME_COMMON_PAGE_LOAD_METRICS_PAGE_LOAD_METRICS_UTIL_H_
diff --git a/chrome/common/page_load_metrics/test/page_load_metrics_test_util.cc b/chrome/common/page_load_metrics/test/page_load_metrics_test_util.cc new file mode 100644 index 0000000..4b587e8 --- /dev/null +++ b/chrome/common/page_load_metrics/test/page_load_metrics_test_util.cc
@@ -0,0 +1,79 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/common/page_load_metrics/test/page_load_metrics_test_util.h" + +#include "chrome/common/page_load_metrics/page_load_metrics.mojom.h" +#include "chrome/common/page_load_metrics/page_load_metrics_util.h" + +using page_load_metrics::OptionalMin; + +void PopulateRequiredTimingFields( + page_load_metrics::mojom::PageLoadTiming* inout_timing) { + if (inout_timing->paint_timing->first_meaningful_paint && + !inout_timing->paint_timing->first_contentful_paint) { + inout_timing->paint_timing->first_contentful_paint = + inout_timing->paint_timing->first_meaningful_paint; + } + if ((inout_timing->paint_timing->first_text_paint || + inout_timing->paint_timing->first_image_paint || + inout_timing->paint_timing->first_contentful_paint) && + !inout_timing->paint_timing->first_paint) { + inout_timing->paint_timing->first_paint = + OptionalMin(OptionalMin(inout_timing->paint_timing->first_text_paint, + inout_timing->paint_timing->first_image_paint), + inout_timing->paint_timing->first_contentful_paint); + } + if (inout_timing->paint_timing->first_paint && + !inout_timing->document_timing->first_layout) { + inout_timing->document_timing->first_layout = + inout_timing->paint_timing->first_paint; + } + if (inout_timing->document_timing->load_event_start && + !inout_timing->document_timing->dom_content_loaded_event_start) { + inout_timing->document_timing->dom_content_loaded_event_start = + inout_timing->document_timing->load_event_start; + } + if (inout_timing->document_timing->first_layout && + !inout_timing->parse_timing->parse_start) { + inout_timing->parse_timing->parse_start = + inout_timing->document_timing->first_layout; + } + if (inout_timing->document_timing->dom_content_loaded_event_start && + !inout_timing->parse_timing->parse_stop) { + inout_timing->parse_timing->parse_stop = + inout_timing->document_timing->dom_content_loaded_event_start; + } + if (inout_timing->parse_timing->parse_stop && + !inout_timing->parse_timing->parse_start) { + inout_timing->parse_timing->parse_start = + inout_timing->parse_timing->parse_stop; + } + if (inout_timing->parse_timing->parse_start && + !inout_timing->response_start) { + inout_timing->response_start = inout_timing->parse_timing->parse_start; + } + if (inout_timing->parse_timing->parse_start) { + if (!inout_timing->parse_timing->parse_blocked_on_script_load_duration) + inout_timing->parse_timing->parse_blocked_on_script_load_duration = + base::TimeDelta(); + if (!inout_timing->parse_timing + ->parse_blocked_on_script_execution_duration) { + inout_timing->parse_timing->parse_blocked_on_script_execution_duration = + base::TimeDelta(); + } + if (!inout_timing->parse_timing + ->parse_blocked_on_script_load_from_document_write_duration) { + inout_timing->parse_timing + ->parse_blocked_on_script_load_from_document_write_duration = + base::TimeDelta(); + } + if (!inout_timing->parse_timing + ->parse_blocked_on_script_execution_from_document_write_duration) { + inout_timing->parse_timing + ->parse_blocked_on_script_execution_from_document_write_duration = + base::TimeDelta(); + } + } +}
diff --git a/chrome/common/page_load_metrics/test/page_load_metrics_test_util.h b/chrome/common/page_load_metrics/test/page_load_metrics_test_util.h new file mode 100644 index 0000000..cd0d9c1a --- /dev/null +++ b/chrome/common/page_load_metrics/test/page_load_metrics_test_util.h
@@ -0,0 +1,19 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_COMMON_PAGE_LOAD_METRICS_TEST_PAGE_LOAD_METRICS_TEST_UTIL_H_ +#define CHROME_COMMON_PAGE_LOAD_METRICS_TEST_PAGE_LOAD_METRICS_TEST_UTIL_H_ + +namespace page_load_metrics { +namespace mojom { +class PageLoadTiming; +} // namespace mojom +} // namespace page_load_metrics + +// Helper that fills in any timing fields that page load metrics requires but +// that are currently missing. +void PopulateRequiredTimingFields( + page_load_metrics::mojom::PageLoadTiming* inout_timing); + +#endif // CHROME_COMMON_PAGE_LOAD_METRICS_TEST_PAGE_LOAD_METRICS_TEST_UTIL_H_
diff --git a/chrome/common/page_load_metrics/test/weak_mock_timer.cc b/chrome/common/page_load_metrics/test/weak_mock_timer.cc new file mode 100644 index 0000000..0fe222af --- /dev/null +++ b/chrome/common/page_load_metrics/test/weak_mock_timer.cc
@@ -0,0 +1,25 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/common/page_load_metrics/test/weak_mock_timer.h" + +namespace page_load_metrics { +namespace test { + +WeakMockTimer::WeakMockTimer() + : MockTimer(false /* retain_user_task */, false /* is_repeating */) {} + +WeakMockTimerProvider::WeakMockTimerProvider() {} +WeakMockTimerProvider::~WeakMockTimerProvider() {} + +base::MockTimer* WeakMockTimerProvider::GetMockTimer() const { + return timer_.get(); +} + +void WeakMockTimerProvider::SetMockTimer(base::WeakPtr<WeakMockTimer> timer) { + timer_ = timer; +} + +} // namespace test +} // namespace page_load_metrics
diff --git a/chrome/common/page_load_metrics/test/weak_mock_timer.h b/chrome/common/page_load_metrics/test/weak_mock_timer.h new file mode 100644 index 0000000..185b586 --- /dev/null +++ b/chrome/common/page_load_metrics/test/weak_mock_timer.h
@@ -0,0 +1,44 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_COMMON_PAGE_LOAD_METRICS_TEST_WEAK_MOCK_TIMER_H_ +#define CHROME_COMMON_PAGE_LOAD_METRICS_TEST_WEAK_MOCK_TIMER_H_ + +#include "base/macros.h" +#include "base/memory/weak_ptr.h" +#include "base/timer/mock_timer.h" + +namespace page_load_metrics { +namespace test { + +// WeakMockTimer is a MockTimer that allows clients to keep WeakPtr<>s to it. +class WeakMockTimer : public base::MockTimer, + public base::SupportsWeakPtr<WeakMockTimer> { + public: + WeakMockTimer(); + + private: + DISALLOW_COPY_AND_ASSIGN(WeakMockTimer); +}; + +// WeakMockTimerProvider is a testing helper class that test classes can inherit +// from to provide basic MockTimer tracking capabilities. +class WeakMockTimerProvider { + public: + WeakMockTimerProvider(); + virtual ~WeakMockTimerProvider(); + + base::MockTimer* GetMockTimer() const; + void SetMockTimer(base::WeakPtr<WeakMockTimer> timer); + + private: + base::WeakPtr<WeakMockTimer> timer_; + + DISALLOW_COPY_AND_ASSIGN(WeakMockTimerProvider); +}; + +} // namespace test +} // namespace page_load_metrics + +#endif // CHROME_COMMON_PAGE_LOAD_METRICS_TEST_WEAK_MOCK_TIMER_H_
diff --git a/chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc b/chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc index 9996789b..64352d06 100644 --- a/chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc +++ b/chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc
@@ -207,8 +207,8 @@ return timing; } -std::unique_ptr<base::Timer> MetricsRenderFrameObserver::CreateTimer() const { - return base::WrapUnique(new base::OneShotTimer); +std::unique_ptr<base::Timer> MetricsRenderFrameObserver::CreateTimer() { + return base::MakeUnique<base::OneShotTimer>(); } std::unique_ptr<PageTimingSender>
diff --git a/chrome/renderer/page_load_metrics/metrics_render_frame_observer.h b/chrome/renderer/page_load_metrics/metrics_render_frame_observer.h index 78b8b3f..db52f07 100644 --- a/chrome/renderer/page_load_metrics/metrics_render_frame_observer.h +++ b/chrome/renderer/page_load_metrics/metrics_render_frame_observer.h
@@ -47,7 +47,7 @@ void SendMetrics(); virtual bool ShouldSendMetrics() const; virtual mojom::PageLoadTimingPtr GetTiming() const; - virtual std::unique_ptr<base::Timer> CreateTimer() const; + virtual std::unique_ptr<base::Timer> CreateTimer(); virtual std::unique_ptr<PageTimingSender> CreatePageTimingSender(); virtual bool HasNoRenderFrame() const;
diff --git a/chrome/renderer/page_load_metrics/metrics_render_frame_observer_unittest.cc b/chrome/renderer/page_load_metrics/metrics_render_frame_observer_unittest.cc index 90f0859..8dbb975 100644 --- a/chrome/renderer/page_load_metrics/metrics_render_frame_observer_unittest.cc +++ b/chrome/renderer/page_load_metrics/metrics_render_frame_observer_unittest.cc
@@ -11,6 +11,7 @@ #include "base/time/time.h" #include "base/timer/mock_timer.h" #include "chrome/common/page_load_metrics/page_load_timing.h" +#include "chrome/common/page_load_metrics/test/weak_mock_timer.h" #include "chrome/renderer/page_load_metrics/fake_page_timing_sender.h" #include "testing/gtest/include/gtest/gtest.h" @@ -19,14 +20,15 @@ // Implementation of the MetricsRenderFrameObserver class we're testing, // with the GetTiming() and ShouldSendMetrics() methods stubbed out to make // the rest of the class more testable. -class TestMetricsRenderFrameObserver : public MetricsRenderFrameObserver { +class TestMetricsRenderFrameObserver : public MetricsRenderFrameObserver, + public test::WeakMockTimerProvider { public: TestMetricsRenderFrameObserver() : MetricsRenderFrameObserver(nullptr) {} - std::unique_ptr<base::Timer> CreateTimer() const override { - if (!mock_timer_) - ADD_FAILURE() << "CreateTimer() called, but no MockTimer available."; - return std::move(mock_timer_); + std::unique_ptr<base::Timer> CreateTimer() override { + auto timer = base::MakeUnique<test::WeakMockTimer>(); + SetMockTimer(timer->AsWeakPtr()); + return std::move(timer); } std::unique_ptr<PageTimingSender> CreatePageTimingSender() override { @@ -34,11 +36,6 @@ new FakePageTimingSender(&validator_)); } - void set_mock_timer(std::unique_ptr<base::Timer> timer) { - ASSERT_EQ(nullptr, mock_timer_); - mock_timer_ = std::move(timer); - } - void ExpectPageLoadTiming(const mojom::PageLoadTiming& timing) { SetFakePageLoadTiming(timing); validator_.ExpectPageLoadTiming(timing); @@ -65,18 +62,14 @@ private: FakePageTimingSender::PageTimingValidator validator_; mutable mojom::PageLoadTimingPtr fake_timing_; - mutable std::unique_ptr<base::Timer> mock_timer_; }; typedef testing::Test MetricsRenderFrameObserverTest; TEST_F(MetricsRenderFrameObserverTest, NoMetrics) { TestMetricsRenderFrameObserver observer; - base::MockTimer* mock_timer = new base::MockTimer(false, false); - observer.set_mock_timer(base::WrapUnique(mock_timer)); - observer.DidChangePerformanceTiming(); - ASSERT_FALSE(mock_timer->IsRunning()); + ASSERT_EQ(nullptr, observer.GetMockTimer()); } TEST_F(MetricsRenderFrameObserverTest, SingleMetric) { @@ -84,21 +77,19 @@ base::TimeDelta first_layout = base::TimeDelta::FromMillisecondsD(10); TestMetricsRenderFrameObserver observer; - base::MockTimer* mock_timer = new base::MockTimer(false, false); - observer.set_mock_timer(base::WrapUnique(mock_timer)); mojom::PageLoadTiming timing; page_load_metrics::InitPageLoadTimingForTest(&timing); timing.navigation_start = nav_start; observer.ExpectPageLoadTiming(timing); observer.DidCommitProvisionalLoad(true, false); - mock_timer->Fire(); + observer.GetMockTimer()->Fire(); timing.document_timing->first_layout = first_layout; observer.ExpectPageLoadTiming(timing); observer.DidChangePerformanceTiming(); - mock_timer->Fire(); + observer.GetMockTimer()->Fire(); } TEST_F(MetricsRenderFrameObserverTest, MultipleMetrics) { @@ -108,22 +99,20 @@ base::TimeDelta load_event = base::TimeDelta::FromMillisecondsD(2); TestMetricsRenderFrameObserver observer; - base::MockTimer* mock_timer = new base::MockTimer(false, false); - observer.set_mock_timer(base::WrapUnique(mock_timer)); mojom::PageLoadTiming timing; page_load_metrics::InitPageLoadTimingForTest(&timing); timing.navigation_start = nav_start; observer.ExpectPageLoadTiming(timing); observer.DidCommitProvisionalLoad(true, false); - mock_timer->Fire(); + observer.GetMockTimer()->Fire(); timing.document_timing->first_layout = first_layout; timing.document_timing->dom_content_loaded_event_start = dom_event; observer.ExpectPageLoadTiming(timing); observer.DidChangePerformanceTiming(); - mock_timer->Fire(); + observer.GetMockTimer()->Fire(); // At this point, we should have triggered the generation of two metrics. // Verify and reset the observer's expectations before moving on to the next @@ -134,7 +123,7 @@ observer.ExpectPageLoadTiming(timing); observer.DidChangePerformanceTiming(); - mock_timer->Fire(); + observer.GetMockTimer()->Fire(); // Verify and reset the observer's expectations before moving on to the next // part of the test. @@ -146,7 +135,7 @@ // this invocation to generate any additional metrics. observer.SetFakePageLoadTiming(timing); observer.DidChangePerformanceTiming(); - ASSERT_FALSE(mock_timer->IsRunning()); + ASSERT_FALSE(observer.GetMockTimer()->IsRunning()); } TEST_F(MetricsRenderFrameObserverTest, MultipleNavigations) { @@ -156,22 +145,20 @@ base::TimeDelta load_event = base::TimeDelta::FromMillisecondsD(2); TestMetricsRenderFrameObserver observer; - base::MockTimer* mock_timer = new base::MockTimer(false, false); - observer.set_mock_timer(base::WrapUnique(mock_timer)); mojom::PageLoadTiming timing; page_load_metrics::InitPageLoadTimingForTest(&timing); timing.navigation_start = nav_start; observer.ExpectPageLoadTiming(timing); observer.DidCommitProvisionalLoad(true, false); - mock_timer->Fire(); + observer.GetMockTimer()->Fire(); timing.document_timing->first_layout = first_layout; timing.document_timing->dom_content_loaded_event_start = dom_event; timing.document_timing->load_event_start = load_event; observer.ExpectPageLoadTiming(timing); observer.DidChangePerformanceTiming(); - mock_timer->Fire(); + observer.GetMockTimer()->Fire(); // At this point, we should have triggered the generation of two metrics. // Verify and reset the observer's expectations before moving on to the next @@ -186,12 +173,11 @@ page_load_metrics::InitPageLoadTimingForTest(&timing_2); timing_2.navigation_start = nav_start_2; - base::MockTimer* mock_timer2 = new base::MockTimer(false, false); - observer.set_mock_timer(base::WrapUnique(mock_timer2)); + observer.SetMockTimer(nullptr); observer.ExpectPageLoadTiming(timing_2); observer.DidCommitProvisionalLoad(true, false); - mock_timer2->Fire(); + observer.GetMockTimer()->Fire(); timing_2.document_timing->first_layout = first_layout_2; timing_2.document_timing->dom_content_loaded_event_start = dom_event_2; @@ -199,7 +185,7 @@ observer.ExpectPageLoadTiming(timing_2); observer.DidChangePerformanceTiming(); - mock_timer2->Fire(); + observer.GetMockTimer()->Fire(); } } // namespace page_load_metrics
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index 627c81ce..7c5e1dad 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn
@@ -1522,8 +1522,6 @@ "../browser/ntp_snippets/content_suggestions_service_factory_browsertest.cc", "../browser/page_load_metrics/observers/ads_page_load_metrics_observer_browsertest.cc", "../browser/page_load_metrics/observers/https_engagement_metrics/https_engagement_page_load_metrics_observer_browsertest.cc", - "../browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.cc", - "../browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.h", "../browser/page_load_metrics/page_load_metrics_browsertest.cc", "../browser/password_manager/credential_manager_browsertest.cc", "../browser/password_manager/password_manager_browsertest.cc", @@ -3323,6 +3321,8 @@ "../common/media_router/media_source_helper_unittest.cc", "../common/media_router/media_source_unittest.cc", "../common/origin_trials/chrome_origin_trial_policy_unittest.cc", + "../common/page_load_metrics/test/weak_mock_timer.cc", + "../common/page_load_metrics/test/weak_mock_timer.h", "../common/partial_circular_buffer_unittest.cc", "../common/pref_names_util_unittest.cc", "../common/search/instant_types_unittest.cc",
diff --git a/chrome/test/base/testing_profile.cc b/chrome/test/base/testing_profile.cc index bbff5e9c..58041030 100644 --- a/chrome/test/base/testing_profile.cc +++ b/chrome/test/base/testing_profile.cc
@@ -25,6 +25,8 @@ #include "chrome/browser/bookmarks/chrome_bookmark_client.h" #include "chrome/browser/bookmarks/managed_bookmark_service_factory.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h" +#include "chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_factory.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/favicon/favicon_service_factory.h" @@ -985,6 +987,19 @@ return nullptr; } +content::BrowsingDataRemoverDelegate* +TestingProfile::GetBrowsingDataRemoverDelegate() { + // TestingProfile contains a real BrowsingDataRemover from BrowserContext. + // Since ChromeBrowsingDataRemoverDelegate is just a Chrome-specific extension + // of BrowsingDataRemover, we include it here for consistency. + // + // This is not a problem, since ChromeBrowsingDataRemoverDelegate mostly + // just serves as an interface to deletion mechanisms of various browsing + // data backends, which are already mocked if considered too heavy-weight + // for TestingProfile. + return ChromeBrowsingDataRemoverDelegateFactory::GetForProfile(this); +} + net::URLRequestContextGetter* TestingProfile::CreateRequestContext( content::ProtocolHandlerMap* protocol_handlers, content::URLRequestInterceptorScopedVector request_interceptors) {
diff --git a/chrome/test/base/testing_profile.h b/chrome/test/base/testing_profile.h index fcb80e0..9fff4af 100644 --- a/chrome/test/base/testing_profile.h +++ b/chrome/test/base/testing_profile.h
@@ -249,6 +249,8 @@ content::SSLHostStateDelegate* GetSSLHostStateDelegate() override; content::PermissionManager* GetPermissionManager() override; content::BackgroundSyncController* GetBackgroundSyncController() override; + content::BrowsingDataRemoverDelegate* GetBrowsingDataRemoverDelegate() + override; net::URLRequestContextGetter* CreateRequestContext( content::ProtocolHandlerMap* protocol_handlers, content::URLRequestInterceptorScopedVector request_interceptors) override;
diff --git a/chromecast/browser/cast_browser_context.cc b/chromecast/browser/cast_browser_context.cc index 0fe6501..2a8e6ce 100644 --- a/chromecast/browser/cast_browser_context.cc +++ b/chromecast/browser/cast_browser_context.cc
@@ -137,6 +137,11 @@ return nullptr; } +content::BrowsingDataRemoverDelegate* +CastBrowserContext::GetBrowsingDataRemoverDelegate() { + return nullptr; +} + net::URLRequestContextGetter* CastBrowserContext::CreateRequestContext( content::ProtocolHandlerMap* protocol_handlers, content::URLRequestInterceptorScopedVector request_interceptors) {
diff --git a/chromecast/browser/cast_browser_context.h b/chromecast/browser/cast_browser_context.h index 028196f..d14f3342 100644 --- a/chromecast/browser/cast_browser_context.h +++ b/chromecast/browser/cast_browser_context.h
@@ -40,6 +40,8 @@ content::SSLHostStateDelegate* GetSSLHostStateDelegate() override; content::PermissionManager* GetPermissionManager() override; content::BackgroundSyncController* GetBackgroundSyncController() override; + content::BrowsingDataRemoverDelegate* GetBrowsingDataRemoverDelegate() + override; net::URLRequestContextGetter* CreateRequestContext( content::ProtocolHandlerMap* protocol_handlers, content::URLRequestInterceptorScopedVector request_interceptors) override;
diff --git a/components/password_manager/content/browser/credential_manager_impl.cc b/components/password_manager/content/browser/credential_manager_impl.cc index 608ccbb..9d903e7 100644 --- a/components/password_manager/content/browser/credential_manager_impl.cc +++ b/components/password_manager/content/browser/credential_manager_impl.cc
@@ -83,7 +83,7 @@ // is only available on HTTPS origins. auto form_fetcher = base::MakeUnique<FormFetcherImpl>( PasswordStore::FormDigest(*observed_form), client_, false, false); - form_manager_ = base::MakeUnique<CredentialManagerPasswordFormManager>( + form_manager_ = base::MakeRefCounted<CredentialManagerPasswordFormManager>( client_, GetDriver(), *observed_form, std::move(form), this, nullptr, std::move(form_fetcher)); }
diff --git a/components/password_manager/content/browser/credential_manager_impl.h b/components/password_manager/content/browser/credential_manager_impl.h index 6db0b99..dd8acbe 100644 --- a/components/password_manager/content/browser/credential_manager_impl.h +++ b/components/password_manager/content/browser/credential_manager_impl.h
@@ -9,6 +9,7 @@ #include "base/callback.h" #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" #include "components/password_manager/content/common/credential_manager.mojom.h" #include "components/password_manager/core/browser/credential_manager_password_form_manager.h" @@ -84,7 +85,7 @@ virtual base::WeakPtr<PasswordManagerDriver> GetDriver(); PasswordManagerClient* client_; - std::unique_ptr<CredentialManagerPasswordFormManager> form_manager_; + scoped_refptr<CredentialManagerPasswordFormManager> form_manager_; // Set to false to disable automatic signing in. BooleanPrefMember auto_signin_enabled_;
diff --git a/components/password_manager/content/browser/credential_manager_impl_unittest.cc b/components/password_manager/content/browser/credential_manager_impl_unittest.cc index 0e7a132f..5feb29ed 100644 --- a/components/password_manager/content/browser/credential_manager_impl_unittest.cc +++ b/components/password_manager/content/browser/credential_manager_impl_unittest.cc
@@ -81,7 +81,7 @@ ~MockPasswordManagerClient() override {} bool PromptUserToSaveOrUpdatePassword( - std::unique_ptr<PasswordFormManager> manager, + scoped_refptr<PasswordFormManager> manager, bool update_password) override { manager_.swap(manager); PromptUserToSavePasswordPtr(manager_.get()); @@ -136,7 +136,7 @@ private: TestingPrefServiceSimple prefs_; PasswordStore* store_; - std::unique_ptr<PasswordFormManager> manager_; + scoped_refptr<PasswordFormManager> manager_; DISALLOW_COPY_AND_ASSIGN(MockPasswordManagerClient); };
diff --git a/components/password_manager/core/browser/credential_manager_password_form_manager.cc b/components/password_manager/core/browser/credential_manager_password_form_manager.cc index 4406d19..ffdeb11a 100644 --- a/components/password_manager/core/browser/credential_manager_password_form_manager.cc +++ b/components/password_manager/core/browser/credential_manager_password_form_manager.cc
@@ -45,9 +45,6 @@ GrabFetcher(std::move(form_fetcher)); } -CredentialManagerPasswordFormManager::~CredentialManagerPasswordFormManager() { -} - void CredentialManagerPasswordFormManager::ProcessMatches( const std::vector<const PasswordForm*>& non_federated, size_t filtered_count) { @@ -68,6 +65,8 @@ weak_factory_.GetWeakPtr())); } +CredentialManagerPasswordFormManager::~CredentialManagerPasswordFormManager() {} + void CredentialManagerPasswordFormManager::NotifyDelegate() { delegate_->OnProvisionalSaveComplete(); }
diff --git a/components/password_manager/core/browser/credential_manager_password_form_manager.h b/components/password_manager/core/browser/credential_manager_password_form_manager.h index 4816e17..556bb18 100644 --- a/components/password_manager/core/browser/credential_manager_password_form_manager.h +++ b/components/password_manager/core/browser/credential_manager_password_form_manager.h
@@ -6,6 +6,7 @@ #define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_CREDENTIAL_MANAGER_PASSWORD_FORM_MANAGER_H_ #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" #include "components/password_manager/core/browser/password_form_manager.h" @@ -46,12 +47,14 @@ CredentialManagerPasswordFormManagerDelegate* delegate, std::unique_ptr<FormSaver> form_saver, std::unique_ptr<FormFetcher> form_fetcher); - ~CredentialManagerPasswordFormManager() override; void ProcessMatches( const std::vector<const autofill::PasswordForm*>& non_federated, size_t filtered_count) override; + protected: + ~CredentialManagerPasswordFormManager() override; + private: // Calls OnProvisionalSaveComplete on |delegate_|. void NotifyDelegate();
diff --git a/components/password_manager/core/browser/credential_manager_password_form_manager_unittest.cc b/components/password_manager/core/browser/credential_manager_password_form_manager_unittest.cc index 88caa117..df684b27 100644 --- a/components/password_manager/core/browser/credential_manager_password_form_manager_unittest.cc +++ b/components/password_manager/core/browser/credential_manager_password_form_manager_unittest.cc
@@ -50,12 +50,14 @@ TEST_F(CredentialManagerPasswordFormManagerTest, AbortEarly) { PasswordForm observed_form; MockDelegate delegate; - auto form_manager = base::MakeUnique<CredentialManagerPasswordFormManager>( - &client_, driver_.AsWeakPtr(), observed_form, - base::MakeUnique<PasswordForm>(observed_form), &delegate, - base::MakeUnique<StubFormSaver>(), base::MakeUnique<FakeFormFetcher>()); + auto form_manager = + base::MakeRefCounted<CredentialManagerPasswordFormManager>( + &client_, driver_.AsWeakPtr(), observed_form, + base::MakeUnique<PasswordForm>(observed_form), &delegate, + base::MakeUnique<StubFormSaver>(), + base::MakeUnique<FakeFormFetcher>()); - auto deleter = [&form_manager]() { form_manager.reset(); }; + auto deleter = [&form_manager]() { form_manager = nullptr; }; // Simulate that the PasswordStore responded to the FormFetcher. As a result, // |form_manager| should call the delegate's OnProvisionalSaveComplete, which
diff --git a/components/password_manager/core/browser/password_form_manager.h b/components/password_manager/core/browser/password_form_manager.h index 2bc3bb2..932fb67c 100644 --- a/components/password_manager/core/browser/password_form_manager.h +++ b/components/password_manager/core/browser/password_form_manager.h
@@ -13,6 +13,7 @@ #include <vector> #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" #include "base/optional.h" #include "base/strings/string16.h" @@ -38,7 +39,8 @@ // This class helps with filling the observed form (both HTML and from HTTP // auth) and with saving/updating the stored information about it. -class PasswordFormManager : public FormFetcher::Consumer { +class PasswordFormManager : public FormFetcher::Consumer, + public base::RefCounted<PasswordFormManager> { public: // |password_manager| owns |this|, |client| and |driver| serve to // communicate with embedder, |observed_form| is the associated form |this| @@ -56,7 +58,6 @@ const autofill::PasswordForm& observed_form, std::unique_ptr<FormSaver> form_saver, FormFetcher* form_fetcher); - ~PasswordFormManager() override; // Flags describing the result of comparing two forms as performed by // DoesMatch. Individual flags are only relevant for HTML forms, but @@ -255,12 +256,16 @@ void GrabFetcher(std::unique_ptr<FormFetcher> fetcher); protected: + ~PasswordFormManager() override; + // FormFetcher::Consumer: void ProcessMatches( const std::vector<const autofill::PasswordForm*>& non_federated, size_t filtered_count) override; private: + friend class base::RefCounted<PasswordFormManager>; + // ManagerAction - What does the manager do with this form? Either it // fills it, or it doesn't. If it doesn't fill it, that's either // because it has no match or it is disabled via the AUTOCOMPLETE=off
diff --git a/components/password_manager/core/browser/password_form_manager_unittest.cc b/components/password_manager/core/browser/password_form_manager_unittest.cc index e4ed9f70..ebef05d 100644 --- a/components/password_manager/core/browser/password_form_manager_unittest.cc +++ b/components/password_manager/core/browser/password_form_manager_unittest.cc
@@ -387,9 +387,9 @@ saved_match_.form_data.fields.push_back(field); password_manager_.reset(new PasswordManager(&client_)); - form_manager_.reset(new PasswordFormManager( + form_manager_ = base::MakeRefCounted<PasswordFormManager>( password_manager_.get(), &client_, client_.driver(), observed_form_, - base::MakeUnique<NiceMock<MockFormSaver>>(), &fake_form_fetcher_)); + base::MakeUnique<NiceMock<MockFormSaver>>(), &fake_form_fetcher_); } // Save saved_match() for observed_form() where |observed_form_data|, @@ -406,9 +406,10 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager( - password_manager(), client(), client()->driver(), form, - base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), form, + base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher)); PasswordForm match = CreateSavedMatch(false); match.generation_upload_status = status; match.times_used = times_used; @@ -437,8 +438,8 @@ if (field_type) { // Show the password generation popup to check that the generation vote // would be ignored. - form_manager.set_generation_element(saved_match()->password_element); - form_manager.set_generation_popup_was_shown(true); + form_manager->set_generation_element(saved_match()->password_element); + form_manager->set_generation_popup_was_shown(true); expect_generation_vote = *field_type != autofill::ACCOUNT_CREATION_PASSWORD; @@ -459,9 +460,9 @@ StartUploadRequest(_, _, _, _, _)) .Times(0); } - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( form_to_save, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); - form_manager.Save(); + form_manager->Save(); Mock::VerifyAndClearExpectations( client()->mock_driver()->mock_autofill_download_manager()); } @@ -495,9 +496,10 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager( - password_manager(), client(), client()->driver(), *observed_form(), - base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *observed_form(), + base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher)); fetcher.SetNonFederated({saved_match()}, 0u); // User submits current and new credentials to the observed form. @@ -507,21 +509,21 @@ submitted_form.password_value = saved_match()->password_value; submitted_form.new_password_value = ASCIIToUTF16("test2"); submitted_form.preferred = true; - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( submitted_form, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); // Successful login. The PasswordManager would instruct PasswordFormManager // to update. - EXPECT_FALSE(form_manager.IsNewLogin()); + EXPECT_FALSE(form_manager->IsNewLogin()); EXPECT_FALSE( - form_manager.is_possible_change_password_form_without_username()); + form_manager->is_possible_change_password_form_without_username()); // By now, the PasswordFormManager should have promoted the new password // value already to be the current password, and should no longer maintain // any info about the new password value. EXPECT_EQ(submitted_form.new_password_value, - form_manager.pending_credentials().password_value); - EXPECT_TRUE(form_manager.pending_credentials().new_password_value.empty()); + form_manager->pending_credentials().password_value); + EXPECT_TRUE(form_manager->pending_credentials().new_password_value.empty()); std::map<base::string16, autofill::ServerFieldType> expected_types; expected_types[ASCIIToUTF16("full_name")] = autofill::UNKNOWN_TYPE; @@ -557,13 +559,13 @@ switch (field_type) { case autofill::NEW_PASSWORD: - form_manager.Update(*saved_match()); + form_manager->Update(*saved_match()); break; case autofill::PROBABLY_NEW_PASSWORD: - form_manager.OnNoInteraction(true /* it is an update */); + form_manager->OnNoInteraction(true /* it is an update */); break; case autofill::NOT_NEW_PASSWORD: - form_manager.OnNopeUpdateClicked(); + form_manager->OnNopeUpdateClicked(); break; default: NOTREACHED(); @@ -638,9 +640,10 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager( - password_manager(), client(), client()->driver(), form, - base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), form, + base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher)); fetcher.SetNonFederated(std::vector<const PasswordForm*>(), 0u); @@ -649,13 +652,13 @@ if (interaction == SAVE) expected_available_field_types.insert(autofill::PASSWORD); - form_manager.set_is_manual_generation(is_manual_generation); + form_manager->set_is_manual_generation(is_manual_generation); base::string16 generation_element = is_change_password_form ? form.new_password_element : form.password_element; - form_manager.set_generation_element(generation_element); - form_manager.set_generation_popup_was_shown(true); - form_manager.set_has_generated_password(has_generated_password); + form_manager->set_generation_element(generation_element); + form_manager->set_generation_popup_was_shown(true); + form_manager->set_has_generated_password(has_generated_password); // Figure out expected generation event type. autofill::AutofillUploadContents::Field::PasswordGenerationType @@ -676,17 +679,17 @@ form_structure.FormSignatureAsStr(), expected_generation_types), false, expected_available_field_types, std::string(), true)); - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( submitted_form, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); switch (interaction) { case SAVE: - form_manager.Save(); + form_manager->Save(); break; case NEVER: - form_manager.OnNeverClicked(); + form_manager->OnNeverClicked(); break; case NO_INTERACTION: - form_manager.OnNoInteraction(false /* not an update prompt*/); + form_manager->OnNoInteraction(false /* not an update prompt*/); break; } Mock::VerifyAndClearExpectations( @@ -750,9 +753,10 @@ const char* filled_username, const char* filled_password, const char* submitted_password = nullptr) { - PasswordFormManager form_manager( - password_manager(), client(), client()->driver(), *observed_form(), - base::MakeUnique<NiceMock<MockFormSaver>>(), fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *observed_form(), + base::MakeUnique<NiceMock<MockFormSaver>>(), fetcher)); EXPECT_CALL(*client()->mock_driver()->mock_autofill_download_manager(), StartUploadRequest(_, _, _, _, _)) @@ -788,13 +792,13 @@ submitted_password ? base::ASCIIToUTF16(submitted_password) : base::ASCIIToUTF16(filled_password); - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( submitted_form, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); if (submit_result == SimulatedSubmitResult::PASSED) { - form_manager.LogSubmitPassed(); - form_manager.Save(); + form_manager->LogSubmitPassed(); + form_manager->Save(); } else { - form_manager.LogSubmitFailed(); + form_manager->LogSubmitFailed(); } } } @@ -811,7 +815,7 @@ // Define |fake_form_fetcher_| before |form_manager_|, because the former // needs to outlive the latter. FakeFormFetcher fake_form_fetcher_; - std::unique_ptr<PasswordFormManager> form_manager_; + scoped_refptr<PasswordFormManager> form_manager_; }; class PasswordFormManagerFillOnAccountSelectTest @@ -930,9 +934,10 @@ observed_form()->signon_realm = "http://accounts.google.com"; FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager(password_manager(), client(), - client()->driver(), *observed_form(), - base::MakeUnique<MockFormSaver>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *observed_form(), + base::MakeUnique<MockFormSaver>(), &fetcher)); // Doesn't apply because it is just a PSL match of the observed form. PasswordForm blacklisted_psl = *observed_form(); @@ -975,12 +980,12 @@ saved_match()}; fetcher.SetNonFederated(matches, 0u); - EXPECT_TRUE(form_manager.IsBlacklisted()); - EXPECT_THAT(form_manager.blacklisted_matches(), + EXPECT_TRUE(form_manager->IsBlacklisted()); + EXPECT_THAT(form_manager->blacklisted_matches(), UnorderedElementsAre(Pointee(blacklisted_match), Pointee(blacklisted_match2))); - EXPECT_EQ(1u, form_manager.best_matches().size()); - EXPECT_EQ(*saved_match(), *form_manager.preferred_match()); + EXPECT_EQ(1u, form_manager->best_matches().size()); + EXPECT_EQ(*saved_match(), *form_manager->preferred_match()); } // Test that even in the presence of blacklisted matches, the non-blacklisted @@ -1070,9 +1075,10 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager(password_manager(), client(), - client()->driver(), *observed_form(), - base::MakeUnique<MockFormSaver>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *observed_form(), + base::MakeUnique<MockFormSaver>(), &fetcher)); fetcher.SetNonFederated(std::vector<const PasswordForm*>(), 0u); // User enters current and new credentials to the observed form. @@ -1081,28 +1087,28 @@ credentials.password_value = ASCIIToUTF16("oldpassword"); credentials.new_password_value = ASCIIToUTF16("newpassword"); credentials.preferred = true; - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( credentials, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); // Successful login. The PasswordManager would instruct PasswordFormManager // to save, which should know this is a new login. - EXPECT_TRUE(form_manager.IsNewLogin()); - EXPECT_EQ(credentials.origin, form_manager.pending_credentials().origin); + EXPECT_TRUE(form_manager->IsNewLogin()); + EXPECT_EQ(credentials.origin, form_manager->pending_credentials().origin); EXPECT_EQ(credentials.signon_realm, - form_manager.pending_credentials().signon_realm); - EXPECT_EQ(credentials.action, form_manager.pending_credentials().action); - EXPECT_TRUE(form_manager.pending_credentials().preferred); + form_manager->pending_credentials().signon_realm); + EXPECT_EQ(credentials.action, form_manager->pending_credentials().action); + EXPECT_TRUE(form_manager->pending_credentials().preferred); EXPECT_EQ(credentials.username_value, - form_manager.pending_credentials().username_value); + form_manager->pending_credentials().username_value); // By this point, the PasswordFormManager should have promoted the new // password value to be the current password, and should have wiped the // password element name: it is likely going to be different on a login // form, so it is not worth remembering them. EXPECT_EQ(credentials.new_password_value, - form_manager.pending_credentials().password_value); - EXPECT_TRUE(form_manager.pending_credentials().password_element.empty()); - EXPECT_TRUE(form_manager.pending_credentials().new_password_value.empty()); + form_manager->pending_credentials().password_value); + EXPECT_TRUE(form_manager->pending_credentials().password_element.empty()); + EXPECT_TRUE(form_manager->pending_credentials().new_password_value.empty()); } TEST_F(PasswordFormManagerTest, TestUpdatePassword) { @@ -1153,9 +1159,10 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager(password_manager(), client(), - client()->driver(), *observed_form(), - base::MakeUnique<MockFormSaver>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *observed_form(), + base::MakeUnique<MockFormSaver>(), &fetcher)); fetcher.SetNonFederated({saved_match()}, 0u); // User submits current and new credentials to the observed form. @@ -1164,28 +1171,28 @@ credentials.password_value = saved_match()->password_value; credentials.new_password_value = ASCIIToUTF16("test2"); credentials.preferred = true; - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( credentials, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); // Successful login. The PasswordManager would instruct PasswordFormManager // to save, and since this is an update, it should know not to save as a new // login. - EXPECT_FALSE(form_manager.IsNewLogin()); + EXPECT_FALSE(form_manager->IsNewLogin()); // By now, the PasswordFormManager should have promoted the new password value // already to be the current password, and should no longer maintain any info // about the new password. EXPECT_EQ(credentials.new_password_value, - form_manager.pending_credentials().password_value); - EXPECT_TRUE(form_manager.pending_credentials().new_password_element.empty()); - EXPECT_TRUE(form_manager.pending_credentials().new_password_value.empty()); + form_manager->pending_credentials().password_value); + EXPECT_TRUE(form_manager->pending_credentials().new_password_element.empty()); + EXPECT_TRUE(form_manager->pending_credentials().new_password_value.empty()); // Trigger saving to exercise some special case handling for updating. PasswordForm new_credentials; - EXPECT_CALL(MockFormSaver::Get(&form_manager), Update(_, _, _, nullptr)) + EXPECT_CALL(MockFormSaver::Get(form_manager.get()), Update(_, _, _, nullptr)) .WillOnce(testing::SaveArg<0>(&new_credentials)); - form_manager.Save(); + form_manager->Save(); // No meta-information should be updated, only the password. EXPECT_EQ(credentials.new_password_value, new_credentials.password_value); @@ -1204,9 +1211,10 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager(password_manager(), client(), - client()->driver(), observed, - base::MakeUnique<MockFormSaver>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), observed, + base::MakeUnique<MockFormSaver>(), &fetcher)); PasswordForm saved_form = observed; saved_form.origin = GURL("https://accounts.google.com/a/OtherLoginAuth"); @@ -1214,8 +1222,8 @@ fetcher.SetNonFederated({&saved_form}, 0u); // Different paths for action / origin are okay. - EXPECT_EQ(1u, form_manager.best_matches().size()); - EXPECT_EQ(*form_manager.best_matches().begin()->second, saved_form); + EXPECT_EQ(1u, form_manager->best_matches().size()); + EXPECT_EQ(*form_manager->best_matches().begin()->second, saved_form); } // Test that saved empty action URL is updated with the submitted action URL. @@ -1375,9 +1383,10 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager(password_manager(), client(), - client()->driver(), signup_form, - base::MakeUnique<MockFormSaver>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), signup_form, + base::MakeUnique<MockFormSaver>(), &fetcher)); EXPECT_CALL(*(client()->mock_driver()), AllowPasswordGenerationForForm(_)); PasswordForm simulated_result = CreateSavedMatch(false); fetcher.SetNonFederated({&simulated_result}, 0u); @@ -1527,9 +1536,10 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager(password_manager(), client(), - client()->driver(), encountered_form, - base::MakeUnique<MockFormSaver>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), encountered_form, + base::MakeUnique<MockFormSaver>(), &fetcher)); PasswordForm incomplete_form; incomplete_form.origin = GURL("http://accounts.google.com/LoginAuth"); @@ -1555,16 +1565,16 @@ // Feed the incomplete credentials to the manager. fetcher.SetNonFederated({&incomplete_form}, 0u); - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( complete_form, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); // By now that form has been used once. complete_form.times_used = 1; obsolete_form.times_used = 1; // Check that PasswordStore receives an update request with the complete form. - EXPECT_CALL(MockFormSaver::Get(&form_manager), + EXPECT_CALL(MockFormSaver::Get(form_manager.get()), Update(complete_form, _, _, Pointee(obsolete_form))); - form_manager.Save(); + form_manager->Save(); } // Test that public-suffix-matched credentials score lower than same-origin @@ -1706,10 +1716,11 @@ PasswordFormManager::RESULT_ACTION_MATCH); // Then when the observed form has an invalid URL: PasswordForm valid_action_form(*observed_form()); - PasswordFormManager invalid_manager( - password_manager(), client(), client()->driver(), invalid_action_form, - base::MakeUnique<MockFormSaver>(), fake_form_fetcher()); - EXPECT_EQ(0, invalid_manager.DoesManage(valid_action_form, nullptr) & + scoped_refptr<PasswordFormManager> invalid_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), invalid_action_form, + base::MakeUnique<MockFormSaver>(), fake_form_fetcher())); + EXPECT_EQ(0, invalid_manager->DoesManage(valid_action_form, nullptr) & PasswordFormManager::RESULT_ACTION_MATCH); } @@ -1723,10 +1734,11 @@ PasswordFormManager::RESULT_ACTION_MATCH); // Then when the observed form has an empty URL: PasswordForm valid_action_form(*observed_form()); - PasswordFormManager empty_action_manager( - password_manager(), client(), client()->driver(), empty_action_form, - base::MakeUnique<MockFormSaver>(), fake_form_fetcher()); - EXPECT_EQ(0, empty_action_manager.DoesManage(valid_action_form, nullptr) & + scoped_refptr<PasswordFormManager> empty_action_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), empty_action_form, + base::MakeUnique<MockFormSaver>(), fake_form_fetcher())); + EXPECT_EQ(0, empty_action_manager->DoesManage(valid_action_form, nullptr) & PasswordFormManager::RESULT_ACTION_MATCH); } @@ -1739,10 +1751,11 @@ // The other way round: observing a non-HTML form, don't match a HTML form. PasswordForm html_form(*observed_form()); - PasswordFormManager non_html_manager( - password_manager(), client(), kNoDriver, non_html_form, - base::MakeUnique<MockFormSaver>(), fake_form_fetcher()); - EXPECT_EQ(0, non_html_manager.DoesManage(html_form, nullptr) & + scoped_refptr<PasswordFormManager> non_html_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), kNoDriver, non_html_form, + base::MakeUnique<MockFormSaver>(), fake_form_fetcher())); + EXPECT_EQ(0, non_html_manager->DoesManage(html_form, nullptr) & PasswordFormManager::RESULT_HTML_ATTRIBUTES_MATCH); } @@ -1776,16 +1789,18 @@ PasswordForm secure_observed_form(*observed_form()); secure_observed_form.origin = GURL("https://accounts.google.com/a/LoginAuth"); - PasswordFormManager secure_manager( - password_manager(), client(), client()->driver(), secure_observed_form, - base::MakeUnique<MockFormSaver>(), fake_form_fetcher()); + scoped_refptr<PasswordFormManager> secure_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), + secure_observed_form, base::MakeUnique<MockFormSaver>(), + fake_form_fetcher())); // Also for HTTPS in the observed form, and HTTP in the compared form, an // exact path match is expected. - EXPECT_EQ(0, secure_manager.DoesManage(form_longer_path, nullptr) & + EXPECT_EQ(0, secure_manager->DoesManage(form_longer_path, nullptr) & PasswordFormManager::RESULT_HTML_ATTRIBUTES_MATCH); // Not even upgrade to HTTPS in the compared form should help. form_longer_path.origin = GURL("https://accounts.google.com/a/LoginAuth/sec"); - EXPECT_EQ(0, secure_manager.DoesManage(form_longer_path, nullptr) & + EXPECT_EQ(0, secure_manager->DoesManage(form_longer_path, nullptr) & PasswordFormManager::RESULT_HTML_ATTRIBUTES_MATCH); } @@ -1873,9 +1888,10 @@ // Don't vote for the username field yet. FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager( - password_manager(), client(), client()->driver(), *saved_match(), - base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *saved_match(), + base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher)); fetcher.SetNonFederated(std::vector<const PasswordForm*>(), 0u); PasswordForm form_to_save(*saved_match()); @@ -1888,18 +1904,19 @@ EXPECT_CALL( *client()->mock_driver()->mock_autofill_download_manager(), StartUploadRequest(_, false, expected_available_field_types, _, true)); - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( form_to_save, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); - form_manager.Save(); + form_manager->Save(); } TEST_F(PasswordFormManagerTest, UploadFormData_NewPassword_Blacklist) { // Do not upload a vote if the user is blacklisting the form. FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager blacklist_form_manager( - password_manager(), client(), client()->driver(), *saved_match(), - base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher); + scoped_refptr<PasswordFormManager> blacklist_form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *saved_match(), + base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher)); fetcher.SetNonFederated(std::vector<const PasswordForm*>(), 0u); autofill::ServerFieldTypeSet expected_available_field_types; @@ -1908,7 +1925,7 @@ EXPECT_CALL(*client()->mock_driver()->mock_autofill_download_manager(), StartUploadRequest(_, _, expected_available_field_types, _, true)) .Times(0); - blacklist_form_manager.PermanentlyBlacklist(); + blacklist_form_manager->PermanentlyBlacklist(); } TEST_F(PasswordFormManagerTest, UploadPasswordForm) { @@ -1993,9 +2010,10 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager(password_manager(), client(), - client()->driver(), form, - base::MakeUnique<MockFormSaver>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), form, + base::MakeUnique<MockFormSaver>(), &fetcher)); // Suddenly, the frame and its driver disappear. client()->KillDriver(); @@ -2073,10 +2091,11 @@ base::ASCIIToUTF16("new_pwd"); FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager manager_creds( - password_manager(), client(), client()->driver(), - observed_change_password_form, base::MakeUnique<MockFormSaver>(), - &fetcher); + scoped_refptr<PasswordFormManager> manager_creds( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), + observed_change_password_form, base::MakeUnique<MockFormSaver>(), + &fetcher)); autofill::PasswordFormFillData fill_data; EXPECT_CALL(*client()->mock_driver(), FillPasswordForm(_)) @@ -2084,7 +2103,7 @@ PasswordForm result = CreateSavedMatch(false); fetcher.SetNonFederated({&result}, 0u); - EXPECT_EQ(1u, manager_creds.best_matches().size()); + EXPECT_EQ(1u, manager_creds->best_matches().size()); EXPECT_EQ(0u, fill_data.additional_logins.size()); EXPECT_TRUE(fill_data.wait_for_username); } @@ -2110,9 +2129,10 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager(password_manager(), client(), - client()->driver(), *observed_form(), - base::MakeUnique<MockFormSaver>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *observed_form(), + base::MakeUnique<MockFormSaver>(), &fetcher)); fetcher.SetNonFederated({saved_match()}, 0u); @@ -2122,29 +2142,29 @@ credentials.password_value = saved_match()->password_value; credentials.new_password_value = ASCIIToUTF16("test2"); credentials.preferred = true; - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( credentials, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); // Successful login. The PasswordManager would instruct PasswordFormManager // to save, and since this is an update, it should know not to save as a new // login. - EXPECT_FALSE(form_manager.IsNewLogin()); + EXPECT_FALSE(form_manager->IsNewLogin()); EXPECT_FALSE( - form_manager.is_possible_change_password_form_without_username()); + form_manager->is_possible_change_password_form_without_username()); // By now, the PasswordFormManager should have promoted the new password value // already to be the current password, and should no longer maintain any info // about the new password value. EXPECT_EQ(credentials.new_password_value, - form_manager.pending_credentials().password_value); - EXPECT_TRUE(form_manager.pending_credentials().new_password_value.empty()); + form_manager->pending_credentials().password_value); + EXPECT_TRUE(form_manager->pending_credentials().new_password_value.empty()); // Trigger saving to exercise some special case handling during updating. PasswordForm new_credentials; - EXPECT_CALL(MockFormSaver::Get(&form_manager), Update(_, _, _, nullptr)) + EXPECT_CALL(MockFormSaver::Get(form_manager.get()), Update(_, _, _, nullptr)) .WillOnce(SaveArg<0>(&new_credentials)); - form_manager.Update(*saved_match()); + form_manager->Update(*saved_match()); // No meta-information should be updated, only the password. EXPECT_EQ(credentials.new_password_value, new_credentials.password_value); @@ -2175,9 +2195,10 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager(password_manager(), client(), - client()->driver(), *observed_form(), - base::MakeUnique<MockFormSaver>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *observed_form(), + base::MakeUnique<MockFormSaver>(), &fetcher)); fetcher.SetNonFederated({saved_match()}, 0u); @@ -2188,30 +2209,31 @@ credentials.password_value = saved_match()->password_value; credentials.new_password_value = ASCIIToUTF16("test2"); credentials.preferred = true; - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( credentials, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); // Successful login. The PasswordManager would instruct PasswordFormManager // to save, and since this is an update, it should know not to save as a new // login. - EXPECT_FALSE(form_manager.IsNewLogin()); - EXPECT_TRUE(form_manager.is_possible_change_password_form_without_username()); + EXPECT_FALSE(form_manager->IsNewLogin()); + EXPECT_TRUE( + form_manager->is_possible_change_password_form_without_username()); // By now, the PasswordFormManager should have promoted the new password value // already to be the current password, and should no longer maintain any info // about the new password value. EXPECT_EQ(saved_match()->username_value, - form_manager.pending_credentials().username_value); + form_manager->pending_credentials().username_value); EXPECT_EQ(credentials.new_password_value, - form_manager.pending_credentials().password_value); - EXPECT_TRUE(form_manager.pending_credentials().new_password_value.empty()); + form_manager->pending_credentials().password_value); + EXPECT_TRUE(form_manager->pending_credentials().new_password_value.empty()); // Trigger saving to exercise some special case handling during updating. PasswordForm new_credentials; - EXPECT_CALL(MockFormSaver::Get(&form_manager), Update(_, _, _, nullptr)) + EXPECT_CALL(MockFormSaver::Get(form_manager.get()), Update(_, _, _, nullptr)) .WillOnce(SaveArg<0>(&new_credentials)); - form_manager.Update(form_manager.pending_credentials()); + form_manager->Update(form_manager->pending_credentials()); // No other information than password value should be updated. In particular // not the username. @@ -2230,21 +2252,22 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager(password_manager(), client(), - client()->driver(), form, - base::MakeUnique<MockFormSaver>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), form, + base::MakeUnique<MockFormSaver>(), &fetcher)); // The creation of |fetcher| keeps it waiting for store results. This test // keeps the fetcher waiting on purpose. PasswordForm submitted_form(form); submitted_form.password_value += ASCIIToUTF16("add stuff, make it different"); - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( submitted_form, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); base::HistogramTester histogram_tester; - EXPECT_CALL(MockFormSaver::Get(&form_manager), - WipeOutdatedCopies(form_manager.pending_credentials(), _, _)); - form_manager.WipeStoreCopyIfOutdated(); + EXPECT_CALL(MockFormSaver::Get(form_manager.get()), + WipeOutdatedCopies(form_manager->pending_credentials(), _, _)); + form_manager->WipeStoreCopyIfOutdated(); histogram_tester.ExpectUniqueSample("PasswordManager.StoreReadyWhenWiping", 0, 1); } @@ -2363,9 +2386,10 @@ TEST_F(PasswordFormManagerTest, TestUpdatePSLMatchedCredentials) { FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager(password_manager(), client(), - client()->driver(), *observed_form(), - base::MakeUnique<MockFormSaver>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *observed_form(), + base::MakeUnique<MockFormSaver>(), &fetcher)); fetcher.SetNonFederated({saved_match(), psl_saved_match()}, 0u); // User submits a credentials with an old username and a new password. @@ -2373,24 +2397,24 @@ credentials.username_value = saved_match()->username_value; credentials.password_value = ASCIIToUTF16("new_password"); credentials.preferred = true; - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( credentials, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); // Successful login. The PasswordManager would instruct PasswordFormManager // to save, and since this is an update, it should know not to save as a new // login. - EXPECT_FALSE(form_manager.IsNewLogin()); + EXPECT_FALSE(form_manager->IsNewLogin()); // Trigger saving to exercise some special case handling during updating. PasswordForm new_credentials; std::vector<autofill::PasswordForm> credentials_to_update; - EXPECT_CALL(MockFormSaver::Get(&form_manager), Update(_, _, _, nullptr)) + EXPECT_CALL(MockFormSaver::Get(form_manager.get()), Update(_, _, _, nullptr)) .WillOnce(testing::DoAll(SaveArg<0>(&new_credentials), SaveArgPointee<2>(&credentials_to_update))); EXPECT_CALL(*client()->mock_driver()->mock_autofill_download_manager(), StartUploadRequest(_, false, _, _, true)); - form_manager.Save(); + form_manager->Save(); // No meta-information should be updated, only the password. EXPECT_EQ(credentials.password_value, new_credentials.password_value); @@ -2415,9 +2439,10 @@ TestNotUpdatePSLMatchedCredentialsWithAnotherUsername) { FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager(password_manager(), client(), - client()->driver(), *observed_form(), - base::MakeUnique<MockFormSaver>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *observed_form(), + base::MakeUnique<MockFormSaver>(), &fetcher)); psl_saved_match()->username_value += ASCIIToUTF16("1"); fetcher.SetNonFederated({saved_match(), psl_saved_match()}, 0u); @@ -2426,23 +2451,23 @@ credentials.username_value = saved_match()->username_value; credentials.password_value = ASCIIToUTF16("new_password"); credentials.preferred = true; - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( credentials, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); // Successful login. The PasswordManager would instruct PasswordFormManager // to save, and since this is an update, it should know not to save as a new // login. - EXPECT_FALSE(form_manager.IsNewLogin()); + EXPECT_FALSE(form_manager->IsNewLogin()); // Trigger saving to exercise some special case handling during updating. PasswordForm new_credentials; - EXPECT_CALL(MockFormSaver::Get(&form_manager), + EXPECT_CALL(MockFormSaver::Get(form_manager.get()), Update(_, _, Pointee(IsEmpty()), nullptr)) .WillOnce(testing::SaveArg<0>(&new_credentials)); EXPECT_CALL(*client()->mock_driver()->mock_autofill_download_manager(), StartUploadRequest(_, false, _, _, true)); - form_manager.Save(); + form_manager->Save(); // No meta-information should be updated, only the password. EXPECT_EQ(credentials.password_value, new_credentials.password_value); @@ -2456,9 +2481,10 @@ TestNotUpdatePSLMatchedCredentialsWithAnotherPassword) { FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager(password_manager(), client(), - client()->driver(), *observed_form(), - base::MakeUnique<MockFormSaver>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *observed_form(), + base::MakeUnique<MockFormSaver>(), &fetcher)); psl_saved_match()->password_value += ASCIIToUTF16("1"); fetcher.SetNonFederated({saved_match(), psl_saved_match()}, 0u); @@ -2467,23 +2493,23 @@ credentials.username_value = saved_match()->username_value; credentials.password_value = ASCIIToUTF16("new_password"); credentials.preferred = true; - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( credentials, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); // Successful login. The PasswordManager would instruct PasswordFormManager // to save, and since this is an update, it should know not to save as a new // login. - EXPECT_FALSE(form_manager.IsNewLogin()); + EXPECT_FALSE(form_manager->IsNewLogin()); // Trigger saving to exercise some special case handling during updating. PasswordForm new_credentials; - EXPECT_CALL(MockFormSaver::Get(&form_manager), + EXPECT_CALL(MockFormSaver::Get(form_manager.get()), Update(_, _, Pointee(IsEmpty()), nullptr)) .WillOnce(testing::SaveArg<0>(&new_credentials)); EXPECT_CALL(*client()->mock_driver()->mock_autofill_download_manager(), StartUploadRequest(_, false, _, _, true)); - form_manager.Save(); + form_manager->Save(); // No meta-information should be updated, only the password. EXPECT_EQ(credentials.password_value, new_credentials.password_value); @@ -2496,9 +2522,10 @@ TEST_F(PasswordFormManagerTest, TestNotUpdateWhenOnlyPSLMatched) { FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager(password_manager(), client(), - client()->driver(), *observed_form(), - base::MakeUnique<MockFormSaver>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *observed_form(), + base::MakeUnique<MockFormSaver>(), &fetcher)); fetcher.SetNonFederated({psl_saved_match()}, 0u); // User submits a credentials with an old username and a new password. @@ -2506,18 +2533,18 @@ credentials.username_value = saved_match()->username_value; credentials.password_value = ASCIIToUTF16("new_password"); credentials.preferred = true; - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( credentials, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); - EXPECT_TRUE(form_manager.IsNewLogin()); + EXPECT_TRUE(form_manager->IsNewLogin()); // PSL matched credential should not be updated, since we are not sure that // this is the same credential as submitted one. PasswordForm new_credentials; - EXPECT_CALL(MockFormSaver::Get(&form_manager), Save(_, _, nullptr)) + EXPECT_CALL(MockFormSaver::Get(form_manager.get()), Save(_, _, nullptr)) .WillOnce(testing::SaveArg<0>(&new_credentials)); - form_manager.Save(); + form_manager->Save(); EXPECT_EQ(credentials.password_value, new_credentials.password_value); EXPECT_EQ(credentials.username_value, new_credentials.username_value); @@ -2652,9 +2679,10 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager(password_manager(), client(), - client()->driver(), *observed_form(), - base::MakeUnique<MockFormSaver>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *observed_form(), + base::MakeUnique<MockFormSaver>(), &fetcher)); fetcher.SetNonFederated({saved_match()}, 0u); // User submits current and new credentials to the observed form. @@ -2662,20 +2690,21 @@ submitted_form.password_value = saved_match()->password_value; submitted_form.new_password_value = ASCIIToUTF16("test2"); submitted_form.preferred = true; - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( submitted_form, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); // Successful login. The PasswordManager would instruct PasswordFormManager // to update. - EXPECT_FALSE(form_manager.IsNewLogin()); - EXPECT_TRUE(form_manager.is_possible_change_password_form_without_username()); + EXPECT_FALSE(form_manager->IsNewLogin()); + EXPECT_TRUE( + form_manager->is_possible_change_password_form_without_username()); // By now, the PasswordFormManager should have promoted the new password // value already to be the current password, and should no longer maintain // any info about the new password value. EXPECT_EQ(submitted_form.new_password_value, - form_manager.pending_credentials().password_value); - EXPECT_TRUE(form_manager.pending_credentials().new_password_value.empty()); + form_manager->pending_credentials().password_value); + EXPECT_TRUE(form_manager->pending_credentials().new_password_value.empty()); std::map<base::string16, autofill::ServerFieldType> expected_types; expected_types[observed_form()->password_element] = autofill::PASSWORD; @@ -2699,7 +2728,7 @@ false, expected_available_field_types, expected_login_signature, true)); - form_manager.Update(*saved_match()); + form_manager->Update(*saved_match()); } // Checks uploading a vote about the usage of the password generation popup. @@ -2735,14 +2764,15 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager( - password_manager(), client(), client()->driver(), form, - base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), form, + base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher)); base::string16 generation_element = form.password_element; if (found_generation_element) - form_manager.SaveGenerationFieldDetectedByClassifier(generation_element); + form_manager->SaveGenerationFieldDetectedByClassifier(generation_element); else - form_manager.SaveGenerationFieldDetectedByClassifier(base::string16()); + form_manager->SaveGenerationFieldDetectedByClassifier(base::string16()); fetcher.SetNonFederated(std::vector<const PasswordForm*>(), 0u); @@ -2754,9 +2784,9 @@ found_generation_element, generation_element), false, _, _, true)); - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( submitted_form, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); - form_manager.Save(); + form_manager->Save(); } } @@ -2772,9 +2802,10 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager( - password_manager(), client(), client()->driver(), form, - base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), form, + base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher)); fetcher.SetNonFederated(std::vector<const PasswordForm*>(), 0u); DCHECK_EQ(3U, form.form_data.fields.size()); @@ -2793,9 +2824,9 @@ StartUploadRequest( CheckFieldPropertiesMasksUpload(expected_field_properties), false, _, _, true)); - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( submitted_form, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); - form_manager.Save(); + form_manager->Save(); } TEST_F(PasswordFormManagerTest, TestSavingAPIFormsWithSamePassword) { @@ -2807,9 +2838,10 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager(password_manager(), client(), - client()->driver(), *observed_form(), - base::MakeUnique<MockFormSaver>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *observed_form(), + base::MakeUnique<MockFormSaver>(), &fetcher)); fetcher.SetNonFederated({saved_match()}, 0u); // User submits new credentials with the same password as in already saved @@ -2820,16 +2852,16 @@ credentials.password_value = saved_match()->password_value; credentials.preferred = true; - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( credentials, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); - EXPECT_TRUE(form_manager.IsNewLogin()); + EXPECT_TRUE(form_manager->IsNewLogin()); PasswordForm new_credentials; - EXPECT_CALL(MockFormSaver::Get(&form_manager), Save(_, _, nullptr)) + EXPECT_CALL(MockFormSaver::Get(form_manager.get()), Save(_, _, nullptr)) .WillOnce(SaveArg<0>(&new_credentials)); - form_manager.Save(); + form_manager->Save(); EXPECT_EQ(saved_match()->username_value + ASCIIToUTF16("1"), new_credentials.username_value); @@ -2873,9 +2905,10 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager( - password_manager(), client(), client()->driver(), form, - base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), form, + base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher)); PasswordForm form_to_save(form); form_to_save.preferred = true; @@ -2903,9 +2936,9 @@ false /* expect_generation_vote */), false, expected_available_field_types, std::string(), true)); - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( form_to_save, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); - form_manager.Save(); + form_manager->Save(); } TEST_F(PasswordFormManagerFillOnAccountSelectTest, ProcessFrame) { @@ -2961,11 +2994,12 @@ observed.scheme = kCorrectScheme; FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager( - password_manager(), client(), - (kCorrectScheme == PasswordForm::SCHEME_HTML ? client()->driver() - : nullptr), - observed, base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), + (kCorrectScheme == PasswordForm::SCHEME_HTML ? client()->driver() + : nullptr), + observed, base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher)); PasswordForm match = *saved_match(); match.scheme = kCorrectScheme; @@ -2974,20 +3008,20 @@ non_match.scheme = kWrongScheme; // First try putting the correct scheme first in returned matches. - static_cast<FormFetcher::Consumer*>(&form_manager) + static_cast<FormFetcher::Consumer*>(form_manager.get()) ->ProcessMatches({&match, &non_match}, 0u); - EXPECT_EQ(1u, form_manager.best_matches().size()); + EXPECT_EQ(1u, form_manager->best_matches().size()); EXPECT_EQ(kCorrectScheme, - form_manager.best_matches().begin()->second->scheme); + form_manager->best_matches().begin()->second->scheme); // Now try putting the correct scheme last in returned matches. - static_cast<FormFetcher::Consumer*>(&form_manager) + static_cast<FormFetcher::Consumer*>(form_manager.get()) ->ProcessMatches({&non_match, &match}, 0u); - EXPECT_EQ(1u, form_manager.best_matches().size()); + EXPECT_EQ(1u, form_manager->best_matches().size()); EXPECT_EQ(kCorrectScheme, - form_manager.best_matches().begin()->second->scheme); + form_manager->best_matches().begin()->second->scheme); } } } @@ -3132,13 +3166,13 @@ MockFormFetcher fetcher; FormFetcher::Consumer* added_consumer = nullptr; EXPECT_CALL(fetcher, AddConsumer(_)).WillOnce(SaveArg<0>(&added_consumer)); - auto form_manager = base::MakeUnique<PasswordFormManager>( + auto form_manager = base::MakeRefCounted<PasswordFormManager>( password_manager(), client(), client()->driver(), *observed_form(), base::MakeUnique<MockFormSaver>(), &fetcher); EXPECT_EQ(form_manager.get(), added_consumer); EXPECT_CALL(fetcher, RemoveConsumer(form_manager.get())); - form_manager.reset(); + form_manager = nullptr; } // Check that if asked to take ownership of the same FormFetcher which it had @@ -3147,17 +3181,18 @@ TEST_F(PasswordFormManagerTest, GrabFetcher_Same) { auto fetcher = base::MakeUnique<MockFormFetcher>(); fetcher->Fetch(); - PasswordFormManager form_manager( - password_manager(), client(), client()->driver(), *observed_form(), - base::MakeUnique<MockFormSaver>(), fetcher.get()); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *observed_form(), + base::MakeUnique<MockFormSaver>(), fetcher.get())); EXPECT_CALL(*fetcher, AddConsumer(_)).Times(0); EXPECT_CALL(*fetcher, RemoveConsumer(_)).Times(0); - form_manager.GrabFetcher(std::move(fetcher)); + form_manager->GrabFetcher(std::move(fetcher)); // There will be a RemoveConsumer call as soon as form_manager goes out of // scope, but the test needs to ensure that there is none as a result of // GrabFetcher. - Mock::VerifyAndClearExpectations(form_manager.form_fetcher()); + Mock::VerifyAndClearExpectations(form_manager->form_fetcher()); } // Check that if asked to take ownership of a different FormFetcher than which @@ -3189,15 +3224,16 @@ FormFetcher::Consumer* added_consumer = nullptr; EXPECT_CALL(old_fetcher, AddConsumer(_)) .WillOnce(SaveArg<0>(&added_consumer)); - PasswordFormManager form_manager( - password_manager(), client(), client()->driver(), *observed_form(), - base::MakeUnique<MockFormSaver>(), &old_fetcher); - EXPECT_EQ(&form_manager, added_consumer); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *observed_form(), + base::MakeUnique<MockFormSaver>(), &old_fetcher)); + EXPECT_EQ(form_manager.get(), added_consumer); auto new_fetcher = base::MakeUnique<MockFormFetcher>(); - EXPECT_CALL(*new_fetcher, AddConsumer(&form_manager)); - EXPECT_CALL(old_fetcher, RemoveConsumer(&form_manager)); - form_manager.GrabFetcher(std::move(new_fetcher)); + EXPECT_CALL(*new_fetcher, AddConsumer(form_manager.get())); + EXPECT_CALL(old_fetcher, RemoveConsumer(form_manager.get())); + form_manager->GrabFetcher(std::move(new_fetcher)); } TEST_F(PasswordFormManagerTest, UploadSignInForm_WithAutofillTypes) { @@ -3214,9 +3250,10 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager( - password_manager(), client(), client()->driver(), *observed_form(), - base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *observed_form(), + base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher)); fetcher.SetNonFederated(std::vector<const PasswordForm*>(), 0u); PasswordForm form_to_save(*observed_form()); @@ -3229,9 +3266,9 @@ client()->mock_driver()->mock_autofill_manager(); EXPECT_CALL(*mock_autofill_manager, StartUploadProcessPtr(_, _, true)) .WillOnce(WithArg<0>(SaveToUniquePtr(&uploaded_form_structure))); - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( form_to_save, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); - form_manager.Save(); + form_manager->Save(); ASSERT_EQ(2u, uploaded_form_structure->field_count()); autofill::ServerFieldTypeSet expected_types = {autofill::PASSWORD}; @@ -3251,9 +3288,10 @@ FakeFormFetcher fetcher; fetcher.Fetch(); - PasswordFormManager form_manager( - password_manager(), client(), client()->driver(), *observed_form(), - base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher); + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( + password_manager(), client(), client()->driver(), *observed_form(), + base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher)); fetcher.SetNonFederated(std::vector<const PasswordForm*>(), 0u); PasswordForm form_to_save(*observed_form()); @@ -3263,9 +3301,9 @@ auto* mock_autofill_manager = client()->mock_driver()->mock_autofill_manager(); EXPECT_CALL(*mock_autofill_manager, StartUploadProcessPtr(_, _, _)).Times(0); - form_manager.ProvisionallySave( + form_manager->ProvisionallySave( form_to_save, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); - form_manager.Save(); + form_manager->Save(); } TEST_F(PasswordFormManagerTest, @@ -3274,12 +3312,12 @@ fake_form_fetcher()->set_did_complete_querying_suppressed_forms(false); fake_form_fetcher()->Fetch(); - std::unique_ptr<PasswordFormManager> form_manager = - base::MakeUnique<PasswordFormManager>( + scoped_refptr<PasswordFormManager> form_manager( + base::MakeRefCounted<PasswordFormManager>( password_manager(), client(), client()->driver(), *observed_form(), - base::MakeUnique<NiceMock<MockFormSaver>>(), fake_form_fetcher()); + base::MakeUnique<NiceMock<MockFormSaver>>(), fake_form_fetcher())); fake_form_fetcher()->SetNonFederated(std::vector<const PasswordForm*>(), 0u); - form_manager.reset(); + form_manager = nullptr; histogram_tester.ExpectUniqueSample( "PasswordManager.QueryingSuppressedAccountsFinished", false, 1); @@ -3472,12 +3510,12 @@ fetcher.set_did_complete_querying_suppressed_forms(true); fetcher.Fetch(); - std::unique_ptr<PasswordFormManager> form_manager = - base::MakeUnique<PasswordFormManager>( + scoped_refptr<PasswordFormManager> form_manager = + base::MakeRefCounted<PasswordFormManager>( password_manager(), client(), client()->driver(), https_observed_form, base::MakeUnique<NiceMock<MockFormSaver>>(), &fetcher); fetcher.SetNonFederated(std::vector<const PasswordForm*>(), 0u); - form_manager.reset(); + form_manager = nullptr; histogram_tester.ExpectUniqueSample( "PasswordManager.QueryingSuppressedAccountsFinished", true, 1);
diff --git a/components/password_manager/core/browser/password_manager.cc b/components/password_manager/core/browser/password_manager.cc index 804aa5d..df1208d 100644 --- a/components/password_manager/core/browser/password_manager.cc +++ b/components/password_manager/core/browser/password_manager.cc
@@ -238,7 +238,7 @@ // If there is no corresponding PasswordFormManager, we create one. This is // not the common case, and should only happen when there is a bug in our // ability to detect forms. - auto manager = base::MakeUnique<PasswordFormManager>( + auto manager = base::MakeRefCounted<PasswordFormManager>( this, client_, driver->AsWeakPtr(), form, base::WrapUnique(new FormSaverImpl(client_->GetPasswordStore())), nullptr); @@ -349,11 +349,9 @@ return; } - std::unique_ptr<PasswordFormManager> manager; - // Transfer ownership of the manager from |pending_login_managers_| to + // Copy ownership of the manager from |pending_login_managers_| to // |manager|. - manager.swap(*matched_manager_it); - pending_login_managers_.erase(matched_manager_it); + scoped_refptr<PasswordFormManager> manager(*matched_manager_it); PasswordForm submitted_form(form); submitted_form.preferred = true; @@ -410,7 +408,7 @@ void PasswordManager::DropFormManagers() { pending_login_managers_.clear(); - provisional_save_manager_.reset(); + provisional_save_manager_ = nullptr; all_visible_forms_.clear(); } @@ -567,7 +565,7 @@ if (logger) logger->LogFormSignatures(Logger::STRING_ADDING_SIGNATURE, *iter); - auto manager = base::MakeUnique<PasswordFormManager>( + auto manager = base::MakeRefCounted<PasswordFormManager>( this, client_, (driver ? driver->AsWeakPtr() : base::WeakPtr<PasswordManagerDriver>()), *iter, base::WrapUnique(new FormSaverImpl(client_->GetPasswordStore())), @@ -603,7 +601,7 @@ RecordFailure(MATCHING_NOT_COMPLETE, provisional_save_manager_->observed_form().origin, logger.get()); - provisional_save_manager_.reset(); + provisional_save_manager_ = nullptr; return false; } return true; @@ -650,7 +648,7 @@ if (logger) logger->LogMessage(Logger::STRING_DECISION_DROP); provisional_save_manager_->LogSubmitFailed(); - provisional_save_manager_.reset(); + provisional_save_manager_ = nullptr; return; } @@ -687,7 +685,7 @@ all_visible_forms_[i]); logger->LogMessage(Logger::STRING_DECISION_DROP); } - provisional_save_manager_.reset(); + provisional_save_manager_ = nullptr; // Clear all_visible_forms_ once we found the match. all_visible_forms_.clear(); return; @@ -754,7 +752,7 @@ RecordFailure(SYNC_CREDENTIAL, provisional_save_manager_->observed_form().origin, logger.get()); - provisional_save_manager_.reset(); + provisional_save_manager_ = nullptr; return; } } @@ -794,7 +792,7 @@ if (provisional_save_manager_->has_generated_password()) { client_->AutomaticPasswordSave(std::move(provisional_save_manager_)); } else { - provisional_save_manager_.reset(); + provisional_save_manager_ = nullptr; } } }
diff --git a/components/password_manager/core/browser/password_manager.h b/components/password_manager/core/browser/password_manager.h index b85165d..30990f69 100644 --- a/components/password_manager/core/browser/password_manager.h +++ b/components/password_manager/core/browser/password_manager.h
@@ -13,6 +13,7 @@ #include "base/callback.h" #include "base/gtest_prod_util.h" #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "base/observer_list.h" #include "base/strings/string16.h" #include "build/build_config.h" @@ -184,7 +185,7 @@ #if defined(UNIT_TEST) // TODO(crbug.com/639786): Replace using this by quering the factory for // mocked PasswordFormManagers. - const std::vector<std::unique_ptr<PasswordFormManager>>& + const std::vector<scoped_refptr<PasswordFormManager>>& pending_login_managers() { return pending_login_managers_; } @@ -266,7 +267,7 @@ // When a form is "seen" on a page, a PasswordFormManager is created // and stored in this collection until user navigates away from page. - std::vector<std::unique_ptr<PasswordFormManager>> pending_login_managers_; + std::vector<scoped_refptr<PasswordFormManager>> pending_login_managers_; // When the user submits a password/credential, this contains the // PasswordFormManager for the form in question until we deem the login @@ -274,7 +275,7 @@ // send the PasswordFormManager back to the pending_login_managers_ set. // Scoped in case PasswordManager gets deleted (e.g tab closes) between the // time a user submits a login form and gets to the next page. - std::unique_ptr<PasswordFormManager> provisional_save_manager_; + scoped_refptr<PasswordFormManager> provisional_save_manager_; // The embedder-level client. Must outlive this class. PasswordManagerClient* const client_;
diff --git a/components/password_manager/core/browser/password_manager_client.h b/components/password_manager/core/browser/password_manager_client.h index 7004aee79..405202d 100644 --- a/components/password_manager/core/browser/password_manager_client.h +++ b/components/password_manager/core/browser/password_manager_client.h
@@ -9,6 +9,7 @@ #include "base/callback.h" #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "components/autofill/core/common/password_form.h" #include "components/password_manager/core/browser/credentials_filter.h" #include "components/password_manager/core/browser/password_store.h" @@ -91,7 +92,7 @@ // and form_to_save.pending_credentials() should correspond to the credential // that was overidden. virtual bool PromptUserToSaveOrUpdatePassword( - std::unique_ptr<PasswordFormManager> form_to_save, + scoped_refptr<PasswordFormManager> form_to_save, bool update_password) = 0; // Informs the embedder of a password forms that the user should choose from. @@ -136,7 +137,7 @@ // Called when a password is saved in an automated fashion. Embedder may // inform the user that this save has occured. virtual void AutomaticPasswordSave( - std::unique_ptr<PasswordFormManager> saved_form_manager) = 0; + scoped_refptr<PasswordFormManager> saved_form_manager) = 0; // Called when a password is autofilled. |best_matches| contains the // PasswordForm into which a password was filled: the client may choose to
diff --git a/components/password_manager/core/browser/password_manager_unittest.cc b/components/password_manager/core/browser/password_manager_unittest.cc index d0895e8c8..f1b9c07 100644 --- a/components/password_manager/core/browser/password_manager_unittest.cc +++ b/components/password_manager/core/browser/password_manager_unittest.cc
@@ -61,9 +61,8 @@ MOCK_CONST_METHOD0(IsSavingAndFillingEnabledForCurrentPage, bool()); MOCK_CONST_METHOD0(DidLastPageLoadEncounterSSLErrors, bool()); MOCK_CONST_METHOD0(GetPasswordStore, PasswordStore*()); - // The code inside EXPECT_CALL for PromptUserToSaveOrUpdatePasswordPtr owns - // the PasswordFormManager* argument. - MOCK_METHOD1(PromptUserToSaveOrUpdatePasswordPtr, void(PasswordFormManager*)); + MOCK_METHOD2(PromptUserToSaveOrUpdatePassword, + bool(scoped_refptr<PasswordFormManager>, bool)); MOCK_METHOD1(NotifySuccessfulLoginWithExistingPassword, void(const autofill::PasswordForm&)); MOCK_METHOD0(AutomaticPasswordSaveIndicator, void()); @@ -72,15 +71,8 @@ MOCK_METHOD0(GetDriver, PasswordManagerDriver*()); MOCK_CONST_METHOD0(GetStoreResultFilter, const MockStoreResultFilter*()); - // Workaround for std::unique_ptr<> lacking a copy constructor. - bool PromptUserToSaveOrUpdatePassword( - std::unique_ptr<PasswordFormManager> manager, - bool update_password) override { - PromptUserToSaveOrUpdatePasswordPtr(manager.release()); - return false; - } void AutomaticPasswordSave( - std::unique_ptr<PasswordFormManager> manager) override { + scoped_refptr<PasswordFormManager> manager) override { AutomaticPasswordSaveIndicator(); } @@ -110,8 +102,6 @@ arg0->OnGetPasswordStoreResults(std::vector<std::unique_ptr<PasswordForm>>()); } -ACTION_P(SaveToScopedPtr, scoped) { scoped->reset(arg0); } - } // namespace class PasswordManagerTest : public testing::Test { @@ -271,9 +261,9 @@ .WillRepeatedly(Return(true)); OnPasswordFormSubmitted(form); - std::unique_ptr<PasswordFormManager> form_manager_to_save; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save))); + scoped_refptr<PasswordFormManager> form_manager_to_save; + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager_to_save), Return(false))); // Now the password manager waits for the navigation to complete. observed.clear(); @@ -313,7 +303,7 @@ // consent by using the generated password. The form should be saved once // navigation occurs. The client will be informed that automatic saving has // occured. - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)).Times(0); PasswordForm form_to_save; EXPECT_CALL(*store_, AddLogin(_)).WillOnce(SaveArg<0>(&form_to_save)); EXPECT_CALL(client_, AutomaticPasswordSaveIndicator()); @@ -349,9 +339,9 @@ OnPasswordFormSubmitted(form); // We still expect an add, since we didn't have a good match. - std::unique_ptr<PasswordFormManager> form_manager_to_save; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save))); + scoped_refptr<PasswordFormManager> form_manager_to_save; + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager_to_save), Return(false))); // Now the password manager waits for the navigation to complete. observed.clear(); @@ -375,7 +365,7 @@ // No message from the renderer that a password was submitted. No // expected calls. - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)).Times(0); observed.clear(); manager()->OnPasswordFormsParsed(&driver_, observed); manager()->OnPasswordFormsRendered(&driver_, observed, true); @@ -397,9 +387,9 @@ .WillRepeatedly(Return(true)); OnPasswordFormSubmitted(form); - std::unique_ptr<PasswordFormManager> form_manager_to_save; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save))); + scoped_refptr<PasswordFormManager> form_manager_to_save; + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager_to_save), Return(false))); observed.clear(); manager()->OnPasswordFormsParsed(&driver_, observed); @@ -446,9 +436,9 @@ .WillRepeatedly(Return(true)); OnPasswordFormSubmitted(second_form); - std::unique_ptr<PasswordFormManager> form_manager_to_save; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save))); + scoped_refptr<PasswordFormManager> form_manager_to_save; + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager_to_save), Return(false))); // Navigation after form submit, no forms appear. observed.clear(); manager()->OnPasswordFormsParsed(&driver_, observed); @@ -477,9 +467,9 @@ OnPasswordFormSubmitted(form); // Expect info bar to appear: - std::unique_ptr<PasswordFormManager> form_manager_to_save; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save))); + scoped_refptr<PasswordFormManager> form_manager_to_save; + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager_to_save), Return(false))); // The form reappears, but is not visible in the layout: manager()->OnPasswordFormsParsed(&driver_, observed); @@ -551,7 +541,7 @@ observed.push_back(MakeTwitterFailedLoginForm()); // A PasswordForm appears, and is visible in the layout: // No expected calls to the PasswordStore... - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)).Times(0); EXPECT_CALL(client_, AutomaticPasswordSaveIndicator()).Times(0); EXPECT_CALL(*store_, AddLogin(_)).Times(0); EXPECT_CALL(*store_, UpdateLogin(_)).Times(0); @@ -571,7 +561,7 @@ manager()->OnPasswordFormsRendered(&driver_, observed, true); // User should not be prompted and password should not be saved. - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)).Times(0); EXPECT_CALL(*store_, AddLogin(_)).Times(0); // Prefs are needed for failure logging about sync credentials. EXPECT_CALL(client_, GetPrefs()).WillRepeatedly(Return(nullptr)); @@ -598,7 +588,7 @@ // |observed_form| and not the |stored_form| what is passed to ShouldSave. observed_form.username_element += ASCIIToUTF16("1"); observed.push_back(observed_form); - EXPECT_CALL(driver_, FillPasswordForm(_)).Times(2); + EXPECT_CALL(driver_, FillPasswordForm(_)).Times(3); // Simulate that |form| is already in the store, making this an update. EXPECT_CALL(*store_, GetLogins(_, _)) .WillRepeatedly(WithArg<1>(InvokeConsumer(stored_form))); @@ -684,7 +674,7 @@ // Because the user successfully uses an updated sync password, Chrome should // remove the obsolete copy of it. EXPECT_CALL(*store_, RemoveLogin(form)); - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)).Times(0); observed.clear(); manager()->OnPasswordFormsParsed(&driver_, observed); manager()->OnPasswordFormsRendered(&driver_, observed, true); @@ -721,7 +711,7 @@ first_form.origin = GURL("http://www.xda-developers.com/"); first_form.action = GURL("http://forum.xda-developers.com/login.php"); - // |second_form|'s action differs only with it's scheme i.e. *https://*. + // |second_form|'s action differs only with it's scheme i.e. "https://". PasswordForm second_form(first_form); second_form.action = GURL("https://forum.xda-developers.com/login.php"); @@ -742,7 +732,7 @@ observed.push_back(second_form); // Verify that no prompt to save the password is shown. - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)).Times(0); manager()->OnPasswordFormsParsed(&driver_, observed); manager()->OnPasswordFormsRendered(&driver_, observed, true); } @@ -814,9 +804,9 @@ // Make sure |PromptUserToSaveOrUpdatePassword| gets called, and the resulting // form manager is saved. - std::unique_ptr<PasswordFormManager> form_manager_to_save; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save))); + scoped_refptr<PasswordFormManager> form_manager_to_save; + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager_to_save), Return(false))); EXPECT_CALL(client_, GetMainFrameURL()) .WillRepeatedly(ReturnRef(insecure_form.origin)); @@ -829,7 +819,7 @@ // Expect no further calls to |ProptUserToSaveOrUpdatePassword| due to // insecure origin. - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)).Times(0); // Trigger call to |ProvisionalSavePassword| by rendering a page without // forms. @@ -865,9 +855,9 @@ .WillRepeatedly(Return(true)); OnPasswordFormSubmitted(form); - std::unique_ptr<PasswordFormManager> form_manager_to_save; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save))); + scoped_refptr<PasswordFormManager> form_manager_to_save; + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager_to_save), Return(false))); // Now the password manager waits for the login to complete successfully. observed.clear(); @@ -898,9 +888,9 @@ .WillRepeatedly(Return(true)); OnPasswordFormSubmitted(form); - std::unique_ptr<PasswordFormManager> form_manager_to_save; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save))); + scoped_refptr<PasswordFormManager> form_manager_to_save; + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager_to_save), Return(false))); // Now the password manager waits for the navigation to complete. observed.clear(); @@ -941,19 +931,23 @@ PasswordForm form(MakeSimpleForm()); observed.push_back(form); EXPECT_CALL(*store_, GetLogins(_, _)) - .WillOnce(WithArg<1>(InvokeEmptyConsumerWithForms())); + .Times(2) + .WillRepeatedly(WithArg<1>(InvokeEmptyConsumerWithForms())); manager()->OnPasswordFormsParsed(&driver_, observed); manager()->OnPasswordFormsRendered(&driver_, observed, true); EXPECT_CALL(client_, IsSavingAndFillingEnabledForCurrentPage()) .WillRepeatedly(Return(true)); - std::unique_ptr<PasswordFormManager> form_manager_to_save; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save))); + scoped_refptr<PasswordFormManager> form_manager_to_save; + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager_to_save), Return(false))); manager()->OnInPageNavigation(&driver_, form); + // Checks |form_manager_to_save| is still in |pending_login_managers_|. + EXPECT_EQ(1u, manager()->pending_login_managers().size()); + // Simulate saving the form, as if the info bar was accepted. EXPECT_CALL(*store_, AddLogin(FormMatches(form))); ASSERT_TRUE(form_manager_to_save); @@ -980,9 +974,9 @@ // Prefs are needed for failure logging about blacklisting. EXPECT_CALL(client_, GetPrefs()).WillRepeatedly(Return(nullptr)); - std::unique_ptr<PasswordFormManager> form_manager_to_save; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save))); + scoped_refptr<PasswordFormManager> form_manager_to_save; + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager_to_save), Return(false))); manager()->OnInPageNavigation(&driver_, form); EXPECT_TRUE(form_manager_to_save->IsBlacklisted()); @@ -1015,9 +1009,9 @@ .WillRepeatedly(Return(true)); OnPasswordFormSubmitted(submitted_form); - std::unique_ptr<PasswordFormManager> form_manager_to_save; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save))); + scoped_refptr<PasswordFormManager> form_manager_to_save; + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager_to_save), Return(false))); // Now the password manager waits for the navigation to complete. observed.clear(); @@ -1068,9 +1062,9 @@ .WillRepeatedly(Return(true)); OnPasswordFormSubmitted(submitted_form); - std::unique_ptr<PasswordFormManager> form_manager_to_save; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save))); + scoped_refptr<PasswordFormManager> form_manager_to_save; + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager_to_save), Return(false))); // Now the password manager waits for the navigation to complete. observed.clear(); @@ -1120,9 +1114,9 @@ .WillRepeatedly(Return(true)); OnPasswordFormSubmitted(form); - std::unique_ptr<PasswordFormManager> form_manager_to_save; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save))); + scoped_refptr<PasswordFormManager> form_manager_to_save; + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager_to_save), Return(false))); // Now the password manager waits for the navigation to complete. observed.clear(); @@ -1153,7 +1147,7 @@ autofill::PasswordForm updated_form; autofill::PasswordForm notified_form; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)).Times(0); EXPECT_CALL(*store_, UpdateLogin(_)).WillOnce(SaveArg<0>(&updated_form)); EXPECT_CALL(client_, NotifySuccessfulLoginWithExistingPassword(_)) .WillOnce(SaveArg<0>(¬ified_form)); @@ -1193,9 +1187,9 @@ static_cast<FormFetcherImpl*>(form_manager->form_fetcher()) ->OnGetPasswordStoreResults(std::vector<std::unique_ptr<PasswordForm>>()); - std::unique_ptr<PasswordFormManager> form_manager_to_save; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save))); + scoped_refptr<PasswordFormManager> form_manager_to_save; + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager_to_save), Return(false))); // Now the password manager waits for the navigation to complete. observed.clear(); @@ -1222,7 +1216,7 @@ manager()->SetHasGeneratedPasswordForForm(&driver_, form, true); // Do not save generated password when the password form reappears. - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)).Times(0); EXPECT_CALL(*store_, AddLogin(_)).Times(0); EXPECT_CALL(client_, AutomaticPasswordSaveIndicator()).Times(0); @@ -1254,7 +1248,7 @@ OnPasswordFormSubmitted(form); // Do not save generated password when the password form reappears. - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)).Times(0); EXPECT_CALL(*store_, AddLogin(_)).Times(0); EXPECT_CALL(client_, AutomaticPasswordSaveIndicator()).Times(0); @@ -1288,7 +1282,7 @@ OnPasswordFormSubmitted(form); // No infobar or prompt is shown if submission fails. - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)).Times(0); EXPECT_CALL(client_, AutomaticPasswordSaveIndicator()).Times(0); // Simulate submission failing, with the same form being visible after @@ -1320,9 +1314,9 @@ OnPasswordFormSubmitted(form); // Verify that a normal prompt is shown instead of the force saving UI. - std::unique_ptr<PasswordFormManager> form_to_save; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_to_save))); + scoped_refptr<PasswordFormManager> form_to_save; + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_to_save), Return(false))); EXPECT_CALL(client_, AutomaticPasswordSaveIndicator()).Times(0); // Simulate a successful submission. @@ -1350,7 +1344,7 @@ form.username_value = ASCIIToUTF16("new_username"); OnPasswordFormSubmitted(form); - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)).Times(0); PasswordForm form_to_save; EXPECT_CALL(*store_, AddLogin(_)).WillOnce(SaveArg<0>(&form_to_save)); EXPECT_CALL(client_, AutomaticPasswordSaveIndicator()); @@ -1413,12 +1407,12 @@ .WillRepeatedly(WithArg<1>(InvokeEmptyConsumerWithForms())); EXPECT_CALL(*store_, GetLoginsForSameOrganizationName(_, _)); } - std::unique_ptr<PasswordFormManager> form_manager; + scoped_refptr<PasswordFormManager> form_manager; if (found_matched_logins_in_store) { - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager))); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager), Return(false))); } else { - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)).Times(0); } EXPECT_CALL(client_, AutomaticPasswordSaveIndicator()) .Times(found_matched_logins_in_store ? 0 : 1); @@ -1485,11 +1479,11 @@ manager()->OnPasswordFormsParsed(&driver_, observed); manager()->OnPasswordFormsRendered(&driver_, observed, true); - std::unique_ptr<PasswordFormManager> form_manager_to_save; + scoped_refptr<PasswordFormManager> form_manager_to_save; EXPECT_CALL(client_, IsSavingAndFillingEnabledForCurrentPage()) .WillRepeatedly(Return(true)); - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save))); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager_to_save), Return(false))); manager()->OnPasswordFormForceSaveRequested(&driver_, form); ASSERT_TRUE(form_manager_to_save); EXPECT_EQ(form.password_value, @@ -1514,7 +1508,7 @@ EXPECT_CALL(client_, IsSavingAndFillingEnabledForCurrentPage()) .WillRepeatedly(Return(true)); - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)).Times(0); manager()->OnPasswordFormForceSaveRequested(&driver_, empty_password_form); } @@ -1555,7 +1549,7 @@ .WillRepeatedly(Return(true)); OnPasswordFormSubmitted(form); - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)).Times(0); observed.clear(); manager()->OnPasswordFormsParsed(&driver_, observed); manager()->OnPasswordFormsRendered(&driver_, observed, true); @@ -1591,7 +1585,7 @@ PasswordForm saved_form; PasswordForm saved_notified_form; EXPECT_CALL(*store_, UpdateLogin(_)).WillOnce(SaveArg<0>(&saved_form)); - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0); + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)).Times(0); EXPECT_CALL(client_, NotifySuccessfulLoginWithExistingPassword(_)) .WillOnce(SaveArg<0>(&saved_notified_form)); EXPECT_CALL(*store_, AddLogin(_)).Times(0); @@ -1629,9 +1623,9 @@ filled_form.password_value = ASCIIToUTF16("new_password"); OnPasswordFormSubmitted(filled_form); - std::unique_ptr<PasswordFormManager> form_manager_to_save; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save))); + scoped_refptr<PasswordFormManager> form_manager_to_save; + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager_to_save), Return(false))); observed_forms.clear(); manager()->OnPasswordFormsParsed(&driver_, observed_forms); @@ -1680,9 +1674,9 @@ observed[0].new_password_value.clear(); // Check success of the submission. - std::unique_ptr<PasswordFormManager> form_manager_to_save; - EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)) - .WillOnce(WithArg<0>(SaveToScopedPtr(&form_manager_to_save))); + scoped_refptr<PasswordFormManager> form_manager_to_save; + EXPECT_CALL(client_, PromptUserToSaveOrUpdatePassword(_, _)) + .WillOnce(DoAll(SaveArg<0>(&form_manager_to_save), Return(false))); manager()->OnPasswordFormsParsed(&driver_, observed); manager()->OnPasswordFormsRendered(&driver_, observed, true);
diff --git a/components/password_manager/core/browser/stub_password_manager_client.cc b/components/password_manager/core/browser/stub_password_manager_client.cc index 309c877e..432d115 100644 --- a/components/password_manager/core/browser/stub_password_manager_client.cc +++ b/components/password_manager/core/browser/stub_password_manager_client.cc
@@ -16,7 +16,7 @@ StubPasswordManagerClient::~StubPasswordManagerClient() {} bool StubPasswordManagerClient::PromptUserToSaveOrUpdatePassword( - std::unique_ptr<PasswordFormManager> form_to_save, + scoped_refptr<PasswordFormManager> form_to_save, bool update_password) { return false; } @@ -41,7 +41,7 @@ void StubPasswordManagerClient::NotifyStorePasswordCalled() {} void StubPasswordManagerClient::AutomaticPasswordSave( - std::unique_ptr<PasswordFormManager> saved_manager) {} + scoped_refptr<PasswordFormManager> saved_manager) {} PrefService* StubPasswordManagerClient::GetPrefs() { return nullptr;
diff --git a/components/password_manager/core/browser/stub_password_manager_client.h b/components/password_manager/core/browser/stub_password_manager_client.h index 77c740b..4cfb70e 100644 --- a/components/password_manager/core/browser/stub_password_manager_client.h +++ b/components/password_manager/core/browser/stub_password_manager_client.h
@@ -22,7 +22,7 @@ // PasswordManagerClient: bool PromptUserToSaveOrUpdatePassword( - std::unique_ptr<PasswordFormManager> form_to_save, + scoped_refptr<PasswordFormManager> form_to_save, bool update_password) override; bool PromptUserToChooseCredentials( std::vector<std::unique_ptr<autofill::PasswordForm>> local_forms, @@ -37,7 +37,7 @@ const autofill::PasswordForm& form) override; void NotifyStorePasswordCalled() override; void AutomaticPasswordSave( - std::unique_ptr<PasswordFormManager> saved_manager) override; + scoped_refptr<PasswordFormManager> saved_manager) override; PrefService* GetPrefs() override; PasswordStore* GetPasswordStore() const override; const GURL& GetLastCommittedEntryURL() const override;
diff --git a/components/password_manager/sync/browser/sync_credentials_filter_unittest.cc b/components/password_manager/sync/browser/sync_credentials_filter_unittest.cc index 2830c55..1ae27e47 100644 --- a/components/password_manager/sync/browser/sync_credentials_filter_unittest.cc +++ b/components/password_manager/sync/browser/sync_credentials_filter_unittest.cc
@@ -96,12 +96,13 @@ CredentialsFilterTest() : password_manager_(&client_), pending_(SimpleGaiaForm("user@gmail.com")), - form_manager_(&password_manager_, - &client_, - driver_.AsWeakPtr(), - pending_, - base::MakeUnique<StubFormSaver>(), - &fetcher_), + form_manager_(base::MakeRefCounted<PasswordFormManager>( + &password_manager_, + &client_, + driver_.AsWeakPtr(), + pending_, + base::MakeUnique<StubFormSaver>(), + &fetcher_)), filter_(&client_, base::Bind(&SyncUsernameTestBase::sync_service, base::Unretained(this)), @@ -138,7 +139,7 @@ } fetcher_.SetNonFederated(matches, 0u); - form_manager_.ProvisionallySave( + form_manager_->ProvisionallySave( pending_, PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES); } @@ -148,7 +149,7 @@ StubPasswordManagerDriver driver_; PasswordForm pending_; FakeFormFetcher fetcher_; - PasswordFormManager form_manager_; + scoped_refptr<PasswordFormManager> form_manager_; SyncCredentialsFilter filter_; }; @@ -286,7 +287,7 @@ base::UserActionTester tester; SavePending(LoginState::EXISTING); - filter_.ReportFormLoginSuccess(form_manager_); + filter_.ReportFormLoginSuccess(*form_manager_); EXPECT_EQ(1, tester.GetActionCount(kFilledAndLoginActionName)); } @@ -296,7 +297,7 @@ base::UserActionTester tester; SavePending(LoginState::NEW); - filter_.ReportFormLoginSuccess(form_manager_); + filter_.ReportFormLoginSuccess(*form_manager_); EXPECT_EQ(0, tester.GetActionCount(kFilledAndLoginActionName)); } @@ -308,7 +309,7 @@ base::UserActionTester tester; SavePending(LoginState::EXISTING); - filter_.ReportFormLoginSuccess(form_manager_); + filter_.ReportFormLoginSuccess(*form_manager_); EXPECT_EQ(0, tester.GetActionCount(kFilledAndLoginActionName)); } @@ -319,7 +320,7 @@ base::UserActionTester tester; SavePending(LoginState::EXISTING); - filter_.ReportFormLoginSuccess(form_manager_); + filter_.ReportFormLoginSuccess(*form_manager_); EXPECT_EQ(0, tester.GetActionCount(kFilledAndLoginActionName)); } @@ -329,7 +330,7 @@ base::UserActionTester tester; SavePending(LoginState::EXISTING); - filter_.ReportFormLoginSuccess(form_manager_); + filter_.ReportFormLoginSuccess(*form_manager_); EXPECT_EQ(0, tester.GetActionCount(kFilledAndLoginActionName)); }
diff --git a/content/browser/browser_context.cc b/content/browser/browser_context.cc index 0f23bbec..a7db3b40 100644 --- a/content/browser/browser_context.cc +++ b/content/browser/browser_context.cc
@@ -574,8 +574,4 @@ return salt; } -BrowsingDataRemoverDelegate* BrowserContext::GetBrowsingDataRemoverDelegate() { - return nullptr; -} - } // namespace content
diff --git a/content/browser/download/download_manager_impl_unittest.cc b/content/browser/download/download_manager_impl_unittest.cc index c43e4df..b7eb3d24 100644 --- a/content/browser/download/download_manager_impl_unittest.cc +++ b/content/browser/download/download_manager_impl_unittest.cc
@@ -320,6 +320,7 @@ MOCK_METHOD0(GetSSLHostStateDelegate, SSLHostStateDelegate*()); MOCK_METHOD0(GetPermissionManager, PermissionManager*()); MOCK_METHOD0(GetBackgroundSyncController, BackgroundSyncController*()); + MOCK_METHOD0(GetBrowsingDataRemoverDelegate, BrowsingDataRemoverDelegate*()); MOCK_METHOD0(CreateMediaRequestContext, net::URLRequestContextGetter*()); MOCK_METHOD2(CreateMediaRequestContextForStoragePartition,
diff --git a/content/public/browser/browser_context.h b/content/public/browser/browser_context.h index 7a895f2..6108d81 100644 --- a/content/public/browser/browser_context.h +++ b/content/public/browser/browser_context.h
@@ -240,7 +240,7 @@ // Returns the BrowsingDataRemoverDelegate for this context. This will be // called once per context. It's valid to return nullptr. - virtual BrowsingDataRemoverDelegate* GetBrowsingDataRemoverDelegate(); + virtual BrowsingDataRemoverDelegate* GetBrowsingDataRemoverDelegate() = 0; // Creates the main net::URLRequestContextGetter. It's called only once. virtual net::URLRequestContextGetter* CreateRequestContext(
diff --git a/content/public/test/test_browser_context.cc b/content/public/test/test_browser_context.cc index ddcefea..cce6843c 100644 --- a/content/public/test/test_browser_context.cc +++ b/content/public/test/test_browser_context.cc
@@ -130,6 +130,13 @@ return background_sync_controller_.get(); } +BrowsingDataRemoverDelegate* +TestBrowserContext::GetBrowsingDataRemoverDelegate() { + // Most BrowsingDataRemover tests do not require a delegate + // (not even a mock one). + return nullptr; +} + net::URLRequestContextGetter* TestBrowserContext::CreateRequestContext( content::ProtocolHandlerMap* protocol_handlers, content::URLRequestInterceptorScopedVector request_interceptors) {
diff --git a/content/public/test/test_browser_context.h b/content/public/test/test_browser_context.h index 1f878d3b..7a6ea77 100644 --- a/content/public/test/test_browser_context.h +++ b/content/public/test/test_browser_context.h
@@ -57,6 +57,7 @@ SSLHostStateDelegate* GetSSLHostStateDelegate() override; PermissionManager* GetPermissionManager() override; BackgroundSyncController* GetBackgroundSyncController() override; + BrowsingDataRemoverDelegate* GetBrowsingDataRemoverDelegate() override; net::URLRequestContextGetter* CreateRequestContext( ProtocolHandlerMap* protocol_handlers, URLRequestInterceptorScopedVector request_interceptors) override;
diff --git a/content/shell/browser/shell_browser_context.cc b/content/shell/browser/shell_browser_context.cc index c0d11128..85c708f6 100644 --- a/content/shell/browser/shell_browser_context.cc +++ b/content/shell/browser/shell_browser_context.cc
@@ -223,4 +223,9 @@ return background_sync_controller_.get(); } +BrowsingDataRemoverDelegate* +ShellBrowserContext::GetBrowsingDataRemoverDelegate() { + return nullptr; +} + } // namespace content
diff --git a/content/shell/browser/shell_browser_context.h b/content/shell/browser/shell_browser_context.h index 1e8b69b..660c602 100644 --- a/content/shell/browser/shell_browser_context.h +++ b/content/shell/browser/shell_browser_context.h
@@ -56,6 +56,7 @@ SSLHostStateDelegate* GetSSLHostStateDelegate() override; PermissionManager* GetPermissionManager() override; BackgroundSyncController* GetBackgroundSyncController() override; + BrowsingDataRemoverDelegate* GetBrowsingDataRemoverDelegate() override; net::URLRequestContextGetter* CreateRequestContext( ProtocolHandlerMap* protocol_handlers, URLRequestInterceptorScopedVector request_interceptors) override;
diff --git a/device/bluetooth/bluetooth_low_energy_device_mac.mm b/device/bluetooth/bluetooth_low_energy_device_mac.mm index 8e730550..e80b9c5 100644 --- a/device/bluetooth/bluetooth_low_energy_device_mac.mm +++ b/device/bluetooth/bluetooth_low_energy_device_mac.mm
@@ -20,8 +20,7 @@ #include "device/bluetooth/bluetooth_remote_gatt_descriptor_mac.h" #include "device/bluetooth/bluetooth_remote_gatt_service_mac.h" -using device::BluetoothDevice; -using device::BluetoothLowEnergyDeviceMac; +namespace device { BluetoothLowEnergyDeviceMac::BluetoothLowEnergyDeviceMac( BluetoothAdapterMac* adapter, @@ -184,7 +183,7 @@ } void BluetoothLowEnergyDeviceMac::ConnectToServiceInsecurely( - const device::BluetoothUUID& uuid, + const BluetoothUUID& uuid, const ConnectToServiceCallback& callback, const ConnectToServiceErrorCallback& error_callback) { NOTIMPLEMENTED(); @@ -245,8 +244,8 @@ } if (discovery_pending_count_ == 0) { for (auto it = gatt_services_.begin(); it != gatt_services_.end(); ++it) { - device::BluetoothRemoteGattService* gatt_service = it->second.get(); - device::BluetoothRemoteGattServiceMac* gatt_service_mac = + BluetoothRemoteGattService* gatt_service = it->second.get(); + BluetoothRemoteGattServiceMac* gatt_service_mac = static_cast<BluetoothRemoteGattServiceMac*>(gatt_service); gatt_service_mac->DiscoverCharacteristics(); } @@ -442,7 +441,7 @@ } } -device::BluetoothAdapterMac* BluetoothLowEnergyDeviceMac::GetMacAdapter() { +BluetoothAdapterMac* BluetoothLowEnergyDeviceMac::GetMacAdapter() { return static_cast<BluetoothAdapterMac*>(this->adapter_); } @@ -450,12 +449,12 @@ return peripheral_; } -device::BluetoothRemoteGattServiceMac* +BluetoothRemoteGattServiceMac* BluetoothLowEnergyDeviceMac::GetBluetoothRemoteGattServiceMac( CBService* cb_service) const { for (auto it = gatt_services_.begin(); it != gatt_services_.end(); ++it) { - device::BluetoothRemoteGattService* gatt_service = it->second.get(); - device::BluetoothRemoteGattServiceMac* gatt_service_mac = + BluetoothRemoteGattService* gatt_service = it->second.get(); + BluetoothRemoteGattServiceMac* gatt_service_mac = static_cast<BluetoothRemoteGattServiceMac*>(gatt_service); if (gatt_service_mac->GetService() == cb_service) return gatt_service_mac; @@ -463,14 +462,14 @@ return nullptr; } -device::BluetoothRemoteGattDescriptorMac* +BluetoothRemoteGattDescriptorMac* BluetoothLowEnergyDeviceMac::GetBluetoothRemoteGattDescriptorMac( CBDescriptor* cb_descriptor) const { CBCharacteristic* cb_characteristic = cb_descriptor.characteristic; - device::BluetoothRemoteGattServiceMac* gatt_service = + BluetoothRemoteGattServiceMac* gatt_service = GetBluetoothRemoteGattServiceMac(cb_characteristic.service); DCHECK(gatt_service); - device::BluetoothRemoteGattCharacteristicMac* gatt_characteristic = + BluetoothRemoteGattCharacteristicMac* gatt_characteristic = gatt_service->GetBluetoothRemoteGattCharacteristicMac(cb_characteristic); DCHECK(gatt_characteristic); return gatt_characteristic->GetBluetoothRemoteGattDescriptorMac( @@ -505,8 +504,6 @@ DidFailToConnectGatt(BluetoothDevice::ConnectErrorCode::ERROR_FAILED); } -namespace device { - std::ostream& operator<<(std::ostream& out, const BluetoothLowEnergyDeviceMac& device) { // TODO(crbug.com/703878): Should use @@ -518,4 +515,5 @@ << &device << ", " << is_gatt_connected << ", \"" << name.value_or("Unnamed device") << "\">"; } + } // namespace device
diff --git a/extensions/browser/api/mime_handler_private/mime_handler_private.cc b/extensions/browser/api/mime_handler_private/mime_handler_private.cc index dd5219f..a104f250 100644 --- a/extensions/browser/api/mime_handler_private/mime_handler_private.cc +++ b/extensions/browser/api/mime_handler_private/mime_handler_private.cc
@@ -65,27 +65,27 @@ std::move(request)); } -void MimeHandlerServiceImpl::GetStreamInfo( - const GetStreamInfoCallback& callback) { +void MimeHandlerServiceImpl::GetStreamInfo(GetStreamInfoCallback callback) { if (!stream_) { - callback.Run(mime_handler::StreamInfoPtr()); + std::move(callback).Run(mime_handler::StreamInfoPtr()); return; } - callback.Run(mojo::ConvertTo<mime_handler::StreamInfoPtr>(*stream_)); + std::move(callback).Run( + mojo::ConvertTo<mime_handler::StreamInfoPtr>(*stream_)); } -void MimeHandlerServiceImpl::AbortStream(const AbortStreamCallback& callback) { +void MimeHandlerServiceImpl::AbortStream(AbortStreamCallback callback) { if (!stream_) { - callback.Run(); + std::move(callback).Run(); return; } stream_->Abort(base::Bind(&MimeHandlerServiceImpl::OnStreamClosed, - weak_factory_.GetWeakPtr(), callback)); + weak_factory_.GetWeakPtr(), + base::Passed(&callback))); } -void MimeHandlerServiceImpl::OnStreamClosed( - const AbortStreamCallback& callback) { - callback.Run(); +void MimeHandlerServiceImpl::OnStreamClosed(AbortStreamCallback callback) { + std::move(callback).Run(); } } // namespace extensions
diff --git a/extensions/browser/api/mime_handler_private/mime_handler_private.h b/extensions/browser/api/mime_handler_private/mime_handler_private.h index 3e202a2..e2340b8 100644 --- a/extensions/browser/api/mime_handler_private/mime_handler_private.h +++ b/extensions/browser/api/mime_handler_private/mime_handler_private.h
@@ -28,11 +28,11 @@ friend class MimeHandlerServiceImplTest; // mime_handler::MimeHandlerService overrides. - void GetStreamInfo(const GetStreamInfoCallback& callback) override; - void AbortStream(const AbortStreamCallback& callback) override; + void GetStreamInfo(GetStreamInfoCallback callback) override; + void AbortStream(AbortStreamCallback callback) override; // Invoked by the callback used to abort |stream_|. - void OnStreamClosed(const AbortStreamCallback& callback); + void OnStreamClosed(AbortStreamCallback callback); // A handle to the stream being handled by the MimeHandlerViewGuest. base::WeakPtr<StreamContainer> stream_;
diff --git a/extensions/common/BUILD.gn b/extensions/common/BUILD.gn index a5e96fe8..b2d03da 100644 --- a/extensions/common/BUILD.gn +++ b/extensions/common/BUILD.gn
@@ -34,9 +34,6 @@ "//url/mojo:url_mojom_gurl", ] - # TODO(crbug.com/714018): Convert the implementation to use OnceCallback. - use_once_callback = false - # TODO(crbug.com/699569): Convert to use the new JS bindings. use_new_js_bindings = false }
diff --git a/extensions/common/api/BUILD.gn b/extensions/common/api/BUILD.gn index 3321a45..c71bee80 100644 --- a/extensions/common/api/BUILD.gn +++ b/extensions/common/api/BUILD.gn
@@ -30,9 +30,6 @@ "mime_handler.mojom", ] - # TODO(crbug.com/714018): Convert the implementation to use OnceCallback. - use_once_callback = false - # TODO(crbug.com/699569): Convert to use the new JS bindings. use_new_js_bindings = false }
diff --git a/extensions/utility/utility_handler.cc b/extensions/utility/utility_handler.cc index 1f2e6e1..ff3299d 100644 --- a/extensions/utility/utility_handler.cc +++ b/extensions/utility/utility_handler.cc
@@ -42,12 +42,13 @@ // extensions::mojom::ExtensionUnpacker: void Unzip(const base::FilePath& file, const base::FilePath& path, - const UnzipCallback& callback) override { + UnzipCallback callback) override { std::unique_ptr<base::DictionaryValue> manifest; if (UnzipFileManifestIntoPath(file, path, &manifest)) { - callback.Run(UnzipFileIntoPath(file, path, std::move(manifest))); + std::move(callback).Run( + UnzipFileIntoPath(file, path, std::move(manifest))); } else { - callback.Run(false); + std::move(callback).Run(false); } } @@ -55,7 +56,7 @@ const std::string& extension_id, Manifest::Location location, int32_t creation_flags, - const UnpackCallback& callback) override { + UnpackCallback callback) override { CHECK_GT(location, Manifest::INVALID_LOCATION); CHECK_LT(location, Manifest::NUM_LOCATIONS); DCHECK(ExtensionsClient::Get()); @@ -65,9 +66,9 @@ Unpacker unpacker(path.DirName(), path, extension_id, location, creation_flags); if (unpacker.Run()) { - callback.Run(base::string16(), unpacker.TakeParsedManifest()); + std::move(callback).Run(base::string16(), unpacker.TakeParsedManifest()); } else { - callback.Run(unpacker.error_message(), nullptr); + std::move(callback).Run(unpacker.error_message(), nullptr); } } @@ -113,13 +114,13 @@ } private: - void Parse(const std::string& xml, const ParseCallback& callback) override { + void Parse(const std::string& xml, ParseCallback callback) override { UpdateManifest manifest; if (manifest.Parse(xml)) { - callback.Run(manifest.results()); + std::move(callback).Run(manifest.results()); } else { LOG(WARNING) << "Error parsing update manifest:\n" << manifest.errors(); - callback.Run(base::nullopt); + std::move(callback).Run(base::nullopt); } }
diff --git a/headless/lib/browser/headless_browser_context_impl.cc b/headless/lib/browser/headless_browser_context_impl.cc index d680daa..3da6febd 100644 --- a/headless/lib/browser/headless_browser_context_impl.cc +++ b/headless/lib/browser/headless_browser_context_impl.cc
@@ -237,6 +237,11 @@ return nullptr; } +content::BrowsingDataRemoverDelegate* +HeadlessBrowserContextImpl::GetBrowsingDataRemoverDelegate() { + return nullptr; +} + net::URLRequestContextGetter* HeadlessBrowserContextImpl::CreateRequestContext( content::ProtocolHandlerMap* protocol_handlers, content::URLRequestInterceptorScopedVector request_interceptors) {
diff --git a/headless/lib/browser/headless_browser_context_impl.h b/headless/lib/browser/headless_browser_context_impl.h index e6a77f1..b06766a4 100644 --- a/headless/lib/browser/headless_browser_context_impl.h +++ b/headless/lib/browser/headless_browser_context_impl.h
@@ -64,6 +64,8 @@ content::SSLHostStateDelegate* GetSSLHostStateDelegate() override; content::PermissionManager* GetPermissionManager() override; content::BackgroundSyncController* GetBackgroundSyncController() override; + content::BrowsingDataRemoverDelegate* GetBrowsingDataRemoverDelegate() + override; net::URLRequestContextGetter* CreateRequestContext( content::ProtocolHandlerMap* protocol_handlers, content::URLRequestInterceptorScopedVector request_interceptors) override;
diff --git a/ios/chrome/browser/passwords/ios_chrome_password_manager_client.h b/ios/chrome/browser/passwords/ios_chrome_password_manager_client.h index b0cbd424..2e5aa6c4 100644 --- a/ios/chrome/browser/passwords/ios_chrome_password_manager_client.h +++ b/ios/chrome/browser/passwords/ios_chrome_password_manager_client.h
@@ -22,11 +22,11 @@ // Shows UI to prompt the user to save the password. - (void)showSavePasswordInfoBar: - (std::unique_ptr<password_manager::PasswordFormManager>)formToSave; + (scoped_refptr<password_manager::PasswordFormManager>)formToSave; // Shows UI to prompt the user to update the password. - (void)showUpdatePasswordInfoBar: - (std::unique_ptr<password_manager::PasswordFormManager>)formToUpdate; + (scoped_refptr<password_manager::PasswordFormManager>)formToUpdate; @property(readonly, nonatomic) ios::ChromeBrowserState* browserState; @@ -46,14 +46,14 @@ // password_manager::PasswordManagerClient implementation. password_manager::PasswordSyncState GetPasswordSyncState() const override; bool PromptUserToSaveOrUpdatePassword( - std::unique_ptr<password_manager::PasswordFormManager> form_to_save, + scoped_refptr<password_manager::PasswordFormManager> form_to_save, bool update_password) override; bool PromptUserToChooseCredentials( std::vector<std::unique_ptr<autofill::PasswordForm>> local_forms, const GURL& origin, const CredentialsCallback& callback) override; void AutomaticPasswordSave( - std::unique_ptr<password_manager::PasswordFormManager> saved_form_manager) + scoped_refptr<password_manager::PasswordFormManager> saved_form_manager) override; bool IsIncognito() const override; PrefService* GetPrefs() override;
diff --git a/ios/chrome/browser/passwords/ios_chrome_password_manager_client.mm b/ios/chrome/browser/passwords/ios_chrome_password_manager_client.mm index 6c7c82a..19f20bc 100644 --- a/ios/chrome/browser/passwords/ios_chrome_password_manager_client.mm +++ b/ios/chrome/browser/passwords/ios_chrome_password_manager_client.mm
@@ -69,7 +69,7 @@ } bool IOSChromePasswordManagerClient::PromptUserToSaveOrUpdatePassword( - std::unique_ptr<PasswordFormManager> form_to_save, + scoped_refptr<PasswordFormManager> form_to_save, bool update_password) { if (form_to_save->IsBlacklisted()) return false; @@ -84,7 +84,7 @@ } void IOSChromePasswordManagerClient::AutomaticPasswordSave( - std::unique_ptr<PasswordFormManager> saved_form_manager) { + scoped_refptr<PasswordFormManager> saved_form_manager) { NOTIMPLEMENTED(); }
diff --git a/ios/chrome/browser/passwords/ios_chrome_password_manager_infobar_delegate.h b/ios/chrome/browser/passwords/ios_chrome_password_manager_infobar_delegate.h index afc444a..d85fd60 100644 --- a/ios/chrome/browser/passwords/ios_chrome_password_manager_infobar_delegate.h +++ b/ios/chrome/browser/passwords/ios_chrome_password_manager_infobar_delegate.h
@@ -8,6 +8,7 @@ #include <memory> #include "base/macros.h" +#include "base/memory/ref_counted.h" #include "components/infobars/core/confirm_infobar_delegate.h" #include "components/password_manager/core/browser/password_manager_metrics_util.h" @@ -26,7 +27,7 @@ protected: IOSChromePasswordManagerInfoBarDelegate( bool is_smart_lock_branding_enabled, - std::unique_ptr<password_manager::PasswordFormManager> form_manager); + scoped_refptr<password_manager::PasswordFormManager> form_manager); password_manager::PasswordFormManager* form_to_save() const { return form_to_save_.get(); @@ -50,7 +51,7 @@ // The password_manager::PasswordFormManager managing the form we're asking // the user about, and should save as per their decision. - std::unique_ptr<password_manager::PasswordFormManager> form_to_save_; + scoped_refptr<password_manager::PasswordFormManager> form_to_save_; // Used to track the results we get from the info bar. password_manager::metrics_util::UIDismissalReason infobar_response_;
diff --git a/ios/chrome/browser/passwords/ios_chrome_password_manager_infobar_delegate.mm b/ios/chrome/browser/passwords/ios_chrome_password_manager_infobar_delegate.mm index db7cea28..b82afeb 100644 --- a/ios/chrome/browser/passwords/ios_chrome_password_manager_infobar_delegate.mm +++ b/ios/chrome/browser/passwords/ios_chrome_password_manager_infobar_delegate.mm
@@ -27,7 +27,7 @@ IOSChromePasswordManagerInfoBarDelegate:: IOSChromePasswordManagerInfoBarDelegate( bool is_smart_lock_branding_enabled, - std::unique_ptr<password_manager::PasswordFormManager> form_to_save) + scoped_refptr<password_manager::PasswordFormManager> form_to_save) : form_to_save_(std::move(form_to_save)), infobar_response_(password_manager::metrics_util::NO_DIRECT_INTERACTION), is_smart_lock_branding_enabled_(is_smart_lock_branding_enabled) {}
diff --git a/ios/chrome/browser/passwords/ios_chrome_save_password_infobar_delegate.h b/ios/chrome/browser/passwords/ios_chrome_save_password_infobar_delegate.h index 3d931742..47bfb4dc 100644 --- a/ios/chrome/browser/passwords/ios_chrome_save_password_infobar_delegate.h +++ b/ios/chrome/browser/passwords/ios_chrome_save_password_infobar_delegate.h
@@ -30,14 +30,14 @@ static void Create( bool is_smart_lock_branding_enabled, infobars::InfoBarManager* infobar_manager, - std::unique_ptr<password_manager::PasswordFormManager> form_to_save); + scoped_refptr<password_manager::PasswordFormManager> form_to_save); ~IOSChromeSavePasswordInfoBarDelegate() override; private: IOSChromeSavePasswordInfoBarDelegate( bool is_smart_lock_branding_enabled, - std::unique_ptr<password_manager::PasswordFormManager> form_to_save); + scoped_refptr<password_manager::PasswordFormManager> form_to_save); // ConfirmInfoBarDelegate implementation. infobars::InfoBarDelegate::InfoBarIdentifier GetIdentifier() const override;
diff --git a/ios/chrome/browser/passwords/ios_chrome_save_password_infobar_delegate.mm b/ios/chrome/browser/passwords/ios_chrome_save_password_infobar_delegate.mm index 4d0cec2e..7e444c5f3 100644 --- a/ios/chrome/browser/passwords/ios_chrome_save_password_infobar_delegate.mm +++ b/ios/chrome/browser/passwords/ios_chrome_save_password_infobar_delegate.mm
@@ -22,7 +22,7 @@ void IOSChromeSavePasswordInfoBarDelegate::Create( bool is_smart_lock_branding_enabled, infobars::InfoBarManager* infobar_manager, - std::unique_ptr<PasswordFormManager> form_to_save) { + scoped_refptr<PasswordFormManager> form_to_save) { DCHECK(infobar_manager); auto delegate = base::WrapUnique(new IOSChromeSavePasswordInfoBarDelegate( is_smart_lock_branding_enabled, std::move(form_to_save))); @@ -34,7 +34,7 @@ IOSChromeSavePasswordInfoBarDelegate::IOSChromeSavePasswordInfoBarDelegate( bool is_smart_lock_branding_enabled, - std::unique_ptr<PasswordFormManager> form_to_save) + scoped_refptr<PasswordFormManager> form_to_save) : IOSChromePasswordManagerInfoBarDelegate(is_smart_lock_branding_enabled, std::move(form_to_save)) {}
diff --git a/ios/chrome/browser/passwords/ios_chrome_update_password_infobar_delegate.h b/ios/chrome/browser/passwords/ios_chrome_update_password_infobar_delegate.h index d720ba2..cdd99bf5 100644 --- a/ios/chrome/browser/passwords/ios_chrome_update_password_infobar_delegate.h +++ b/ios/chrome/browser/passwords/ios_chrome_update_password_infobar_delegate.h
@@ -27,7 +27,7 @@ static void Create( bool is_smart_lock_branding_enabled, infobars::InfoBarManager* infobar_manager, - std::unique_ptr<password_manager::PasswordFormManager> form_to_save); + scoped_refptr<password_manager::PasswordFormManager> form_to_save); ~IOSChromeUpdatePasswordInfoBarDelegate() override; @@ -49,7 +49,7 @@ private: IOSChromeUpdatePasswordInfoBarDelegate( bool is_smart_lock_branding_enabled, - std::unique_ptr<password_manager::PasswordFormManager> form_to_save); + scoped_refptr<password_manager::PasswordFormManager> form_to_save); // Returns the string with the branded title of the password manager (e.g. // "Google Smart Lock for Passwords").
diff --git a/ios/chrome/browser/passwords/ios_chrome_update_password_infobar_delegate.mm b/ios/chrome/browser/passwords/ios_chrome_update_password_infobar_delegate.mm index 19afa48..0929fcf 100644 --- a/ios/chrome/browser/passwords/ios_chrome_update_password_infobar_delegate.mm +++ b/ios/chrome/browser/passwords/ios_chrome_update_password_infobar_delegate.mm
@@ -27,7 +27,7 @@ void IOSChromeUpdatePasswordInfoBarDelegate::Create( bool is_smart_lock_branding_enabled, infobars::InfoBarManager* infobar_manager, - std::unique_ptr<PasswordFormManager> form_manager) { + scoped_refptr<PasswordFormManager> form_manager) { DCHECK(infobar_manager); auto delegate = base::WrapUnique(new IOSChromeUpdatePasswordInfoBarDelegate( is_smart_lock_branding_enabled, std::move(form_manager))); @@ -43,7 +43,7 @@ IOSChromeUpdatePasswordInfoBarDelegate::IOSChromeUpdatePasswordInfoBarDelegate( bool is_smart_lock_branding_enabled, - std::unique_ptr<PasswordFormManager> form_manager) + scoped_refptr<PasswordFormManager> form_manager) : IOSChromePasswordManagerInfoBarDelegate(is_smart_lock_branding_enabled, std::move(form_manager)) { selected_account_ = form_to_save()->preferred_match()->username_value;
diff --git a/ios/chrome/browser/passwords/password_controller.mm b/ios/chrome/browser/passwords/password_controller.mm index 2545cb3..608285e5 100644 --- a/ios/chrome/browser/passwords/password_controller.mm +++ b/ios/chrome/browser/passwords/password_controller.mm
@@ -18,6 +18,7 @@ #include "base/mac/foundation_util.h" #include "base/mac/scoped_nsobject.h" #include "base/memory/ptr_util.h" +#include "base/memory/ref_counted.h" #include "base/strings/string16.h" #include "base/strings/sys_string_conversions.h" #include "base/strings/utf_string_conversions.h" @@ -118,7 +119,7 @@ // Displays infobar for |form| with |type|. If |type| is UPDATE, the user // is prompted to update the password. If |type| is SAVE, the user is prompted // to save the password. -- (void)showInfoBarForForm:(std::unique_ptr<PasswordFormManager>)form +- (void)showInfoBarForForm:(scoped_refptr<PasswordFormManager>)form infoBarType:(PasswordInfoBarType)type; @end @@ -642,14 +643,13 @@ #pragma mark - PasswordManagerClientDelegate -- (void)showSavePasswordInfoBar: - (std::unique_ptr<PasswordFormManager>)formToSave { +- (void)showSavePasswordInfoBar:(scoped_refptr<PasswordFormManager>)formToSave { [self showInfoBarForForm:std::move(formToSave) infoBarType:PasswordInfoBarType::SAVE]; } - (void)showUpdatePasswordInfoBar: - (std::unique_ptr<PasswordFormManager>)formToUpdate { + (scoped_refptr<PasswordFormManager>)formToUpdate { [self showInfoBarForForm:std::move(formToUpdate) infoBarType:PasswordInfoBarType::UPDATE]; } @@ -852,7 +852,7 @@ #pragma mark - Private methods -- (void)showInfoBarForForm:(std::unique_ptr<PasswordFormManager>)form +- (void)showInfoBarForForm:(scoped_refptr<PasswordFormManager>)form infoBarType:(PasswordInfoBarType)type { if (!webStateObserverBridge_ || !webStateObserverBridge_->web_state()) return;
diff --git a/ios/chrome/browser/snapshots/BUILD.gn b/ios/chrome/browser/snapshots/BUILD.gn index a1db577d2..062cc82d 100644 --- a/ios/chrome/browser/snapshots/BUILD.gn +++ b/ios/chrome/browser/snapshots/BUILD.gn
@@ -9,6 +9,8 @@ "snapshot_cache.h", "snapshot_cache.mm", "snapshot_cache_internal.h", + "snapshot_cache_web_state_list_observer.h", + "snapshot_cache_web_state_list_observer.mm", "snapshot_manager.h", "snapshot_manager.mm", "snapshot_overlay.h", @@ -19,26 +21,12 @@ deps = [ "//base", "//ios/chrome/browser", + "//ios/chrome/browser/tabs", "//ios/chrome/browser/ui", + "//ios/chrome/browser/web_state_list", "//ios/web", ] - public_deps = [ - ":snapshots_arc", - ] - allow_circular_includes_from = [ ":snapshots_arc" ] libs = [ "QuartzCore.framework" ] -} - -source_set("snapshots_arc") { - sources = [ - "snapshot_cache_web_state_list_observer.h", - "snapshot_cache_web_state_list_observer.mm", - ] - deps = [ - "//base", - "//ios/chrome/browser/tabs", - "//ios/chrome/browser/web_state_list", - ] configs += [ "//build/config/compiler:enable_arc" ] }
diff --git a/ios/chrome/browser/snapshots/lru_cache.mm b/ios/chrome/browser/snapshots/lru_cache.mm index ffbf1dd..5116cfe1 100644 --- a/ios/chrome/browser/snapshots/lru_cache.mm +++ b/ios/chrome/browser/snapshots/lru_cache.mm
@@ -7,52 +7,38 @@ #include <stddef.h> #include <memory> +#include <unordered_map> -#include "base/containers/hash_tables.h" #include "base/containers/mru_cache.h" -#include "base/mac/scoped_nsobject.h" #include "base/macros.h" +#include "base/memory/ptr_util.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif namespace { struct NSObjectEqualTo { - bool operator()(const base::scoped_nsprotocol<id<NSObject>>& obj1, - const base::scoped_nsprotocol<id<NSObject>>& obj2) const { + bool operator()(id<NSObject> obj1, id<NSObject> obj2) const { return [obj1 isEqual:obj2]; } }; struct NSObjectHash { - std::size_t operator()( - const base::scoped_nsprotocol<id<NSObject>>& obj) const { - return [obj hash]; - } + std::size_t operator()(id<NSObject> obj) const { return [obj hash]; } }; template <class KeyType, class ValueType, class HashType> struct MRUCacheNSObjectHashMap { - typedef base::hash_map<KeyType, ValueType, HashType, NSObjectEqualTo> Type; + using Type = + std::unordered_map<KeyType, ValueType, HashType, NSObjectEqualTo>; }; -class NSObjectMRUCache - : public base::MRUCacheBase<base::scoped_nsprotocol<id<NSObject>>, - base::scoped_nsprotocol<id<NSObject>>, - NSObjectHash, - MRUCacheNSObjectHashMap> { - private: - typedef base::MRUCacheBase<base::scoped_nsprotocol<id<NSObject>>, - base::scoped_nsprotocol<id<NSObject>>, - NSObjectHash, - MRUCacheNSObjectHashMap> - ParentType; - - public: - explicit NSObjectMRUCache(typename ParentType::size_type max_size) - : ParentType(max_size) {} - - private: - DISALLOW_COPY_AND_ASSIGN(NSObjectMRUCache); -}; +using NSObjectMRUCache = base::MRUCacheBase<id<NSObject>, + id<NSObject>, + NSObjectHash, + MRUCacheNSObjectHashMap>; } // namespace @@ -60,39 +46,30 @@ std::unique_ptr<NSObjectMRUCache> _cache; } -@synthesize maxCacheSize = _maxCacheSize; - -- (instancetype)init { - NOTREACHED(); // Use initWithCacheSize: instead. - return nil; -} - - (instancetype)initWithCacheSize:(NSUInteger)maxCacheSize { - self = [super init]; - if (self) { - _maxCacheSize = maxCacheSize; - _cache.reset(new NSObjectMRUCache(self.maxCacheSize)); + if ((self = [super init])) { + _cache = base::MakeUnique<NSObjectMRUCache>(maxCacheSize); } return self; } +- (NSUInteger)maxCacheSize { + return _cache->max_size(); +} + - (id)objectForKey:(id<NSObject>)key { - base::scoped_nsprotocol<id<NSObject>> keyObj([key retain]); - auto it = _cache->Get(keyObj); + auto it = _cache->Get(key); if (it == _cache->end()) return nil; - return it->second.get(); + return it->second; } - (void)setObject:(id<NSObject>)value forKey:(NSObject*)key { - base::scoped_nsprotocol<id<NSObject>> keyObj([key copy]); - base::scoped_nsprotocol<id<NSObject>> valueObj([value retain]); - _cache->Put(keyObj, valueObj); + _cache->Put([key copy], value); } - (void)removeObjectForKey:(id<NSObject>)key { - base::scoped_nsprotocol<id<NSObject>> keyObj([key retain]); - auto it = _cache->Peek(keyObj); + auto it = _cache->Peek(key); if (it != _cache->end()) _cache->Erase(it); }
diff --git a/ios/chrome/browser/snapshots/snapshot_cache.h b/ios/chrome/browser/snapshots/snapshot_cache.h index d75fd9e..afb74dd9 100644 --- a/ios/chrome/browser/snapshots/snapshot_cache.h +++ b/ios/chrome/browser/snapshots/snapshot_cache.h
@@ -13,14 +13,14 @@ // A singleton providing an in-memory and on-disk cache of tab snapshots. // A snapshot is a full-screen image of the contents of the page at the current -// scroll offset and zoom level, used to stand in for the UIWebView if it has +// scroll offset and zoom level, used to stand in for the WKWebView if it has // been purged from memory or when quickly switching tabs. // Persists to disk on a background thread each time a snapshot changes. @interface SnapshotCache : NSObject // Track session IDs to not release on low memory and to reload on // |UIApplicationDidBecomeActiveNotification|. -@property(nonatomic, retain) NSSet* pinnedIDs; +@property(nonatomic, strong) NSSet* pinnedIDs; + (SnapshotCache*)sharedInstance;
diff --git a/ios/chrome/browser/snapshots/snapshot_cache.mm b/ios/chrome/browser/snapshots/snapshot_cache.mm index ab03a7f..34be527 100644 --- a/ios/chrome/browser/snapshots/snapshot_cache.mm +++ b/ios/chrome/browser/snapshots/snapshot_cache.mm
@@ -13,8 +13,6 @@ #include "base/location.h" #include "base/logging.h" #include "base/mac/bind_objc_block.h" -#include "base/mac/objc_property_releaser.h" -#include "base/mac/scoped_cftyperef.h" #include "base/mac/scoped_nsobject.h" #include "base/strings/sys_string_conversions.h" #include "base/task_runner_util.h" @@ -92,8 +90,7 @@ scale:[SnapshotCache snapshotScaleForDevice]]; } -void WriteImageToDisk(const base::scoped_nsobject<UIImage>& image, - const base::FilePath& filePath) { +void WriteImageToDisk(UIImage* image, const base::FilePath& filePath) { base::ThreadRestrictions::AssertIOAllowed(); if (!image) return; @@ -115,18 +112,16 @@ } } -void ConvertAndSaveGreyImage( - const base::FilePath& colorPath, - const base::FilePath& greyPath, - const base::scoped_nsobject<UIImage>& cachedImage) { +void ConvertAndSaveGreyImage(const base::FilePath& color_image_path, + const base::FilePath& grey_image_path, + UIImage* color_image) { base::ThreadRestrictions::AssertIOAllowed(); - base::scoped_nsobject<UIImage> colorImage = cachedImage; - if (!colorImage) - colorImage.reset([ReadImageFromDisk(colorPath) retain]); - if (!colorImage) + if (!color_image) + color_image = ReadImageFromDisk(color_image_path); + if (!color_image) return; - base::scoped_nsobject<UIImage> greyImage([GreyImage(colorImage) retain]); - WriteImageToDisk(greyImage, greyPath); + UIImage* grey_image = GreyImage(color_image); + WriteImageToDisk(grey_image, grey_image_path); } } // anonymous namespace @@ -134,25 +129,22 @@ @implementation SnapshotCache { // Cache to hold color snapshots in memory. n.b. Color snapshots are not // kept in memory on tablets. - base::scoped_nsobject<LRUCache> lruCache_; + LRUCache* lruCache_; // Temporary dictionary to hold grey snapshots for tablet side swipe. This // will be nil before -createGreyCache is called and after -removeGreyCache // is called. - base::scoped_nsobject<NSMutableDictionary> greyImageDictionary_; - NSSet* pinnedIDs_; + NSMutableDictionary<NSString*, UIImage*>* greyImageDictionary_; // Session ID of most recent pending grey snapshot request. - base::scoped_nsobject<NSString> mostRecentGreySessionId_; + NSString* mostRecentGreySessionId_; // Block used by pending request for a grey snapshot. - base::scoped_nsprotocol<GreyBlock> mostRecentGreyBlock_; + GreyBlock mostRecentGreyBlock_; - // Session ID and correspoinding UIImage for the snapshot that will likely + // Session ID and corresponding UIImage for the snapshot that will likely // be requested to be saved to disk when the application is backgrounded. - base::scoped_nsobject<NSString> backgroundingImageSessionId_; - base::scoped_nsobject<UIImage> backgroundingColorImage_; - - base::mac::ObjCPropertyReleaser propertyReleaser_SnapshotCache_; + NSString* backgroundingImageSessionId_; + UIImage* backgroundingColorImage_; } @synthesize pinnedIDs = pinnedIDs_; @@ -165,9 +157,7 @@ - (id)init { if ((self = [super init])) { DCHECK_CURRENTLY_ON(web::WebThread::UI); - propertyReleaser_SnapshotCache_.Init(self, [SnapshotCache class]); - - lruCache_.reset([[LRUCache alloc] initWithCacheSize:kLRUCacheMaxCapacity]); + lruCache_ = [[LRUCache alloc] initWithCacheSize:kLRUCacheMaxCapacity]; [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(handleLowMemory) @@ -200,7 +190,6 @@ removeObserver:self name:UIApplicationDidBecomeActiveNotification object:nil]; - [super dealloc]; } + (CGFloat)snapshotScaleForDevice { @@ -230,12 +219,12 @@ base::PostTaskAndReplyWithResult( web::WebThread::GetTaskRunnerForThread(web::WebThread::FILE_USER_BLOCKING) .get(), - FROM_HERE, base::BindBlock(^base::scoped_nsobject<UIImage>() { + FROM_HERE, base::BindBlockArc(^base::scoped_nsobject<UIImage>() { // Retrieve the image on a high priority thread. - return base::scoped_nsobject<UIImage>([ReadImageFromDisk( - [SnapshotCache imagePathForSessionID:sessionID]) retain]); + return base::scoped_nsobject<UIImage>( + ReadImageFromDisk([SnapshotCache imagePathForSessionID:sessionID])); }), - base::BindBlock(^(base::scoped_nsobject<UIImage> image) { + base::BindBlockArc(^(base::scoped_nsobject<UIImage> image) { if (image) [lruCache_ setObject:image forKey:sessionID]; if (callback) @@ -252,9 +241,8 @@ // Save the image to disk. web::WebThread::PostBlockingPoolSequencedTask( - kSequenceToken, FROM_HERE, base::BindBlock(^{ - base::scoped_nsobject<UIImage> scoped_image([image retain]); - WriteImageToDisk(scoped_image, + kSequenceToken, FROM_HERE, base::BindBlockArc(^{ + WriteImageToDisk(image, [SnapshotCache imagePathForSessionID:sessionID]); })); } @@ -264,8 +252,7 @@ [lruCache_ removeObjectForKey:sessionID]; web::WebThread::PostBlockingPoolSequencedTask( - kSequenceToken, FROM_HERE, - base::BindBlock(^{ + kSequenceToken, FROM_HERE, base::BindBlockArc(^{ base::FilePath imagePath = [SnapshotCache imagePathForSessionID:sessionID]; base::DeleteFile(imagePath, false); @@ -330,8 +317,7 @@ // Copying the date, as the block must copy the value, not the reference. const base::Time dateCopy = date; web::WebThread::PostBlockingPoolSequencedTask( - kSequenceToken, FROM_HERE, - base::BindBlock(^{ + kSequenceToken, FROM_HERE, base::BindBlockArc(^{ std::set<base::FilePath> filesToKeep; for (NSString* sessionID : liveSessionIds) { base::FilePath curImagePath = @@ -362,14 +348,14 @@ DCHECK_CURRENTLY_ON(web::WebThread::UI); if (!sessionID) return; - backgroundingImageSessionId_.reset([sessionID copy]); - backgroundingColorImage_.reset([[lruCache_ objectForKey:sessionID] retain]); + backgroundingImageSessionId_ = [sessionID copy]; + backgroundingColorImage_ = [lruCache_ objectForKey:sessionID]; } - (void)handleLowMemory { DCHECK_CURRENTLY_ON(web::WebThread::UI); - base::scoped_nsobject<NSMutableDictionary> dictionary( - [[NSMutableDictionary alloc] initWithCapacity:2]); + NSMutableDictionary<NSString*, UIImage*>* dictionary = + [NSMutableDictionary dictionaryWithCapacity:2]; for (NSString* sessionID in pinnedIDs_) { UIImage* image = [lruCache_ objectForKey:sessionID]; if (image) @@ -396,7 +382,7 @@ if (greyImage) [greyImageDictionary_ setObject:greyImage forKey:sessionID]; if ([sessionID isEqualToString:mostRecentGreySessionId_]) { - mostRecentGreyBlock_.get()(greyImage); + mostRecentGreyBlock_(greyImage); [self clearGreySessionInfo]; } } @@ -411,39 +397,40 @@ base::PostTaskAndReplyWithResult( web::WebThread::GetTaskRunnerForThread(web::WebThread::FILE_USER_BLOCKING) .get(), - FROM_HERE, base::BindBlock(^base::scoped_nsobject<UIImage>() { - base::scoped_nsobject<UIImage> result([image retain]); + FROM_HERE, base::BindBlockArc(^base::scoped_nsobject<UIImage>() { + base::scoped_nsobject<UIImage> result(image); // If the image is not in the cache, load it from disk. - if (!result) - result.reset([ReadImageFromDisk( - [SnapshotCache imagePathForSessionID:sessionID]) retain]); + if (!result) { + result.reset(ReadImageFromDisk( + [SnapshotCache imagePathForSessionID:sessionID])); + } if (result) - result.reset([GreyImage(result) retain]); + result.reset(GreyImage(result)); return result; }), - base::BindBlock(^(base::scoped_nsobject<UIImage> greyImage) { + base::BindBlockArc(^(base::scoped_nsobject<UIImage> greyImage) { [self saveGreyImage:greyImage forKey:sessionID]; })); } - (void)createGreyCache:(NSArray*)sessionIDs { DCHECK_CURRENTLY_ON(web::WebThread::UI); - greyImageDictionary_.reset( - [[NSMutableDictionary alloc] initWithCapacity:kGreyInitialCapacity]); + greyImageDictionary_ = + [NSMutableDictionary dictionaryWithCapacity:kGreyInitialCapacity]; for (NSString* sessionID in sessionIDs) [self loadGreyImageAsync:sessionID]; } - (void)removeGreyCache { DCHECK_CURRENTLY_ON(web::WebThread::UI); - greyImageDictionary_.reset(); + greyImageDictionary_ = nil; [self clearGreySessionInfo]; } - (void)clearGreySessionInfo { DCHECK_CURRENTLY_ON(web::WebThread::UI); - mostRecentGreySessionId_.reset(); - mostRecentGreyBlock_.reset(); + mostRecentGreySessionId_ = nil; + mostRecentGreyBlock_ = nil; } - (void)greyImageForSessionID:(NSString*)sessionID @@ -455,8 +442,8 @@ callback(image); [self clearGreySessionInfo]; } else { - mostRecentGreySessionId_.reset([sessionID copy]); - mostRecentGreyBlock_.reset([callback copy]); + mostRecentGreySessionId_ = [sessionID copy]; + mostRecentGreyBlock_ = [callback copy]; } } @@ -474,7 +461,7 @@ base::PostTaskAndReplyWithResult( web::WebThread::GetTaskRunnerForThread(web::WebThread::FILE_USER_BLOCKING) .get(), - FROM_HERE, base::BindBlock(^base::scoped_nsobject<UIImage>() { + FROM_HERE, base::BindBlockArc(^base::scoped_nsobject<UIImage>() { // Retrieve the image on a high priority thread. // Loading the file into NSData is more reliable. // -imageWithContentsOfFile would ocassionally claim the image was not a @@ -489,9 +476,9 @@ return base::scoped_nsobject<UIImage>(); DCHECK(callback); return base::scoped_nsobject<UIImage>( - [[UIImage imageWithData:imageData] retain]); + [UIImage imageWithData:imageData]); }), - base::BindBlock(^(base::scoped_nsobject<UIImage> image) { + base::BindBlockArc(^(base::scoped_nsobject<UIImage> image) { if (!image) { [self retrieveImageForSessionID:sessionID callback:^(UIImage* local_image) { @@ -517,14 +504,18 @@ // The color image may still be in memory. Verify the sessionID matches. if (backgroundingColorImage_) { if (![backgroundingImageSessionId_ isEqualToString:sessionID]) { - backgroundingColorImage_.reset(); - backgroundingImageSessionId_.reset(); + backgroundingImageSessionId_ = nil; + backgroundingColorImage_ = nil; } } - web::WebThread::PostBlockingPoolTask( - FROM_HERE, base::Bind(&ConvertAndSaveGreyImage, colorImagePath, - greyImagePath, backgroundingColorImage_)); + // Copy the UIImage* so that the block does not reference |self|. + UIImage* backgroundingColorImage = backgroundingColorImage_; + web::WebThread::PostBlockingPoolTask(FROM_HERE, base::BindBlockArc(^{ + ConvertAndSaveGreyImage( + colorImagePath, greyImagePath, + backgroundingColorImage); + })); } @end
diff --git a/ios/chrome/browser/snapshots/snapshot_manager.h b/ios/chrome/browser/snapshots/snapshot_manager.h index fed63a6..50cf315 100644 --- a/ios/chrome/browser/snapshots/snapshot_manager.h +++ b/ios/chrome/browser/snapshots/snapshot_manager.h
@@ -7,7 +7,6 @@ #import <UIKit/UIKit.h> - // Snapshot manager for contents of a tab. A snapshot is a full-screen image // of the contents of the page at the current scroll offset and zoom level, // used to stand in for the web view if it has been purged from memory or when @@ -42,7 +41,7 @@ callback:(void (^)(UIImage*))callback; // Stores the supplied thumbnail for the specified |sessionID|. -- (void)setImage:(UIImage*)img withSessionID:(NSString*)sessionID; +- (void)setImage:(UIImage*)image withSessionID:(NSString*)sessionID; // Removes the cached thumbnail for the specified |sessionID|. - (void)removeImageWithSessionID:(NSString*)sessionID;
diff --git a/ios/chrome/browser/snapshots/snapshot_manager.mm b/ios/chrome/browser/snapshots/snapshot_manager.mm index 71c61f6..84f684de 100644 --- a/ios/chrome/browser/snapshots/snapshot_manager.mm +++ b/ios/chrome/browser/snapshots/snapshot_manager.mm
@@ -11,6 +11,10 @@ #import "ios/chrome/browser/snapshots/snapshot_cache.h" #import "ios/chrome/browser/snapshots/snapshot_overlay.h" +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + namespace { // Returns YES if |view| or any view it contains is a WKWebView. BOOL ViewHierarchyContainsWKWebView(UIView* view) { @@ -96,8 +100,8 @@ callback:callback]; } -- (void)setImage:(UIImage*)img withSessionID:(NSString*)sessionID { - [[SnapshotCache sharedInstance] setImage:img withSessionID:sessionID]; +- (void)setImage:(UIImage*)image withSessionID:(NSString*)sessionID { + [[SnapshotCache sharedInstance] setImage:image withSessionID:sessionID]; } - (void)removeImageWithSessionID:(NSString*)sessionID {
diff --git a/ios/chrome/browser/snapshots/snapshot_overlay.h b/ios/chrome/browser/snapshots/snapshot_overlay.h index 2e23693..6253795e 100644 --- a/ios/chrome/browser/snapshots/snapshot_overlay.h +++ b/ios/chrome/browser/snapshots/snapshot_overlay.h
@@ -18,11 +18,11 @@ - (instancetype)init NS_UNAVAILABLE; // The overlay view. -@property(nonatomic, readonly) UIView* view; +@property(nonatomic, strong, readonly) UIView* view; // Y offset for the overlay view. Used to adjust the y position of |_view| // within the snapshot. -@property(nonatomic, readonly) CGFloat yOffset; +@property(nonatomic, assign, readonly) CGFloat yOffset; @end
diff --git a/ios/chrome/browser/snapshots/snapshot_overlay.mm b/ios/chrome/browser/snapshots/snapshot_overlay.mm index d41b8a6..b936f4e 100644 --- a/ios/chrome/browser/snapshots/snapshot_overlay.mm +++ b/ios/chrome/browser/snapshots/snapshot_overlay.mm
@@ -5,31 +5,24 @@ #import "ios/chrome/browser/snapshots/snapshot_overlay.h" #include "base/logging.h" -#import "base/mac/scoped_nsobject.h" -@implementation SnapshotOverlay { - base::scoped_nsobject<UIView> _view; -} +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif +@implementation SnapshotOverlay + +@synthesize view = _view; @synthesize yOffset = _yOffset; - (instancetype)initWithView:(UIView*)view yOffset:(CGFloat)yOffset { self = [super init]; if (self) { DCHECK(view); - _view.reset([view retain]); + _view = view; _yOffset = yOffset; } return self; } -- (instancetype)init { - NOTREACHED(); - return nil; -} - -- (UIView*)view { - return _view; -} - @end
diff --git a/ios/chrome/browser/snapshots/snapshots_util.mm b/ios/chrome/browser/snapshots/snapshots_util.mm index 83fea37..822dc11 100644 --- a/ios/chrome/browser/snapshots/snapshots_util.mm +++ b/ios/chrome/browser/snapshots/snapshots_util.mm
@@ -15,6 +15,10 @@ #include "base/strings/stringprintf.h" #include "base/task_scheduler/post_task.h" +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + namespace { const char* kOrientationDescriptions[] = { "LandscapeLeft",
diff --git a/ios/chrome/browser/ui/payments/payment_request_edit_view_controller.mm b/ios/chrome/browser/ui/payments/payment_request_edit_view_controller.mm index 9539bc97..523bdb16 100644 --- a/ios/chrome/browser/ui/payments/payment_request_edit_view_controller.mm +++ b/ios/chrome/browser/ui/payments/payment_request_edit_view_controller.mm
@@ -322,8 +322,16 @@ UIPickerView* pickerView = [[UIPickerView alloc] initWithFrame:CGRectZero]; pickerView.delegate = self; pickerView.dataSource = self; + pickerView.accessibilityIdentifier = + [NSString stringWithFormat:@"%@_pickerView", field.label]; [self.pickerViews setObject:pickerView forKey:key]; item.inputView = pickerView; + + // Set UIPickerView's default selected row. + [pickerView reloadAllComponents]; + [pickerView selectRow:[options indexOfObject:field.value] + inComponent:0 + animated:NO]; } // Reload the item.
diff --git a/ios/showcase/payments/BUILD.gn b/ios/showcase/payments/BUILD.gn index dd19e96..3b3bc8b 100644 --- a/ios/showcase/payments/BUILD.gn +++ b/ios/showcase/payments/BUILD.gn
@@ -36,6 +36,7 @@ deps = [ "//base", "//components/strings", + "//ios/chrome/app/strings:ios_strings_grit", "//ios/chrome/browser/ui", "//ios/chrome/browser/ui/autofill:autofill_ui", "//ios/chrome/browser/ui/payments:payments_ui",
diff --git a/ios/showcase/payments/sc_payments_editor_coordinator.mm b/ios/showcase/payments/sc_payments_editor_coordinator.mm index 7bd18082..ba652e23 100644 --- a/ios/showcase/payments/sc_payments_editor_coordinator.mm +++ b/ios/showcase/payments/sc_payments_editor_coordinator.mm
@@ -28,6 +28,8 @@ @property(nonatomic, strong) ProtocolAlerter* alerter; +@property(nonatomic, strong) EditorField* province; + @end @implementation SCPaymentsEditorCoordinator @@ -37,6 +39,7 @@ @synthesize paymentRequestEditViewController = _paymentRequestEditViewController; @synthesize alerter = _alerter; +@synthesize province = _province; - (void)start { self.alerter = [[ProtocolAlerter alloc] initWithProtocols:@[ @@ -55,6 +58,12 @@ self.alerter)]; [self.paymentRequestEditViewController setValidatorDelegate:self]; [self.paymentRequestEditViewController loadModel]; + // Set the options for the province field after the model is loaded. + NSArray<NSString*>* options = @[ @"Ontario", @"Quebec" ]; + self.province.value = options[1]; + self.province.enabled = YES; + [self.paymentRequestEditViewController setOptions:options + forEditorField:self.province]; [self.baseViewController pushViewController:self.paymentRequestEditViewController animated:YES]; @@ -76,6 +85,13 @@ value:@"CAN" required:YES]; [country setDisplayValue:@"Canada"]; + self.province = [[EditorField alloc] + initWithAutofillUIType:AutofillUITypeProfileHomeAddressState + fieldType:EditorFieldTypeTextField + label:@"Province" + value:@"Loading..." + required:YES]; + self.province.enabled = NO; EditorField* address = [[EditorField alloc] initWithAutofillUIType:AutofillUITypeProfileHomeAddressStreet fieldType:EditorFieldTypeTextField @@ -88,8 +104,14 @@ label:@"Postal Code" value:@"" required:NO]; + EditorField* save = [[EditorField alloc] + initWithAutofillUIType:AutofillUITypeCreditCardSaveToChrome + fieldType:EditorFieldTypeSwitch + label:@"Save" + value:@"YES" + required:NO]; - return @[ name, country, address, postalCode ]; + return @[ name, country, self.province, address, postalCode, save ]; } #pragma mark - PaymentRequestEditViewControllerDataSource @@ -111,12 +133,7 @@ - (NSString*)paymentRequestEditViewController: (PaymentRequestEditViewController*)controller validateField:(EditorField*)field { - if (field.value.length) - return nil; - else if (field.isRequired) - return @"Field is required"; - else - return nil; + return (!field.value.length && field.isRequired) ? @"Field is required" : nil; } @end
diff --git a/ios/showcase/payments/sc_payments_editor_egtest.mm b/ios/showcase/payments/sc_payments_editor_egtest.mm index cabca8d9..757415af 100644 --- a/ios/showcase/payments/sc_payments_editor_egtest.mm +++ b/ios/showcase/payments/sc_payments_editor_egtest.mm
@@ -9,6 +9,7 @@ #import "ios/chrome/browser/ui/autofill/autofill_edit_accessory_view.h" #import "ios/chrome/browser/ui/payments/payment_request_edit_view_controller.h" #include "ios/chrome/browser/ui/ui_util.h" +#include "ios/chrome/grit/ios_strings.h" #import "ios/showcase/test/showcase_eg_utils.h" #import "ios/showcase/test/showcase_test_case.h" #include "ui/base/l10n/l10n_util.h" @@ -108,6 +109,12 @@ nil)] assertWithMatcher:grey_notNil()]; + [[EarlGrey selectElementWithMatcher:grey_accessibilityLabel(@"Province*")] + assertWithMatcher:grey_notNil()]; + [[EarlGrey + selectElementWithMatcher:grey_accessibilityID(@"Province_textField")] + assertWithMatcher:grey_text(@"Quebec")]; + [[EarlGrey selectElementWithMatcher:grey_accessibilityLabel(@"Address*")] assertWithMatcher:grey_notNil()]; [[EarlGrey @@ -119,6 +126,28 @@ [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Postal Code_textField")] assertWithMatcher:grey_text(@"")]; + + [[EarlGrey + selectElementWithMatcher:grey_allOf(grey_accessibilityLabel(@"Save"), + grey_accessibilityValue( + l10n_util::GetNSString( + IDS_IOS_SETTING_ON)), + nil)] + assertWithMatcher:grey_notNil()]; +} + +// Tests if the expected input view for the province field is displaying, when +// the field is focused, and that the expected row is selected. +- (void)testVerifyProvinceFieldInputView { + // Tap the province textfield. + [[EarlGrey + selectElementWithMatcher:grey_accessibilityID(@"Province_textField")] + performAction:grey_tap()]; + + // Assert that a UIPicker view is displaying and the expected row is selected. + [[EarlGrey + selectElementWithMatcher:grey_accessibilityID(@"Province_pickerView")] + assertWithMatcher:grey_pickerColumnSetToValue(0, @"Quebec")]; } // Tests if tapping the selector field notifies the delegate. @@ -193,6 +222,16 @@ [[[EarlGrey selectElementWithMatcher:InputAccessoryViewNextButton()] assertWithMatcher:grey_enabled()] performAction:grey_tap()]; + // Assert the province textfield is focused. + AssertTextFieldWithAccessibilityIDIsFirstResponder(@"Province_textField"); + + // Assert the input accessory view's previous button is enabled. + [[EarlGrey selectElementWithMatcher:InputAccessoryViewPreviousButton()] + assertWithMatcher:grey_enabled()]; + // Assert the input accessory view's next button is enabled and tap it. + [[[EarlGrey selectElementWithMatcher:InputAccessoryViewNextButton()] + assertWithMatcher:grey_enabled()] performAction:grey_tap()]; + // Assert the address textfield is focused. AssertTextFieldWithAccessibilityIDIsFirstResponder(@"Address_textField"); @@ -239,12 +278,12 @@ kWarningMessageAccessibilityID)] assertWithMatcher:grey_notVisible()]; - // Assert the name textfield is focused. - AssertTextFieldWithAccessibilityIDIsFirstResponder(@"Name_textField"); + // Assert the province textfield is focused. + AssertTextFieldWithAccessibilityIDIsFirstResponder(@"Province_textField"); - // Assert the input accessory view's previous button is disabled. + // Assert the input accessory view's previous button is enabled. [[EarlGrey selectElementWithMatcher:InputAccessoryViewPreviousButton()] - assertWithMatcher:grey_not(grey_enabled())]; + assertWithMatcher:grey_enabled()]; // Assert the input accessory view's next button is enabled. [[EarlGrey selectElementWithMatcher:InputAccessoryViewNextButton()] assertWithMatcher:grey_enabled()]; @@ -265,6 +304,15 @@ [[EarlGrey selectElementWithMatcher:grey_accessibilityID(@"Name_textField")] performAction:grey_typeText(@"\n")]; + // Assert the province textfield is focused. + AssertTextFieldWithAccessibilityIDIsFirstResponder(@"Province_textField"); + + // The standard keyboard does not display for the province field. Instead, tap + // the address textfield. + [[EarlGrey + selectElementWithMatcher:grey_accessibilityID(@"Address_textField")] + performAction:grey_tap()]; + // Assert the address textfield is focused. AssertTextFieldWithAccessibilityIDIsFirstResponder(@"Address_textField");
diff --git a/mojo/edk/system/ports/node.cc b/mojo/edk/system/ports/node.cc index f6217a7..ec9b441 100644 --- a/mojo/edk/system/ports/node.cc +++ b/mojo/edk/system/ports/node.cc
@@ -429,11 +429,23 @@ // this node. When the message is forwarded, these ports will get transferred // following the usual method. If the message cannot be accepted, then the // newly bound ports will simply be closed. - for (size_t i = 0; i < message->num_ports(); ++i) { - int rv = AcceptPort(message->ports()[i], message->port_descriptors()[i]); - if (rv != OK) - return rv; + Event::PortDescriptor& descriptor = message->port_descriptors()[i]; + if (descriptor.referring_node_name == kInvalidNodeName) { + // If the referring node name is invalid, this descriptor can be ignored + // and the port should already exist locally. + if (!GetPort(message->ports()[i])) + return ERROR_PORT_UNKNOWN; + } else { + int rv = AcceptPort(message->ports()[i], descriptor); + if (rv != OK) + return rv; + + // Ensure that the referring node is wiped out of this descriptor. This + // allows the event to be forwarded across multiple local hops without + // attempting to accept the port more than once. + descriptor.referring_node_name = kInvalidNodeName; + } } bool has_next_message = false; @@ -993,7 +1005,6 @@ // by a proxy. Otherwise, use the next outgoing sequence number. if (message->sequence_num() == 0) message->set_sequence_num(port->next_sequence_num_to_send++); - #if DCHECK_IS_ON() std::ostringstream ports_buf; for (size_t i = 0; i < message->num_ports(); ++i) { @@ -1033,10 +1044,16 @@ } } - Event::PortDescriptor* port_descriptors = message->port_descriptors(); - for (size_t i = 0; i < message->num_ports(); ++i) { - WillSendPort(LockedPort(ports[i].get()), port->peer_node_name, - message->ports() + i, port_descriptors + i); + if (port->peer_node_name != name_) { + // We only bother to proxy and rewrite ports in the event if it's going to + // be routed to an external node. This substantially reduces the amount of + // port churn in the system, as many port-carrying events are routed at + // least 1 or 2 intra-node hops before (if ever) being routed externally. + Event::PortDescriptor* port_descriptors = message->port_descriptors(); + for (size_t i = 0; i < message->num_ports(); ++i) { + WillSendPort(LockedPort(ports[i].get()), port->peer_node_name, + message->ports() + i, port_descriptors + i); + } } for (size_t i = 0; i < message->num_ports(); ++i)
diff --git a/mojo/edk/system/ports/ports_unittest.cc b/mojo/edk/system/ports/ports_unittest.cc index e20fda38..d4aa9db 100644 --- a/mojo/edk/system/ports/ports_unittest.cc +++ b/mojo/edk/system/ports/ports_unittest.cc
@@ -1411,7 +1411,10 @@ ScopedMessage message; PortRef X, Y; - EXPECT_EQ(OK, node1.node().CreatePortPair(&X, &Y)); + EXPECT_EQ(OK, node0.node().CreateUninitializedPort(&X)); + EXPECT_EQ(OK, node1.node().CreateUninitializedPort(&Y)); + EXPECT_EQ(OK, node0.node().InitializePort(X, node1.name(), Y.name())); + EXPECT_EQ(OK, node1.node().InitializePort(Y, node0.name(), X.name())); // Block the merge from proceeding until we can do something stupid with port // C. This avoids the test logic racing with async merge logic. @@ -1422,18 +1425,20 @@ // Move C to a new port E. This is not a sane use of Node's public API but // is still hypothetically possible. It allows us to force a merge failure - // because C will be in an invalid state by the term the merge is processed. + // because C will be in an invalid state by the time the merge is processed. // As a result, B should be closed. - EXPECT_EQ(OK, node1.SendStringMessageWithPort(X, "foo", C)); + EXPECT_EQ(OK, node1.SendStringMessageWithPort(Y, "foo", C)); node1.Unblock(); - ASSERT_TRUE(node1.ReadMessage(Y, &message)); + WaitForIdle(); + + ASSERT_TRUE(node0.ReadMessage(X, &message)); ASSERT_EQ(1u, message->num_ports()); PortRef E; - ASSERT_EQ(OK, node1.node().GetPort(message->ports()[0], &E)); + ASSERT_EQ(OK, node0.node().GetPort(message->ports()[0], &E)); - EXPECT_EQ(OK, node1.node().ClosePort(X)); + EXPECT_EQ(OK, node0.node().ClosePort(X)); EXPECT_EQ(OK, node1.node().ClosePort(Y)); WaitForIdle(); @@ -1446,7 +1451,7 @@ // Close A, D, and E. EXPECT_EQ(OK, node0.node().ClosePort(A)); EXPECT_EQ(OK, node1.node().ClosePort(D)); - EXPECT_EQ(OK, node1.node().ClosePort(E)); + EXPECT_EQ(OK, node0.node().ClosePort(E)); WaitForIdle();
diff --git a/services/service_manager/service_manager.cc b/services/service_manager/service_manager.cc index ed3adad..1575a305 100644 --- a/services/service_manager/service_manager.cc +++ b/services/service_manager/service_manager.cc
@@ -323,19 +323,26 @@ class InterfaceProviderImpl : public mojom::InterfaceProvider { public: - InterfaceProviderImpl(const std::string& spec, + InterfaceProviderImpl(Instance* instance, + const std::string& spec, const Identity& source_identity, const Identity& target_identity, service_manager::ServiceManager* service_manager, mojom::InterfaceProviderPtr target, mojom::InterfaceProviderRequest source_request) - : spec_(spec), + : instance_(instance), + spec_(spec), source_identity_(source_identity), target_identity_(target_identity), service_manager_(service_manager), target_(std::move(target)), source_binding_(this, std::move(source_request)) {} - ~InterfaceProviderImpl() override {} + ~InterfaceProviderImpl() override { + target_.set_connection_error_handler(base::Bind( + &InterfaceProviderImpl::OnConnectionError, base::Unretained(this))); + source_binding_.set_connection_error_handler(base::Bind( + &InterfaceProviderImpl::OnConnectionError, base::Unretained(this))); + } private: // mojom::InterfaceProvider: @@ -366,6 +373,12 @@ return true; } + void OnConnectionError() { + // Deletes |this|. + instance_->filters_.erase(this); + } + + Instance* const instance_; const std::string spec_; const Identity source_identity_; const Identity target_identity_; @@ -449,9 +462,10 @@ const Identity& source, mojom::InterfaceProviderRequest source_request, mojom::InterfaceProviderPtr target) override { - filters_.push_back(base::MakeUnique<InterfaceProviderImpl>( - spec, source, identity_, service_manager_, std::move(target), - std::move(source_request))); + auto* filter = new InterfaceProviderImpl( + this, spec, source, identity_, service_manager_, std::move(target), + std::move(source_request)); + filters_[filter] = base::WrapUnique(filter); } // mojom::PIDReceiver: @@ -633,7 +647,8 @@ base::ProcessId pid_ = base::kNullProcessId; State state_; - std::vector<std::unique_ptr<InterfaceProviderImpl>> filters_; + std::map<InterfaceProviderImpl*, std::unique_ptr<InterfaceProviderImpl>> + filters_; // The number of outstanding OnBindInterface requests which are in flight. int pending_service_connections_ = 0;
diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptModule.cpp b/third_party/WebKit/Source/bindings/core/v8/ScriptModule.cpp index 6dbdb6a..ca0f630 100644 --- a/third_party/WebKit/Source/bindings/core/v8/ScriptModule.cpp +++ b/third_party/WebKit/Source/bindings/core/v8/ScriptModule.cpp
@@ -68,9 +68,11 @@ DCHECK(!IsNull()); v8::Local<v8::Context> context = script_state->GetContext(); - bool success = NewLocal(script_state->GetIsolate()) - ->Instantiate(context, &ResolveModuleCallback); - if (!success) { + bool success; + if (!NewLocal(script_state->GetIsolate()) + ->InstantiateModule(context, &ResolveModuleCallback) + .To(&success) || + !success) { DCHECK(try_catch.HasCaught()); return ScriptValue(script_state, try_catch.Exception()); }
diff --git a/third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLPlugInElementCustom.cpp b/third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLPlugInElementCustom.cpp index dbac540..a4482fa9 100644 --- a/third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLPlugInElementCustom.cpp +++ b/third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLPlugInElementCustom.cpp
@@ -47,13 +47,24 @@ void GetScriptableObjectProperty( const AtomicString& name, const v8::PropertyCallbackInfo<v8::Value>& info) { + ScriptState* state = ScriptState::Current(info.GetIsolate()); + if (!state->World().IsMainWorld()) { + if (state->World().IsIsolatedWorld()) { + UseCounter::Count(CurrentExecutionContext(info.GetIsolate()), + UseCounter::kPluginInstanceAccessFromIsolatedWorld); + } + // The plugin system cannot deal with multiple worlds, so block any + // non-main world access. + return; + } + UseCounter::Count(CurrentExecutionContext(info.GetIsolate()), + UseCounter::kPluginInstanceAccessFromMainWorld); + HTMLPlugInElement* impl = ElementType::toImpl(info.Holder()); v8::Local<v8::Object> instance = impl->PluginWrapper(); if (instance.IsEmpty()) return; - ScriptState* state = ScriptState::Current(info.GetIsolate()); - v8::Local<v8::String> v8_name = V8String(info.GetIsolate(), name); if (!V8CallBoolean(instance->HasOwnProperty(state->GetContext(), v8_name))) return; @@ -62,14 +73,6 @@ if (!instance->Get(state->GetContext(), v8_name).ToLocal(&value)) return; - if (state->World().IsIsolatedWorld()) { - UseCounter::Count(CurrentExecutionContext(info.GetIsolate()), - UseCounter::kPluginInstanceAccessFromIsolatedWorld); - } else if (state->World().IsMainWorld()) { - UseCounter::Count(CurrentExecutionContext(info.GetIsolate()), - UseCounter::kPluginInstanceAccessFromMainWorld); - } - V8SetReturnValue(info, value); } @@ -79,6 +82,12 @@ v8::Local<v8::Value> value, const v8::PropertyCallbackInfo<v8::Value>& info) { DCHECK(!value.IsEmpty()); + ScriptState* state = ScriptState::Current(info.GetIsolate()); + if (!state->World().IsMainWorld()) { + // The plugin system cannot deal with multiple worlds, so block any + // non-main world access. + return; + } HTMLPlugInElement* impl = ElementType::toImpl(info.Holder()); v8::Local<v8::Object> instance = impl->PluginWrapper(); @@ -87,10 +96,8 @@ // Don't intercept any of the properties of the HTMLPluginElement. v8::Local<v8::String> v8_name = V8String(info.GetIsolate(), name); - if (!V8CallBoolean(instance->HasOwnProperty( - info.GetIsolate()->GetCurrentContext(), v8_name)) && - V8CallBoolean(info.Holder()->Has(info.GetIsolate()->GetCurrentContext(), - v8_name))) { + if (!V8CallBoolean(instance->HasOwnProperty(state->GetContext(), v8_name)) && + V8CallBoolean(info.Holder()->Has(state->GetContext(), v8_name))) { return; } @@ -103,8 +110,8 @@ // DOM element will also be set. For plugin's that don't intercept the call // (all except gTalk) this makes no difference at all. For gTalk the fact // that the property on the DOM element also gets set is inconsequential. - V8CallBoolean(instance->CreateDataProperty( - info.GetIsolate()->GetCurrentContext(), v8_name, value)); + V8CallBoolean( + instance->CreateDataProperty(state->GetContext(), v8_name, value)); V8SetReturnValue(info, value); }
diff --git a/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h b/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h index 94a95891..f7ad2ce 100644 --- a/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h +++ b/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h
@@ -1280,35 +1280,6 @@ } template <> -inline CSSIdentifierValue::CSSIdentifierValue(TextCombine e) - : CSSValue(kIdentifierClass) { - switch (e) { - case TextCombine::kNone: - value_id_ = CSSValueNone; - break; - case TextCombine::kAll: - value_id_ = CSSValueAll; - break; - } -} - -template <> -inline TextCombine CSSIdentifierValue::ConvertTo() const { - switch (value_id_) { - case CSSValueNone: - return TextCombine::kNone; - case CSSValueAll: - case CSSValueHorizontal: // -webkit-text-combine - return TextCombine::kAll; - default: - break; - } - - NOTREACHED(); - return TextCombine::kNone; -} - -template <> inline CSSIdentifierValue::CSSIdentifierValue(TextEmphasisPosition position) : CSSValue(kIdentifierClass) { switch (position) { @@ -1444,41 +1415,6 @@ } template <> -inline CSSIdentifierValue::CSSIdentifierValue(TextOrientation e) - : CSSValue(kIdentifierClass) { - switch (e) { - case TextOrientation::kSideways: - value_id_ = CSSValueSideways; - break; - case TextOrientation::kMixed: - value_id_ = CSSValueMixed; - break; - case TextOrientation::kUpright: - value_id_ = CSSValueUpright; - break; - } -} - -template <> -inline TextOrientation CSSIdentifierValue::ConvertTo() const { - switch (value_id_) { - case CSSValueSideways: - case CSSValueSidewaysRight: - return TextOrientation::kSideways; - case CSSValueMixed: - case CSSValueVerticalRight: // -webkit-text-orientation - return TextOrientation::kMixed; - case CSSValueUpright: - return TextOrientation::kUpright; - default: - break; - } - - NOTREACHED(); - return TextOrientation::kMixed; -} - -template <> inline CSSIdentifierValue::CSSIdentifierValue(FontDescription::Kerning kerning) : CSSValue(kIdentifierClass) { switch (kerning) {
diff --git a/third_party/WebKit/Source/core/css/CSSProperties.json5 b/third_party/WebKit/Source/core/css/CSSProperties.json5 index de8f83a..f7342bc 100644 --- a/third_party/WebKit/Source/core/css/CSSProperties.json5 +++ b/third_party/WebKit/Source/core/css/CSSProperties.json5
@@ -524,11 +524,12 @@ custom_value: true, inherited: true, priority: "High", - field_template: "storage_only", - type_name: "TextOrientation", - default_value: "TextOrientation::kMixed", + field_template: "keyword", + keywords: ["sideways", "mixed", "upright"], + default_value: "mixed", field_size: 2, field_group: "rare-inherited", + getter: "GetTextOrientation" }, { name: "-webkit-text-orientation", @@ -2030,9 +2031,9 @@ name: "text-combine-upright", inherited: true, name_for_methods: "TextCombine", - field_template: "storage_only", - type_name: "TextCombine", - default_value: "TextCombine::kNone", + field_template: "keyword", + keywords: ["none", "all"], + default_value: "none", field_size: 1, field_group: "rare-inherited", }, @@ -2593,7 +2594,6 @@ name: "-webkit-text-combine", inherited: true, name_for_methods: "TextCombine", - type_name: "TextCombine", }, { name: "-webkit-text-emphasis-color",
diff --git a/third_party/WebKit/Source/core/css/CSSValueIDMappings.h b/third_party/WebKit/Source/core/css/CSSValueIDMappings.h index cca59a6f..6f131b4 100644 --- a/third_party/WebKit/Source/core/css/CSSValueIDMappings.h +++ b/third_party/WebKit/Source/core/css/CSSValueIDMappings.h
@@ -34,6 +34,13 @@ } template <> +inline ETextCombine CssValueIDToPlatformEnum(CSSValueID v) { + if (v == CSSValueHorizontal) // -webkit-text-combine + return ETextCombine::kAll; + return detail::cssValueIDToPlatformEnumGenerated<ETextCombine>(v); +} + +template <> inline ETextAlign CssValueIDToPlatformEnum(CSSValueID v) { if (v == CSSValueWebkitAuto) // Legacy -webkit-auto. Eqiuvalent to start. return ETextAlign::kStart; @@ -43,6 +50,15 @@ } template <> +inline ETextOrientation CssValueIDToPlatformEnum(CSSValueID v) { + if (v == CSSValueSidewaysRight) // Legacy -webkit-auto. Eqiuvalent to start. + return ETextOrientation::kSideways; + if (v == CSSValueVerticalRight) + return ETextOrientation::kMixed; + return detail::cssValueIDToPlatformEnumGenerated<ETextOrientation>(v); +} + +template <> inline WritingMode CssValueIDToPlatformEnum(CSSValueID v) { switch (v) { case CSSValueHorizontalTb:
diff --git a/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp b/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp index 3c2274f..afd43d17 100644 --- a/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp +++ b/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
@@ -3304,12 +3304,12 @@ case CSSPropertyWebkitWritingMode: return CSSIdentifierValue::Create(style.GetWritingMode()); case CSSPropertyWebkitTextCombine: - if (style.GetTextCombine() == TextCombine::kAll) + if (style.TextCombine() == ETextCombine::kAll) return CSSIdentifierValue::Create(CSSValueHorizontal); case CSSPropertyTextCombineUpright: - return CSSIdentifierValue::Create(style.GetTextCombine()); + return CSSIdentifierValue::Create(style.TextCombine()); case CSSPropertyWebkitTextOrientation: - if (style.GetTextOrientation() == TextOrientation::kMixed) + if (style.GetTextOrientation() == ETextOrientation::kMixed) return CSSIdentifierValue::Create(CSSValueVerticalRight); case CSSPropertyTextOrientation: return CSSIdentifierValue::Create(style.GetTextOrientation());
diff --git a/third_party/WebKit/Source/core/css/resolver/FontBuilder.cpp b/third_party/WebKit/Source/core/css/resolver/FontBuilder.cpp index 56601c2e..0e05ce9 100644 --- a/third_party/WebKit/Source/core/css/resolver/FontBuilder.cpp +++ b/third_party/WebKit/Source/core/css/resolver/FontBuilder.cpp
@@ -242,11 +242,11 @@ return FontOrientation::kHorizontal; switch (style.GetTextOrientation()) { - case TextOrientation::kMixed: + case ETextOrientation::kMixed: return FontOrientation::kVerticalMixed; - case TextOrientation::kUpright: + case ETextOrientation::kUpright: return FontOrientation::kVerticalUpright; - case TextOrientation::kSideways: + case ETextOrientation::kSideways: return FontOrientation::kVerticalRotated; default: NOTREACHED();
diff --git a/third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp b/third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp index 909904e..0ce638345 100644 --- a/third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp +++ b/third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp
@@ -827,14 +827,14 @@ StyleResolverState& state, const CSSValue& value) { state.SetTextOrientation( - ToCSSIdentifierValue(value).ConvertTo<TextOrientation>()); + ToCSSIdentifierValue(value).ConvertTo<ETextOrientation>()); } void StyleBuilderFunctions::applyValueCSSPropertyWebkitTextOrientation( StyleResolverState& state, const CSSValue& value) { state.SetTextOrientation( - ToCSSIdentifierValue(value).ConvertTo<TextOrientation>()); + ToCSSIdentifierValue(value).ConvertTo<ETextOrientation>()); } void StyleBuilderFunctions::applyValueCSSPropertyVariable(
diff --git a/third_party/WebKit/Source/core/css/resolver/StyleResolverState.h b/third_party/WebKit/Source/core/css/resolver/StyleResolverState.h index 7ea8f17..c1eea48 100644 --- a/third_party/WebKit/Source/core/css/resolver/StyleResolverState.h +++ b/third_party/WebKit/Source/core/css/resolver/StyleResolverState.h
@@ -201,7 +201,7 @@ style_->SetWritingMode(new_writing_mode); font_builder_.DidChangeWritingMode(); } - void SetTextOrientation(TextOrientation text_orientation) { + void SetTextOrientation(ETextOrientation text_orientation) { if (style_->SetTextOrientation(text_orientation)) font_builder_.DidChangeTextOrientation(); }
diff --git a/third_party/WebKit/Source/core/dom/Document.h b/third_party/WebKit/Source/core/dom/Document.h index eb72c7c..92c1459 100644 --- a/third_party/WebKit/Source/core/dom/Document.h +++ b/third_party/WebKit/Source/core/dom/Document.h
@@ -1631,8 +1631,6 @@ ViewportDescription legacy_viewport_description_; Length viewport_default_min_width_; - ReferrerPolicy referrer_policy_; - DocumentTiming document_timing_; Member<MediaQueryMatcher> media_query_matcher_; bool write_recursion_is_too_deep_;
diff --git a/third_party/WebKit/Source/core/editing/LayoutSelection.cpp b/third_party/WebKit/Source/core/editing/LayoutSelection.cpp index 0ad66bbc..2467fa8 100644 --- a/third_party/WebKit/Source/core/editing/LayoutSelection.cpp +++ b/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
@@ -334,7 +334,7 @@ ClearSelection(); return; } - + DCHECK(!selection.IsNone()); // Use the rightmost candidate for the start of the selection, and the // leftmost candidate for the end of the selection. Example: foo <a>bar</a>. // Imagine that a line wrap occurs after 'foo', and that 'bar' is selected. @@ -342,26 +342,22 @@ // code will think that content on the line containing 'foo' is selected // and will fill the gap before 'bar'. PositionInFlatTree start_pos = selection.Start(); - PositionInFlatTree candidate = MostForwardCaretPosition(start_pos); - if (IsVisuallyEquivalentCandidate(candidate)) - start_pos = candidate; + const PositionInFlatTree most_forward_start = + MostForwardCaretPosition(start_pos); + if (IsVisuallyEquivalentCandidate(most_forward_start)) + start_pos = most_forward_start; PositionInFlatTree end_pos = selection.end(); - candidate = MostBackwardCaretPosition(end_pos); - if (IsVisuallyEquivalentCandidate(candidate)) - end_pos = candidate; + const PositionInFlatTree most_backward = MostBackwardCaretPosition(end_pos); + if (IsVisuallyEquivalentCandidate(most_backward)) + end_pos = most_backward; - // We can get into a state where the selection endpoints map to the same - // |VisiblePosition| when a selection is deleted because we don't yet notify - // the |FrameSelection| of text removal. - if (start_pos.IsNull() || end_pos.IsNull() || - selection.VisibleStart().DeepEquivalent() == - selection.VisibleEnd().DeepEquivalent()) - return; + DCHECK(start_pos.IsNotNull()); + DCHECK(end_pos.IsNotNull()); DCHECK_LE(start_pos, end_pos); LayoutObject* start_layout_object = start_pos.AnchorNode()->GetLayoutObject(); LayoutObject* end_layout_object = end_pos.AnchorNode()->GetLayoutObject(); - if (!start_layout_object || !end_layout_object) - return; + DCHECK(start_layout_object); + DCHECK(end_layout_object); DCHECK(start_layout_object->View() == end_layout_object->View()); const SelectionPaintRange new_range(
diff --git a/third_party/WebKit/Source/core/style/ComputedStyle.h b/third_party/WebKit/Source/core/style/ComputedStyle.h index c8810947..a2e98be 100644 --- a/third_party/WebKit/Source/core/style/ComputedStyle.h +++ b/third_party/WebKit/Source/core/style/ComputedStyle.h
@@ -1669,15 +1669,6 @@ bool QuotesDataEquivalent(const ComputedStyle&) const; - // text-combine-upright (aka -webkit-text-combine, -epub-text-combine) - static TextCombine InitialTextCombine() { return TextCombine::kNone; } - TextCombine GetTextCombine() const { - return static_cast<TextCombine>(rare_inherited_data_->text_combine_); - } - void SetTextCombine(TextCombine v) { - SET_VAR(rare_inherited_data_, text_combine_, static_cast<unsigned>(v)); - } - // text-justify static TextJustify InitialTextJustify() { return kTextJustifyAuto; } TextJustify GetTextJustify() const { @@ -1688,14 +1679,7 @@ } // text-orientation (aka -webkit-text-orientation, -epub-text-orientation) - static TextOrientation InitialTextOrientation() { - return TextOrientation::kMixed; - } - TextOrientation GetTextOrientation() const { - return static_cast<TextOrientation>( - rare_inherited_data_->text_orientation_); - } - bool SetTextOrientation(TextOrientation); + bool SetTextOrientation(ETextOrientation); // text-shadow static ShadowList* InitialTextShadow() { return 0; } @@ -2192,7 +2176,7 @@ } // Text-combine utility functions. - bool HasTextCombine() const { return GetTextCombine() != TextCombine::kNone; } + bool HasTextCombine() const { return TextCombine() != ETextCombine::kNone; } // Grid utility functions. const Vector<GridTrackSize>& GridAutoRepeatColumns() const { @@ -3595,7 +3579,7 @@ } inline bool ComputedStyle::SetTextOrientation( - TextOrientation text_orientation) { + ETextOrientation text_orientation) { if (compareEqual(rare_inherited_data_->text_orientation_, static_cast<unsigned>(text_orientation))) return false;
diff --git a/third_party/WebKit/Source/core/style/ComputedStyleConstants.h b/third_party/WebKit/Source/core/style/ComputedStyleConstants.h index 0b70801..7f769ed 100644 --- a/third_party/WebKit/Source/core/style/ComputedStyleConstants.h +++ b/third_party/WebKit/Source/core/style/ComputedStyleConstants.h
@@ -118,8 +118,6 @@ kLength }; -enum class TextCombine { kNone, kAll }; - enum EFillAttachment { kScrollBackgroundAttachment, kLocalBackgroundAttachment, @@ -270,8 +268,6 @@ kCustom }; -enum class TextOrientation { kMixed, kUpright, kSideways }; - enum TextOverflow { kTextOverflowClip = 0, kTextOverflowEllipsis }; enum class EImageRendering {
diff --git a/third_party/WebKit/Source/core/style/CounterDirectives.cpp b/third_party/WebKit/Source/core/style/CounterDirectives.cpp index 1203c541..f922b6bc 100644 --- a/third_party/WebKit/Source/core/style/CounterDirectives.cpp +++ b/third_party/WebKit/Source/core/style/CounterDirectives.cpp
@@ -32,12 +32,4 @@ a.IsReset() == b.IsReset() && a.ResetValue() == b.ResetValue(); } -std::unique_ptr<CounterDirectiveMap> Clone( - const CounterDirectiveMap& counter_directives) { - std::unique_ptr<CounterDirectiveMap> result = - WTF::WrapUnique(new CounterDirectiveMap); - *result = counter_directives; - return result; -} - } // namespace blink
diff --git a/third_party/WebKit/Source/core/style/CounterDirectives.h b/third_party/WebKit/Source/core/style/CounterDirectives.h index fecde4c..e4ecc2f 100644 --- a/third_party/WebKit/Source/core/style/CounterDirectives.h +++ b/third_party/WebKit/Source/core/style/CounterDirectives.h
@@ -99,9 +99,13 @@ return !(a == b); } -typedef HashMap<AtomicString, CounterDirectives> CounterDirectiveMap; - -std::unique_ptr<CounterDirectiveMap> Clone(const CounterDirectiveMap&); +// Not to be deleted through a pointer to HashMap. +class CounterDirectiveMap : public HashMap<AtomicString, CounterDirectives> { + public: + std::unique_ptr<CounterDirectiveMap> Clone() const { + return WTF::WrapUnique(new CounterDirectiveMap(*this)); + } +}; } // namespace blink
diff --git a/third_party/WebKit/Source/core/style/StyleRareNonInheritedData.cpp b/third_party/WebKit/Source/core/style/StyleRareNonInheritedData.cpp index 5ec1022..c5f5bec 100644 --- a/third_party/WebKit/Source/core/style/StyleRareNonInheritedData.cpp +++ b/third_party/WebKit/Source/core/style/StyleRareNonInheritedData.cpp
@@ -154,7 +154,7 @@ grid_item_(o.grid_item_), scroll_snap_(o.scroll_snap_), content_(o.content_ ? o.content_->Clone() : nullptr), - counter_directives_(o.counter_directives_ ? Clone(*o.counter_directives_) + counter_directives_(o.counter_directives_ ? o.counter_directives_->Clone() : nullptr), animations_(o.animations_ ? o.animations_->Clone() : nullptr), transitions_(o.transitions_ ? o.transitions_->Clone() : nullptr),
diff --git a/third_party/WebKit/Source/modules/exported/BUILD.gn b/third_party/WebKit/Source/modules/exported/BUILD.gn index 68da1a31..751fd2e 100644 --- a/third_party/WebKit/Source/modules/exported/BUILD.gn +++ b/third_party/WebKit/Source/modules/exported/BUILD.gn
@@ -14,6 +14,7 @@ "WebSpeechGrammar.cpp", "WebSpeechRecognitionHandle.cpp", "WebSpeechRecognitionResult.cpp", + "WebStorageEventDispatcherImpl.cpp", ] defines = [ "BLINK_MODULES_IMPLEMENTATION=1" ]
diff --git a/third_party/WebKit/Source/web/WebStorageEventDispatcherImpl.cpp b/third_party/WebKit/Source/modules/exported/WebStorageEventDispatcherImpl.cpp similarity index 100% rename from third_party/WebKit/Source/web/WebStorageEventDispatcherImpl.cpp rename to third_party/WebKit/Source/modules/exported/WebStorageEventDispatcherImpl.cpp
diff --git a/third_party/WebKit/Source/web/BUILD.gn b/third_party/WebKit/Source/web/BUILD.gn index 0449433..c2442c88 100644 --- a/third_party/WebKit/Source/web/BUILD.gn +++ b/third_party/WebKit/Source/web/BUILD.gn
@@ -170,7 +170,6 @@ "WebSharedWorkerImpl.h", "WebSharedWorkerReportingProxyImpl.cpp", "WebSharedWorkerReportingProxyImpl.h", - "WebStorageEventDispatcherImpl.cpp", "WebSurroundingText.cpp", "WebTextCheckingCompletionImpl.cpp", "WebTextCheckingCompletionImpl.h",
diff --git a/tools/gn/bootstrap/bootstrap.py b/tools/gn/bootstrap/bootstrap.py index 6f2f5b1..0b03d26 100755 --- a/tools/gn/bootstrap/bootstrap.py +++ b/tools/gn/bootstrap/bootstrap.py
@@ -487,6 +487,7 @@ 'base/sys_info.cc', 'base/task_runner.cc', 'base/task_scheduler/delayed_task_manager.cc', + 'base/task_scheduler/environment_config.cc', 'base/task_scheduler/post_task.cc', 'base/task_scheduler/priority_queue.cc', 'base/task_scheduler/scheduler_lock_impl.cc',
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index e0d8cd6..ed6d6f24 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml
@@ -90907,6 +90907,13 @@ </affected-histogram> </histogram_suffixes> +<histogram_suffixes name="PageLoadMetricsAfterBuffering" separator="."> + <suffix name="AfterBuffering" + label="Recorded after buffering of timing updates in the browser + process has completed."/> + <affected-histogram name="PageLoad.Internal.OutOfOrderInterFrameTiming"/> +</histogram_suffixes> + <histogram_suffixes name="PageLoadMetricsAfterPaint" separator="."> <suffix name="AfterPaint" label="Limited to the duration of time starting after first paint, for
diff --git a/tools/perf/page_sets/system_health/browsing_stories.py b/tools/perf/page_sets/system_health/browsing_stories.py index 7d57323..79fe111f 100644 --- a/tools/perf/page_sets/system_health/browsing_stories.py +++ b/tools/perf/page_sets/system_health/browsing_stories.py
@@ -562,6 +562,7 @@ ITEMS_TO_VISIT = 4 +@decorators.Disabled('android') # crbug.com/728081 class BrowseTOIMobileStory(_ArticleBrowsingStory): NAME = 'browse:news:toi' URL = 'http://m.timesofindia.com'