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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 13875006: Attempting to fix perf regression on Linux of nacl_helper-data/data. (Closed)

Created:
7 years, 8 months ago by rpetterson
Modified:
7 years, 8 months ago
Reviewers:
raymes1, Cris Neckar
CC:
chromium-reviews, piman+watch_chromium.org, raymes+watch_chromium.org, yusukes+watch_chromium.org, yzshen+watch_chromium.org, ihf+watch_chromium.org
Visibility:
Public.

Description

Attempting to fix perf regression on Linux of nacl_helper-data/data. Revert 195188 - Add interface to set the sub resource crash key from Flash BUG=N/A TEST=N/A Review URL: https://chromiumcodereview.appspot.com/14311004 TBR=cdn@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=195344

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -20 lines) Patch
M ppapi/api/private/ppb_flash.idl View 1 chunk +1 line, -6 lines 0 comments Download
M ppapi/c/private/ppb_flash.h View 2 chunks +2 lines, -6 lines 0 comments Download
M ppapi/proxy/flash_resource.cc View 2 chunks +3 lines, -8 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
rpetterson
7 years, 8 months ago (2013-04-20 00:01:46 UTC) #1
raymes1
Did this have any impact? This change should have been fairly innocuous. On Fri, Apr ...
7 years, 8 months ago (2013-04-20 20:57:29 UTC) #2
rpetterson
7 years, 8 months ago (2013-04-22 17:46:38 UTC) #3 
I think these questions have been resolved at this point, but please let me
know if there are other qeustions. Apologies for reverting your change
while trying to diagnose the problem. Hopefully you've had a chance to
recommit at this point.

Thanks,

Rachel
On Apr 20, 2013 1:57 PM, "Raymes Khoury" <raymes@google.com> wrote:

> Did this have any impact? This change should have been fairly innocuous.
>
> On Fri, Apr 19, 2013 at 5:01 PM,  <rlp@chromium.org> wrote:
> > Reviewers: Cris Neckar,
> >
> > Description:
> > Attempting to fix perf regression on Linux of nacl_helper-data/data.
> >
> > Revert 195188 - Add interface to set the sub resource crash key from
> Flash
> >
> > BUG=N/A
> > TEST=N/A
> >
> > Review URL: https://chromiumcodereview.appspot.com/14311004
> >
> > TBR=cdn@chromium.org
> >
> > Please review this at https://codereview.chromium.org/13875006/
> >
> > SVN Base: svn://svn.chromium.org/chrome/trunk/src/
> >
> > Affected files:
> >   M     ppapi/api/private/ppb_flash.idl
> >   M     ppapi/c/private/ppb_flash.h
> >   M     ppapi/proxy/flash_resource.cc
> >
> >
> > Index: ppapi/api/private/ppb_flash.idl
> > ===================================================================
> > --- ppapi/api/private/ppb_flash.idl     (revision 195342)
> > +++ ppapi/api/private/ppb_flash.idl     (working copy)
> > @@ -103,12 +103,7 @@
> >    /**
> >     * Specifies the document URL which contains the flash instance.
> >     */
> > -  PP_FLASHCRASHKEY_URL = 1,
> > -
> > -  /**
> > -   * Specifies the URL of the current swf.
> > -   */
> > -  PP_FLASHCRASHKEY_RESOURCE_URL = 2
> > +  PP_FLASHCRASHKEY_URL = 1
> >  };
> >
> >  /**
> > Index: ppapi/c/private/ppb_flash.h
> > ===================================================================
> > --- ppapi/c/private/ppb_flash.h (revision 195342)
> > +++ ppapi/c/private/ppb_flash.h (working copy)
> > @@ -3,7 +3,7 @@
> >   * found in the LICENSE file.
> >   */
> >
> > -/* From private/ppb_flash.idl modified Thu Apr 18 15:06:12 2013. */
> > +/* From private/ppb_flash.idl modified Thu Mar 28 10:30:53 2013. */
> >
> >  #ifndef PPAPI_C_PRIVATE_PPB_FLASH_H_
> >  #define PPAPI_C_PRIVATE_PPB_FLASH_H_
> > @@ -118,11 +118,7 @@
> >    /**
> >     * Specifies the document URL which contains the flash instance.
> >     */
> > -  PP_FLASHCRASHKEY_URL = 1,
> > -  /**
> > -   * Specifies the URL of the current swf.
> > -   */
> > -  PP_FLASHCRASHKEY_RESOURCE_URL = 2
> > +  PP_FLASHCRASHKEY_URL = 1
> >  } PP_FlashCrashKey;
> >  PP_COMPILE_ASSERT_SIZE_IN_BYTES(PP_FlashCrashKey, 4);
> >  /**
> > Index: ppapi/proxy/flash_resource.cc
> > ===================================================================
> > --- ppapi/proxy/flash_resource.cc       (revision 195342)
> > +++ ppapi/proxy/flash_resource.cc       (working copy)
> > @@ -7,7 +7,6 @@
> >  #include <cmath>
> >
> >  #include "base/containers/mru_cache.h"
> > -#include "base/debug/crash_logging.h"
> >  #include "base/lazy_instance.h"
> >  #include "base/time.h"
> >  #include "ppapi/c/pp_errors.h"
> > @@ -84,18 +83,14 @@
> >  PP_Bool FlashResource::SetCrashData(PP_Instance instance,
> >                                      PP_FlashCrashKey key,
> >                                      PP_Var value) {
> > -  StringVar* url_string_var(StringVar::FromPPVar(value));
> > -  if (!url_string_var)
> > -    return PP_FALSE;
> >    switch (key) {
> >      case PP_FLASHCRASHKEY_URL: {
> > +      StringVar* url_string_var(StringVar::FromPPVar(value));
> > +      if (!url_string_var)
> > +        return PP_FALSE;
> >        PluginGlobals::Get()->SetActiveURL(url_string_var->value());
> >        return PP_TRUE;
> >      }
> > -    case PP_FLASHCRASHKEY_RESOURCE_URL: {
> > -      base::debug::SetCrashKeyValue("subresource_url",
> > url_string_var->value());
> > -      return PP_TRUE;
> > -    }
> >    }
> >    return PP_FALSE;
> >  }
> >
> >
>

Powered by Google App Engine
This is Rietveld 408576698