ScreenCapture for Android phase1, part II

chrome/browser/media/webrtc/screen_capture_infobar_delegate_android.h

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_MEDIA_WEBRTC_SCREEN_CAPTURE_INFOBAR_DELEGATE_ANDROID_H_
+#define CHROME_BROWSER_MEDIA_WEBRTC_SCREEN_CAPTURE_INFOBAR_DELEGATE_ANDROID_H_
+
+#include "chrome/browser/media/media_access_handler.h"
+#include "components/infobars/core/confirm_infobar_delegate.h"
+
+namespace content {
+class WebContents;
+}
+
+// This class configures an infobar shown when a page requests access to a
+// user's device screen. The user is shown a message asking whether they wish to
+// share their screen with the current page, and buttons to give access to the
+// screen to the page, or to deny access to it.

[Peter Kasting, 2016/09/20 23:50:29]

Nit: We can make this less wordy and less passive; how about just:

An infobar that allows the user to share their screen with the current page.

[braveyao, 2016/09/21 00:35:55]

Done.

+class ScreenCaptureInfoBarDelegateAndroid : public ConfirmInfoBarDelegate {
+ public:
+  typedef content::MediaResponseCallback ResponseCallback;

[Peter Kasting, 2016/09/20 23:50:29]

Nit: Does this really buy much over just using the original type directly?

If it really does, prefer a type alias to a typedef.

[braveyao, 2016/09/21 00:35:55]

Done.

+
+  // Static method to create the object

[Peter Kasting, 2016/09/20 23:50:29]

Nit: This comment adds nothing to the function declaration.  Something like this
instead:

Creates a screen capture infobar and delegate and adds the infobar to the
InfoBarService associated with |web_contents|.

[braveyao, 2016/09/21 00:35:55]

Done.

+  static void Create(content::WebContents* web_contents,
+                     const content::MediaStreamRequest& request,
+                     const ResponseCallback& callback);
+
+  explicit ScreenCaptureInfoBarDelegateAndroid(

[Peter Kasting, 2016/09/20 23:50:29]

Do not make the constructor public; that's the point of the Create() method.

[braveyao, 2016/09/21 00:35:55]

Done.

+      content::WebContents* web_contents,
+      const content::MediaStreamRequest& request,
+      const ResponseCallback& callback);
+  ~ScreenCaptureInfoBarDelegateAndroid() override;
+
+  // ConfirmInfoBarDelegate:

[Peter Kasting, 2016/09/20 23:50:29]

Nit: These should be private.

[braveyao, 2016/09/21 00:35:55]

Done.

+  infobars::InfoBarDelegate::InfoBarIdentifier GetIdentifier() const override;
+  ScreenCaptureInfoBarDelegateAndroid* AsScreenCaptureInfoBarDelegateAndroid()
+      override;
+  base::string16 GetMessageText() const override;
+  int GetIconId() const override;
+  base::string16 GetButtonLabel(InfoBarButton button) const override;
+  bool Accept() override;
+  bool Cancel() override;
+  void InfoBarDismissed() override;
+
+ private:
+  void RunCallback(content::MediaStreamRequestResult result);

[Peter Kasting, 2016/09/20 23:50:29]

Nit: This deserves a comment, since the implementation is nontrivial.

[braveyao, 2016/09/21 00:35:55]

Done.

+
+  content::WebContents* web_contents_;
+  const content::MediaStreamRequest request_;
+  ResponseCallback callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScreenCaptureInfoBarDelegateAndroid);
+};
+
+#endif  // CHROME_BROWSER_MEDIA_WEBRTC_SCREEN_CAPTURE_INFOBAR_DELEGATE_ANDROID_H_