Good catch, Azeem! Should be a super easy fix, which is nice :-)

Just fyi this may take a little longer to fix correctly than initially thought.  This is because when the suite is disabled, every feature is put in the state kUnavailableSuiteDisabled.  Furthermore, when Messages is enabled while Beto suite is *disabled* (via the PWA), pageContentData (https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/multidevice_page/multidevice_subpage.html?type=cs&q=doesAndroidMessagesRequireSetup_&g=0&l=88) does not change, which is how the ui knows when to update.   

Currently i keep track of the last feature state on the JS side, but it isn't correct in a few of edge user flows (concerning only *disabled* toggle button or *disabled* paper button)  (One case example - disable Beto suite on the main settings page, then enable Messages by pairing with host, then go into multidevice settings page -- instead of a disabled toggle button, it is a disabled 'set up' paper button).

What we should do instead is to propagate the messages pairing state https://cs.chromium.org/chromium/src/chromeos/services/multidevice_setup/public/cpp/android_sms_pairing_state_tracker.h?gsn=AndroidSmsAppHelperDelegate&g=0&l=32, which in combination with the messages FeatureState currently, will let us display the correct *disabled* buttons.  It's a bit hairy, but if yall got any other ideas, lmk pls !

That makes sense - good catch. I think a simpler solution could be to consider the FeatureState to be kFurtherSetupRequired when setup is required, regardless of whether the suite is disabled. This would only require a change in feature_state_manager_impl.cc and in the JS/HTML to change the UI based on this state.

The pros here is that no additional plumbing needed to propagate an extra value into the WebUI. The con is that the FeatureState is a bit ambiguous, since technically the feature is unavailable due to the disabled suite AND requires setup. We could probably mitigate this potential confusion by adding an extra comment to multidevice_setup.mojom. What do you think?

Actually, thinking about this more, I don't think my proposal in comment #5 is the right way to go here, since this will interfere with the ability to know whether pairing was lost (i.e.,  issue 918943 ).

It looks like you will indeed need to pass some extra debug data into the WebUI page. You'll likely want to modify MultideviceHandler [1] to pass this data to the JS layer. Let me know when you start up on this so I can help you understand the basics.

[1] https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/settings/chromeos/multidevice_handler.h

I think we shouldn't try to set the feature state to kFurtherSetupRequired when suite is disabled. Because the messages feature state is used to determine whether or not to install the PWA here: https://cs.chromium.org/chromium/src/chromeos/services/multidevice_setup/android_sms_app_installing_status_observer.cc?rcl=5120c7e4466c232c0d035d88ff9e75eb90ae4e3a&l=74 and this would cause the PWA to be installed even if the suite is disabled.

This issue as originally reported is about the "Setup" button showing up as enabled even when the suite is disabled. This may have actually been partially fixed by this line change https://cs.chromium.org/chromium/src/chromeos/services/multidevice_setup/android_sms_app_installing_status_observer.cc?rcl=5120c7e4466c232c0d035d88ff9e75eb90ae4e3a&l=74 that i added in a different CL. The only issue remaining here is that we show a disabled toggle button instead of a disabled "Setup" button when the suite is disabled. I believe this is the issue @hsuregan is referring to in the comment above. The user won't be able to interact with the button whether we show a disabled toggle button or a disabled "Setup" button so it's arguable whether the added complexity of plumbing the pairing state is worth the improvement.

The other option is to add an intermediate state, kUnavailableSuiteDisabledButRequiresFurtherSetup and show a disabled "Setup" button in that case. But we'd also have to modify android_sms_app_installing_status_observer.cc so that we don't install the PWA in this intermediate state.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/23cc6e08adc7dbee6dc7bed99ef9bd74c3b3950c

commit 23cc6e08adc7dbee6dc7bed99ef9bd74c3b3950c
Author: Regan Hsu <hsuregan@chromium.org>
Date: Fri Jan 18 02:55:28 2019

[CrOS MultiDevice] Add disabled Messages 'Set Up' button.

This CL changes the disabled toggle button to a disabled
'Set Up' button in the case that the suite is disabled,
but Messages has not been set up yet.


Bug:  900798 
Change-Id: Icecb3f5b0f6840ec61f22e3a1143c2135c2f105e
Reviewed-on: https://chromium-review.googlesource.com/c/1406158
Commit-Queue: Regan Hsu <hsuregan@chromium.org>
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623973}
[modify] https://crrev.com/23cc6e08adc7dbee6dc7bed99ef9bd74c3b3950c/chrome/browser/resources/settings/multidevice_page/multidevice_constants.js
[modify] https://crrev.com/23cc6e08adc7dbee6dc7bed99ef9bd74c3b3950c/chrome/browser/resources/settings/multidevice_page/multidevice_subpage.html
[modify] https://crrev.com/23cc6e08adc7dbee6dc7bed99ef9bd74c3b3950c/chrome/browser/resources/settings/multidevice_page/multidevice_subpage.js
[modify] https://crrev.com/23cc6e08adc7dbee6dc7bed99ef9bd74c3b3950c/chrome/browser/ui/webui/settings/chromeos/multidevice_handler.cc
[modify] https://crrev.com/23cc6e08adc7dbee6dc7bed99ef9bd74c3b3950c/chrome/browser/ui/webui/settings/chromeos/multidevice_handler.h
[modify] https://crrev.com/23cc6e08adc7dbee6dc7bed99ef9bd74c3b3950c/chrome/browser/ui/webui/settings/chromeos/multidevice_handler_unittest.cc
[modify] https://crrev.com/23cc6e08adc7dbee6dc7bed99ef9bd74c3b3950c/chrome/browser/ui/webui/settings/md_settings_ui.cc
[modify] https://crrev.com/23cc6e08adc7dbee6dc7bed99ef9bd74c3b3950c/chrome/test/data/webui/settings/multidevice_subpage_tests.js