Commit 9998653e authored by Elly Fong-Jones's avatar Elly Fong-Jones Committed by Commit Bot

bubbles: remove some metrics-gathering hooks

This change:
1) Removes BubbleController::GetName and its various implementations
2) Removes BubbleController::GetVisibleTime and its various implementations
3) Cleans up ExtensionInstalledBubbleBrowserTest to not depend on GetName

Bug: 496955
Change-Id: Iee271ea4d4dbae96f8dd77e2ce2be87bd36ecc3f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1954260
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarMichael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#722642}
parent 11f4508e
...@@ -297,10 +297,6 @@ bool ExtensionInstalledBubble::ShouldClose(BubbleCloseReason reason) const { ...@@ -297,10 +297,6 @@ bool ExtensionInstalledBubble::ShouldClose(BubbleCloseReason reason) const {
return reason != BUBBLE_CLOSE_NAVIGATED; return reason != BUBBLE_CLOSE_NAVIGATED;
} }
std::string ExtensionInstalledBubble::GetName() const {
return "ExtensionInstalled";
}
const content::RenderFrameHost* ExtensionInstalledBubble::OwningFrame() const { const content::RenderFrameHost* ExtensionInstalledBubble::OwningFrame() const {
return nullptr; return nullptr;
} }
......
...@@ -89,7 +89,6 @@ class ExtensionInstalledBubble : public BubbleDelegate { ...@@ -89,7 +89,6 @@ class ExtensionInstalledBubble : public BubbleDelegate {
// BubbleDelegate: // BubbleDelegate:
std::unique_ptr<BubbleUi> BuildBubbleUi() override; std::unique_ptr<BubbleUi> BuildBubbleUi() override;
bool ShouldClose(BubbleCloseReason reason) const override; bool ShouldClose(BubbleCloseReason reason) const override;
std::string GetName() const override;
const content::RenderFrameHost* OwningFrame() const override; const content::RenderFrameHost* OwningFrame() const override;
// Returns false if the bubble could not be shown immediately, because of an // Returns false if the bubble could not be shown immediately, because of an
......
...@@ -41,11 +41,10 @@ class ExtensionInstalledBubbleBrowserTest ...@@ -41,11 +41,10 @@ class ExtensionInstalledBubbleBrowserTest
BubbleController* GetExtensionBubbleControllerFromManager( BubbleController* GetExtensionBubbleControllerFromManager(
BubbleManager* manager) const { BubbleManager* manager) const {
for (auto& controller : manager->controllers_) { // TODO(https://crbug.com/496955): This test class should not be aware of BubbleController
if (controller->GetName() == "ExtensionInstalled") // or its internals. Figure out an alternate strategy to find the needed BubbleUi instance.
return controller.get(); return manager->controllers_.size() ? manager->controllers_[0].get()
} : nullptr;
return nullptr;
} }
BubbleUi* GetBubbleUiFromManager(BubbleManager* manager) const { BubbleUi* GetBubbleUiFromManager(BubbleManager* manager) const {
......
...@@ -19,10 +19,6 @@ ChooserBubbleDelegate::ChooserBubbleDelegate( ...@@ -19,10 +19,6 @@ ChooserBubbleDelegate::ChooserBubbleDelegate(
ChooserBubbleDelegate::~ChooserBubbleDelegate() {} ChooserBubbleDelegate::~ChooserBubbleDelegate() {}
std::string ChooserBubbleDelegate::GetName() const {
return "ChooserBubble";
}
const content::RenderFrameHost* ChooserBubbleDelegate::OwningFrame() const { const content::RenderFrameHost* ChooserBubbleDelegate::OwningFrame() const {
return owning_frame_; return owning_frame_;
} }
...@@ -25,7 +25,6 @@ class ChooserBubbleDelegate : public BubbleDelegate { ...@@ -25,7 +25,6 @@ class ChooserBubbleDelegate : public BubbleDelegate {
~ChooserBubbleDelegate() override; ~ChooserBubbleDelegate() override;
// BubbleDelegate: // BubbleDelegate:
std::string GetName() const override;
std::unique_ptr<BubbleUi> BuildBubbleUi() override; std::unique_ptr<BubbleUi> BuildBubbleUi() override;
const content::RenderFrameHost* OwningFrame() const override; const content::RenderFrameHost* OwningFrame() const override;
......
...@@ -27,27 +27,11 @@ bool BubbleController::CloseBubble(BubbleCloseReason reason) { ...@@ -27,27 +27,11 @@ bool BubbleController::CloseBubble(BubbleCloseReason reason) {
return manager_->CloseBubble(this->AsWeakPtr(), reason); return manager_->CloseBubble(this->AsWeakPtr(), reason);
} }
bool BubbleController::UpdateBubbleUi() {
DCHECK(thread_checker_.CalledOnValidThread());
if (!bubble_ui_)
return false;
return delegate_->UpdateBubbleUi(bubble_ui_.get());
}
std::string BubbleController::GetName() const {
return delegate_->GetName();
}
base::TimeDelta BubbleController::GetVisibleTime() const {
return base::TimeTicks::Now() - show_time_;
}
void BubbleController::Show() { void BubbleController::Show() {
DCHECK(!bubble_ui_); DCHECK(!bubble_ui_);
bubble_ui_ = delegate_->BuildBubbleUi(); bubble_ui_ = delegate_->BuildBubbleUi();
DCHECK(bubble_ui_); DCHECK(bubble_ui_);
bubble_ui_->Show(AsWeakPtr()); bubble_ui_->Show(AsWeakPtr());
show_time_ = base::TimeTicks::Now();
} }
void BubbleController::UpdateAnchorPosition() { void BubbleController::UpdateAnchorPosition() {
......
...@@ -30,16 +30,6 @@ class BubbleController : public base::SupportsWeakPtr<BubbleController> { ...@@ -30,16 +30,6 @@ class BubbleController : public base::SupportsWeakPtr<BubbleController> {
// Calls CloseBubble on the associated BubbleManager. // Calls CloseBubble on the associated BubbleManager.
bool CloseBubble(BubbleCloseReason reason); bool CloseBubble(BubbleCloseReason reason);
// Calls UpdateBubbleUi on the associated BubbleManager.
// Returns true if the UI was updated.
bool UpdateBubbleUi();
// Used to identify a bubble for collecting metrics.
std::string GetName() const;
// Used for collecting metrics.
base::TimeDelta GetVisibleTime() const;
private: private:
friend class BubbleManager; friend class BubbleManager;
friend class ExtensionInstalledBubbleBrowserTest; friend class ExtensionInstalledBubbleBrowserTest;
...@@ -67,9 +57,6 @@ class BubbleController : public base::SupportsWeakPtr<BubbleController> { ...@@ -67,9 +57,6 @@ class BubbleController : public base::SupportsWeakPtr<BubbleController> {
// Verify that functions that affect the UI are done on the same thread. // Verify that functions that affect the UI are done on the same thread.
base::ThreadChecker thread_checker_; base::ThreadChecker thread_checker_;
// To keep track of the amount of time a bubble was visible.
base::TimeTicks show_time_;
DISALLOW_COPY_AND_ASSIGN(BubbleController); DISALLOW_COPY_AND_ASSIGN(BubbleController);
}; };
......
...@@ -42,9 +42,6 @@ class BubbleDelegate { ...@@ -42,9 +42,6 @@ class BubbleDelegate {
// Return true to indicate the UI was updated. // Return true to indicate the UI was updated.
virtual bool UpdateBubbleUi(BubbleUi* bubble_ui); virtual bool UpdateBubbleUi(BubbleUi* bubble_ui);
// Used to identify a bubble for collecting metrics.
virtual std::string GetName() const = 0;
// If this returns non-null, the bubble will be closed when the returned frame // If this returns non-null, the bubble will be closed when the returned frame
// is destroyed. // is destroyed.
virtual const content::RenderFrameHost* OwningFrame() const = 0; virtual const content::RenderFrameHost* OwningFrame() const = 0;
......
...@@ -49,10 +49,6 @@ class MockBubbleDelegate : public BubbleDelegate { ...@@ -49,10 +49,6 @@ class MockBubbleDelegate : public BubbleDelegate {
return std::move(bubble_ui_); return std::move(bubble_ui_);
} }
MOCK_METHOD1(UpdateBubbleUi, bool(BubbleUi*));
std::string GetName() const override { return "MockBubble"; }
// To verify destructor call. // To verify destructor call.
MOCK_METHOD0(Destroyed, void()); MOCK_METHOD0(Destroyed, void());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment