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

Issue 218693003: Generate FrameTree::uniqueName with a URL attribute value. (Closed)

Created:
6 years, 8 months ago by tkent
Modified:
6 years, 8 months ago
Reviewers:
Nate Chapin
CC:
blink-reviews, Erik Dahlström (inactive), pdr.
Visibility:
Public.

Description

Generate FrameTree::uniqueName with a URL attribute value. This CL fixes a bug that navigating back can swap plugin contents. Bug detail: When a page is loaded by back navigation, we try to load the last contents into frames. SVG content in <object> is also represented as a frame internally. The last contents state is stored in a HistoryItem tree, and HistoryItems for sub-frames are identified by 'uniqueName,' which depends on frame creation order if frames don't have name attributes. So, if frame creation orders of the first visit and the second visit are different, we could get incorrect HistoryItem in WebFrameImpl:: createChildFrame. How to fix: This CL tries to generate unique uniqueName even if a frame has no name attribute. We use an attribute value of Element:: subResourceAttributeName. The test was created by ed@opera.com. BUG=352762 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170728

Patch Set 1 : #

Total comments: 2

Patch Set 2 : remove unload handler #

Unified diffs Side-by-side diffs Delta from patch set Stats (+50 lines, -5 lines) Patch
M LayoutTests/http/tests/navigation/reload-subframe-object-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
A LayoutTests/svg/as-object/history-navigation.html View 1 1 chunk +21 lines, -0 lines 0 comments Download
A LayoutTests/svg/as-object/history-navigation-expected.html View 1 chunk +4 lines, -0 lines 0 comments Download
A LayoutTests/svg/as-object/resources/left.svg View 1 chunk +3 lines, -0 lines 0 comments Download
A LayoutTests/svg/as-object/resources/left-right.html View 1 chunk +8 lines, -0 lines 0 comments Download
A LayoutTests/svg/as-object/resources/right.svg View 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/page/FrameTree.h View 1 chunk +3 lines, -1 line 0 comments Download
M Source/core/page/FrameTree.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/web/WebFrameImpl.cpp View 1 1 chunk +5 lines, -1 line 0 comments Download

Messages

Total messages: 14 (0 generated)
tkent
Nate, would you review this please?
6 years, 8 months ago (2014-04-02 23:24:25 UTC) #1
Nate Chapin
LGTM with a test nit https://codereview.chromium.org/218693003/diff/20001/LayoutTests/svg/as-object/history-navigation.html File LayoutTests/svg/as-object/history-navigation.html (right): https://codereview.chromium.org/218693003/diff/20001/LayoutTests/svg/as-object/history-navigation.html#newcode8 LayoutTests/svg/as-object/history-navigation.html:8: onunload = function() {}; ...
6 years, 8 months ago (2014-04-02 23:31:16 UTC) #2
tkent
https://codereview.chromium.org/218693003/diff/20001/LayoutTests/svg/as-object/history-navigation.html File LayoutTests/svg/as-object/history-navigation.html (right): https://codereview.chromium.org/218693003/diff/20001/LayoutTests/svg/as-object/history-navigation.html#newcode8 LayoutTests/svg/as-object/history-navigation.html:8: onunload = function() {}; On 2014/04/02 23:31:17, Nate Chapin ...
6 years, 8 months ago (2014-04-03 00:26:36 UTC) #3
tkent
The CQ bit was checked by tkent@chromium.org
6 years, 8 months ago (2014-04-03 00:33:04 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tkent@chromium.org/218693003/30001
6 years, 8 months ago (2014-04-03 00:33:14 UTC) #5
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 8 months ago (2014-04-03 01:32:29 UTC) #6
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on win_blink_rel
6 years, 8 months ago (2014-04-03 01:32:30 UTC) #7
tkent
The CQ bit was checked by tkent@chromium.org
6 years, 8 months ago (2014-04-03 01:48:14 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tkent@chromium.org/218693003/30001
6 years, 8 months ago (2014-04-03 01:48:20 UTC) #9
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 8 months ago (2014-04-03 02:17:07 UTC) #10
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on win_blink_rel
6 years, 8 months ago (2014-04-03 02:17:07 UTC) #11
tkent
The CQ bit was checked by tkent@chromium.org
6 years, 8 months ago (2014-04-03 02:24:49 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tkent@chromium.org/218693003/30001
6 years, 8 months ago (2014-04-03 02:24:54 UTC) #13
commit-bot: I haz the power
6 years, 8 months ago (2014-04-03 03:47:08 UTC) #14
Message was sent while issue was closed.
Change committed as 170728

Powered by Google App Engine
This is Rietveld 408576698