Re-add support for Chromecast #2493

bsclifton · 2019-05-24T17:27:57Z

Fixes brave/brave-browser#209
Related brave/brave-browser#4631

Revert "Merge pull request #2119 from brave/revert_enabling_chromecast"
This reverts commit 29ea1df, reversing
changes made to c08710a.

Submitter Checklist:

Test Plan:

First Launch

Launch Brave with a new profile
Verify that Cast option is not available in the hamburger menu
Open wireshark, filter traffic for udp.port == 1900 and verify that no traffic is generated by Brave.

Settings Page

Open brave://settings and verify that Extensions > Media Router is disabled
Toggle the switch, relaunch.
Navigate to the hamburger menu and check Cast is available
Click Cast, and verify if you're able to cast your screen to a Chrome-cast device (Note: that signature verification takes some time on first try, so wait for ~10-20 secs before trying this)
Toggle the switch back off, relaunch and verify Cast is not available in the hamburger menu.

Upgrade

Open Brave-Browser without this change
Navigate to brave://flags and enable media router
Relaunch and verify that ChromeCast doesn't work
Open Brave-Browser (use the same profile) with the change
Navigate to brave://settings and confirm Media Router preference is enabled
Verify Cast works as expected.

Settings UI

Navigate to brave://settings
Toggle the Media Router Setting
Verify that a fixed bottom bar requesting a relaunch.
Toggle the Media Router Setting and verify that the bar disappears
Toggle it back on, and navigate to a different page. The bar should disappear
Navigate to brave://settings and the bar should re-appear.

Sync with brave://flags

Navigate to brave flags, set Load Media Router Component Extension to enabled
Verify that Media Router setting in, brave://settings is also enabled
Navigate to brave flags, set Load Media Router Component Extension to disabled
Verify that Media Router setting in, brave://settings is also disabled

Navigate to brave flags, set Media Router Setting to enabled
Verify that the Media Router Flag, in brave://flags is enabled
Navigate to brave flags, set Media Router Setting to disabled
Verify that the Media Router Flag, in brave://flags is disabled

Reviewer Checklist:

New files have MPL-2.0 license header.
Request a security/privacy review as needed.
Adequate test coverage exists to prevent regressions
Verify test plan is specified in PR before merging to source

After-merge Checklist:

The associated issue milestone is set to the smallest version that the
changes has landed on.
All relevant documentation has been updated.

bsclifton · 2019-05-24T17:28:53Z

Errors from prior merge attempt:

12:53:51  NETWORK AUDIT FAIL: https://clients2.google.com/service/update2/crx?os=linux&arch=x64&os_arch=x86_64&nacl_arch=x86-64&prod=chromiumcrx&prodchannel=unknown&prodversion=73.0.64.26&lang=en-US&acceptformat=crx2,crx3&x=id%3Dpkedcjkdefgpdelpbcmbmeomcjbeemfm%26v%3D0.0.0.0%26installedby%3Dother%26uc%26ping%3Dr%253D-1%2526e%253D1
12:53:51  NETWORK AUDIT FAIL: https://clients2.google.com/service/update2/crx?os=linux&arch=x64&os_arch=x86_64&nacl_arch=x86-64&prod=chromiumcrx&prodchannel=unknown&prodversion=73.0.64.26&lang=en-US&acceptformat=crx2,crx3&x=id%3Dpkedcjkdefgpdelpbcmbmeomcjbeemfm%26v%3D0.0.0.0%26installedby%3Dother%26uc%26ping%3Dr%253D-1%2526e%253D1
12:53:51  NETWORK AUDIT FAIL: https://extensionupdater.brave.com/service/update2/crx?os=linux&arch=x64&os_arch=x86_64&nacl_arch=x86-64&prod=chromiumcrx&prodchannel=unknown&prodversion=73.0.64.26&lang=en-US&acceptformat=crx2,crx3&x=id%3Dpkedcjkdefgpdelpbcmbmeomcjbeemfm%26v%3D0.0.0.0%26installedby%3Dother%26uc%26ping%3Dr%253D-1%2526e%253D1
12:53:51  NETWORK AUDIT FAIL: http://redirector.gvt1.com/edgedl/chromewebstore/L2Nocm9tZV9leHRlbnNpb24vYmxvYnMvMjJlQUFXRC12Ny1ldUFnMXF3SDlXZDlFZw/7319.128.0.1_pkedcjkdefgpdelpbcmbmeomcjbeemfm.crx
12:53:51  NETWORK AUDIT FAIL: http://redirector.gvt1.com/edgedl/chromewebstore/L2Nocm9tZV9leHRlbnNpb24vYmxvYnMvMjJlQUFXRC12Ny1ldUFnMXF3SDlXZDlFZw/7319.128.0.1_pkedcjkdefgpdelpbcmbmeomcjbeemfm.crx
12:53:51  NETWORK AUDIT FAIL: http://r6---sn-nx5e6ne6.gvt1.com/edgedl/chromewebstore/L2Nocm9tZV9leHRlbnNpb24vYmxvYnMvMjJlQUFXRC12Ny1ldUFnMXF3SDlXZDlFZw/7319.128.0.1_pkedcjkdefgpdelpbcmbmeomcjbeemfm.crx?cms_redirect=yes&mip=34.219.185.75&mm=28&mn=sn-nx5e6ne6&ms=nvh&mt=1553863827&mv=m&pl=12&shardbypass=yes

simonhong · 2019-06-18T02:45:25Z

As @jumde mentioned, case is still not working due to Disabling extension pkedcjkdefgpdelpbcmbmeomcjbeemfm ('Chrome Media Router') due to install verification failure.

and @jumde tested it is enabled with below patch.

index 32b499da4..9672b8987 100644
--- a/browser/extensions/brave_extension_management.cc
+++ b/browser/extensions/brave_extension_management.cc
@@ -16,6 +16,7 @@
 #include "brave/browser/extensions/brave_tor_client_updater.h"
 #include "chrome/browser/extensions/external_policy_loader.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/extensions/install_verifier.h"
 #include "components/prefs/pref_service.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/extension.h"
@@ -41,6 +42,9 @@ void BraveExtensionManagement::RegisterBraveExtensions() {
       *base::CommandLine::ForCurrentProcess();
   if (!command_line.HasSwitch(switches::kDisableTorClientUpdaterExtension))
     g_brave_browser_process->tor_client_updater()->Register();
+  std::set<std::string> ids;
+  ids.insert("pkedcjkdefgpdelpbcmbmeomcjbeemfm");
+  InstallVerifier::Get(profile_)->AddProvisional(ids);
 }

~~With above patch, Disabling extension pkedcjkdefgpdelpbcmbmeomcjbeemfm ('Chrome Media Router') due to install verification failure. is not visible but still can't get available device list.~~ Works with clean profile.

jumde · 2019-06-18T03:04:20Z

As @jumde mentioned, case is still not working due to Disabling extension pkedcjkdefgpdelpbcmbmeomcjbeemfm ('Chrome Media Router') due to install verification failure.

and @jumde tested it is enabled with below patch.

index 32b499da4..9672b8987 100644
--- a/browser/extensions/brave_extension_management.cc
+++ b/browser/extensions/brave_extension_management.cc
@@ -16,6 +16,7 @@
 #include "brave/browser/extensions/brave_tor_client_updater.h"
 #include "chrome/browser/extensions/external_policy_loader.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/extensions/install_verifier.h"
 #include "components/prefs/pref_service.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/extension.h"
@@ -41,6 +42,9 @@ void BraveExtensionManagement::RegisterBraveExtensions() {
       *base::CommandLine::ForCurrentProcess();
   if (!command_line.HasSwitch(switches::kDisableTorClientUpdaterExtension))
     g_brave_browser_process->tor_client_updater()->Register();
+  std::set<std::string> ids;
+  ids.insert("pkedcjkdefgpdelpbcmbmeomcjbeemfm");
+  InstallVerifier::Get(profile_)->AddProvisional(ids);
 }

With above patch, Disabling extension pkedcjkdefgpdelpbcmbmeomcjbeemfm ('Chrome Media Router') due to install verification failure. is not visible but still can't get available device list.

@simonhong - With this patch I checked that the signature check fails in https://cs.chromium.org/chromium/src/chrome/browser/extensions/install_verifier.cc?l=631. I'm not sure if we're okay with that.

simonhong · 2019-06-18T03:13:46Z

@jumde I edited right before your last comment :)
Device list is fetched properly with clean profile.

diracdeltas · 2019-06-20T00:49:49Z

common/network_constants.cc

@@ -38,6 +42,9 @@ const char kCRLSetPrefix3[] =
 const char kCRLSetPrefix4[] =
    "*://storage.googleapis.com/update-delta/hfnkpimlhhgieaddgfemjhofmfblmnib"
    "/*crxd";
+const char kGoogleAPIPrefix[] = "https://www.googleapis.com/";


out of curiosity, why https instead of * here?

it was not connecting over http but I agree, using * would help if the code fallsback to the non-secure url

simonhong · 2019-06-25T00:34:42Z

@bridiver added cause this PR adds new patch file.

petemill

Thanks for making the restart notification persistent. Minor changes requested to the JS api

petemill · 2019-07-11T22:55:04Z

browser/ui/webui/settings/brave_default_extensions_handler.cc

@@ -63,10 +64,29 @@ void BraveDefaultExtensionsHandler::RegisterMessages() {
      flags_ui::kRestartBrowser,
      base::BindRepeating(&BraveDefaultExtensionsHandler::HandleRestartBrowser,
                          base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(
+      "showRelaunchToast",


👍 on the pattern but please can we call the command getRestartNeeded so that:

it's about the data, not the UI implementation (toast)

it's specific about getting data, and not performing an action

petemill · 2019-07-11T23:01:55Z

browser/resources/settings/brave_default_extensions_page/brave_default_extensions_page.js


  /** @override */
  created: function() {
    this.browserProxy_ = settings.BraveDefaultExtensionsBrowserProxyImpl.getInstance();
+    this.browserProxy_.showRelaunchToast().then(show => {
+      this.hideRestartToast = !show;
+      this.$.needsRestart.hidden = this.hideRestartToast;


This may be a micro-optimization, so consider it a nit: I'd feel more comfortable if this didn't require a DOM change on 99.9% of all Settings page loads (i.e. page loads which don't have the toast showing).
This can be achieved by either:

Preferred: Wrap the element in an <if which tests hideRestartToast (though positive variable name is better, e.g. showRestartToast

Similar pattern to what you have: Create a css class called something like pageNeedsRestart, and only add this class if the variable comes back true.

simonhong · 2019-07-12T00:52:00Z

browser/ui/webui/settings/brave_default_extensions_handler.h


  bool IsExtensionInstalled(const std::string& extension_id) const;
  void OnInstallResult(const std::string& pref_name,
      bool success, const std::string& error,
      extensions::webstore_install::Result result);

  Profile* profile_ = nullptr;
+  bool restartNeeded = false;


nit: restart_needed_ is the chromium style.

simonhong · 2019-07-12T00:52:41Z

browser/ui/webui/settings/brave_default_extensions_handler.cc

+  args->GetBoolean(0, &enabled);
+
+  restartNeeded = !restartNeeded;
+  if (enabled) {


It would be good to add some comments why this is needed.

jumde · 2019-07-15T02:08:07Z

patches/chrome-browser-extensions-install_verifier.cc.patch

+                                          disable_reason::DisableReason* reason,
+                                          base::string16* error) const {
+   CHECK(extension);
+  ReturnFalseIfExtensionShouldSkipSignatureChecking(extension->id())


This happens because for Media Router the signature fetch and check happens out of order. This is likely a chromium bug, but not seen because chromium uses ENFORCE instead of ENFORCE_STRICT by default.(https://cs.chromium.org/chromium/src/chrome/browser/extensions/chrome_content_verifier_delegate.cc?l=110)

But, ENFORCE_STRICT (https://cs.chromium.org/chromium/src/extensions/browser/content_verifier_delegate.h?g=0&l=42) is better for security.

We have multiple options here:

Use ENFORCE over ENFORCE_STRICT similar to chromium.

SkipSignatureVerification for MediaRouter as seen in this PR.

RedoSignature check after the fetch is complete.

Thoughts? @bridiver @diracdeltas @tomlowenthal

cc: @bsclifton @simonhong

Option 2 or 3 sound great to me- I'd vote for option 2 as my choice 😄

i prefer 3 since Google Chrome uses ENFORCE_STRICT (https://cs.chromium.org/chromium/src/chrome/browser/extensions/chrome_content_verifier_delegate.cc?l=68); so IIUC, 3 is the only option that gives us parity with Google Chrome security-wise.

@diracdeltas - ENFORCE_STRICT is only used when field trials are enabled, I don't think Chrome has field trials enabled for release builds. Let me know if I'm missing something.

@diracdeltas I thought it was the other way around? Chromium uses ENFORCE so my preference is to follow that and not patch here

@diracdeltas @bridiver - ENFORCE_STRICT works well for the extensions I have tested, but I understand the concern that we might run into web-compat issues with an experimental feature.

I have a patch for 3 but, I'm okay with 1 to avoid any web-compat risk we might run into.

bridiver · 2019-07-15T21:26:29Z

patches/chrome-browser-ui-webui-flags_ui.cc.patch

+       !args->GetString(1, &enable_str))
+     return;
+
+  SyncFeatureFlagWithBravePrefs(entry_internal_name, Profile::FromWebUI(web_ui()));


I'm confused, I thought we were doing a lazy check to sync with the pref?

if we need notification at the time this changes then we should add a pref observer for kEnabledLabsExperiments

I think I could have named it better, the intention of this call is to update the temporary pref kBraveEnabledMediaRouter that we use in brave://settings when flags are toggled. So toggling the flag correctly toggles the Media Router switch in brave://settings.

this can be done without a patch using a chromium_src override for SetFeatureEntryEnabled which I thought is what we had before and maybe this is why I'm confused here

bridiver · 2019-07-15T21:28:43Z

browser/net/brave_common_static_redirect_network_delegate_helper.cc

@@ -45,13 +45,42 @@ int OnBeforeURLRequest_CommonStaticRedirectWork(
    const ResponseCallback& next_callback,
    std::shared_ptr<BraveRequestInfo> ctx) {
  GURL::Replacements replacements;
+  static URLPattern googleapis_pattern(
+    URLPattern::SCHEME_HTTP | URLPattern::SCHEME_HTTPS, kGoogleAPIPrefix);


lint 4 spaces

bridiver · 2019-07-15T21:33:02Z

browser/ui/webui/settings/brave_default_extensions_handler.cc

+      base::Value(restart_needed_));
+}
+
+void BraveDefaultExtensionsHandler::HandleRestartBrowser(


I believe the correct way to handle this is to just link to kChromeUIRestartURL

Navigation to kChromeUIRestartURL internally calls chrome::AttemptRestart: https://cs.chromium.org/chromium/src/chrome/browser/browser_about_handler.cc?l=98.

We can link to kChromeUIRestartURL in javascript directly but I was just trying to avoid chances of regression if kChromeUIRestartURL changes in future.

I think it is better to use kChromeUIRestartURL than to duplicate the logic here. Not sure why kChromeUIRestartURL would change in the future

@bridiver just noting that the same functionality in chrome://flags does not link to kChromeUIRestartUI but sends a message to a message handler via chrome.send too and calls AttemptRestart. I wonder what would happen to session restore if the tab was navigated away from the settings page to kChromeUIRestartURL. The settings page may not restore, which would be unfortunate. Although I did spot comments around this being a non-navigation url, so maybe it would restore... Still, I wonder why the Flags page doesn't use it.

@petemill because a lot of other things happen in FlagsDOMHandler::HandleRestartBrowser before the restart. The only thing we're doing here is calling AttemptRestart so it's functionally equivalent

DCHECK(flags_storage_); #if defined(OS_CHROMEOS) // On ChromeOS be less intrusive and restart inside the user session after // we apply the newly selected flags. base::CommandLine user_flags(base::CommandLine::NO_PROGRAM); about_flags::ConvertFlagsToSwitches(flags_storage_.get(), &user_flags, flags_ui::kAddSentinels); // Apply additional switches from policy that should not be dropped when // applying flags.. chromeos::UserSessionManager::MaybeAppendPolicySwitches( Profile::FromWebUI(web_ui())->GetPrefs(), &user_flags); base::CommandLine::StringVector flags; // argv[0] is the program name |base::CommandLine::NO_PROGRAM|. flags.assign(user_flags.argv().begin() + 1, user_flags.argv().end()); VLOG(1) << "Restarting to apply per-session flags..."; AccountId account_id = user_manager::UserManager::Get()->GetActiveUser()->GetAccountId(); chromeos::UserSessionManager::GetInstance()->SetSwitchesForUser( account_id, chromeos::UserSessionManager::CommandLineSwitchesType:: kPolicyAndFlagsAndKioskControl, flags); #endif chrome::AttemptRestart(); }

they need all of that code to run on chromeos before attempting the restart

bridiver · 2019-07-15T21:36:28Z

browser/ui/webui/settings/brave_default_extensions_handler.cc

+
+
+  AllowJavascript();
+  ResolveJavascriptCallback(args->GetList()[0].Clone(),


why are you cloning this? Also args->GetList()[0] is not the correct or safe way to get this value.

std::string callback_id; args->GetString(0, &callback_id);

and then pass in base::Value(callback_id)

bridiver · 2019-07-15T21:37:46Z

browser/ui/webui/settings/brave_default_extensions_handler.cc

+  // Adding the media router extension on explicit toggle to prevent
+  // any suprise connections on upgrade
+  if (enabled) {
+    extensions::ExtensionPrefs::Get(profile_)->SetExtensionEnabled(


we shouldn't be directly calling ExtensionPrefs::SetExtensionEnabled. You should enable/disable using the ExtensionRegistry

bridiver · 2019-07-17T20:53:38Z

browser/net/brave_common_static_redirect_network_delegate_helper_unittest.cc

+TEST_F(BraveCommonStaticRedirectNetworkDelegateHelperTest,
+    RedirectChromecastDownload) {
+  net::TestDelegate test_delegate;
+  GURL url(


how do we know this still works if the extension version changes?

changed to random_hash and random_version, the pattern https://github.com/brave/brave-core/pull/2493/files#diff-6337bc2627a3b8782b369e66dc193cb9R47 should catch all changes to hash and version

jumde · 2019-07-18T23:28:52Z

chromium_src/chrome/browser/about_flags.cc

+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#define SetFeatureEntryEnabled SetFeatureEntryEnabled_ChromiumImpl
+#include "../../../../chrome/browser/about_flags.cc"  // NOLINT


Needed because SetFeatureEntryEnabled in about_flags.cc calls, SetFeatureEnabled in flags_state.cc

petemill

Found a minor issue with the restart prompt. Since I've already provided feedback about that which may have led to this issue, I've implemented a proposed change in #2974. Apart from that it all looks good.

Fixes brave/brave-browser#209

petemill

Cast off! ⚓️

bsclifton self-assigned this May 24, 2019

bsclifton force-pushed the readd-chromecast branch from 2d7b50e to 889b4bf Compare May 28, 2019 19:36

bsclifton mentioned this pull request May 28, 2019

Re-add support for Chromecast brave/brave-browser#4631

Closed

29 tasks

bsclifton requested a review from jumde May 28, 2019 23:16

bsclifton added this to the 0.67.x - Nightly milestone May 28, 2019

bsclifton mentioned this pull request May 29, 2019

Enable Chromecast support brave/brave-browser#209

Closed

jumde force-pushed the readd-chromecast branch 2 times, most recently from 24f1944 to 63cea2e Compare June 10, 2019 21:37

simonhong modified the milestones: 0.67.x - Dev, 0.68.x - Nightly Jun 12, 2019

simonhong force-pushed the readd-chromecast branch from c292858 to 3d390cd Compare June 18, 2019 11:41

simonhong changed the title ~~WIP: Re-add support for Chromecast~~ Re-add support for Chromecast Jun 18, 2019

simonhong marked this pull request as ready for review June 18, 2019 11:45

bsclifton added CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS labels Jun 18, 2019

bsclifton mentioned this pull request Jun 18, 2019

Adding Chrome Cast per https://github.com/brave/brave-browser/issues/209 brave/extension-whitelist#17

Closed

diracdeltas reviewed Jun 20, 2019

View reviewed changes

jumde force-pushed the readd-chromecast branch 4 times, most recently from 9ec2881 to 1b13d24 Compare June 24, 2019 22:23

simonhong requested a review from bridiver June 25, 2019 00:34

jumde force-pushed the readd-chromecast branch from ea15f56 to 1b16b5b Compare June 25, 2019 21:54

bsclifton requested a review from bbondy June 25, 2019 22:23

jumde requested a review from diracdeltas June 25, 2019 22:30

bsclifton assigned jumde Jul 11, 2019

petemill requested changes Jul 11, 2019

View reviewed changes

simonhong reviewed Jul 12, 2019

View reviewed changes

jumde force-pushed the readd-chromecast branch 2 times, most recently from c6c6a27 to 10047e1 Compare July 13, 2019 19:26

jumde reviewed Jul 15, 2019

View reviewed changes

jumde force-pushed the readd-chromecast branch from f5da576 to 969710f Compare July 15, 2019 18:52

bridiver reviewed Jul 15, 2019

View reviewed changes

jumde force-pushed the readd-chromecast branch 2 times, most recently from 9bde2f6 to 40ccf3d Compare July 17, 2019 06:28

bridiver reviewed Jul 17, 2019

View reviewed changes

jumde force-pushed the readd-chromecast branch from 302d3a0 to 6a45e60 Compare July 18, 2019 23:14

jumde reviewed Jul 18, 2019

View reviewed changes

simonhong force-pushed the readd-chromecast branch from 14e207c to fb00111 Compare July 22, 2019 06:58

bridiver previously approved these changes Jul 22, 2019

View reviewed changes

petemill mentioned this pull request Jul 22, 2019

Settings: Observe when media router settings change and show or hide restart prompt accordingly #2974

Closed

petemill requested changes Jul 22, 2019

View reviewed changes

jumde dismissed bridiver’s stale review via 931087a July 23, 2019 04:49

jumde force-pushed the readd-chromecast branch from fb00111 to 931087a Compare July 23, 2019 04:49

Re-add support for Chromecast

e0ed3de

Fixes brave/brave-browser#209

jumde force-pushed the readd-chromecast branch from 931087a to e0ed3de Compare July 23, 2019 15:38

petemill approved these changes Jul 23, 2019

View reviewed changes

bsclifton merged commit f56db6d into master Jul 23, 2019

bsclifton modified the milestones: 0.68.x - Dev, 0.69.x - Nightly Jul 23, 2019

bsclifton deleted the readd-chromecast branch August 6, 2019 17:58



		AllowJavascript();
		ResolveJavascriptCallback(args->GetList()[0].Clone(),

Re-add support for Chromecast #2493

Re-add support for Chromecast #2493

Conversation

bsclifton commented May 24, 2019 • edited by jumde Loading

Submitter Checklist:

Test Plan:

Reviewer Checklist:

After-merge Checklist:

bsclifton commented May 24, 2019

simonhong commented Jun 18, 2019 • edited Loading

jumde commented Jun 18, 2019

simonhong commented Jun 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonhong commented Jun 25, 2019

petemill left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bridiver Jul 15, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jumde Jul 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petemill left a comment

Choose a reason for hiding this comment

petemill left a comment

Choose a reason for hiding this comment

bsclifton commented May 24, 2019 •

edited by jumde

Loading

simonhong commented Jun 18, 2019 •

edited

Loading

bridiver Jul 15, 2019 •

edited

Loading

jumde Jul 18, 2019 •

edited

Loading