diff --git a/chrome/browser/permissions/permission_context_base.h b/chrome/browser/permissions/permission_context_base.h index d39cd53..e42a665 100644 --- a/chrome/browser/permissions/permission_context_base.h +++ b/chrome/browser/permissions/permission_context_base.h
@@ -42,8 +42,6 @@ // - Define your new permission in the ContentSettingsType enum. // - Create a class that inherits from PermissionContextBase and passes the // new permission. -// - Inherit from PermissionInfobarDelegate and implement -// |GetMessageText| // - Edit the PermissionRequestImpl methods to add the new text. // - Hit several asserts for the missing plumbing and fix them :) // After this you can override several other methods to customize behavior, @@ -70,9 +68,8 @@ // PermissionContextBase can use it. static const char kPermissionsKillSwitchBlockedValue[]; - // The renderer is requesting permission to push messages. - // When the answer to a permission request has been determined, |callback| - // should be called with the result. + // |callback| is called upon resolution of the request, but not if a prompt + // is shown and ignored. virtual void RequestPermission(content::WebContents* web_contents, const PermissionRequestID& id, const GURL& requesting_frame, @@ -117,9 +114,8 @@ const GURL& requesting_origin, const GURL& embedding_origin) const; - // Decide whether the permission should be granted. - // Calls PermissionDecided if permission can be decided non-interactively, - // or NotifyPermissionSet if permission decided by presenting an infobar. + // Called if generic checks (existing content setting, embargo, etc.) fail to + // resolve a permission request. The default implementation prompts the user. virtual void DecidePermission(content::WebContents* web_contents, const PermissionRequestID& id, const GURL& requesting_origin, @@ -127,15 +123,8 @@ bool user_gesture, const BrowserPermissionCallback& callback); - // Called when permission is granted without interactively asking the user. - void PermissionDecided(const PermissionRequestID& id, - const GURL& requesting_origin, - const GURL& embedding_origin, - bool user_gesture, - const BrowserPermissionCallback& callback, - bool persist, - ContentSetting content_setting); - + // Updates stored content setting if persist is set, updates tab indicators + // and runs the callback to finish the request. virtual void NotifyPermissionSet(const PermissionRequestID& id, const GURL& requesting_origin, const GURL& embedding_origin, @@ -185,6 +174,16 @@ const BrowserPermissionCallback& callback, bool permission_blocked); + // This is the callback for PermissionRequestImpl and is called once the user + // allows/blocks/dismisses a permission prompt. + void PermissionDecided(const PermissionRequestID& id, + const GURL& requesting_origin, + const GURL& embedding_origin, + bool user_gesture, + const BrowserPermissionCallback& callback, + bool persist, + ContentSetting content_setting); + // Called when the user has made a permission decision. This is a hook for // descendent classes to do appropriate things they might need to do when this // happens.
diff --git a/chrome/browser/ui/page_info/page_info_ui.cc b/chrome/browser/ui/page_info/page_info_ui.cc index 4182652..a140037 100644 --- a/chrome/browser/ui/page_info/page_info_ui.cc +++ b/chrome/browser/ui/page_info/page_info_ui.cc
@@ -474,6 +474,11 @@ const gfx::ImageSkia PageInfoUI::GetCertificateIcon() { return gfx::CreateVectorIcon(kCertificateIcon, 16, gfx::kChromeIconGrey); } + +// static +const gfx::ImageSkia PageInfoUI::GetSiteSettingsIcon() { + return gfx::CreateVectorIcon(kSettingsIcon, 16, gfx::kChromeIconGrey); +} #endif // static
diff --git a/chrome/browser/ui/page_info/page_info_ui.h b/chrome/browser/ui/page_info/page_info_ui.h index e2343e30..0dd659f 100644 --- a/chrome/browser/ui/page_info/page_info_ui.h +++ b/chrome/browser/ui/page_info/page_info_ui.h
@@ -196,8 +196,11 @@ // Returns the connection icon ID for the given connection |status|. static int GetConnectionIconID(PageInfo::SiteConnectionStatus status); #else - // Returns the icon for the Certificate area. + // Returns the icon for the page Certificate. static const gfx::ImageSkia GetCertificateIcon(); + + // Returns the icon for the button / link to Site settings. + static const gfx::ImageSkia GetSiteSettingsIcon(); #endif // Return true if the given ContentSettingsType is in PageInfoUI.
diff --git a/chrome/browser/ui/views/hover_button.cc b/chrome/browser/ui/views/hover_button.cc index 3ac9555..ba8f42f 100644 --- a/chrome/browser/ui/views/hover_button.cc +++ b/chrome/browser/ui/views/hover_button.cc
@@ -13,6 +13,7 @@ #include "ui/views/animation/ink_drop_ripple.h" #include "ui/views/background.h" #include "ui/views/border.h" +#include "ui/views/controls/styled_label.h" #include "ui/views/focus/focus_manager.h" #include "ui/views/layout/grid_layout.h" @@ -26,6 +27,34 @@ horz_spacing); } +// Sets the accessible name of |parent| to the text from |first| and |second|. +// Also set the combined text as the tooltip for |parent| if either |first| or +// |second|'s text is cut off or elided. +void SetTooltipAndAccessibleName(views::Button* parent, + views::StyledLabel* first, + views::Label* second, + const gfx::Rect& available_space, + int taken_width) { + const base::string16 accessible_name = + second == nullptr ? first->text() + : base::JoinString({first->text(), second->text()}, + base::ASCIIToUTF16("\n")); + + // |views::StyledLabel|s only add tooltips for any links they may have. + // However, since |HoverButton| will never insert a link inside its child + // |StyledLabel|, decide whether it needs a tooltip by checking whether the + // available space is smaller than its preferred size. + bool first_truncated = first->GetPreferredSize().width() > + (available_space.width() - taken_width); + base::string16 second_tooltip; + if (second != nullptr) + second->GetTooltipText(gfx::Point(), &second_tooltip); + parent->SetTooltipText(first_truncated || !second_tooltip.empty() + ? accessible_name + : base::string16()); + parent->SetAccessibleName(accessible_name); +} + } // namespace HoverButton::HoverButton(views::ButtonListener* button_listener, @@ -102,9 +131,14 @@ columns->AddColumn(views::GridLayout::CENTER, views::GridLayout::CENTER, kFixed, views::GridLayout::USE_PREF, 0, 0); columns->AddPaddingColumn(kFixed, icon_label_spacing); - columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER, + columns->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL, kStretchy, views::GridLayout::USE_PREF, 0, 0); + taken_width_ = GetInsets().width() + icon_view->GetPreferredSize().width() + + icon_label_spacing; + + // Make sure hovering over the icon also hovers the |HoverButton|. + icon_view->set_can_process_events_within_subtree(false); // Don't cover |icon_view| when the ink drops are being painted. |LabelButton| // already does this with its own image. icon_view->SetPaintToLayer(); @@ -112,36 +146,36 @@ // Split the two rows evenly between the total height minus the padding. const int row_height = (total_height - remaining_vert_spacing * 2) / num_labels; - grid_layout->StartRow(0, 0, row_height); + grid_layout->StartRow(0, kColumnSetId, row_height); grid_layout->AddView(icon_view.release(), 1, num_labels); - title_ = new views::Label(title); - grid_layout->AddView(title_); + title_ = new views::StyledLabel(title, nullptr); + // Size without a maximum width to get a single line label. + title_->SizeToFit(0); + // |views::StyledLabel|s are all multi-line. With a layout manager, + // |StyledLabel| will try use the available space to size itself, and long + // titles will wrap to the next line (for smaller |HoverButton|s, this will + // also cover up |subtitle_|). Wrap it in a parent view with no layout manager + // to ensure it keeps its original size set by SizeToFit() above. Long titles + // will then be truncated. + views::View* title_wrapper = new views::View; + title_wrapper->AddChildView(title_); + // Hover the whole button when hovering |title_|. This is OK because |title_| + // will never have a link in it. + title_wrapper->set_can_process_events_within_subtree(false); + grid_layout->AddView(title_wrapper); if (!subtitle.empty()) { - grid_layout->StartRow(0, 0, row_height); + grid_layout->StartRow(0, kColumnSetId, row_height); subtitle_ = new views::Label(subtitle, views::style::CONTEXT_BUTTON, STYLE_SECONDARY); subtitle_->SetHorizontalAlignment(gfx::ALIGN_LEFT); subtitle_->SetAutoColorReadabilityEnabled(false); grid_layout->SkipColumns(1); grid_layout->AddView(subtitle_); - - const base::string16 accessible_name = - base::JoinString({title, subtitle}, base::ASCIIToUTF16("\n")); - // Only set a tooltip if either |title_| or |subtitle_| are too long. - base::string16 title_tooltip, subtitle_tooltip; - title_->GetTooltipText(gfx::Point(), &title_tooltip); - subtitle_->GetTooltipText(gfx::Point(), &subtitle_tooltip); - if (!title_tooltip.empty() || !subtitle_tooltip.empty()) { - // Setting the tooltip text also happens to set the accessible name. - SetTooltipText(accessible_name); - } else { - SetAccessibleName(accessible_name); - } - } else { - SetAccessibleName(title); } + SetTooltipAndAccessibleName(this, title_, subtitle_, GetLocalBounds(), + taken_width_); // Since this constructor doesn't use the |LabelButton|'s image / label Views, // make sure the minimum size is correct according to the |grid_layout|. @@ -156,6 +190,21 @@ subtitle_->SetElideBehavior(elide_behavior); } +void HoverButton::SetTitleTextWithHintRange(const base::string16& title_text, + const gfx::Range& range) { + DCHECK(title_); + title_->SetText(title_text); + + if (range.IsValid()) { + views::StyledLabel::RangeStyleInfo style_info; + style_info.text_style = STYLE_SECONDARY; + title_->AddStyleRange(range, style_info); + } + title_->SizeToFit(0); + SetTooltipAndAccessibleName(this, title_, subtitle_, GetLocalBounds(), + taken_width_); +} + void HoverButton::StateChanged(ButtonState old_state) { LabelButton::StateChanged(old_state); @@ -190,13 +239,31 @@ return highlight; } +void HoverButton::Layout() { + LabelButton::Layout(); + + // Vertically center |title_| manually since it doesn't have a LayoutManager. + if (title_) { + DCHECK(title_->parent()); + int y_center = title_->parent()->height() / 2 - title_->size().height() / 2; + title_->SetPosition(gfx::Point(title_->x(), y_center)); + } +} + views::View* HoverButton::GetTooltipHandlerForPoint(const gfx::Point& point) { if (!HitTestPoint(point)) return nullptr; // If possible, take advantage of the |views::Label| tooltip behavior, which // only sets the tooltip when the text is too long. - if (title_ && subtitle_) + if (title_) return this; - return title_ ? title_ : label(); + return label(); +} + +void HoverButton::OnBoundsChanged(const gfx::Rect& previous_bounds) { + if (title_) { + SetTooltipAndAccessibleName(this, title_, subtitle_, GetLocalBounds(), + taken_width_); + } }
diff --git a/chrome/browser/ui/views/hover_button.h b/chrome/browser/ui/views/hover_button.h index ce5496eb..775308c 100644 --- a/chrome/browser/ui/views/hover_button.h +++ b/chrome/browser/ui/views/hover_button.h
@@ -16,6 +16,7 @@ namespace views { class ButtonListener; class Label; +class StyledLabel; class View; } // namespace views @@ -42,6 +43,15 @@ ~HoverButton() override; + // Updates the title text, and applies the secondary style to the text + // specified by |range|. If |range| is invalid, no style is applied. This + // method is only supported for |HoverButton|s created with a title and + // subtitle. + void SetTitleTextWithHintRange(const base::string16& title_text, + const gfx::Range& range); + + // This method is only supported for |HoverButton|s created with a title and + // non-empty subtitle. void SetSubtitleElideBehavior(gfx::ElideBehavior elide_behavior); protected: @@ -55,12 +65,18 @@ const override; // views::View: + void Layout() override; views::View* GetTooltipHandlerForPoint(const gfx::Point& point) override; + void OnBoundsChanged(const gfx::Rect& previous_bounds) override; private: - views::Label* title_; + views::StyledLabel* title_; views::Label* subtitle_; + // The horizontal space the padding and icon take up. Used for calculating the + // available space for |title_|, if it exists. + int taken_width_ = 0; + DISALLOW_COPY_AND_ASSIGN(HoverButton); };
diff --git a/chrome/browser/ui/views/page_info/page_info_bubble_view.cc b/chrome/browser/ui/views/page_info/page_info_bubble_view.cc index 40e42c67..061e522 100644 --- a/chrome/browser/ui/views/page_info/page_info_bubble_view.cc +++ b/chrome/browser/ui/views/page_info/page_info_bubble_view.cc
@@ -30,6 +30,7 @@ #include "chrome/browser/ui/views/collected_cookies_views.h" #include "chrome/browser/ui/views/harmony/chrome_layout_provider.h" #include "chrome/browser/ui/views/harmony/chrome_typography.h" +#include "chrome/browser/ui/views/hover_button.h" #include "chrome/browser/ui/views/page_info/chosen_object_row.h" #include "chrome/browser/ui/views/page_info/non_accessible_image_view.h" #include "chrome/browser/ui/views/page_info/permission_selector_row.h" @@ -94,6 +95,10 @@ constexpr int kMinBubbleWidth = 320; constexpr int kMaxBubbleWidth = 1000; +bool UseHarmonyStyle() { + return ui::MaterialDesignController::IsSecondaryUiMaterial(); +} + // Adds a ColumnSet on |layout| with a single View column and padding columns // on either side of it with |margin| width. void AddColumnWithSideMargin(GridLayout* layout, int margin, int id) { @@ -104,57 +109,122 @@ column_set->AddPaddingColumn(0, margin); } -// Creates a section containing a title, icon, and link. Used to display -// Cookies and Certificate information. +// Creates a section containing a title, icon, and link. Used to display Cookies +// and Certificate information. Hovering over the |subtitle_text| will show the +// |tooltip_text|. // *----------------------------------------------* -// | Icon | Title (Cookies/Certificate) | +// | Icon | Title |title_resource_id| string | // |----------------------------------------------| -// | | Link | +// | | Link |subtitle_text| | // *----------------------------------------------* -views::View* CreateInspectLinkSection(const gfx::ImageSkia& image_icon, - const int title_id, - views::Link* link) { - views::View* new_view = new views::View(); +views::View* CreateMoreInfoLinkSection(views::LinkListener* listener, + const gfx::ImageSkia& image_icon, + int title_resource_id, + const base::string16& subtitle_text, + int click_target_id, + const base::string16& tooltip_text, + views::Link** link) { + *link = new views::Link(subtitle_text); + (*link)->set_id(click_target_id); + (*link)->set_listener(listener); + (*link)->SetUnderline(false); + (*link)->SetTooltipText(tooltip_text); + views::View* new_view = new views::View(); GridLayout* layout = GridLayout::CreateAndInstall(new_view); + ChromeLayoutProvider* provider = ChromeLayoutProvider::Get(); + const int side_margin = + provider->GetInsetsMetric(views::INSETS_DIALOG_SUBSECTION).left(); + const int vert_spacing = + provider->GetDistanceMetric(DISTANCE_CONTROL_LIST_VERTICAL) / 2; const int column = 0; views::ColumnSet* column_set = layout->AddColumnSet(column); + column_set->AddPaddingColumn(0, side_margin); column_set->AddColumn(GridLayout::CENTER, GridLayout::CENTER, 0, GridLayout::FIXED, PageInfoBubbleView::kIconColumnWidth, 0); - column_set->AddPaddingColumn(0, - ChromeLayoutProvider::Get()->GetDistanceMetric( - views::DISTANCE_RELATED_LABEL_HORIZONTAL)); + column_set->AddPaddingColumn( + 0, provider->GetDistanceMetric(views::DISTANCE_RELATED_LABEL_HORIZONTAL)); column_set->AddColumn(GridLayout::LEADING, GridLayout::FILL, 0, GridLayout::USE_PREF, 0, 0); + column_set->AddPaddingColumn(0, side_margin); - layout->StartRow(1, column); + layout->StartRowWithPadding(1, column, 0, vert_spacing); views::ImageView* icon = new NonAccessibleImageView(); icon->SetImage(image_icon); layout->AddView(icon); views::Label* title_label = new views::Label( - l10n_util::GetStringUTF16(title_id), CONTEXT_BODY_TEXT_LARGE); + l10n_util::GetStringUTF16(title_resource_id), CONTEXT_BODY_TEXT_LARGE); layout->AddView(title_label); layout->StartRow(1, column); layout->SkipColumns(1); - layout->AddView(link); + layout->AddView(*link); + layout->AddPaddingRow(0, vert_spacing); return new_view; } -views::View* CreateSiteSettingsLink(const int side_margin, - views::LinkListener* listener) { +// Formats strings and returns the |gfx::Range| of the newly inserted string. +gfx::Range GetRangeForFormatString(int string_id, + const base::string16& insert_string, + base::string16* final_string) { + size_t offset; + *final_string = l10n_util::GetStringFUTF16(string_id, insert_string, &offset); + return gfx::Range(offset, offset + insert_string.length()); +} + +// Creates a button that formats the string given by |title_resource_id| with +// |secondary_text| and displays the latter part in the secondary text color. +std::unique_ptr<HoverButton> CreateMoreInfoButton( + views::ButtonListener* listener, + const gfx::ImageSkia& image_icon, + int title_resource_id, + const base::string16& secondary_text, + int click_target_id, + const base::string16& tooltip_text) { + auto icon = std::make_unique<NonAccessibleImageView>(); + icon->SetImage(image_icon); + auto button = std::make_unique<HoverButton>( + listener, std::move(icon), base::string16(), base::string16()); + + if (secondary_text.empty()) { + button->SetTitleTextWithHintRange( + l10n_util::GetStringUTF16(title_resource_id), + gfx::Range::InvalidRange()); + } else { + base::string16 title_text; + gfx::Range secondary_text_range = + GetRangeForFormatString(title_resource_id, secondary_text, &title_text); + button->SetTitleTextWithHintRange(title_text, secondary_text_range); + } + + button->set_id(click_target_id); + button->SetTooltipText(tooltip_text); + return button; +} + +std::unique_ptr<views::View> CreateSiteSettingsLink( + const int side_margin, + PageInfoBubbleView* listener) { + const base::string16& tooltip = + l10n_util::GetStringUTF16(IDS_PAGE_INFO_SITE_SETTINGS_TOOLTIP); + if (UseHarmonyStyle()) { + return CreateMoreInfoButton( + listener, PageInfoUI::GetSiteSettingsIcon(), + IDS_PAGE_INFO_SITE_SETTINGS_LINK, base::string16(), + PageInfoBubbleView::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_SITE_SETTINGS, + tooltip); + } views::Link* site_settings_link = new views::Link( l10n_util::GetStringUTF16(IDS_PAGE_INFO_SITE_SETTINGS_LINK)); site_settings_link->set_id( - PageInfoBubbleView::VIEW_ID_PAGE_INFO_LINK_SITE_SETTINGS); - site_settings_link->SetTooltipText( - l10n_util::GetStringUTF16(IDS_PAGE_INFO_SITE_SETTINGS_TOOLTIP)); + PageInfoBubbleView::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_SITE_SETTINGS); + site_settings_link->SetTooltipText(tooltip); site_settings_link->set_listener(listener); site_settings_link->SetUnderline(false); - views::View* link_section = new views::View(); + auto link_section = std::make_unique<views::View>(); link_section->SetLayoutManager(new views::BoxLayout( views::BoxLayout::kHorizontal, gfx::Insets(0, side_margin))); link_section->AddChildView(site_settings_link); @@ -520,7 +590,8 @@ profile_(profile), header_(nullptr), site_settings_view_(nullptr), - cookie_dialog_link_(nullptr), + cookie_link_legacy_(nullptr), + cookie_button_(nullptr), weak_factory_(this) { g_shown_bubble_type = BUBBLE_PAGE_INFO; g_page_info_bubble = this; @@ -537,7 +608,17 @@ // bubble. const int side_margin = margins().left(); DCHECK_EQ(margins().left(), margins().right()); - set_margins(gfx::Insets(margins().top(), 0, margins().bottom(), 0)); + + ChromeLayoutProvider* layout_provider = ChromeLayoutProvider::Get(); + + // In Harmony, the last view is a HoverButton, which overrides the bottom + // dialog inset in favor of its own. Note the multi-button value is used here + // assuming that the "Cookies" & "Site settings" buttons will always be shown. + const int hover_list_spacing = + layout_provider->GetDistanceMetric(DISTANCE_CONTENT_LIST_VERTICAL_MULTI); + const int bottom_margin = + UseHarmonyStyle() ? hover_list_spacing : margins().bottom(); + set_margins(gfx::Insets(margins().top(), 0, bottom_margin, 0)); GridLayout* layout = GridLayout::CreateAndInstall(this); constexpr int kColumnId = 0; @@ -556,25 +637,23 @@ layout->StartRow(0, kColumnId); layout->AddView(new views::Separator()); - ChromeLayoutProvider* layout_provider = ChromeLayoutProvider::Get(); - const int vertical_spacing = layout_provider->GetDistanceMetric( - views::DISTANCE_UNRELATED_CONTROL_VERTICAL); - layout->StartRowWithPadding(0, kColumnId, 0, vertical_spacing); - site_settings_view_ = CreateSiteSettingsView(side_margin); + // The views inside |site_settings_view_| have their own padding, so subtract + // that from the actual padding needed to get the correct value. + const int vertical_spacing = + layout_provider->GetDistanceMetric( + views::DISTANCE_UNRELATED_CONTROL_VERTICAL) - + layout_provider->GetDistanceMetric(DISTANCE_CONTROL_LIST_VERTICAL) / 2; + layout->StartRowWithPadding( + 0, kColumnId, 0, + UseHarmonyStyle() ? hover_list_spacing : vertical_spacing); + site_settings_view_ = CreateSiteSettingsView(); layout->AddView(site_settings_view_); - layout->StartRowWithPadding(0, kColumnId, 0, vertical_spacing); - layout->AddView(CreateSiteSettingsLink(side_margin, this)); + layout->StartRowWithPadding(0, kColumnId, 0, + UseHarmonyStyle() ? 0 : vertical_spacing); + layout->AddView(CreateSiteSettingsLink(side_margin, this).release()); - if (!ui::MaterialDesignController::IsSecondaryUiMaterial()) { - // In non-material, titles are inset from the dialog margin. Ensure the - // horizontal insets match. - set_title_margins(gfx::Insets( - layout_provider->GetInsetsMetric(views::INSETS_DIALOG).top(), - side_margin, 0, side_margin)); - } views::BubbleDialogDelegateView::CreateBubble(this); - presenter_.reset(new PageInfo( this, profile, TabSpecificContentSettings::FromWebContents(web_contents), web_contents, url, security_info)); @@ -644,19 +723,19 @@ GetWidget()->Close(); presenter_->OnWhitelistPasswordReuseButtonPressed(web_contents()); break; + case PageInfoBubbleView::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_SITE_SETTINGS: + case PageInfoBubbleView::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_COOKIE_DIALOG: + case PageInfoBubbleView:: + VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_CERTIFICATE_VIEWER: + HandleMoreInfoRequest(button); + break; default: NOTREACHED(); } } void PageInfoBubbleView::LinkClicked(views::Link* source, int event_flags) { - // The bubble closes automatically when the collected cookies dialog or the - // certificate viewer opens. So delay handling of the link clicked to avoid - // a crash in the base class which needs to complete the mouse event handling. - content::BrowserThread::PostTask( - content::BrowserThread::UI, FROM_HERE, - base::BindOnce(&PageInfoBubbleView::HandleLinkClickedAsync, - weak_factory_.GetWeakPtr(), source)); + HandleMoreInfoRequest(source); } gfx::Size PageInfoBubbleView::CalculatePreferredSize() const { @@ -676,17 +755,28 @@ } void PageInfoBubbleView::SetCookieInfo(const CookieInfoList& cookie_info_list) { - // This method gets called each time site data is updated, so if the cookie - // link already exists, replace the text instead of creating a new one. - if (cookie_dialog_link_ == nullptr) { - // Create the link for the Cookies section. - cookie_dialog_link_ = new views::Link( - l10n_util::GetPluralStringFUTF16(IDS_PAGE_INFO_NUM_COOKIES, 0)); - cookie_dialog_link_->set_id( - PageInfoBubbleView::VIEW_ID_PAGE_INFO_LINK_COOKIE_DIALOG); - cookie_dialog_link_->set_listener(this); - cookie_dialog_link_->SetUnderline(false); + // Calculate the number of cookies used by this site. |cookie_info_list| + // should only ever have 2 items: first- and third-party cookies. + DCHECK_EQ(cookie_info_list.size(), 2u); + int total_allowed = 0; + for (const auto& i : cookie_info_list) { + total_allowed += i.allowed; + } + // Get the string to display the number of cookies. + base::string16 num_cookies_text; + if (UseHarmonyStyle()) { + num_cookies_text = l10n_util::GetPluralStringFUTF16( + IDS_PAGE_INFO_NUM_COOKIES_PARENTHESIZED, total_allowed); + } else { + num_cookies_text = l10n_util::GetPluralStringFUTF16( + IDS_PAGE_INFO_NUM_COOKIES, total_allowed); + } + + // Create the cookie link / button if it doesn't yet exist. This method gets + // called each time site data is updated, so if it *does* already exist, skip + // this part and just update the text. + if (cookie_link_legacy_ == nullptr && cookie_button_ == nullptr) { // Get the icon. PageInfoUI::PermissionInfo info; info.type = CONTENT_SETTINGS_TYPE_COOKIES; @@ -697,22 +787,34 @@ const gfx::ImageSkia icon = PageInfoUI::GetPermissionIcon(info).AsImageSkia(); - site_settings_view_->AddChildView(CreateInspectLinkSection( - icon, IDS_PAGE_INFO_COOKIES, cookie_dialog_link_)); + const base::string16& tooltip = + l10n_util::GetStringUTF16(IDS_PAGE_INFO_COOKIES_TOOLTIP); + + if (UseHarmonyStyle()) { + cookie_button_ = + CreateMoreInfoButton( + this, icon, IDS_PAGE_INFO_COOKIES_BUTTON_TEXT, num_cookies_text, + VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_COOKIE_DIALOG, tooltip) + .release(); + site_settings_view_->AddChildView(cookie_button_); + } else { + site_settings_view_->AddChildView(CreateMoreInfoLinkSection( + this, icon, IDS_PAGE_INFO_COOKIES, num_cookies_text, + VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_COOKIE_DIALOG, tooltip, + &cookie_link_legacy_)); + } } - // Calculate the number of cookies used by this site. |cookie_info_list| - // should only ever have 2 items: first- and third-party cookies. - DCHECK_EQ(cookie_info_list.size(), 2u); - int total_allowed = 0; - for (const auto& i : cookie_info_list) { - total_allowed += i.allowed; + // Update the text displaying the number of allowed cookies. + DCHECK((cookie_link_legacy_ == nullptr) != (cookie_button_ == nullptr)); + if (UseHarmonyStyle()) { + base::string16 button_text; + gfx::Range styled_range = GetRangeForFormatString( + IDS_PAGE_INFO_COOKIES_BUTTON_TEXT, num_cookies_text, &button_text); + cookie_button_->SetTitleTextWithHintRange(button_text, styled_range); + } else { + cookie_link_legacy_->SetText(num_cookies_text); } - base::string16 label_text = l10n_util::GetPluralStringFUTF16( - IDS_PAGE_INFO_NUM_COOKIES, total_allowed); - cookie_dialog_link_->SetText(label_text); - cookie_dialog_link_->SetTooltipText( - l10n_util::GetStringUTF16(IDS_PAGE_INFO_COOKIES_TOOLTIP)); Layout(); SizeToContents(); @@ -797,7 +899,7 @@ // their full width. If the combobox selection is changed, this may make the // widths inconsistent again, but that is OK since the widths will be updated // on the next time the bubble is opened. - if (ui::MaterialDesignController::IsSecondaryUiMaterial()) { + if (UseHarmonyStyle()) { const int maximum_width = ChromeLayoutProvider::Get()->GetDistanceMetric( views::DISTANCE_BUTTON_MAX_LINKABLE_WIDTH); int combobox_width = 0; @@ -862,29 +964,37 @@ // validity of the Certificate. const bool valid_identity = (identity_info.identity_status != PageInfo::SITE_IDENTITY_STATUS_ERROR); - const base::string16 link_title = l10n_util::GetStringUTF16( - valid_identity ? IDS_PAGE_INFO_CERTIFICATE_VALID_LINK - : IDS_PAGE_INFO_CERTIFICATE_INVALID_LINK); - - // Create the link to add to the Certificate Section. - views::Link* certificate_viewer_link = new views::Link(link_title); - certificate_viewer_link->set_id( - PageInfoBubbleView::VIEW_ID_PAGE_INFO_LINK_CERTIFICATE_VIEWER); - certificate_viewer_link->set_listener(this); - certificate_viewer_link->SetUnderline(false); + base::string16 tooltip; if (valid_identity) { - certificate_viewer_link->SetTooltipText(l10n_util::GetStringFUTF16( + tooltip = l10n_util::GetStringFUTF16( IDS_PAGE_INFO_CERTIFICATE_VALID_LINK_TOOLTIP, - base::UTF8ToUTF16(certificate_->issuer().GetDisplayName()))); + base::UTF8ToUTF16(certificate_->issuer().GetDisplayName())); } else { - certificate_viewer_link->SetTooltipText(l10n_util::GetStringUTF16( - IDS_PAGE_INFO_CERTIFICATE_INVALID_LINK_TOOLTIP)); + tooltip = l10n_util::GetStringUTF16( + IDS_PAGE_INFO_CERTIFICATE_INVALID_LINK_TOOLTIP); } // Add the Certificate Section. - site_settings_view_->AddChildView(CreateInspectLinkSection( - PageInfoUI::GetCertificateIcon(), IDS_PAGE_INFO_CERTIFICATE, - certificate_viewer_link)); + if (UseHarmonyStyle()) { + const base::string16 secondary_text = l10n_util::GetStringUTF16( + valid_identity ? IDS_PAGE_INFO_CERTIFICATE_VALID_PARENTHESIZED + : IDS_PAGE_INFO_CERTIFICATE_INVALID_PARENTHESIZED); + site_settings_view_->AddChildView( + CreateMoreInfoButton( + this, PageInfoUI::GetCertificateIcon(), + IDS_PAGE_INFO_CERTIFICATE_BUTTON_TEXT, secondary_text, + VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_CERTIFICATE_VIEWER, tooltip) + .release()); + } else { + const base::string16 link_title = l10n_util::GetStringUTF16( + valid_identity ? IDS_PAGE_INFO_CERTIFICATE_VALID_LINK + : IDS_PAGE_INFO_CERTIFICATE_INVALID_LINK); + views::Link* certificate_viewer_link = nullptr; + site_settings_view_->AddChildView(CreateMoreInfoLinkSection( + this, PageInfoUI::GetCertificateIcon(), IDS_PAGE_INFO_CERTIFICATE, + link_title, VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_CERTIFICATE_VIEWER, + tooltip, &certificate_viewer_link)); + } } if (identity_info.show_change_password_buttons) { @@ -897,12 +1007,10 @@ SizeToContents(); } -views::View* PageInfoBubbleView::CreateSiteSettingsView(int side_margin) { +views::View* PageInfoBubbleView::CreateSiteSettingsView() { views::View* site_settings_view = new views::View(); - views::BoxLayout* box_layout = new views::BoxLayout( - views::BoxLayout::kVertical, gfx::Insets(0, side_margin), - ChromeLayoutProvider::Get()->GetDistanceMetric( - DISTANCE_CONTROL_LIST_VERTICAL)); + views::BoxLayout* box_layout = + new views::BoxLayout(views::BoxLayout::kVertical); site_settings_view->SetLayoutManager(box_layout); box_layout->set_cross_axis_alignment( views::BoxLayout::CROSS_AXIS_ALIGNMENT_STRETCH); @@ -910,22 +1018,33 @@ return site_settings_view; } -void PageInfoBubbleView::HandleLinkClickedAsync(views::Link* source) { +void PageInfoBubbleView::HandleMoreInfoRequest(views::View* source) { + // The bubble closes automatically when the collected cookies dialog or the + // certificate viewer opens. So delay handling of the link clicked to avoid + // a crash in the base class which needs to complete the mouse event handling. + content::BrowserThread::PostTask( + content::BrowserThread::UI, FROM_HERE, + base::BindOnce(&PageInfoBubbleView::HandleMoreInfoRequestAsync, + weak_factory_.GetWeakPtr(), source->id())); +} + +void PageInfoBubbleView::HandleMoreInfoRequestAsync(int view_id) { // All switch cases require accessing web_contents(), so we check it here. if (web_contents() == nullptr || web_contents()->IsBeingDestroyed()) { return; } - switch (source->id()) { - case PageInfoBubbleView::VIEW_ID_PAGE_INFO_LINK_SITE_SETTINGS: + switch (view_id) { + case PageInfoBubbleView::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_SITE_SETTINGS: presenter_->OpenSiteSettingsView(); break; - case PageInfoBubbleView::VIEW_ID_PAGE_INFO_LINK_COOKIE_DIALOG: + case PageInfoBubbleView::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_COOKIE_DIALOG: // Count how often the Collected Cookies dialog is opened. presenter_->RecordPageInfoAction( PageInfo::PAGE_INFO_COOKIES_DIALOG_OPENED); new CollectedCookiesViews(web_contents()); break; - case PageInfoBubbleView::VIEW_ID_PAGE_INFO_LINK_CERTIFICATE_VIEWER: { + case PageInfoBubbleView:: + VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_CERTIFICATE_VIEWER: { gfx::NativeWindow top_window = web_contents()->GetTopLevelNativeWindow(); if (certificate_ && top_window) { presenter_->RecordPageInfoAction(
diff --git a/chrome/browser/ui/views/page_info/page_info_bubble_view.h b/chrome/browser/ui/views/page_info/page_info_bubble_view.h index a0a233b..1c300c0 100644 --- a/chrome/browser/ui/views/page_info/page_info_bubble_view.h +++ b/chrome/browser/ui/views/page_info/page_info_bubble_view.h
@@ -25,6 +25,7 @@ class GURL; class Browser; class BubbleHeaderView; +class HoverButton; class Profile; namespace content { @@ -81,9 +82,9 @@ VIEW_ID_PAGE_INFO_BUTTON_WHITELIST_PASSWORD_REUSE, VIEW_ID_PAGE_INFO_LABEL_SECURITY_DETAILS, VIEW_ID_PAGE_INFO_LABEL_RESET_CERTIFICATE_DECISIONS, - VIEW_ID_PAGE_INFO_LINK_COOKIE_DIALOG, - VIEW_ID_PAGE_INFO_LINK_SITE_SETTINGS, - VIEW_ID_PAGE_INFO_LINK_CERTIFICATE_VIEWER, + VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_COOKIE_DIALOG, + VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_SITE_SETTINGS, + VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_CERTIFICATE_VIEWER, }; // Creates the appropriate page info bubble for the given |url|. @@ -152,12 +153,15 @@ // Creates the contents of the |site_settings_view_|. The ownership of the // returned view is transferred to the caller. - views::View* CreateSiteSettingsView(int side_margin) WARN_UNUSED_RESULT; + views::View* CreateSiteSettingsView() WARN_UNUSED_RESULT; + + // Posts a task to HandleMoreInfoRequestAsync() below. + void HandleMoreInfoRequest(views::View* source); // Used to asynchronously handle clicks since these calls may cause the // destruction of the settings view and the base class window still needs to // be alive to finish handling the mouse or keyboard click. - void HandleLinkClickedAsync(views::Link* source); + void HandleMoreInfoRequestAsync(int view_id); // The presenter that controls the Page Info UI. std::unique_ptr<PageInfo> presenter_; @@ -173,8 +177,10 @@ // The view that contains the certificate, cookie, and permissions sections. views::View* site_settings_view_; - // The link that opens the "Cookies" dialog. - views::Link* cookie_dialog_link_; + // The link that opens the "Cookies" dialog. Non-harmony mode only. + views::Link* cookie_link_legacy_; + // The bubble that opens the "Cookies" dialog. Harmony mode only. + HoverButton* cookie_button_; // The view that contains the "Permissions" table of the bubble. views::View* permissions_view_;
diff --git a/chrome/browser/ui/views/page_info/page_info_bubble_view_browsertest.cc b/chrome/browser/ui/views/page_info/page_info_bubble_view_browsertest.cc index 97f388f5..0beef86 100644 --- a/chrome/browser/ui/views/page_info/page_info_bubble_view_browsertest.cc +++ b/chrome/browser/ui/views/page_info/page_info_bubble_view_browsertest.cc
@@ -92,7 +92,8 @@ OpenPageInfoBubble(browser); // Get site settings button. views::View* site_settings_button = GetView( - browser, PageInfoBubbleView::VIEW_ID_PAGE_INFO_LINK_SITE_SETTINGS); + browser, + PageInfoBubbleView::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_SITE_SETTINGS); ClickAndWaitForSettingsPageToOpen(site_settings_button); return browser->tab_strip_model()
diff --git a/chrome/browser/ui/views/page_info/page_info_bubble_view_unittest.cc b/chrome/browser/ui/views/page_info/page_info_bubble_view_unittest.cc index 4b602fa..81242b9 100644 --- a/chrome/browser/ui/views/page_info/page_info_bubble_view_unittest.cc +++ b/chrome/browser/ui/views/page_info/page_info_bubble_view_unittest.cc
@@ -9,6 +9,7 @@ #include "build/build_config.h" #include "chrome/browser/ui/exclusive_access/exclusive_access_manager.h" #include "chrome/browser/ui/views/harmony/chrome_layout_provider.h" +#include "chrome/browser/ui/views/hover_button.h" #include "chrome/browser/ui/views/page_info/chosen_object_row.h" #include "chrome/browser/ui/views/page_info/permission_selector_row.h" #include "chrome/browser/usb/usb_chooser_context.h" @@ -23,6 +24,7 @@ #include "device/usb/mock_usb_service.h" #include "testing/gtest/include/gtest/gtest.h" #include "ui/base/l10n/l10n_util.h" +#include "ui/base/material_design/material_design_controller.h" #include "ui/base/ui_base_features.h" #include "ui/events/event_utils.h" #include "ui/views/controls/button/menu_button.h" @@ -66,11 +68,19 @@ return view_->selector_rows_[index].get(); } - // Returns the number of cookies shown on the link to open the collected - // cookies dialog. This link is always shown, so fail if it's not there. + // Returns the number of cookies shown on the link or button to open the + // collected cookies dialog. This should always be shown. base::string16 GetCookiesLinkText() { - EXPECT_TRUE(view_->cookie_dialog_link_); - return view_->cookie_dialog_link_->text(); + if (ui::MaterialDesignController::IsSecondaryUiMaterial()) { + EXPECT_TRUE(view_->cookie_button_); + ui::AXNodeData data; + view_->cookie_button_->GetAccessibleNodeData(&data); + std::string name; + data.GetStringAttribute(ui::AX_ATTR_NAME, &name); + return base::ASCIIToUTF16(name); + } + EXPECT_TRUE(view_->cookie_link_legacy_); + return view_->cookie_link_legacy_->text(); } // Returns the permission label text of the |index|th permission selector row. @@ -334,5 +344,6 @@ base::string16 expected = l10n_util::GetPluralStringFUTF16( IDS_PAGE_INFO_NUM_COOKIES, first_party_cookies.allowed + third_party_cookies.allowed); - EXPECT_EQ(expected, api_->GetCookiesLinkText()); + size_t index = api_->GetCookiesLinkText().find(expected); + EXPECT_NE(std::string::npos, index); }
diff --git a/chromeos/cryptohome/homedir_methods.cc b/chromeos/cryptohome/homedir_methods.cc index 496359c2..eacd659f 100644 --- a/chromeos/cryptohome/homedir_methods.cc +++ b/chromeos/cryptohome/homedir_methods.cc
@@ -25,16 +25,6 @@ HomedirMethods* g_homedir_methods = NULL; -// Fill authorization protobuffer. -void FillAuthorizationProtobuf(const Authorization& auth, - cryptohome::AuthorizationRequest* auth_proto) { - Key* key = auth_proto->mutable_key(); - if (!auth.label.empty()) { - key->mutable_data()->set_label(auth.label); - } - key->set_secret(auth.key); -} - void ParseAuthorizationDataProtobuf( const KeyAuthorizationData& authorization_data_proto, KeyDefinition::AuthorizationData* authorization_data) { @@ -143,15 +133,12 @@ const KeyDefinition& new_key, bool clobber_if_exists, const Callback& callback) override { - cryptohome::AuthorizationRequest auth_proto; cryptohome::AddKeyRequest request; - - FillAuthorizationProtobuf(auth, &auth_proto); KeyDefinitionToKey(new_key, request.mutable_key()); request.set_clobber_if_exists(clobber_if_exists); DBusThreadManager::Get()->GetCryptohomeClient()->AddKeyEx( - id, auth_proto, request, + id, CreateAuthorizationRequest(auth.label, auth.key), request, base::BindOnce(&HomedirMethodsImpl::OnBaseReplyCallback, weak_ptr_factory_.GetWeakPtr(), callback)); } @@ -160,14 +147,12 @@ const Authorization& auth, const std::string& label, const Callback& callback) override { - cryptohome::AuthorizationRequest auth_proto; cryptohome::RemoveKeyRequest request; - FillAuthorizationProtobuf(auth, &auth_proto); request.mutable_key()->mutable_data()->set_label(label); DBusThreadManager::Get()->GetCryptohomeClient()->RemoveKeyEx( - id, auth_proto, request, + id, CreateAuthorizationRequest(auth.label, auth.key), request, base::BindOnce(&HomedirMethodsImpl::OnBaseReplyCallback, weak_ptr_factory_.GetWeakPtr(), callback)); } @@ -177,15 +162,13 @@ const KeyDefinition& new_key, const std::string& signature, const Callback& callback) override { - cryptohome::AuthorizationRequest auth_proto; cryptohome::UpdateKeyRequest pb_update_key; - FillAuthorizationProtobuf(auth, &auth_proto); KeyDefinitionToKey(new_key, pb_update_key.mutable_changes()); pb_update_key.set_authorization_signature(signature); DBusThreadManager::Get()->GetCryptohomeClient()->UpdateKeyEx( - id, auth_proto, pb_update_key, + id, CreateAuthorizationRequest(auth.label, auth.key), pb_update_key, base::BindOnce(&HomedirMethodsImpl::OnBaseReplyCallback, weak_ptr_factory_.GetWeakPtr(), callback)); } @@ -437,6 +420,17 @@ } } +cryptohome::AuthorizationRequest CreateAuthorizationRequest( + const std::string& label, + const std::string& secret) { + cryptohome::AuthorizationRequest auth_request; + Key* key = auth_request.mutable_key(); + if (!label.empty()) + key->mutable_data()->set_label(label); + key->set_secret(secret); + return auth_request; +} + // static void HomedirMethods::Initialize() { if (g_homedir_methods) {
diff --git a/chromeos/cryptohome/homedir_methods.h b/chromeos/cryptohome/homedir_methods.h index eb942f2..f14e8112 100644 --- a/chromeos/cryptohome/homedir_methods.h +++ b/chromeos/cryptohome/homedir_methods.h
@@ -23,6 +23,10 @@ // Converts the given KeyDefinition to a Key. void CHROMEOS_EXPORT KeyDefinitionToKey(const KeyDefinition& key_def, Key* key); +// Creates an AuthorizationRequest from the given secret and label. +AuthorizationRequest CHROMEOS_EXPORT +CreateAuthorizationRequest(const std::string& secret, const std::string& label); + // This class manages calls to Cryptohome service's home directory methods: // Mount, CheckKey, Add/UpdateKey. class CHROMEOS_EXPORT HomedirMethods {
diff --git a/chromeos/login/auth/extended_authenticator_impl.cc b/chromeos/login/auth/extended_authenticator_impl.cc index 7e9ba7c..ce98062 100644 --- a/chromeos/login/auth/extended_authenticator_impl.cc +++ b/chromeos/login/auth/extended_authenticator_impl.cc
@@ -191,42 +191,24 @@ const ResultCallback& success_callback, const UserContext& user_context) { RecordStartMarker("MountEx"); - - cryptohome::Identification id(user_context.GetAccountId()); const Key* const key = user_context.GetKey(); - cryptohome::MountRequest mount; - cryptohome::AuthorizationRequest auth; - cryptohome::Key* auth_key = auth.mutable_key(); - if (!key->GetLabel().empty()) { - auth_key->mutable_data()->set_label(key->GetLabel()); - } - auth_key->set_secret(key->GetSecret()); cryptohome::HomedirMethods::GetInstance()->MountEx( - id, - auth, - mount, - base::Bind(&ExtendedAuthenticatorImpl::OnMountComplete, - this, - "MountEx", - user_context, - success_callback)); + cryptohome::Identification(user_context.GetAccountId()), + cryptohome::CreateAuthorizationRequest(key->GetLabel(), key->GetSecret()), + cryptohome::MountRequest(), + base::Bind(&ExtendedAuthenticatorImpl::OnMountComplete, this, "MountEx", + user_context, success_callback)); } void ExtendedAuthenticatorImpl::DoAuthenticateToCheck( const base::Closure& success_callback, const UserContext& user_context) { RecordStartMarker("CheckKeyEx"); - - cryptohome::Identification id(user_context.GetAccountId()); const Key* const key = user_context.GetKey(); - cryptohome::AuthorizationRequest auth; - cryptohome::Key* auth_key = auth.mutable_key(); - if (!key->GetLabel().empty()) { - auth_key->mutable_data()->set_label(key->GetLabel()); - } - auth_key->set_secret(key->GetSecret()); cryptohome::HomedirMethods::GetInstance()->CheckKeyEx( - id, auth, cryptohome::CheckKeyRequest(), + cryptohome::Identification(user_context.GetAccountId()), + cryptohome::CreateAuthorizationRequest(key->GetLabel(), key->GetSecret()), + cryptohome::CheckKeyRequest(), base::Bind(&ExtendedAuthenticatorImpl::OnOperationComplete, this, "CheckKeyEx", user_context, success_callback)); }
diff --git a/content/common/resource_messages.h b/content/common/resource_messages.h index 56d7ba07..ea29b50a 100644 --- a/content/common/resource_messages.h +++ b/content/common/resource_messages.h
@@ -29,6 +29,7 @@ #include "net/ssl/ssl_info.h" #include "net/traffic_annotation/network_traffic_annotation.h" #include "net/url_request/redirect_info.h" +#include "services/network/public/cpp/cors_error_status.h" #include "services/network/public/cpp/url_loader_status.h" #include "services/network/public/interfaces/fetch_api.mojom.h" #include "third_party/WebKit/public/platform/WebMixedContentContextType.h" @@ -294,6 +295,11 @@ IPC_ENUM_TRAITS_MAX_VALUE(network::mojom::CORSError, network::mojom::CORSError::kLast) +IPC_STRUCT_TRAITS_BEGIN(network::CORSErrorStatus) + IPC_STRUCT_TRAITS_MEMBER(cors_error) + IPC_STRUCT_TRAITS_MEMBER(related_response_headers) +IPC_STRUCT_TRAITS_END() + IPC_STRUCT_TRAITS_BEGIN(network::URLLoaderStatus) IPC_STRUCT_TRAITS_MEMBER(error_code) IPC_STRUCT_TRAITS_MEMBER(exists_in_cache) @@ -301,7 +307,7 @@ IPC_STRUCT_TRAITS_MEMBER(encoded_data_length) IPC_STRUCT_TRAITS_MEMBER(encoded_body_length) IPC_STRUCT_TRAITS_MEMBER(decoded_body_length) - IPC_STRUCT_TRAITS_MEMBER(cors_error) + IPC_STRUCT_TRAITS_MEMBER(cors_error_status) IPC_STRUCT_TRAITS_END() // Resource messages sent from the browser to the renderer.
diff --git a/content/public/common/cors_error_status.typemap b/content/public/common/cors_error_status.typemap new file mode 100644 index 0000000..6b66c33 --- /dev/null +++ b/content/public/common/cors_error_status.typemap
@@ -0,0 +1,13 @@ +# Copyright 2017 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. + +mojom = "//content/public/common/url_loader.mojom" +public_headers = [ "//services/network/public/cpp/cors_error_status.h" ] +traits_headers = [ "//content/common/resource_messages.h" ] +deps = [ + "//content:export", + "//net:net", + "//third_party/WebKit/public:blink_headers", +] +type_mappings = [ "content.mojom.CORSErrorStatus=network::CORSErrorStatus" ]
diff --git a/content/public/common/typemaps.gni b/content/public/common/typemaps.gni index 3dbfcb6c..004ec91 100644 --- a/content/public/common/typemaps.gni +++ b/content/public/common/typemaps.gni
@@ -3,6 +3,7 @@ # found in the LICENSE file. typemaps = [ + "//content/public/common/cors_error_status.typemap", "//content/public/common/manifest.typemap", "//content/public/common/mutable_network_traffic_annotation_tag.typemap", "//content/public/common/referrer.typemap",
diff --git a/content/public/common/url_loader.mojom b/content/public/common/url_loader.mojom index 73af7c8..8e349ae 100644 --- a/content/public/common/url_loader.mojom +++ b/content/public/common/url_loader.mojom
@@ -17,6 +17,9 @@ struct URLRequestRedirectInfo; [Native] +struct CORSErrorStatus; + +[Native] struct URLLoaderStatus; // This enum corresponds to net::RequestPriority. See its comments for details.
diff --git a/content/renderer/loader/cors_url_loader.cc b/content/renderer/loader/cors_url_loader.cc index c8bc8f5..883e0e1 100644 --- a/content/renderer/loader/cors_url_loader.cc +++ b/content/renderer/loader/cors_url_loader.cc
@@ -59,8 +59,8 @@ if (fetch_cors_flag_ && fetch_request_mode_ == FetchRequestMode::kSameOrigin) { - forwarding_client_->OnComplete( - network::URLLoaderStatus(CORSError::kDisallowedByMode)); + forwarding_client_->OnComplete(network::URLLoaderStatus( + network::CORSErrorStatus(CORSError::kDisallowedByMode))); return; } @@ -122,7 +122,9 @@ blink::WebHTTPHeaderMap(response_head.headers.get()), fetch_credentials_mode_, security_origin_); if (cors_error) { - HandleComplete(network::URLLoaderStatus(*cors_error)); + // TODO(toyoshim): Generate related_response_headers here. + network::CORSErrorStatus cors_error_status(*cors_error); + HandleComplete(network::URLLoaderStatus(cors_error_status)); return; } }
diff --git a/content/renderer/loader/cors_url_loader_unittest.cc b/content/renderer/loader/cors_url_loader_unittest.cc index 61fd296..d54dfffc 100644 --- a/content/renderer/loader/cors_url_loader_unittest.cc +++ b/content/renderer/loader/cors_url_loader_unittest.cc
@@ -183,9 +183,9 @@ EXPECT_FALSE(client().has_received_redirect()); EXPECT_FALSE(client().has_received_response()); EXPECT_EQ(net::ERR_FAILED, client().status().error_code); - ASSERT_TRUE(client().status().cors_error); + ASSERT_TRUE(client().status().cors_error_status); EXPECT_EQ(network::mojom::CORSError::kDisallowedByMode, - *client().status().cors_error); + client().status().cors_error_status->cors_error); } TEST_F(CORSURLLoaderTest, CrossOriginRequestWithCORSModeButMissingCORSHeader) { @@ -202,9 +202,9 @@ EXPECT_FALSE(client().has_received_redirect()); EXPECT_FALSE(client().has_received_response()); EXPECT_EQ(net::ERR_FAILED, client().status().error_code); - ASSERT_TRUE(client().status().cors_error); + ASSERT_TRUE(client().status().cors_error_status); EXPECT_EQ(network::mojom::CORSError::kMissingAllowOriginHeader, - *client().status().cors_error); + client().status().cors_error_status->cors_error); } TEST_F(CORSURLLoaderTest, CrossOriginRequestWithCORSMode) { @@ -241,9 +241,9 @@ EXPECT_FALSE(client().has_received_redirect()); EXPECT_FALSE(client().has_received_response()); EXPECT_EQ(net::ERR_FAILED, client().status().error_code); - ASSERT_TRUE(client().status().cors_error); + ASSERT_TRUE(client().status().cors_error_status); EXPECT_EQ(network::mojom::CORSError::kAllowOriginMismatch, - *client().status().cors_error); + client().status().cors_error_status->cors_error); } } // namespace
diff --git a/content/renderer/loader/sync_load_context.cc b/content/renderer/loader/sync_load_context.cc index 8fb5d7f..edd13b3d 100644 --- a/content/renderer/loader/sync_load_context.cc +++ b/content/renderer/loader/sync_load_context.cc
@@ -101,6 +101,8 @@ void SyncLoadContext::OnCompletedRequest( const network::URLLoaderStatus& status) { response_->error_code = status.error_code; + if (status.cors_error_status) + response_->cors_error = status.cors_error_status->cors_error; response_->encoded_data_length = status.encoded_data_length; response_->encoded_body_length = status.encoded_body_length; event_->Signal();
diff --git a/content/renderer/loader/sync_load_response.h b/content/renderer/loader/sync_load_response.h index 1fad58638..160d9bba2 100644 --- a/content/renderer/loader/sync_load_response.h +++ b/content/renderer/loader/sync_load_response.h
@@ -7,7 +7,10 @@ #include <string> +#include "base/optional.h" +#include "content/common/content_export.h" #include "content/public/common/resource_response_info.h" +#include "services/network/public/interfaces/cors.mojom.h" #include "url/gurl.h" namespace content { @@ -19,7 +22,10 @@ ~SyncLoadResponse(); // The response error code. - int error_code; + int error_code = 0; + + // Optional CORS error details. + base::Optional<network::mojom::CORSError> cors_error; // The final URL of the response. This may differ from the request URL in // the case of a server redirect.
diff --git a/content/renderer/loader/web_url_loader_impl.cc b/content/renderer/loader/web_url_loader_impl.cc index 4ae083e4..f9f06d0 100644 --- a/content/renderer/loader/web_url_loader_impl.cc +++ b/content/renderer/loader/web_url_loader_impl.cc
@@ -906,13 +906,15 @@ this, TRACE_EVENT_FLAG_FLOW_IN); if (status.error_code != net::OK) { - WebURLError error(status.error_code, - status.exists_in_cache - ? WebURLError::HasCopyInCache::kTrue - : WebURLError::HasCopyInCache::kFalse, - WebURLError::IsWebSecurityViolation::kFalse, url_); - client_->DidFail(error, total_transfer_size, encoded_body_size, - status.decoded_body_length); + const WebURLError::HasCopyInCache has_copy_in_cache = + status.exists_in_cache ? WebURLError::HasCopyInCache::kTrue + : WebURLError::HasCopyInCache::kFalse; + client_->DidFail( + status.cors_error_status + ? WebURLError(*status.cors_error_status, has_copy_in_cache, url_) + : WebURLError(status.error_code, has_copy_in_cache, + WebURLError::IsWebSecurityViolation::kFalse, url_), + total_transfer_size, encoded_body_size, status.decoded_body_length); } else { client_->DidFinishLoading( (status.completion_time - TimeTicks()).InSecondsF(), @@ -1240,16 +1242,23 @@ // TODO(tc): For file loads, we may want to include a more descriptive // status code or status text. - int error_code = sync_load_response.error_code; + const int error_code = sync_load_response.error_code; if (error_code != net::OK) { - // SyncResourceHandler returns ERR_ABORTED for CORS redirect errors, - // so we treat the error as a web security violation. - const bool is_web_security_violation = error_code == net::ERR_ABORTED; - error = WebURLError(error_code, WebURLError::HasCopyInCache::kFalse, - is_web_security_violation - ? WebURLError::IsWebSecurityViolation::kTrue - : WebURLError::IsWebSecurityViolation::kFalse, - final_url); + if (sync_load_response.cors_error) { + // TODO(toyoshim): Pass CORS error related headers here. + error = + WebURLError(network::CORSErrorStatus(*sync_load_response.cors_error), + WebURLError::HasCopyInCache::kFalse, final_url); + } else { + // SyncResourceHandler returns ERR_ABORTED for CORS redirect errors, + // so we treat the error as a web security violation. + const WebURLError::IsWebSecurityViolation is_web_security_violation = + error_code == net::ERR_ABORTED + ? WebURLError::IsWebSecurityViolation::kTrue + : WebURLError::IsWebSecurityViolation::kFalse; + error = WebURLError(error_code, WebURLError::HasCopyInCache::kFalse, + is_web_security_violation, final_url); + } return; }
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index 691e7310..dd308d9 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc
@@ -2629,7 +2629,7 @@ } void RenderFrameImpl::DidFailProvisionalLoadInternal( - const blink::WebURLError& error, + const WebURLError& error, blink::WebHistoryCommitType commit_type, const base::Optional<std::string>& error_page_content) { TRACE_EVENT1("navigation,benchmark,rail", @@ -2873,7 +2873,7 @@ } void RenderFrameImpl::LoadErrorPage(int reason) { - blink::WebURLError error(reason, frame_->GetDocument().Url()); + WebURLError error(reason, frame_->GetDocument().Url()); std::string error_html; GetContentClient()->renderer()->GetNavigationErrorStrings( @@ -3892,7 +3892,7 @@ } void RenderFrameImpl::DidFailProvisionalLoad( - const blink::WebURLError& error, + const WebURLError& error, blink::WebHistoryCommitType commit_type) { DidFailProvisionalLoadInternal(error, commit_type, base::nullopt); } @@ -4241,7 +4241,7 @@ } } -void RenderFrameImpl::DidFailLoad(const blink::WebURLError& error, +void RenderFrameImpl::DidFailLoad(const WebURLError& error, blink::WebHistoryCommitType commit_type) { TRACE_EVENT1("navigation,rail", "RenderFrameImpl::didFailLoad", "id", routing_id_); @@ -6829,7 +6829,7 @@ void RenderFrameImpl::SendFailedProvisionalLoad( const blink::WebURLRequest& request, - const blink::WebURLError& error, + const WebURLError& error, blink::WebLocalFrame* frame) { bool show_repost_interstitial = (error.reason() == net::ERR_CACHE_MISS &&
diff --git a/services/network/public/cpp/BUILD.gn b/services/network/public/cpp/BUILD.gn index da8d314..a2df10ec 100644 --- a/services/network/public/cpp/BUILD.gn +++ b/services/network/public/cpp/BUILD.gn
@@ -6,6 +6,8 @@ static_library("cpp") { sources = [ + "cors_error_status.cc", + "cors_error_status.h", "net_adapters.cc", "net_adapters.h", "url_loader_status.cc",
diff --git a/services/network/public/cpp/cors_error_status.cc b/services/network/public/cpp/cors_error_status.cc new file mode 100644 index 0000000..c4c395bc --- /dev/null +++ b/services/network/public/cpp/cors_error_status.cc
@@ -0,0 +1,36 @@ +// Copyright 2017 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. + +#include "services/network/public/cpp/cors_error_status.h" + +#include "net/base/net_errors.h" + +namespace network { + +// Note: |cors_error| is initialized to kLast to keep the value inside the +// valid enum value range. The value is meaningless and should be overriden +// immediately by IPC desrtialization code. +CORSErrorStatus::CORSErrorStatus() + : CORSErrorStatus(network::mojom::CORSError::kLast) {} + +CORSErrorStatus::CORSErrorStatus(const CORSErrorStatus& status) = default; + +CORSErrorStatus::CORSErrorStatus(network::mojom::CORSError error) + : cors_error(error) {} + +CORSErrorStatus::CORSErrorStatus( + network::mojom::CORSError error, + scoped_refptr<net::HttpResponseHeaders> headers) + : CORSErrorStatus(error) { + related_response_headers = headers; +} + +CORSErrorStatus::~CORSErrorStatus() = default; + +bool CORSErrorStatus::operator==(const CORSErrorStatus& rhs) const { + return cors_error == rhs.cors_error && + related_response_headers == rhs.related_response_headers; +} + +} // namespace network
diff --git a/services/network/public/cpp/cors_error_status.h b/services/network/public/cpp/cors_error_status.h new file mode 100644 index 0000000..7214251 --- /dev/null +++ b/services/network/public/cpp/cors_error_status.h
@@ -0,0 +1,41 @@ +// Copyright 2017 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 SERVICES_NETWORK_PUBLIC_CPP_CORS_ERROR_STATUS_H_ +#define SERVICES_NETWORK_PUBLIC_CPP_CORS_ERROR_STATUS_H_ + +#include "base/memory/scoped_refptr.h" +#include "net/http/http_response_headers.h" +#include "services/network/public/interfaces/cors.mojom.h" + +namespace network { + +struct CORSErrorStatus { + // This constructor is used by generated IPC serialization code. + // Should not use this explicitly. + // TODO(toyoshim, yhirano): Exploring a way to make this private, and allows + // only serialization code for mojo can access. + CORSErrorStatus(); + + CORSErrorStatus(const CORSErrorStatus& status); + + explicit CORSErrorStatus(network::mojom::CORSError error); + CORSErrorStatus( + network::mojom::CORSError error, + scoped_refptr<net::HttpResponseHeaders> related_response_headers); + + ~CORSErrorStatus(); + + bool operator==(const CORSErrorStatus& rhs) const; + + network::mojom::CORSError cors_error; + + // Partial HTTP response headers including status line that will be useful to + // generate a human readable error message. + scoped_refptr<net::HttpResponseHeaders> related_response_headers; +}; + +} // namespace network + +#endif // SERVICES_NETWORK_PUBLIC_CPP_CORS_ERROR_STATUS_H_
diff --git a/services/network/public/cpp/url_loader_status.cc b/services/network/public/cpp/url_loader_status.cc index e329e86..6b4f7dc 100644 --- a/services/network/public/cpp/url_loader_status.cc +++ b/services/network/public/cpp/url_loader_status.cc
@@ -14,9 +14,9 @@ URLLoaderStatus::URLLoaderStatus(int error_code) : error_code(error_code), completion_time(base::TimeTicks::Now()) {} -URLLoaderStatus::URLLoaderStatus(network::mojom::CORSError error) +URLLoaderStatus::URLLoaderStatus(const CORSErrorStatus& error) : URLLoaderStatus(net::ERR_FAILED) { - cors_error = error; + cors_error_status = error; } URLLoaderStatus::~URLLoaderStatus() = default; @@ -28,7 +28,7 @@ encoded_data_length == rhs.encoded_data_length && encoded_body_length == rhs.encoded_body_length && decoded_body_length == rhs.decoded_body_length && - cors_error == rhs.cors_error; + cors_error_status == rhs.cors_error_status; } } // namespace network
diff --git a/services/network/public/cpp/url_loader_status.h b/services/network/public/cpp/url_loader_status.h index 72485cd6..0c08a8ba 100644 --- a/services/network/public/cpp/url_loader_status.h +++ b/services/network/public/cpp/url_loader_status.h
@@ -10,6 +10,7 @@ #include "base/macros.h" #include "base/optional.h" #include "base/time/time.h" +#include "services/network/public/cpp/cors_error_status.h" #include "services/network/public/interfaces/cors.mojom.h" namespace network { @@ -22,9 +23,9 @@ // |completion_time|. explicit URLLoaderStatus(int error_code); - // Sets ERR_FAILED to |error_code|, |error| to |cors_error|, and + // Sets ERR_FAILED to |error_code|, |error| to |cors_error_status|, and // base::TimeTicks::Now() to |completion_time|. - explicit URLLoaderStatus(network::mojom::CORSError error); + explicit URLLoaderStatus(const CORSErrorStatus& error); ~URLLoaderStatus(); @@ -49,7 +50,7 @@ int64_t decoded_body_length = 0; // Optional CORS error details. - base::Optional<network::mojom::CORSError> cors_error; + base::Optional<CORSErrorStatus> cors_error_status; }; } // namespace network
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index 1f65d2c6..329ce74 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -823,25 +823,40 @@ # ====== Out of Blink CORS related tests BEGIN ====== # TODO(toyoshim, tyoshino, kinuko): Make following tests pass. See crbug.com/736308. # ./Tools/Scripts/run-webkit-tests virtual/outofblink-cors -# 1980 tests ran as expected (1884 passed, 96 didn't), 74 didn't -# including 8 crashes and 1 timeout. -crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/workers/upload-onprogress-event.html [ Crash ] +# Found 2061 tests; running 2060, skipping 1. +# 2060 tests ran as expected (1968 passed, 92 didn't). +crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/workers/upload-onprogress-event.html [ Pass Crash ] +crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/fetch-cors-xhr.https.html [ Failure Crash ] +crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/fetch-event-network-error.https.html [ Crash ] +crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/fetch-request-fallback.https.html [ Crash Failure ] +crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/fetch-request-redirect.https.html [ Crash ] +crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/performance-timeline.https.html [ Crash ] +crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/redirected-response.https.html [ Crash ] +crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/referer.https.html [ Crash ] +crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/referrer-policy-header.https.html [ Crash ] + crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/upload-onload-event.html [ Crash ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/upload-onloadend-event-after-abort.html [ Crash ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/upload-onloadend-event-after-load.html [ Crash ] -crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/upload-onloadstart-event.html [ Crash ] +crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/upload-onloadstart-event.html [ Pass Crash ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/upload-onprogress-event.html [ Crash ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/upload-progress-events.html [ Crash ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/xmlhttprequest-data-url.html [ Crash ] +crbug.com/736308 virtual/outofblink-cors/external/wpt/fetch/api/cors/cors-preflight-status.any.html [ Pass Timeout ] +crbug.com/736308 virtual/outofblink-cors/external/wpt/fetch/api/cors/cors-redirect-preflight.any.html [ Pass Timeout ] +crbug.com/736308 virtual/outofblink-cors/external/wpt/fetch/api/redirect/redirect-location.html [ Pass Timeout ] +crbug.com/736308 virtual/outofblink-cors/external/wpt/fetch/api/redirect/redirect-origin.html [ Pass Timeout ] +crbug.com/736308 virtual/outofblink-cors/external/wpt/fetch/api/redirect/redirect-referrer.html [ Pass Timeout ] +crbug.com/626703 virtual/outofblink-cors/external/wpt/fetch/api/request/request-cache-default-conditional.html [ Pass Timeout ] +crbug.com/626703 virtual/outofblink-cors/external/wpt/fetch/api/response/response-cancel-stream.html [ Pass Timeout ] +crbug.com/626703 virtual/outofblink-cors/external/wpt/fetch/http-cache/status.html [ Pass Timeout ] crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/foreign-fetch-basics.https.html [ Timeout ] crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/sandboxed-iframe-fetch-event.https.html [ Timeout ] crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/fetch-canvas-tainting-cache.https.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https.html [ Failure ] -crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/fetch-cors-xhr.https.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/fetch-event-referrer-policy.https.html [ Failure ] -crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/fetch-request-fallback.https.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/fetch-request-html-imports.https.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/fetch-request-xhr.https.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/external/wpt/service-workers/service-worker/fetch-response-taint.https.html [ Failure ] @@ -850,18 +865,13 @@ crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/chromium/access-control-origin-header-in-isolated-world.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/cross-origin-no-credential-prompt.html [ Failure ] -crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/cross-site-denied-response.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/cross-site-denied-response-sync-2.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/cross-site-denied-response-sync.html [ Failure ] -crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/onerror-event.html [ Failure ] -crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/ontimeout-event-override-after-failure.html [ Failure ] -crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/open-in-body-onload-sync-to-invalid-cross-origin-response-handling.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/origin-whitelisting-all.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/origin-whitelisting-exact-match.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/origin-whitelisting-ip-addresses-with-subdomains.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/origin-whitelisting-removal.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/origin-whitelisting-subdomains.html [ Failure ] -crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/post-blob-content-type-async.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/origin-exact-matching/07.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/origin-exact-matching/08.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/origin-exact-matching/09.html [ Failure ] @@ -907,17 +917,12 @@ crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/origin-whitelisting-ip-addresses.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/post-blob-content-type-sync.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/redirect-cors-origin-null.html [ Failure ] -crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/redirect-cross-origin.html [ Failure ] -crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/redirect-cross-origin-2.html [ Failure ] -crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/simple-cross-origin-denied-events.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/simple-cross-origin-denied-events-post-sync.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/simple-cross-origin-denied-events-sync.html [ Failure ] -crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/simple-cross-origin-progress-events.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/workers/xmlhttprequest-allowed-with-disabled-web-security.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/xhr-to-blob-in-isolated-world.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/xmlhttprequest-allowed-with-disabled-web-security.html [ Failure ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/xmlhttprequest-sync-no-progress-events.html [ Failure ] -crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/xmlhttprequest-unsafe-redirect.html [ Failure ] # ====== Out of Blink CORS related tests END ====== # ====== Fullscreen tests failing with feature policy ====== @@ -3371,9 +3376,6 @@ # Sheriff failure 2017-08-07 crbug.com/708499 [ Linux ] virtual/wheelscrolllatching/fast/compositor-wheel-scroll-latching/touchpad-scroll-impl-to-main.html [ Failure Pass ] -# Sheriff failure 2017-08-11 -crbug.com/626703 virtual/outofblink-cors/external/wpt/fetch/api/response/response-cancel-stream.html [ Pass Timeout ] - # Sheriff failure 2017-08-29 crbug.com/727252 [ Win7 ] external/wpt/media-source/mediasource-endofstream.html [ Pass Timeout ]
diff --git a/third_party/WebKit/Source/core/BUILD.gn b/third_party/WebKit/Source/core/BUILD.gn index e3893d52..52e66d5 100644 --- a/third_party/WebKit/Source/core/BUILD.gn +++ b/third_party/WebKit/Source/core/BUILD.gn
@@ -106,6 +106,7 @@ source_set("prerequisites") { public_deps = [ "//gpu/command_buffer/client:gles2_c_lib", + "//services/network/public/cpp:cpp", "//services/service_manager/public/cpp", "//skia", "//third_party/WebKit/Source/core/inspector:generated",
diff --git a/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp b/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp index 5fd8e58..bffaf82 100644 --- a/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp +++ b/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp
@@ -303,7 +303,7 @@ CSSValue* value) { EditingStyle* selection_style = EditingStyleUtilities::CreateStyleAtSelectionStart( - frame.Selection().ComputeVisibleSelectionInDOMTreeDeprecated()); + frame.Selection().ComputeVisibleSelectionInDOMTree()); if (!selection_style || !selection_style->Style()) return false; @@ -399,12 +399,9 @@ TextGranularity granularity) { const VisibleSelection& selection = CreateVisibleSelectionWithGranularity( SelectionInDOMTree::Builder() - .SetBaseAndExtent(frame.Selection() - .ComputeVisibleSelectionInDOMTreeDeprecated() - .Base(), - frame.Selection() - .ComputeVisibleSelectionInDOMTreeDeprecated() - .Extent()) + .SetBaseAndExtent( + frame.Selection().ComputeVisibleSelectionInDOMTree().Base(), + frame.Selection().ComputeVisibleSelectionInDOMTree().Extent()) .Build(), granularity); const EphemeralRange new_range = selection.ToNormalizedEphemeralRange();
diff --git a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp index d61e619..2dff1ae 100644 --- a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp +++ b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
@@ -180,7 +180,7 @@ const ResourceRequest& request) { DCHECK(out_of_blink_cors_); - // TODO(hintzed) replace this delegation with an implementation that does not + // TODO(toyoshim) replace this delegation with an implementation that does not // perform CORS checks but relies on CORSURLLoader for CORS // (https://crbug.com/736308). StartBlinkCORS(request); @@ -190,7 +190,7 @@ ResourceRequest& request) { DCHECK(out_of_blink_cors_); - // TODO(hintzed) replace this delegation with an implementation that does not + // TODO(toyoshim) replace this delegation with an implementation that does not // perform CORS checks but relies on CORSURLLoader for CORS // (https://crbug.com/736308). DispatchInitialRequestBlinkCORS(request); @@ -202,11 +202,25 @@ network::mojom::FetchCredentialsMode credentials_mode, const ResourceResponse& response, std::unique_ptr<WebDataConsumerHandle> handle) { - // TODO(hintzed) replace this delegation with an implementation that does not - // perform CORS checks but relies on CORSURLLoader for CORS + DCHECK(client_); + // Out of Blink CORS access check is implemented. But we still need some + // additional code to work with unfinished preflight support in Blink. + // TODO(toyoshim): Remove following workaround code to support preflight. // (https://crbug.com/736308). - HandleResponseBlinkCORS(identifier, request_mode, credentials_mode, response, - std::move(handle)); + if (!actual_request_.IsNull()) { + ReportResponseReceived(identifier, response); + HandlePreflightResponse(response); + return; + } + + // TODO(toyoshim): Support Service Worker. (https://crbug.com/736308). + if (response.WasFetchedViaServiceWorker()) { + HandleResponseBlinkCORS(identifier, request_mode, credentials_mode, + response, std::move(handle)); + return; + } + + client_->DidReceiveResponse(identifier, response, std::move(handle)); } bool DocumentThreadableLoader::RedirectReceivedOutOfBlinkCORS( @@ -215,7 +229,7 @@ const ResourceResponse& redirect_response) { DCHECK(out_of_blink_cors_); - // TODO(hintzed) replace this delegation with an implementation that does not + // TODO(toyoshim) replace this delegation with an implementation that does not // perform CORS checks but relies on CORSURLLoader for CORS // (https://crbug.com/736308). return RedirectReceivedBlinkCORS(resource, new_request, redirect_response); @@ -225,7 +239,7 @@ const ResourceRequest& request) { DCHECK(out_of_blink_cors_); - // TODO(hintzed) replace this delegation with an implementation that does not + // TODO(toyoshim) replace this delegation with an implementation that does not // perform CORS checks but relies on CORSURLLoader for CORS // (https://crbug.com/736308). MakeCrossOriginAccessRequestBlinkCORS(request); @@ -393,8 +407,8 @@ ResourceRequest& preflight_request = web_url_request.ToMutableResourceRequest(); - // TODO(tyoshino): Call prepareCrossOriginRequest(preflightRequest) to - // also set the referrer header. + // TODO(tyoshino): Call PrepareCrossOriginRequest(preflight_request) to also + // set the referrer header. if (GetSecurityOrigin()) preflight_request.SetHTTPOrigin(GetSecurityOrigin()); @@ -1142,6 +1156,25 @@ } void DocumentThreadableLoader::DispatchDidFail(const ResourceError& error) { + if (error.CORSErrorStatus()) { + DCHECK(out_of_blink_cors_); + // TODO(toyoshim): Should consider to pass correct arguments instead of + // WebURL() and WebHTTPHeaderMap() to GetErrorString(). + // We still need plumbing required information. + const int response_code = + error.CORSErrorStatus()->related_response_headers + ? error.CORSErrorStatus()->related_response_headers->response_code() + : 0; + GetExecutionContext()->AddConsoleMessage(ConsoleMessage::Create( + kJSMessageSource, kErrorMessageLevel, + "Failed to load " + error.FailingURL() + ": " + + WebCORS::GetErrorString( + error.CORSErrorStatus()->cors_error, KURL(error.FailingURL()), + WebURL(), response_code, WebHTTPHeaderMap(HTTPHeaderMap()), + WebSecurityOrigin(GetSecurityOrigin()), request_context_) + .Utf8() + .data())); + } ThreadableLoaderClient* client = client_; Clear(); client->DidFail(error);
diff --git a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h index b1ddd83..89b55c0 100644 --- a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h +++ b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h
@@ -156,8 +156,8 @@ network::mojom::FetchCredentialsMode, const ResourceResponse&, std::unique_ptr<WebDataConsumerHandle>); - // TODO(hintzed): CORS handled in Blink. Methods below named *BlinkCORS are to - // be removed after https://crbug.com/736308 is fixed (i.e. when CORS is + // TODO(toyoshim): CORS handled in Blink. Methods below named *BlinkCORS are + // to be removed after https://crbug.com/736308 is fixed (i.e. when CORS is // handled out of Blink). void DispatchInitialRequestBlinkCORS(ResourceRequest&); void MakeCrossOriginAccessRequestBlinkCORS(const ResourceRequest&);
diff --git a/third_party/WebKit/Source/platform/BUILD.gn b/third_party/WebKit/Source/platform/BUILD.gn index 7cd2e02..da9d78c2 100644 --- a/third_party/WebKit/Source/platform/BUILD.gn +++ b/third_party/WebKit/Source/platform/BUILD.gn
@@ -640,6 +640,7 @@ "exported/WebTextRun.cpp", "exported/WebThreadSafeData.cpp", "exported/WebURL.cpp", + "exported/WebURLError.cpp", "exported/WebURLLoadTiming.cpp", "exported/WebURLLoaderClient.cpp", "exported/WebURLLoaderTestDelegate.cpp", @@ -1991,6 +1992,7 @@ ":test_support", "//base", "//base/test:test_support", + "//services/network/public/cpp:cpp", "//testing/gtest", "//testing/perf", "//third_party:freetype_harfbuzz",
diff --git a/third_party/WebKit/Source/platform/DEPS b/third_party/WebKit/Source/platform/DEPS index b4de359..14d5cc6 100644 --- a/third_party/WebKit/Source/platform/DEPS +++ b/third_party/WebKit/Source/platform/DEPS
@@ -61,3 +61,9 @@ "-core", "-modules", ] + +specific_include_rules = { + "WebURLError\.cpp": [ + "+net/base/net_errors.h" + ] +}
diff --git a/third_party/WebKit/Source/platform/exported/WebURLError.cpp b/third_party/WebKit/Source/platform/exported/WebURLError.cpp new file mode 100644 index 0000000..47a21a5 --- /dev/null +++ b/third_party/WebKit/Source/platform/exported/WebURLError.cpp
@@ -0,0 +1,37 @@ +// Copyright 2017 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. + +#include "public/platform/WebURLError.h" + +#include "net/base/net_errors.h" + +namespace blink { + +WebURLError::WebURLError(int reason, const WebURL& url) + : reason_(reason), url_(url) { + DCHECK_NE(reason_, 0); +} + +WebURLError::WebURLError(int reason, + HasCopyInCache has_copy_in_cache, + IsWebSecurityViolation is_web_security_violation, + const WebURL& url) + : reason_(reason), + has_copy_in_cache_(has_copy_in_cache == HasCopyInCache::kTrue), + is_web_security_violation_(is_web_security_violation == + IsWebSecurityViolation::kTrue), + url_(url) { + DCHECK_NE(reason_, 0); +} + +WebURLError::WebURLError(const network::CORSErrorStatus& cors_error_status, + HasCopyInCache has_copy_in_cache, + const WebURL& url) + : reason_(net::ERR_FAILED), + has_copy_in_cache_(has_copy_in_cache == HasCopyInCache::kTrue), + is_web_security_violation_(true), + url_(url), + cors_error_status_(cors_error_status) {} + +} // namespace blink
diff --git a/third_party/WebKit/Source/platform/loader/BUILD.gn b/third_party/WebKit/Source/platform/loader/BUILD.gn index ac8260c9..c4be3880 100644 --- a/third_party/WebKit/Source/platform/loader/BUILD.gn +++ b/third_party/WebKit/Source/platform/loader/BUILD.gn
@@ -93,6 +93,7 @@ deps = [ ":make_platform_loader_generated_fetch_initiator_type_names", "//components/link_header_util:link_header_util", + "//services/network/public/cpp:cpp", ] public_deps = [
diff --git a/third_party/WebKit/Source/platform/loader/DEPS b/third_party/WebKit/Source/platform/loader/DEPS index f999846..4f6ea4f 100644 --- a/third_party/WebKit/Source/platform/loader/DEPS +++ b/third_party/WebKit/Source/platform/loader/DEPS
@@ -2,7 +2,7 @@ "+base/metrics/field_trial_params.h", # for fetch/ResourceLoadScheduler.cpp "+base/strings/string_number_conversions.h", # for fetch/ResourceLoadScheduler.cpp "+components/link_header_util", # for LinkHeader.cpp - "+services/network/public/interfaces", # for Fetch API and CORS + "+services/network/public", # for Fetch API and CORS "+third_party/boringssl/src/include/openssl/curve25519.h" # for SubresourceIntegrity.cpp ]
diff --git a/third_party/WebKit/Source/platform/loader/fetch/ResourceError.cpp b/third_party/WebKit/Source/platform/loader/fetch/ResourceError.cpp index 756fe6e..ead9028 100644 --- a/third_party/WebKit/Source/platform/loader/fetch/ResourceError.cpp +++ b/third_party/WebKit/Source/platform/loader/fetch/ResourceError.cpp
@@ -46,7 +46,7 @@ } ResourceError ResourceError::CancelledError(const KURL& url) { - return ResourceError(net::ERR_ABORTED, url); + return ResourceError(net::ERR_ABORTED, url, WTF::nullopt); } ResourceError ResourceError::CancelledDueToAccessCheckError( @@ -69,19 +69,24 @@ } ResourceError ResourceError::CacheMissError(const KURL& url) { - return ResourceError(net::ERR_CACHE_MISS, url); + return ResourceError(net::ERR_CACHE_MISS, url, WTF::nullopt); } ResourceError ResourceError::TimeoutError(const KURL& url) { - return ResourceError(net::ERR_TIMED_OUT, url); + return ResourceError(net::ERR_TIMED_OUT, url, WTF::nullopt); } ResourceError ResourceError::Failure(const KURL& url) { - return ResourceError(net::ERR_FAILED, url); + return ResourceError(net::ERR_FAILED, url, WTF::nullopt); } -ResourceError::ResourceError(int error_code, const KURL& url) - : error_code_(error_code), failing_url_(url) { +ResourceError::ResourceError( + int error_code, + const KURL& url, + WTF::Optional<network::CORSErrorStatus> cors_error_status) + : error_code_(error_code), + failing_url_(url), + cors_error_status_(cors_error_status) { DCHECK_NE(error_code_, 0); InitializeDescription(); } @@ -90,13 +95,15 @@ : error_code_(error.reason()), failing_url_(error.url()), is_access_check_(error.is_web_security_violation()), - has_copy_in_cache_(error.has_copy_in_cache()) { + has_copy_in_cache_(error.has_copy_in_cache()), + cors_error_status_(error.cors_error_status()) { DCHECK_NE(error_code_, 0); InitializeDescription(); } ResourceError ResourceError::Copy() const { - ResourceError error_copy(error_code_, failing_url_.Copy()); + ResourceError error_copy(error_code_, failing_url_.Copy(), + cors_error_status_); error_copy.has_copy_in_cache_ = has_copy_in_cache_; error_copy.localized_description_ = localized_description_.IsolatedCopy(); error_copy.is_access_check_ = is_access_check_; @@ -104,9 +111,16 @@ } ResourceError::operator WebURLError() const { - return WebURLError(error_code_, - has_copy_in_cache_ ? WebURLError::HasCopyInCache::kTrue - : WebURLError::HasCopyInCache::kFalse, + WebURLError::HasCopyInCache has_copy_in_cache = + has_copy_in_cache_ ? WebURLError::HasCopyInCache::kTrue + : WebURLError::HasCopyInCache::kFalse; + + if (cors_error_status_) { + DCHECK_EQ(net::ERR_FAILED, error_code_); + return WebURLError(*cors_error_status_, has_copy_in_cache, failing_url_); + } + + return WebURLError(error_code_, has_copy_in_cache, is_access_check_ ? WebURLError::IsWebSecurityViolation::kTrue : WebURLError::IsWebSecurityViolation::kFalse, @@ -129,6 +143,9 @@ if (a.HasCopyInCache() != b.HasCopyInCache()) return false; + if (a.CORSErrorStatus() != b.CORSErrorStatus()) + return false; + return true; }
diff --git a/third_party/WebKit/Source/platform/loader/fetch/ResourceError.h b/third_party/WebKit/Source/platform/loader/fetch/ResourceError.h index 3b982008..78202fd 100644 --- a/third_party/WebKit/Source/platform/loader/fetch/ResourceError.h +++ b/third_party/WebKit/Source/platform/loader/fetch/ResourceError.h
@@ -34,6 +34,7 @@ #include "platform/wtf/Optional.h" #include "platform/wtf/text/WTFString.h" #include "public/platform/WebURLError.h" +#include "services/network/public/cpp/cors_error_status.h" namespace blink { @@ -60,7 +61,9 @@ ResourceError() = delete; // |error_code| must not be 0. - ResourceError(int error_code, const KURL& failing_url); + ResourceError(int error_code, + const KURL& failing_url, + WTF::Optional<network::CORSErrorStatus>); ResourceError(const WebURLError&); // Makes a deep copy. Useful for when you need to use a ResourceError on @@ -79,6 +82,10 @@ bool WasBlockedByResponse() const; bool ShouldCollapseInitiator() const { return should_collapse_initiator_; } + WTF::Optional<network::CORSErrorStatus> CORSErrorStatus() const { + return cors_error_status_; + } + operator WebURLError() const; static bool Compare(const ResourceError&, const ResourceError&); @@ -95,6 +102,7 @@ bool is_access_check_ = false; bool has_copy_in_cache_ = false; bool should_collapse_initiator_ = false; + WTF::Optional<network::CORSErrorStatus> cors_error_status_; }; inline bool operator==(const ResourceError& a, const ResourceError& b) {
diff --git a/third_party/WebKit/public/platform/DEPS b/third_party/WebKit/public/platform/DEPS index 21113b6..d61d0b88 100644 --- a/third_party/WebKit/public/platform/DEPS +++ b/third_party/WebKit/public/platform/DEPS
@@ -22,6 +22,7 @@ "-public/web", # Enforce to use mojom-shared.h in WebKit/public so that it can compile # inside and outside Blink. + "+services/network/public/cpp/cors_error_status.h", "+services/network/public/interfaces/cors.mojom-shared.h", "+services/network/public/interfaces/fetch_api.mojom-shared.h", "+services/service_manager/public/interfaces",
diff --git a/third_party/WebKit/public/platform/WebURLError.h b/third_party/WebKit/public/platform/WebURLError.h index eb96897..8ef7f9d 100644 --- a/third_party/WebKit/public/platform/WebURLError.h +++ b/third_party/WebKit/public/platform/WebURLError.h
@@ -33,6 +33,8 @@ #include "WebURL.h" #include "base/logging.h" +#include "base/optional.h" +#include "services/network/public/cpp/cors_error_status.h" namespace blink { @@ -50,26 +52,23 @@ WebURLError() = delete; // |reason| must not be 0. - WebURLError(int reason, const WebURL& url) : reason_(reason), url_(url) { - DCHECK_NE(reason_, 0); - } + BLINK_PLATFORM_EXPORT WebURLError(int reason, const WebURL&); // |reason| must not be 0. - WebURLError(int reason, - HasCopyInCache has_copy_in_cache, - IsWebSecurityViolation is_web_security_violation, - const WebURL& url) - : reason_(reason), - has_copy_in_cache_(has_copy_in_cache == HasCopyInCache::kTrue), - is_web_security_violation_(is_web_security_violation == - IsWebSecurityViolation::kTrue), - url_(url) { - DCHECK_NE(reason_, 0); - } + BLINK_PLATFORM_EXPORT WebURLError(int reason, + HasCopyInCache, + IsWebSecurityViolation, + const WebURL&); + BLINK_PLATFORM_EXPORT WebURLError(const network::CORSErrorStatus&, + HasCopyInCache, + const WebURL&); int reason() const { return reason_; } bool has_copy_in_cache() const { return has_copy_in_cache_; } bool is_web_security_violation() const { return is_web_security_violation_; } const WebURL& url() const { return url_; } + const base::Optional<network::CORSErrorStatus> cors_error_status() const { + return cors_error_status_; + } private: // A numeric error code detailing the reason for this error. The value must @@ -85,6 +84,9 @@ // The url that failed to load. WebURL url_; + + // Optional CORS error details. + base::Optional<network::CORSErrorStatus> cors_error_status_; }; } // namespace blink