Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(935)

Unified Diff: third_party/WebKit/Source/modules/media_controls/elements/MediaControlDownloadButtonElement.cpp

Issue 2943983003: chrome/blink: Add functionality for in-product help for media elements. (Closed)
Patch Set: not on pause. Created 3 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/modules/media_controls/elements/MediaControlDownloadButtonElement.cpp
diff --git a/third_party/WebKit/Source/modules/media_controls/elements/MediaControlDownloadButtonElement.cpp b/third_party/WebKit/Source/modules/media_controls/elements/MediaControlDownloadButtonElement.cpp
index 2007c5d81496f008d9a3228e482bb144f32c404f..61130cbdf15553698dcdcd0f923cce4620615eda 100644
--- a/third_party/WebKit/Source/modules/media_controls/elements/MediaControlDownloadButtonElement.cpp
+++ b/third_party/WebKit/Source/modules/media_controls/elements/MediaControlDownloadButtonElement.cpp
@@ -6,6 +6,7 @@
#include "core/InputTypeNames.h"
#include "core/events/Event.h"
+#include "core/frame/LocalFrameClient.h"
#include "core/frame/Settings.h"
#include "core/html/HTMLAnchorElement.h"
#include "core/html/HTMLMediaElement.h"
@@ -14,7 +15,9 @@
#include "core/page/Page.h"
#include "modules/media_controls/MediaControlsImpl.h"
#include "platform/Histogram.h"
+#include "platform/wtf/Functional.h"
#include "public/platform/Platform.h"
+#include "services/service_manager/public/cpp/interface_provider.h"
namespace blink {
@@ -91,11 +94,14 @@ void MediaControlDownloadButtonElement::SetIsWanted(bool wanted) {
if (!IsWanted())
return;
- DCHECK(IsWanted());
if (!show_use_counted_) {
show_use_counted_ = true;
RecordMetrics(DownloadActionMetrics::kShown);
}
+
+ // If the button is wanted, show IPH if possible.
+ MaybeDispatchInProductHelpTrigger();
+ UpdateInProductHelpState();
}
DEFINE_TRACE(MediaControlDownloadButtonElement) {
@@ -132,4 +138,53 @@ void MediaControlDownloadButtonElement::RecordMetrics(
download_action_histogram.Count(static_cast<int>(metric));
}
+void MediaControlDownloadButtonElement::MaybeDispatchInProductHelpTrigger() {
mlamouri (slow - plz ping) 2017/07/13 21:14:27 For what I can see, this setting up the mojo pipe.
Khushal 2017/07/17 17:52:21 MaybeShow would have called UpdateInProductHelpSta
+ if (!media_download_in_product_trigger_observed_ && IsWanted() &&
+ !MediaElement().paused() && MediaElement().GetDocument().GetSettings() &&
+ MediaElement()
+ .GetDocument()
+ .GetSettings()
+ ->GetMediaDownloadInProductHelpEnabled()) {
mlamouri (slow - plz ping) 2017/07/13 21:14:27 For readability, can you do an early return?
Khushal 2017/07/17 17:52:21 Done.
+ DCHECK(!media_in_product_help_);
+
+ media_download_in_product_trigger_observed_ = true;
+ media_in_product_help_ =
+ WTF::MakeUnique<mojom::blink::MediaInProductHelpPtr>();
+ MediaElement()
+ .GetDocument()
+ .GetFrame()
mlamouri (slow - plz ping) 2017/07/13 21:14:27 Can you check that `GetFrame()` doesn't return `nu
Khushal 2017/07/17 17:52:21 Done.
+ ->Client()
+ ->GetInterfaceProvider()
+ ->GetInterface(mojo::MakeRequest(media_in_product_help_.get()));
+ media_in_product_help_->set_connection_error_handler(
+ ConvertToBaseCallback(WTF::Bind(
+ &MediaControlDownloadButtonElement::DismissDownloadInProductHelp,
+ WrapWeakPersistent(this))));
+ GetMediaControls().MaybeShow();
mlamouri (slow - plz ping) 2017/07/13 21:14:27 Does that mean that when a download button will be
Khushal 2017/07/17 17:52:21 I updated the code to take the CanShow aspect into
+ }
+}
+
+void MediaControlDownloadButtonElement::UpdateInProductHelpState() {
mlamouri (slow - plz ping) 2017/07/13 21:14:27 Isn't this method actually triggering the UI?
Khushal 2017/07/17 17:52:21 Restructured to move this into MaybeDispatchInProd
+ if (!media_in_product_help_)
+ return;
+
+ if (static_cast<MediaControlsImpl&>(GetMediaControls()).IsVisible() &&
+ IsWanted()) {
+ (*media_in_product_help_)
+ ->ShowInProductHelpWidget(VisibleBoundsInVisualViewport());
+ return;
+ }
+
+ DismissDownloadInProductHelp();
+}
+
+void MediaControlDownloadButtonElement::DismissDownloadInProductHelp() {
+ media_in_product_help_.reset();
+ static_cast<MediaControlsImpl&>(GetMediaControls()).HideControlsIfNecessary();
+}
+
+bool MediaControlDownloadButtonElement::IsShowingInProductHelp() const {
+ return !!media_in_product_help_;
+}
+
} // namespace blink

Powered by Google App Engine
This is Rietveld 408576698