Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2494713002:
Pass correct resource type to OnDidLoadResourceFromMemoryCache (Closed)
DescriptionPass correct resource type to OnDidLoadResourceFromMemoryCache.
Resource type isn't passed to browser correctly.
More details in https://crbug.com/664135
This is an attempt to fix it.
BUG=664135
Committed: https://crrev.com/6a5b9a93ca59066746f7e46a6f4a8e2594d3bb07
Cr-Commit-Position: refs/heads/master@{#434629}
Patch Set 1 #Patch Set 2 : Add test. #
Messages
Total messages: 32 (17 generated)
Description was changed from ========== Fix DidLoadResourceFromMemoryCache message contents. Resource type wasn't passed to browser correctly. This is an attempt to fix it. BUG=664135 ========== to ========== Fix FrameHostMsg_DidLoadResourceFromMemoryCache message content. Resource type wasn't passed to browser correctly. This is an attempt to fix it. BUG=664135 ==========
The CQ bit was checked by alexilin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Fix FrameHostMsg_DidLoadResourceFromMemoryCache message content. Resource type wasn't passed to browser correctly. This is an attempt to fix it. BUG=664135 ========== to ========== Pass correct resource type to OnDidLoadResourceFromMemoryCache. Resource type isn't passed to browser correctly. More details in https://crbug.com/664135 This is an attempt to fix it. BUG=664135 ==========
alexilin@chromium.org changed reviewers: + mkwst@chromium.org, pasko@chromium.org, tyoshino@chromium.org
Hi all, This is my proposal to how to fix this bug. We got right resource type from browser side after this change. I'm not a blink developer so I can do something wrong. Especially I'm not sure whether we should reset request frameType and requestContext before pass it to dispatchWillSendRequest function.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
I mean, non-owner lgtm
Looks reasonable. Can you add some tests to verify the behavior?
Looks good. Please add some test as Mike suggested.
alexilin@chromium.org changed reviewers: + yoav@yoav.ws
Sorry for delay, I was out of office last week. Concerning tests I couldn't invent more descriptive unittest. It would be worth to add some integration test to check that WebFrameClient sends meaningful message to the browser eventually.
Friendly ping.
On 2016/11/22 17:45:50, alexilin wrote: > Friendly ping. code looks good, but I don't feel like I know gmock well enough to review the tests. Mike? Also, can you run the test on the trybots?
The CQ bit was checked by alexilin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
LGTM. Sorry for _my_ delay. I was sick. :/
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
yoav, tyoshino: ping
On 2016/11/28 08:14:14, alexilin wrote: > yoav, tyoshino: ping LGTM
On 2016/11/28 08:37:28, Yoav Weiss wrote: > On 2016/11/28 08:14:14, alexilin wrote: > > yoav, tyoshino: ping > > LGTM lgtm
The CQ bit was checked by alexilin@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from pasko@chromium.org Link to the patchset: https://codereview.chromium.org/2494713002/#ps20001 (title: "Add test.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480324009952840,
"parent_rev": "46af27e8b43fc95834f7b82d5cb1fcd4a7dccc3c", "commit_rev":
"aa9b31dda8363b41d72dae63615bb0852fac5bf5"}
Message was sent while issue was closed.
Description was changed from ========== Pass correct resource type to OnDidLoadResourceFromMemoryCache. Resource type isn't passed to browser correctly. More details in https://crbug.com/664135 This is an attempt to fix it. BUG=664135 ========== to ========== Pass correct resource type to OnDidLoadResourceFromMemoryCache. Resource type isn't passed to browser correctly. More details in https://crbug.com/664135 This is an attempt to fix it. BUG=664135 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Pass correct resource type to OnDidLoadResourceFromMemoryCache. Resource type isn't passed to browser correctly. More details in https://crbug.com/664135 This is an attempt to fix it. BUG=664135 ========== to ========== Pass correct resource type to OnDidLoadResourceFromMemoryCache. Resource type isn't passed to browser correctly. More details in https://crbug.com/664135 This is an attempt to fix it. BUG=664135 Committed: https://crrev.com/6a5b9a93ca59066746f7e46a6f4a8e2594d3bb07 Cr-Commit-Position: refs/heads/master@{#434629} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/6a5b9a93ca59066746f7e46a6f4a8e2594d3bb07 Cr-Commit-Position: refs/heads/master@{#434629} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
