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

Issue 954543002: <webview>: Fix SrcAttribute and Cleanup (Closed)

Created:
5 years, 10 months ago by Fady Samuel
Modified:
5 years, 10 months ago
Reviewers:
paulmeyer
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

<webview>: Fix SrcAttribute and Cleanup SrcAttribute.setValueIgnoreMutation was calling takeRecords() prior to applying the operation, which meant it didn't actually do the right thing. Mutation callbacks would still get queued up by the mutation observer. This wasn't noticed because WebViewImpl.onLoadCommit only called setValueIgnoreMutation if there was a change in value, and so the mutation observer would not trigger any mutations. This CL fixes this issue and simplifies the code in WebViewImpl.onLoadCommit a bit. This CL moves setting NameAttribute details from WebViewImpl and into the NameAttribute object. This CL also fixes some attribute changes to make sure their mutation handlers don't fire. Finally, this CL moves setupEventProperty to web_view_events.js where it makes more sense to be. BUG=none TBR=lazyboy@chromium.org Committed: https://crrev.com/cab37f85ed91c494e86fc201bf1550b9ba467471 Cr-Commit-Position: refs/heads/master@{#317682}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+43 lines, -38 lines) Patch
M extensions/renderer/resources/guest_view/web_view.js View 3 chunks +5 lines, -33 lines 0 comments Download
M extensions/renderer/resources/guest_view/web_view_attributes.js View 2 chunks +10 lines, -1 line 0 comments Download
M extensions/renderer/resources/guest_view/web_view_events.js View 3 chunks +28 lines, -4 lines 0 comments Download

Messages

Total messages: 13 (6 generated)
Fady Samuel
5 years, 10 months ago (2015-02-23 21:34:51 UTC) #2
paulmeyer
LGTM!
5 years, 10 months ago (2015-02-23 21:50:38 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/954543002/1
5 years, 10 months ago (2015-02-23 21:53:44 UTC) #5
commit-bot: I haz the power
Failed to apply the patch.
5 years, 10 months ago (2015-02-23 23:07:40 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/954543002/1
5 years, 10 months ago (2015-02-23 23:52:13 UTC) #11
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 10 months ago (2015-02-23 23:55:24 UTC) #12
commit-bot: I haz the power
5 years, 10 months ago (2015-02-23 23:55:56 UTC) #13
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/cab37f85ed91c494e86fc201bf1550b9ba467471
Cr-Commit-Position: refs/heads/master@{#317682}

Powered by Google App Engine
This is Rietveld 408576698