Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(188)

Issue 23499003: Improve <webview> autosize: (Closed)

Created:
7 years, 3 months ago by lazyboy
Modified:
7 years, 3 months ago
Reviewers:
Fady Samuel
CC:
chromium-reviews, joi+watch-content_chromium.org, chromium-apps-reviews_chromium.org, darin-cc_chromium.org, jam, extensions-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Improve <webview> autosize: a. Expand/shrink <webview> element when 'sizechange' event fires, to match with the new view size (in shim). b. For SW mode, fix a bug where damage buffer would remain smaller than the view size and would result in crash. Added test for this case. BUG= Test=WebViewTest.AutoSize.* Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=220402

Patch Set 1 #

Total comments: 6

Patch Set 2 : Rework CL. #

Patch Set 3 : Add UMA for autosize. #

Total comments: 1

Patch Set 4 : Move sizechanged handler code to helper. #

Total comments: 2

Patch Set 5 : Address comments. #

Patch Set 6 : Sync. #

Patch Set 7 : Move dirtying autosize state to UpdateGuestAutoSizeState, otherwise we can miss autosize related at… #

Messages

Total messages: 14 (0 generated)
lazyboy
7 years, 3 months ago (2013-08-27 02:09:52 UTC) #1
Fady Samuel
https://codereview.chromium.org/23499003/diff/1/chrome/renderer/resources/extensions/web_view.js File chrome/renderer/resources/extensions/web_view.js (right): https://codereview.chromium.org/23499003/diff/1/chrome/renderer/resources/extensions/web_view.js#newcode118 chrome/renderer/resources/extensions/web_view.js:118: webview.webviewNode_.style.width = webviewEvent.newWidth + 'px'; Check the current bounds ...
7 years, 3 months ago (2013-08-27 14:16:17 UTC) #2
Fady Samuel
Probably also useful to record the UMA action in BrowserPluginGuest::OnSetSize when EnableAutoResize is called to ...
7 years, 3 months ago (2013-08-27 14:18:52 UTC) #3
lazyboy
https://chromiumcodereview.appspot.com/23499003/diff/1/chrome/renderer/resources/extensions/web_view.js File chrome/renderer/resources/extensions/web_view.js (right): https://chromiumcodereview.appspot.com/23499003/diff/1/chrome/renderer/resources/extensions/web_view.js#newcode118 chrome/renderer/resources/extensions/web_view.js:118: webview.webviewNode_.style.width = webviewEvent.newWidth + 'px'; On 2013/08/27 14:16:18, Fady ...
7 years, 3 months ago (2013-08-27 23:22:17 UTC) #4
Fady Samuel
This is really awesome! This will make a bunch of developers happy! :-) Just one ...
7 years, 3 months ago (2013-08-28 19:14:38 UTC) #5
lazyboy
Extracted sizechanged event handler to a separate function. Renamed variable according to the Clean up ...
7 years, 3 months ago (2013-08-28 20:41:17 UTC) #6
Fady Samuel
lgtm after one comment is addressed. Thanks! :-) https://chromiumcodereview.appspot.com/23499003/diff/15001/chrome/test/data/extensions/platform_apps/web_view/autosize/main.js File chrome/test/data/extensions/platform_apps/web_view/autosize/main.js (right): https://chromiumcodereview.appspot.com/23499003/diff/15001/chrome/test/data/extensions/platform_apps/web_view/autosize/main.js#newcode50 chrome/test/data/extensions/platform_apps/web_view/autosize/main.js:50: document.body.appendChild(webview); ...
7 years, 3 months ago (2013-08-28 20:45:27 UTC) #7
lazyboy
https://chromiumcodereview.appspot.com/23499003/diff/15001/chrome/test/data/extensions/platform_apps/web_view/autosize/main.js File chrome/test/data/extensions/platform_apps/web_view/autosize/main.js (right): https://chromiumcodereview.appspot.com/23499003/diff/15001/chrome/test/data/extensions/platform_apps/web_view/autosize/main.js#newcode50 chrome/test/data/extensions/platform_apps/web_view/autosize/main.js:50: document.body.appendChild(webview); On 2013/08/28 20:45:27, Fady Samuel wrote: > Why ...
7 years, 3 months ago (2013-08-28 22:09:37 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/lazyboy@chromium.org/23499003/28001
7 years, 3 months ago (2013-08-28 23:07:11 UTC) #9
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) telemetry_unittests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=191836
7 years, 3 months ago (2013-08-29 02:18:53 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/lazyboy@chromium.org/23499003/28001
7 years, 3 months ago (2013-08-29 02:32:54 UTC) #11
commit-bot: I haz the power
Retried try job too often on linux_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&number=163503
7 years, 3 months ago (2013-08-29 05:54:11 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/lazyboy@chromium.org/23499003/27001
7 years, 3 months ago (2013-08-29 16:40:13 UTC) #13
commit-bot: I haz the power
7 years, 3 months ago (2013-08-29 22:37:59 UTC) #14
Message was sent while issue was closed.
Change committed as 220402

Powered by Google App Engine
This is Rietveld 408576698