Commit c0e0977a authored by Leonard Grey's avatar Leonard Grey Committed by Commit Bot

Mac: use last committed URL when enabling/disabling share menu

Previously, we were using chrome::CanEmailPageLocation to check if share
should be enabled, which checked against the current URL, but user the
last committed URL to actually perform the share.

I'm honestly not sure if the non-Mac email behavior is correct and
would like to side-step that for a moment. On the Mac, we really want
the committed URL since we pass along a screenshot to the share service.

This changes adapts the logic of CanEmailPageLocation to the committed
URL and inlines it in the share menu controller.

Bug: 1095807
Change-Id: Icad4f3766335d0156edf6d1078ea06b328cfed06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2261613Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781955}
parent 528c2a7c
...@@ -48,6 +48,17 @@ NSString* const kOpenSharingSubpaneProtocolValue = @"com.apple.share-services"; ...@@ -48,6 +48,17 @@ NSString* const kOpenSharingSubpaneProtocolValue = @"com.apple.share-services";
NSString* const kRemindersSharingServiceName = NSString* const kRemindersSharingServiceName =
@"com.apple.reminders.RemindersShareExtension"; @"com.apple.reminders.RemindersShareExtension";
bool CanShare() {
Browser* last_active_browser = chrome::FindLastActive();
return last_active_browser &&
last_active_browser->location_bar_model()->ShouldDisplayURL() &&
last_active_browser->tab_strip_model()->GetActiveWebContents() &&
last_active_browser->tab_strip_model()
->GetActiveWebContents()
->GetLastCommittedURL()
.is_valid();
}
} // namespace } // namespace
@implementation ShareMenuController { @implementation ShareMenuController {
...@@ -82,12 +93,7 @@ NSString* const kRemindersSharingServiceName = ...@@ -82,12 +93,7 @@ NSString* const kRemindersSharingServiceName =
[menu removeAllItems]; [menu removeAllItems];
[menu setAutoenablesItems:NO]; [menu setAutoenablesItems:NO];
Browser* lastActiveBrowser = chrome::FindLastActive(); bool canShare = CanShare();
BOOL canShare =
lastActiveBrowser != nullptr &&
// Avoid |CanEmailPageLocation| segfault in interactive UI tests
lastActiveBrowser->tab_strip_model()->GetActiveWebContents() != nullptr &&
chrome::CanEmailPageLocation(lastActiveBrowser);
// Using a real URL instead of empty string to avoid system log about relative // Using a real URL instead of empty string to avoid system log about relative
// URLs in the pasteboard. This URL will not actually be shared to, just used // URLs in the pasteboard. This URL will not actually be shared to, just used
// to fetch sharing services that can handle the NSURL type. // to fetch sharing services that can handle the NSURL type.
...@@ -180,6 +186,7 @@ NSString* const kRemindersSharingServiceName = ...@@ -180,6 +186,7 @@ NSString* const kRemindersSharingServiceName =
// Performs the share action using the sharing service represented by |sender|. // Performs the share action using the sharing service represented by |sender|.
- (void)performShare:(NSMenuItem*)sender { - (void)performShare:(NSMenuItem*)sender {
DCHECK(CanShare());
Browser* browser = chrome::FindLastActive(); Browser* browser = chrome::FindLastActive();
DCHECK(browser); DCHECK(browser);
[self saveTransitionDataFromBrowser:browser]; [self saveTransitionDataFromBrowser:browser];
......
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