Skip to content

Commit

Permalink
WebApp minimum-window width should behave similar to the browser
Browse files Browse the repository at this point in the history
Updating the logic for determining the minimum width of the BrowserView
to use the same minimum width for a primary WebApp instance as it does
a normal/main instance. This by itself doesn't guarantee every control
will always have its expected minimum width, but just like its main
BrowserView equivalent, it greatly reduces the cases while minimizing
complexity.

Partial fixes (Bug) / Full fixes (Fixed)

Bug: 933056, 1110531
Change-Id: Ic25a3345e3b93ffc76ac25fe6b8bf755b1a05ba4
Fixed: 938822, 933056, 1078705, 1104049
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3229971
Reviewed-by: Scott Violet <[email protected]>
Commit-Queue: Hoch Hochkeppel <[email protected]>
Cr-Commit-Position: refs/heads/main@{#933298}
  • Loading branch information
mhochk authored and Chromium LUCI CQ committed Oct 20, 2021
1 parent ae1f551 commit da6677e
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 12 deletions.
8 changes: 8 additions & 0 deletions chrome/browser/ui/views/frame/browser_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,14 @@ class BrowserViewLayoutDelegateImpl : public BrowserViewLayoutDelegate {
return browser_view_->browser()->is_type_normal();
}

bool BrowserIsTypeSystemWebApp() const override {
return browser_view_->browser()->app_controller()->system_app();
}

bool BrowserIsTypeWebApp() const override {
return browser_view_->GetIsWebAppType();
}

bool HasFindBarController() const override {
return browser_view_->browser()->HasFindBarController();
}
Expand Down
4 changes: 3 additions & 1 deletion chrome/browser/ui/views/frame/browser_view_layout.cc
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,9 @@ gfx::Size BrowserViewLayout::GetMinimumSize(const views::View* host) const {
// TODO(pkotwicz): Adjust the minimum height for the find bar.

gfx::Size contents_size(contents_container_->GetMinimumSize());
contents_size.SetToMax(delegate_->BrowserIsTypeNormal()
contents_size.SetToMax((delegate_->BrowserIsTypeNormal() ||
(delegate_->BrowserIsTypeWebApp() &&
!delegate_->BrowserIsTypeSystemWebApp()))
? gfx::Size(kMainBrowserContentsMinimumWidth,
kMainBrowserContentsMinimumHeight)
: kContentsMinimumSize);
Expand Down
15 changes: 8 additions & 7 deletions chrome/browser/ui/views/frame/browser_view_layout.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,14 @@ class WebContentsModalDialogHost;
// The layout manager used in chrome browser.
class BrowserViewLayout : public views::LayoutManager {
public:
// The minimum width for the normal (tabbed) browser window's contents area.
// This should be wide enough that WebUI pages (e.g. chrome://settings) and
// the various associated WebUI dialogs (e.g. Import Bookmarks) can still be
// functional. This value provides a trade-off between browser usability and
// privacy - specifically, the ability to browse in a very small window, even
// on large monitors (which is why a minimum height is not specified). This
// value is used for the main browser window only, not for popups.
// The minimum width for the normal (tabbed or web app) browser window's
// contents area. This should be wide enough that WebUI pages (e.g.
// chrome://settings) and the various associated WebUI dialogs (e.g. Import
// Bookmarks) can still be functional. This value provides a trade-off between
// browser usability and privacy - specifically, the ability to browse in a
// very small window, even on large monitors (which is why a minimum height is
// not specified). This value is used for the main browser window only, not
// for popups.
static constexpr int kMainBrowserContentsMinimumWidth = 500;

// |browser_view| may be null in tests.
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/views/frame/browser_view_layout_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ class BrowserViewLayoutDelegate {
virtual bool SupportsWindowFeature(Browser::WindowFeature feature) const = 0;
virtual gfx::NativeView GetHostView() const = 0;
virtual bool BrowserIsTypeNormal() const = 0;
virtual bool BrowserIsTypeSystemWebApp() const = 0;
virtual bool BrowserIsTypeWebApp() const = 0;
virtual bool HasFindBarController() const = 0;
virtual void MoveWindowForFindBarIfNecessary() const = 0;
};
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/views/frame/browser_view_layout_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ class MockBrowserViewLayoutDelegate : public BrowserViewLayoutDelegate {
}
gfx::NativeView GetHostView() const override { return nullptr; }
bool BrowserIsTypeNormal() const override { return true; }
bool BrowserIsTypeSystemWebApp() const override { return false; }
bool BrowserIsTypeWebApp() const override { return false; }
bool HasFindBarController() const override { return false; }
void MoveWindowForFindBarIfNecessary() const override {}

Expand Down
8 changes: 4 additions & 4 deletions chrome/browser/ui/web_applications/web_app_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ IN_PROC_BROWSER_TEST_F(WebAppBrowserTest, PWASizeIsCorrectlyRestored) {
EXPECT_TRUE(AppBrowserController::IsWebApp(app_browser));
NavigateToURLAndWait(app_browser, app_url);

const gfx::Rect bounds = gfx::Rect(50, 50, 500, 500);
const gfx::Rect bounds = gfx::Rect(50, 50, 550, 550);
app_browser->window()->SetBounds(bounds);
app_browser->window()->Close();

Expand All @@ -552,7 +552,7 @@ IN_PROC_BROWSER_TEST_F(WebAppTabRestoreBrowserTest,
EXPECT_TRUE(AppBrowserController::IsWebApp(app_browser));
NavigateToURLAndWait(app_browser, app_url);

const gfx::Rect bounds = gfx::Rect(50, 50, 500, 500);
const gfx::Rect bounds = gfx::Rect(50, 50, 550, 550);
app_browser->window()->SetBounds(bounds);
app_browser->window()->Close();

Expand Down Expand Up @@ -600,7 +600,7 @@ IN_PROC_BROWSER_TEST_F(WebAppBrowserTest,

const GURL offscope_url =
https_server()->GetURL("offscope.site.test", "/simple.html");
const gfx::Size size(500, 500);
const gfx::Size size(550, 550);

Browser* const popup_browser =
OpenPopupAndWait(app_browser, offscope_url, size);
Expand Down Expand Up @@ -638,7 +638,7 @@ IN_PROC_BROWSER_TEST_F(WebAppBrowserTest,

EXPECT_TRUE(AppBrowserController::IsWebApp(app_browser));

const gfx::Size size(500, 500);
const gfx::Size size(550, 550);
Browser* const popup_browser = OpenPopupAndWait(app_browser, app_url, size);

// The navigation should have happened in a new window.
Expand Down

0 comments on commit da6677e

Please sign in to comment.